diff --git a/lib/tasks/permissions.rake b/lib/tasks/permissions.rake index 416b7a6ef..6d345204f 100644 --- a/lib/tasks/permissions.rake +++ b/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 diff --git a/test/controllers/two_step_verification_exemptions_controller_test.rb b/test/controllers/two_step_verification_exemptions_controller_test.rb index 932713f6e..ac9b0b817 100644 --- a/test/controllers/two_step_verification_exemptions_controller_test.rb +++ b/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 diff --git a/test/factories/oauth_applications.rb b/test/factories/oauth_applications.rb index 58e649f0e..e412eeece 100644 --- a/test/factories/oauth_applications.rb +++ b/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 diff --git a/test/factories/organisation.rb b/test/factories/organisation.rb index bf751b559..990d501bd 100644 --- a/test/factories/organisation.rb +++ b/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 diff --git a/test/integration/users/two_step_verification_test.rb b/test/integration/users/two_step_verification_test.rb index 011f426f3..b37b93d51 100644 --- a/test/integration/users/two_step_verification_test.rb +++ b/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 diff --git a/test/lib/tasks/permissions_test.rb b/test/lib/tasks/permissions_test.rb index 85d674d4e..58a5ea45a 100644 --- a/test/lib/tasks/permissions_test.rb +++ b/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| diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index abd3f12d9..f144cbb3a 100644 --- a/test/policies/user_policy_test.rb +++ b/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]