Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Rake task to remove inappropriately-granted perms #3151

Merged
merged 3 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 2 additions & 4 deletions test/factories/oauth_applications.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/factories/organisation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@

factory :gds_organisation, parent: :organisation do
content_id { Organisation::GDS_ORG_CONTENT_ID }
name { "Government Digital Service" }
end
end
2 changes: 1 addition & 1 deletion test/integration/users/two_step_verification_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
130 changes: 129 additions & 1 deletion test/lib/tasks/permissions_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down 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
2 changes: 1 addition & 1 deletion test/policies/user_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down