diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index 8fc93e6f954828003e94edca423a0b32009fdf29..ed996589489fc66914666e8ff80e41a983954921 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -24,6 +24,7 @@ class PersonalAccessToken < ApplicationRecord
scope :expiring_and_not_notified, ->(date) { where(["revoked = false AND expire_notification_delivered = false AND expires_at >= CURRENT_DATE AND expires_at <= ?", date]) }
scope :expired_today_and_not_notified, -> { where(["revoked = false AND expires_at = CURRENT_DATE AND after_expiry_notification_delivered = false"]) }
scope :inactive, -> { where("revoked = true OR expires_at < CURRENT_DATE") }
+ scope :created_before, -> (date) { where("personal_access_tokens.created_at < :date", date: date) }
scope :last_used_before, -> (date) { where("personal_access_tokens.created_at < :date AND (last_used_at < :date OR last_used_at IS NULL)", date: date) }
scope :with_impersonation, -> { where(impersonation: true) }
scope :without_impersonation, -> { where(impersonation: false) }
diff --git a/lib/gitlab/cleanup/unused_personal_access_tokens.rb b/lib/gitlab/cleanup/personal_access_tokens.rb
similarity index 59%
rename from lib/gitlab/cleanup/unused_personal_access_tokens.rb
rename to lib/gitlab/cleanup/personal_access_tokens.rb
index 97bf808a0ca930d7557981476d283182edd10b03..88665e6ada53161a09c50be6e021cec2a49dd9e9 100644
--- a/lib/gitlab/cleanup/unused_personal_access_tokens.rb
+++ b/lib/gitlab/cleanup/personal_access_tokens.rb
@@ -2,31 +2,23 @@
module Gitlab
module Cleanup
- # Unused active Personal Access Tokens pose a risk to organizations
- # in that they may have been, or may be, leaked to unauthorized
- # individuals. They are likely providing little / no current value
- # because they are not actively being used, and should therefore be
- # proactively revoked.
- class UnusedPersonalAccessTokens
- # By default tokens that haven't been used for over 1 year will
- # be revoked
+ class PersonalAccessTokens
+ # By default tokens that haven't been used for over 1 year will be revoked
DEFAULT_TIME_PERIOD = 1.year
- # To prevent inadvertently revoking actively used tokens, we
- # provide a minimum time
+ # To prevent inadvertently revoking all tokens, we provide a minimum time
MINIMUM_TIME_PERIOD = 1.day
- attr_reader :logger, :last_used_before, :revocation_time, :group
+ attr_reader :logger, :cut_off_date, :revocation_time, :group
- def initialize(last_used_before: DEFAULT_TIME_PERIOD.ago.beginning_of_day, logger: nil, group_full_path:)
- # binding.pry
- # Ensure last_used_before is a Time and far enough in the past
- @last_used_before = last_used_before
+ def initialize(cut_off_date: DEFAULT_TIME_PERIOD.ago.beginning_of_day, logger: nil, group_full_path:)
+ @cut_off_date = cut_off_date
# rubocop: disable CodeReuse/ActiveRecord
@group = Group.find_by_full_path(group_full_path)
# rubocop: enable CodeReuse/ActiveRecord
+
raise "Group with full_path #{group_full_path} not found" unless @group
- raise "Invalid time: #{@last_used_before}" unless @last_used_before <= MINIMUM_TIME_PERIOD.ago
+ raise "Invalid time: #{@cut_off_date}" unless @cut_off_date <= MINIMUM_TIME_PERIOD.ago
# Use a static revocation time to make correlation of revoked
# tokens easier, should it be needed.
@@ -36,27 +28,21 @@ def initialize(last_used_before: DEFAULT_TIME_PERIOD.ago.beginning_of_day, logge
raise "Invalid logger: #{@logger}" unless @logger.respond_to?(:info) && @logger.respond_to?(:warn)
end
- # Revokes unused personal access tokens.
- # A dry run is performed by default, logging what would be
- # revoked. Pass `dry_run: false` explicitly to revoke tokens.
- def run!(dry_run: true)
+ def run!(dry_run: true, revoke_active_tokens: false)
# rubocop:disable Rails/Output
if dry_run
puts "Dry running. No changes will be made"
+ elsif revoke_active_tokens
+ puts "Revoking used and unused access tokens created before #{cut_off_date}..."
else
- puts "Revoking access tokens from before #{last_used_before}..."
+ puts "Revoking access tokens last used and created before #{cut_off_date}..."
end
# rubocop:enable Rails/Output
- logger.info(
- dry_run: dry_run,
- group_full_path: group.full_path,
- message: "Looking for Personal Access Tokens " \
- "last used before #{last_used_before}..."
- )
+ tokens_to_revoke = revocable_tokens(revoke_active_tokens)
# rubocop:disable Cop/InBatches
- revocable_tokens.in_batches do |access_tokens|
+ tokens_to_revoke.in_batches do |access_tokens|
revoke_batch(access_tokens, dry_run)
end
# rubocop:enable Cop/InBatches
@@ -64,12 +50,20 @@ def run!(dry_run: true)
private
- def revocable_tokens
- PersonalAccessToken
- .active
- .owner_is_human
- .last_used_before(last_used_before)
- .for_users(group.users)
+ def revocable_tokens(revoke_active_tokens)
+ if revoke_active_tokens
+ PersonalAccessToken
+ .active
+ .owner_is_human
+ .created_before(cut_off_date)
+ .for_users(group.users)
+ else
+ PersonalAccessToken
+ .active
+ .owner_is_human
+ .last_used_before(cut_off_date)
+ .for_users(group.users)
+ end
end
def revoke_batch(access_tokens, dry_run)
diff --git a/spec/lib/gitlab/cleanup/unused_personal_access_tokens_spec.rb b/spec/lib/gitlab/cleanup/personal_access_tokens_spec.rb
similarity index 67%
rename from spec/lib/gitlab/cleanup/unused_personal_access_tokens_spec.rb
rename to spec/lib/gitlab/cleanup/personal_access_tokens_spec.rb
index eb83ced0e21166a2935444fbf53ef7d604d5cf4e..36c5d0e9b0c718f25f0d135ec28ddc7be8efd3f7 100644
--- a/spec/lib/gitlab/cleanup/unused_personal_access_tokens_spec.rb
+++ b/spec/lib/gitlab/cleanup/personal_access_tokens_spec.rb
@@ -2,7 +2,7 @@
require 'spec_helper'
-RSpec.describe Gitlab::Cleanup::UnusedPersonalAccessTokens do
+RSpec.describe Gitlab::Cleanup::PersonalAccessTokens do
let_it_be(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project_bot) { create(:user, :project_bot) }
@@ -16,6 +16,10 @@
create(:personal_access_token, created_at: last_used_at - 1.minute)
end
+ let!(:old_actively_used_token) do
+ create(:personal_access_token, created_at: last_used_at - 1.minute, last_used_at: 1.day.ago)
+ end
+
let!(:old_unused_token_for_non_group_member) do
create(:personal_access_token, created_at: last_used_at - 1.minute)
end
@@ -37,6 +41,7 @@
before do
group.add_member(old_formerly_used_token.user, Gitlab::Access::DEVELOPER)
+ group.add_member(old_actively_used_token.user, Gitlab::Access::DEVELOPER)
group.add_member(unused_token.user, Gitlab::Access::DEVELOPER)
group.add_member(old_unused_token.user, Gitlab::Access::DEVELOPER)
group.add_member(project_bot, Gitlab::Access::MAINTAINER)
@@ -47,7 +52,7 @@
subject do
described_class.new(
logger: logger,
- last_used_before: last_used_at,
+ cut_off_date: last_used_at,
group_full_path: group_full_path
)
end
@@ -76,16 +81,42 @@
context 'in a real run' do
let(:args) { { dry_run: false } }
- it 'revokes only revocable tokens' do
- subject.run!(**args)
+ context 'when revoking unused tokens' do
+ it 'revokes human-owned tokens created and last used over 1 year ago' do
+ subject.run!(**args)
+
+ expect(PersonalAccessToken.active).to contain_exactly(
+ unused_token,
+ old_actively_used_token,
+ old_unused_project_access_token,
+ old_unused_token_for_non_group_member,
+ old_unused_token_for_subgroup_member
+ )
+ expect(PersonalAccessToken.revoked).to contain_exactly(
+ old_unused_token,
+ old_formerly_used_token
+ )
+ end
+ end
+
+ context 'when revoking used and unused tokens' do
+ let(:args) { { dry_run: false, revoke_active_tokens: true } }
+
+ it 'revokes human-owned tokens created over 1 year ago' do
+ subject.run!(**args)
- expect(PersonalAccessToken.active).to contain_exactly(
- unused_token,
- old_unused_project_access_token,
- old_unused_token_for_non_group_member,
- old_unused_token_for_subgroup_member
- )
- expect(PersonalAccessToken.revoked).to contain_exactly(old_unused_token, old_formerly_used_token)
+ expect(PersonalAccessToken.active).to contain_exactly(
+ unused_token,
+ old_unused_project_access_token,
+ old_unused_token_for_non_group_member,
+ old_unused_token_for_subgroup_member
+ )
+ expect(PersonalAccessToken.revoked).to contain_exactly(
+ old_unused_token,
+ old_actively_used_token,
+ old_formerly_used_token
+ )
+ end
end
it 'updates updated_at' do
diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb
index 9a2a35c08391b4e29e9d3b218678d5bdcc7a6780..68230e81c50acc50b54be70a977a6c1996a46623 100644
--- a/spec/models/personal_access_token_spec.rb
+++ b/spec/models/personal_access_token_spec.rb
@@ -65,6 +65,46 @@
end
end
+ describe '.created_before' do
+ let(:last_used_at) { 1.month.ago.beginning_of_hour }
+ let!(:new_used_token) do
+ create(:personal_access_token,
+ created_at: last_used_at + 1.minute,
+ last_used_at: last_used_at + 1.minute
+ )
+ end
+
+ let!(:old_unused_token) do
+ create(:personal_access_token,
+ created_at: last_used_at - 1.minute
+ )
+ end
+
+ let!(:old_formerly_used_token) do
+ create(:personal_access_token,
+ created_at: last_used_at - 1.minute,
+ last_used_at: last_used_at - 1.minute
+ )
+ end
+
+ let!(:old_still_used_token) do
+ create(:personal_access_token,
+ created_at: last_used_at - 1.minute,
+ last_used_at: 1.minute.ago
+ )
+ end
+
+ subject { described_class.created_before(last_used_at) }
+
+ it do
+ is_expected.to contain_exactly(
+ old_unused_token,
+ old_formerly_used_token,
+ old_still_used_token
+ )
+ end
+ end
+
describe '.last_used_before' do
let(:last_used_at) { 1.month.ago.beginning_of_hour }
let!(:unused_token) { create(:personal_access_token) }