diff --git a/app/controllers/account/manage_permissions_controller.rb b/app/controllers/account/manage_permissions_controller.rb index b24690636..5319510d6 100644 --- a/app/controllers/account/manage_permissions_controller.rb +++ b/app/controllers/account/manage_permissions_controller.rb @@ -4,6 +4,7 @@ class Account::ManagePermissionsController < ApplicationController before_action :authenticate_user! before_action :authorise_user + before_action :allow_no_application_access, only: [:update] def show @application_permissions = all_applications_and_permissions_for(current_user) @@ -21,6 +22,11 @@ def update private + def allow_no_application_access + params[:user] ||= {} + params[:user][:supported_permission_ids] ||= [] + end + def authorise_user authorize %i[account manage_permissions] end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 874474763..328a0e2d1 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -16,7 +16,7 @@ def organisation_options(form_builder) end def organisation_select_options - { include_blank: current_user.publishing_manager? ? false : "None" } + { include_blank: current_user.publishing_manager? ? false : Organisation::NONE } end def user_email_tokens(user = current_user) diff --git a/app/models/event_log.rb b/app/models/event_log.rb index 5c32d83af..d764b97c1 100644 --- a/app/models/event_log.rb +++ b/app/models/event_log.rb @@ -48,6 +48,7 @@ class EventLog < ApplicationRecord ACCESS_GRANTS_DELETED = LogEntry.new(id: 43, description: "Access grants deleted", require_uid: true), ACCESS_TOKENS_DELETED = LogEntry.new(id: 44, description: "Access tokens deleted", require_uid: true), ACCOUNT_DELETED = LogEntry.new(id: 45, description: "Account deleted", require_uid: true), + ORGANISATION_CHANGED = LogEntry.new(id: 46, description: "Organisation changed", require_uid: true, require_initiator: true), # We no longer expire passwords, but we keep this event for history purposes PASSWORD_EXPIRED = LogEntry.new(id: 6, description: "Password expired"), @@ -106,6 +107,10 @@ def self.record_role_change(user, previous_role, new_role, initiator) record_event(user, ROLE_CHANGED, initiator:, trailing_message: "from #{previous_role} to #{new_role}") end + def self.record_organisation_change(user, previous_organisation, new_organisation, initiator) + record_event(user, ORGANISATION_CHANGED, initiator:, trailing_message: "from #{previous_organisation} to #{new_organisation}") + end + def self.record_account_invitation(user, initiator) record_event(user, ACCOUNT_INVITED, initiator:) end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index 0d56b6177..045006516 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -1,5 +1,6 @@ class Organisation < ApplicationRecord GDS_ORG_CONTENT_ID = "af07d5a5-df63-4ddc-9383-6a666845ebe9".freeze + NONE = "None".freeze has_ancestry diff --git a/app/services/user_update.rb b/app/services/user_update.rb index c188c1edc..1da5ec0d0 100644 --- a/app/services/user_update.rb +++ b/app/services/user_update.rb @@ -18,6 +18,7 @@ def call record_update record_permission_changes(old_permissions) record_role_change + record_organisation_change record_2sv_exemption_removed record_2sv_mandated send_two_step_mandated_notification @@ -29,6 +30,8 @@ def call private def filtered_user_params + return user_params unless user_params.key?(:supported_permission_ids) + filter = SupportedPermissionParameterFilter.new(current_user, user, user_params) user_params.merge(supported_permission_ids: filter.filtered_supported_permission_ids) end @@ -87,6 +90,18 @@ def record_role_change ) end + def record_organisation_change + organisation_change = user.previous_changes[:organisation_id] + return unless organisation_change + + EventLog.record_organisation_change( + user, + Organisation.find_by(id: organisation_change.first)&.name || Organisation::NONE, + Organisation.find_by(id: organisation_change.last)&.name || Organisation::NONE, + current_user, + ) + end + def record_2sv_exemption_removed return unless user.require_2sv && user.previous_changes[:reason_for_2sv_exemption] diff --git a/test/services/user_update_test.rb b/test/services/user_update_test.rb index 99485c8b6..a36c87a4a 100644 --- a/test/services/user_update_test.rb +++ b/test/services/user_update_test.rb @@ -78,4 +78,61 @@ class UserUpdateTest < ActionView::TestCase assert_equal 1, EventLog.where(event_id: EventLog::TWO_STEP_MANDATED.id).count end + + should "not lose permissions when supported_permissions are absent from params" do + current_user = create(:superadmin_user) + ip_address = "1.2.3.4" + + affected_user = create(:user) + app = create(:application) + affected_user.grant_application_signin_permission(app) + assert affected_user.has_access_to?(app) + + UserUpdate.new(affected_user, {}, current_user, ip_address).call + + assert affected_user.has_access_to?(app) + end + + should "record when organisation has been changed" do + current_user = create(:superadmin_user) + ip_address = "1.2.3.4" + + organisation_1 = create(:organisation, name: "organisation-1") + organisation_2 = create(:organisation, name: "organisation-2") + affected_user = create(:user, organisation: organisation_1) + + params = { organisation_id: organisation_2.id } + UserUpdate.new(affected_user, params, current_user, ip_address).call + + assert_equal 1, EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).count + assert_equal "from organisation-1 to organisation-2", EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).last.trailing_message + end + + should "record when organisation has been changed from 'None'" do + current_user = create(:superadmin_user) + ip_address = "1.2.3.4" + + organisation = create(:organisation, name: "organisation-name") + affected_user = create(:user, organisation: nil) + + params = { organisation_id: organisation.id } + UserUpdate.new(affected_user, params, current_user, ip_address).call + + assert_equal 1, EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).count + assert_equal "from None to organisation-name", EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).last.trailing_message + end + + should "record when organisation has been changed to 'None'" do + current_user = create(:superadmin_user) + ip_address = "1.2.3.4" + + organisation = create(:organisation, name: "organisation-name") + affected_user = create(:user, organisation:) + + params = { organisation_id: nil } + UserUpdate.new(affected_user, params, current_user, ip_address).call + + assert_equal 1, EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).count + assert_equal "from organisation-name to None", EventLog.where(event_id: EventLog::ORGANISATION_CHANGED.id).last.trailing_message + end end