Merge pull request #3151 from alphagov/add-rake-task-to-revoke-permis…

…sions

Add Rake task to remove inappropriately-granted perms

lib/tasks/permissions.rake

@@ -121,4 +121,75 @@ namespace :permissions do
       end
     end
   end
+
+  desc "Remove inappropriately granted permissions from non-GDS users"
+  task remove_inappropriately_granted_permissions_from_non_gds_users: :environment do
+    supported_permission_ids_to_remove = [
+      { application_name: "Collections publisher", name: SupportedPermission::SIGNIN_NAME },
+      { application_name: "Collections publisher", name: "2i reviewer" },
+      { application_name: "Collections publisher", name: "Coronavirus editor" },
+      { application_name: "Collections publisher", name: "Edit Taxonomy" },
+      { application_name: "Collections publisher", name: "GDS Editor" },
+      { application_name: "Collections publisher", name: "Livestream editor" },
+      { application_name: "Collections publisher", name: "Sidekiq Monitoring" },
+      { application_name: "Collections publisher", name: "Skip review" },
+      { application_name: "Collections publisher", name: "Unreleased feature" },
+      { application_name: "Content Data", name: "view_email_subs" },
+      { application_name: "Content Data", name: "view_siteimprove" },
+      { application_name: "Content Tagger", name: "GDS Editor" },
+      { application_name: "Content Tagger", name: "Tagathon participant" },
+      { application_name: "Content Tagger", name: "Unreleased feature" },
+      { application_name: "Manuals Publisher", name: "gds_editor" },
+      { application_name: "Specialist publisher", name: "gds_editor" },
+      { application_name: "Support", name: "feedex_exporters" },
+      { application_name: "Support", name: "feedex_reviews" },
+    ].map do |application_name_and_name|
+      application_name_and_name => {application_name:, name:}
+      application_id = Doorkeeper::Application.find_by(name: application_name)&.id
+      SupportedPermission.find_by(application_id:, name:)&.id
+    end
+
+    gds_organisation_id = Organisation.find_by(content_id: Organisation::GDS_ORG_CONTENT_ID).id
+    non_gds_user_ids = User.where.not(organisation_id: gds_organisation_id)
+
+    user_application_permissions_to_destroy = UserApplicationPermission.where(
+      user_id: non_gds_user_ids,
+      supported_permission_id: supported_permission_ids_to_remove,
+    )
+
+    number_of_permissions_to_destroy = user_application_permissions_to_destroy.count
+    puts "Number of permissions to remove: #{number_of_permissions_to_destroy}, are you sure? (y)"
+
+    input = $stdin.gets.strip.downcase
+
+    unless input == "y"
+      abort("Aborting")
+    end
+
+    ActiveRecord::Base.transaction do
+      initiator = User.find_by(email: "ynda.jas@digital.cabinet-office.gov.uk")
+
+      puts "Destroying #{number_of_permissions_to_destroy} user application permissions"
+
+      user_application_permissions_to_destroy.each do |user_application_permission|
+        EventLog.record_event(
+          user_application_permission.user,
+          EventLog::PERMISSIONS_REMOVED,
+          initiator:,
+          application_id: user_application_permission.application_id,
+          trailing_message: "(removed inappropriately granted permission from non-GDS user: #{user_application_permission.supported_permission.name})",
+        )
+
+        user_application_permission.destroy!
+      end
+
+      user_application_permissions_remaining = UserApplicationPermission.where(
+        user_id: non_gds_user_ids,
+        supported_permission_id: supported_permission_ids_to_remove,
+      )
+      raise "Failed to destroy given permissions" unless user_application_permissions_remaining.count.zero?
+
+      puts "Destroyed #{number_of_permissions_to_destroy} user application permissions"
+    end
+  end
 end

test/controllers/two_step_verification_exemptions_controller_test.rb

@@ -2,7 +2,7 @@
 
 class TwoStepVerificationExemptionsControllerTest < ActionController::TestCase
   setup do
-    @gds = create(:organisation, content_id: Organisation::GDS_ORG_CONTENT_ID)
+    @gds = create(:gds_organisation)
     @organisation = create(:organisation)
   end
 

test/factories/oauth_applications.rb

@@ -15,15 +15,13 @@
 
     after(:create) do |app, evaluator|
       evaluator.with_non_delegatable_supported_permissions.each do |permission_name|
-        # we create signin in an after_create on application.
-        # this line takes care of tests creating signin in order to look complete or modify delegatable on it.
         app.signin_permission.update(delegatable: false) && next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:supported_permission, application: app, name: permission_name)
       end
 
       evaluator.with_non_delegatable_supported_permissions_not_grantable_from_ui.each do |permission_name|
-        next if permission_name == SupportedPermission::SIGNIN_NAME
+        app.signin_permission.update(delegatable: false, grantable_from_ui: false) && next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:supported_permission, application: app, name: permission_name, grantable_from_ui: false)
       end
@@ -35,7 +33,7 @@
       end
 
       evaluator.with_delegatable_supported_permissions_not_grantable_from_ui.each do |permission_name|
-        next if permission_name == SupportedPermission::SIGNIN_NAME
+        app.signin_permission.update(grantable_from_ui: false) && next if permission_name == SupportedPermission::SIGNIN_NAME
 
         create(:delegatable_supported_permission, application: app, name: permission_name, grantable_from_ui: false)
       end

test/factories/organisation.rb

@@ -8,5 +8,6 @@
 
   factory :gds_organisation, parent: :organisation do
     content_id { Organisation::GDS_ORG_CONTENT_ID }
+    name { "Government Digital Service" }
   end
 end

test/integration/users/two_step_verification_test.rb

@@ -148,7 +148,7 @@ class Users::TwoStepVerificationTest < ActionDispatch::IntegrationTest
   context "Exempting a user from 2sv" do
     context "when logged in as a gds super admin" do
       setup do
-        @gds = create(:organisation, content_id: Organisation::GDS_ORG_CONTENT_ID)
+        @gds = create(:gds_organisation)
         @super_admin = create(:superadmin_user, organisation: @gds)
         @reason_for_exemption = "accessibility reasons"
         @expiry_date = 5.days.from_now.to_date

test/lib/tasks/permissions_test.rb

@@ -4,7 +4,7 @@ class PermissionsTest < ActiveSupport::TestCase
   setup do
     Signon::Application.load_tasks if Rake::Task.tasks.empty?
 
-    @gds_org = create(:organisation, name: "Government Digital Service")
+    @gds_org = create(:gds_organisation)
     @non_gds_org = create(:organisation, name: "Another Department")
     $stdout.stubs(:write)
   end
@@ -110,6 +110,134 @@ class PermissionsTest < ActiveSupport::TestCase
     end
   end
 
+  context "#remove_inappropriately_granted_permissions_from_non_gds_users" do
+    setup do
+      @task = Rake::Task["permissions:remove_inappropriately_granted_permissions_from_non_gds_users"]
+
+      @collections_publisher = application_with_revoke_and_retain_list(
+        name: "Collections publisher",
+        non_signin_permissions_to_revoke: [
+          "2i reviewer", "Coronavirus editor", "Edit Taxonomy", "GDS Editor", "Livestream editor", "Sidekiq Monitoring", "Skip review", "Unreleased feature"
+        ],
+        revoke_signin: true,
+        retain_non_signin_permission: true,
+      )
+      @content_data = application_with_revoke_and_retain_list(
+        name: "Content Data",
+        non_signin_permissions_to_revoke: %w[view_email_subs view_siteimprove],
+        retain_non_signin_permission: true,
+      )
+      @content_tagger = application_with_revoke_and_retain_list(
+        name: "Content Tagger",
+        non_signin_permissions_to_revoke: ["GDS Editor", "Tagathon participant", "Unreleased feature"],
+        retain_non_signin_permission: true,
+      )
+      @manuals_publisher = application_with_revoke_and_retain_list(name: "Manuals Publisher", non_signin_permissions_to_revoke: %w[gds_editor])
+      @specialist_publisher = application_with_revoke_and_retain_list(name: "Specialist Publisher", non_signin_permissions_to_revoke: %w[gds_editor])
+      @support = application_with_revoke_and_retain_list(name: "Support", non_signin_permissions_to_revoke: %w[feedex_exporters feedex_reviews])
+      @app_to_ignore = application_with_revoke_and_retain_list(name: "Ignore me", non_signin_permissions_to_revoke: [], retain_non_signin_permission: true)
+
+      @non_gds_user_1 = create(
+        :user,
+        organisation: @non_gds_org,
+        with_permissions: all_permissions_by_app([@collections_publisher, @content_data, @content_tagger, @app_to_ignore]),
+      )
+      @non_gds_user_2 = create(
+        :user,
+        organisation: @non_gds_org,
+        with_permissions: all_permissions_by_app([@content_tagger, @manuals_publisher, @specialist_publisher, @support, @app_to_ignore]),
+      )
+      @gds_user = create(
+        :user,
+        organisation: @gds_org,
+        with_permissions: all_permissions_by_app([@collections_publisher, @manuals_publisher, @specialist_publisher, @support, @app_to_ignore]),
+      )
+
+      @initiator = create(:user, email: "ynda.jas@digital.cabinet-office.gov.uk")
+    end
+
+    should "remove inapppropriately-granted permissions from non-GDS users, leaving other permissions in place" do
+      string_io = StringIO.new
+      string_io.puts "y"
+      string_io.rewind
+      $stdin = string_io
+
+      @task.invoke
+
+      $stdin = STDIN
+
+      [@collections_publisher, @content_data, @content_tagger, @app_to_ignore].each do |app_hash|
+        assert_equal app_hash[:retained_permissions].map(&:name), @non_gds_user_1.permissions_for(app_hash[:application])
+        assert_event_logs_for_removal(@non_gds_user_1.uid, app_hash[:revoked_permissions])
+        refute_event_logs_for_removal(@non_gds_user_1.uid, app_hash[:retained_permissions])
+      end
+
+      [@content_tagger, @manuals_publisher, @specialist_publisher, @support, @app_to_ignore].each do |app_hash|
+        assert_equal app_hash[:retained_permissions].map(&:name), @non_gds_user_2.permissions_for(app_hash[:application])
+        assert_event_logs_for_removal(@non_gds_user_2.uid, app_hash[:revoked_permissions])
+        refute_event_logs_for_removal(@non_gds_user_2.uid, app_hash[:retained_permissions])
+      end
+
+      [@collections_publisher, @manuals_publisher, @specialist_publisher, @support, @app_to_ignore].each do |app_hash|
+        assert_equal app_hash[:application].supported_permissions.order(:name).pluck(:name), @gds_user.permissions_for(app_hash[:application])
+        refute_event_logs_for_removal(@gds_user.uid, app_hash[:application].supported_permissions)
+      end
+    end
+  end
+
+  def all_permissions_by_app(app_hashes)
+    permissions_hash = {}
+
+    app_hashes.each do |app_hash|
+      application = app_hash[:application]
+      permissions_hash[application] = application.supported_permissions.map(&:name)
+    end
+
+    permissions_hash
+  end
+
+  def application_with_revoke_and_retain_list(name:, non_signin_permissions_to_revoke:, revoke_signin: false, retain_non_signin_permission: false)
+    application = create(:application, name:)
+    hash = { application:, retained_permissions: [], revoked_permissions: [] }
+
+    signin_permission = hash[:application].signin_permission
+
+    if revoke_signin
+      hash[:revoked_permissions].push(signin_permission)
+    else
+      hash[:retained_permissions].push(signin_permission)
+    end
+
+    hash[:retained_permissions].push(create(:supported_permission, application:, name: "Untouchable")) if retain_non_signin_permission
+    hash[:revoked_permissions] = non_signin_permissions_to_revoke.sort.map { |permission_name| create(:supported_permission, application:, name: permission_name) }
+
+    hash
+  end
+
+  def assert_event_logs_for_removal(grantee_uid, permissions)
+    permissions.each do |permission|
+      assert_predicate EventLog.where(
+        uid: grantee_uid,
+        event_id: EventLog::PERMISSIONS_REMOVED.id,
+        initiator: @initiator,
+        application_id: permission.application_id,
+        trailing_message: "(removed inappropriately granted permission from non-GDS user: #{permission.name})",
+      ), :one?
+    end
+  end
+
+  def refute_event_logs_for_removal(grantee_uid, permissions)
+    permissions.each do |permission|
+      assert_empty EventLog.where(
+        uid: grantee_uid,
+        event_id: EventLog::PERMISSIONS_REMOVED.id,
+        initiator: @initiator,
+        application_id: permission.application_id,
+        trailing_message: "(removed inappropriately granted permission from non-GDS user: #{permission.name})",
+      )
+    end
+  end
+
   def user_with_permissions(user_type, organisation, permissions_hash)
     create(user_type, organisation:).tap do |user|
       permissions_hash.to_a.each do |app, permissions|

test/policies/user_policy_test.rb

@@ -9,7 +9,7 @@ class UserPolicyTest < ActiveSupport::TestCase
     @child_organisation = create(:organisation, parent: @parent_organisation)
     @super_org_admin = create(:super_organisation_admin_user, organisation: @parent_organisation)
     @organisation_admin = create(:organisation_admin_user, organisation: @parent_organisation)
-    @gds = create(:organisation, name: "Government Digital Services", content_id: Organisation::GDS_ORG_CONTENT_ID)
+    @gds = create(:gds_organisation)
   end
 
   primary_management_actions = %i[new create assign_organisation]