Skip to content

Commit

Permalink
Add Rake task to remove inappropriately-granted perms
Browse files Browse the repository at this point in the history
These have been identified as inappropriately granted to non-GDS users
as part of an exercise to clean up the data after having fixed the way
delegatable permissions work. We need to remove these permissions from
non-GDS users

We're using quite a few methods in the tests, which might feel a little
overkill for a single-use Rake task, but we opted for this in order to
cut down the number of lines required for all the setup and assertion
logic and limit the chance of mistyping any of the important details

Co-authored-by: George Eaton <george@dxw.com>

User input stubbing taken from: https://medium.com/@bschwartz92/testing-i-o-in-ruby-with-minitest-f775133adf6c
  • Loading branch information
yndajas committed Sep 9, 2024
1 parent b2843be commit 4767d16
Show file tree
Hide file tree
Showing 2 changed files with 199 additions and 0 deletions.
71 changes: 71 additions & 0 deletions lib/tasks/permissions.rake
Original file line number Diff line number Diff line change
Expand Up @@ -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
128 changes: 128 additions & 0 deletions test/lib/tasks/permissions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down

0 comments on commit 4767d16

Please sign in to comment.