From cd7c9984de74c1d8001275e0875a371f7a5c97bd Mon Sep 17 00:00:00 2001
From: Aboobacker MK <akarakath@gitlab.com>
Date: Thu, 12 Sep 2024 11:33:38 +0000
Subject: [PATCH] Add support for organization based filter for PAT finder
Since PAT is now scoped to the organization, it is important to have PAT
to be filtered by organization_id.
---
app/finders/personal_access_tokens_finder.rb | 8 +++
.../pat_organization_filter.yml | 9 +++
.../personal_access_tokens_finder_spec.rb | 65 +++++++++++++++----
3 files changed, 70 insertions(+), 12 deletions(-)
create mode 100644 config/feature_flags/gitlab_com_derisk/pat_organization_filter.yml
diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb
index e52c39e6b9692..88680caa84d8b 100644
--- a/app/finders/personal_access_tokens_finder.rb
+++ b/app/finders/personal_access_tokens_finder.rb
@@ -24,6 +24,7 @@ def execute
tokens = by_last_used_before(tokens)
tokens = by_last_used_after(tokens)
tokens = by_search(tokens)
+ tokens = by_organization(tokens)
tokens = tokens.allow_cross_joins_across_databases(url: "https://gitlab.com/gitlab-org/gitlab/-/issues/436657")
sort(tokens)
@@ -129,4 +130,11 @@ def by_search(tokens)
tokens.search(params[:search])
end
+
+ def by_organization(tokens)
+ return tokens unless params[:organization]
+ return tokens unless Feature.enabled?('pat_organization_filter', current_user)
+
+ tokens.for_organization(params[:organization])
+ end
end
diff --git a/config/feature_flags/gitlab_com_derisk/pat_organization_filter.yml b/config/feature_flags/gitlab_com_derisk/pat_organization_filter.yml
new file mode 100644
index 0000000000000..ae1522233bc35
--- /dev/null
+++ b/config/feature_flags/gitlab_com_derisk/pat_organization_filter.yml
@@ -0,0 +1,9 @@
+---
+name: pat_organization_filter
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/486037
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165620
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/486988
+milestone: '17.4'
+group: group::authentication
+type: gitlab_com_derisk
+default_enabled: false
diff --git a/spec/finders/personal_access_tokens_finder_spec.rb b/spec/finders/personal_access_tokens_finder_spec.rb
index d3ecfe64d5da7..567b089d53875 100644
--- a/spec/finders/personal_access_tokens_finder_spec.rb
+++ b/spec/finders/personal_access_tokens_finder_spec.rb
@@ -6,30 +6,43 @@
using RSpec::Parameterized::TableSyntax
describe '#execute' do
- let_it_be(:admin) { create(:admin) }
- let_it_be(:user) { create(:user) }
+ let_it_be(:organization) { create(:organization) }
+ let_it_be(:admin) { create(:admin, organizations: [organization]) }
+ let_it_be(:user) { create(:user, organizations: [organization]) }
let_it_be(:other_user) { create(:user) }
let_it_be(:project_bot) { create(:user, :project_bot) }
+ let_it_be(:first_organization) { create(:organization) }
+ let_it_be(:second_organization) { create(:organization) }
let_it_be(:tokens) do
{
- active: create(:personal_access_token, user: user, name: 'my_pat_1'),
- active_other: create(:personal_access_token, user: other_user, name: 'my_pat_2'),
- expired: create(:personal_access_token, :expired, user: user),
- revoked: create(:personal_access_token, :revoked, user: user),
- active_impersonation: create(:personal_access_token, :impersonation, user: user),
- expired_impersonation: create(:personal_access_token, :expired, :impersonation, user: user),
- revoked_impersonation: create(:personal_access_token, :revoked, :impersonation, user: user),
- bot: create(:personal_access_token, user: project_bot)
+ active: create(:personal_access_token, user: user, name: 'my_pat_1', organization: organization),
+ active_other: create(:personal_access_token, user: other_user, name: 'my_pat_2', organization: organization),
+ expired: create(:personal_access_token, :expired, user: user, organization: organization),
+ revoked: create(:personal_access_token, :revoked, user: user, organization: organization),
+ active_impersonation: create(:personal_access_token, :impersonation, user: user, organization: organization),
+ expired_impersonation: create(:personal_access_token, :expired, :impersonation, user: user, organization: organization),
+ revoked_impersonation: create(:personal_access_token, :revoked, :impersonation, user: user, organization: organization),
+ bot: create(:personal_access_token, user: project_bot, organization: organization)
}
end
+ let_it_be(:tokens_from_other_organizations) do
+ {
+ with_first_organization: create(:personal_access_token, organization: first_organization),
+ with_second_organization: create(:personal_access_token, organization: second_organization)
+ }
+ end
+
+ let_it_be(:all_tokens) { tokens.merge(tokens_from_other_organizations) }
+
let(:tokens_keys) { tokens.keys }
+ let(:default_params) { { organization: organization } }
let(:params) { {} }
let(:current_user) { admin }
- subject { described_class.new(params, current_user).execute }
+ subject { described_class.new(default_params.merge(params), current_user).execute }
describe 'by current user' do
context 'with no user' do
@@ -283,6 +296,34 @@
end
end
+ describe 'by_organization' do
+ let(:params) { { organization: first_organization } }
+
+ it 'returns tokens by organization' do
+ is_expected.to match_array(PersonalAccessToken.where(organization: first_organization))
+ end
+
+ context 'when orgnzation is not specified' do
+ let(:params) { { organization: nil } }
+
+ it 'returns empty when organization is not specified' do
+ is_expected.to match_array(all_tokens.values)
+ end
+ end
+
+ context 'when the feature flag pat_organization_filter is disabled' do
+ before do
+ stub_feature_flags(pat_organization_filter: false)
+ end
+
+ let(:params) { { organization: first_organization } }
+
+ it 'returns tokens by organization' do
+ is_expected.to match_array(all_tokens.values)
+ end
+ end
+ end
+
describe 'sort' do
where(:sort, :expected_tokens) do
nil | ref(:tokens_keys)
@@ -301,7 +342,7 @@
end
describe 'delegates' do
- subject { described_class.new(params, current_user) }
+ subject { described_class.new(default_params.merge(params), current_user) }
describe '#find_by_id' do
it 'returns token by id' do
--
GitLab