Merge pull request #2308 from alphagov/revoke-api-user-token-when-sus…

…pending

Revoke all access tokens when suspending a user

app/models/user.rb

@@ -159,6 +159,10 @@ def authorised_applications
   end
   alias_method :applications_used, :authorised_applications
 
+  def revoke_all_authorisations
+    authorisations.where(revoked_at: nil).find_each(&:revoke)
+  end
+
   def grant_application_permission(application, supported_permission_name)
     grant_application_permissions(application, [supported_permission_name]).first
   end

db/migrate/20230816164936_revoke_access_tokens_for_suspended_users.rb

@@ -0,0 +1,8 @@
+class RevokeAccessTokensForSuspendedUsers < ActiveRecord::Migration[7.0]
+  def up
+    User.with_status(User::USER_STATUS_SUSPENDED)
+      .find_each(&:revoke_all_authorisations)
+  end
+
+  def down; end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_08_04_094159) do
+ActiveRecord::Schema[7.0].define(version: 2023_08_16_164936) do
   create_table "batch_invitation_application_permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t|
     t.integer "batch_invitation_id", null: false
     t.integer "supported_permission_id", null: false

lib/devise/models/suspendable.rb

@@ -34,6 +34,7 @@ def suspend(reason)
         GovukStatsd.increment("users.suspend")
         update(reason_for_suspension: reason,
                suspended_at: Time.zone.now)
+        revoke_all_authorisations
       end
       # rubocop:enable Rails/SaveBang
 

lib/inactive_users_suspender.rb

@@ -5,6 +5,8 @@ def suspend
       user.reason_for_suspension = reason
       user.save!(validate: false)
 
+      user.revoke_all_authorisations
+
       PermissionUpdater.perform_on(user)
       ReauthEnforcer.perform_on(user)
 

test/lib/inactive_users_suspender_test.rb

@@ -99,4 +99,14 @@ class InactiveUsersSuspenderTest < ActiveSupport::TestCase
 
     InactiveUsersSuspender.new.suspend
   end
+
+  test "revokes all authorisations (`Doorkeeper::AccessToken`s)" do
+    inactive_user = create(:user, current_sign_in_at: 46.days.ago)
+    create(:access_token, resource_owner_id: inactive_user.id)
+    create(:access_token, resource_owner_id: inactive_user.id)
+
+    InactiveUsersSuspender.new.suspend
+
+    assert inactive_user.authorisations.all?(&:revoked?)
+  end
 end

test/models/user_test.rb

@@ -465,6 +465,41 @@ def setup
     assert_not_empty u.errors[:reason_for_suspension]
   end
 
+  test "suspension revokes all authorisations (`Doorkeeper::AccessToken`s)" do
+    create(:access_token, resource_owner_id: @user.id)
+    create(:access_token, resource_owner_id: @user.id)
+
+    @user.suspend("Nothing personal, just needed to suspend a user for testing")
+
+    assert @user.authorisations.all?(&:revoked?)
+  end
+
+  context "#revoke_all_authorisations" do
+    should "revokes all `Doorkeeper::AccessToken`s" do
+      create(:access_token, resource_owner_id: @user.id)
+      create(:access_token, resource_owner_id: @user.id)
+
+      @user.revoke_all_authorisations
+
+      assert @user.authorisations.all?(&:revoked?)
+    end
+
+    should "skips `Doorkeeper::AccessToken`s that are already revoked" do
+      first_revoked_at = create(:access_token, resource_owner_id: @user.id).tap(&:revoke).revoked_at
+      second_revoked_at = create(:access_token, resource_owner_id: @user.id).tap(&:revoke).revoked_at
+      create(:access_token, resource_owner_id: @user.id)
+      create(:access_token, resource_owner_id: @user.id)
+
+      Timecop.travel(1.day.from_now)
+
+      @user.revoke_all_authorisations
+
+      all_revoked_ats = @user.authorisations.map(&:revoked_at).compact
+      assert_includes all_revoked_ats, first_revoked_at
+      assert_includes all_revoked_ats, second_revoked_at
+    end
+  end
+
   test "organisation admin must belong to an organisation" do
     user = build(:user, role: "organisation_admin", organisation_id: nil)