diff --git a/app/finders/personal_access_tokens_finder.rb b/app/finders/personal_access_tokens_finder.rb index e52c39e6b9692d317017c1b01ec82ad332a35342..88680caa84d8b2216214e5dcb0648c841614c9d8 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 0000000000000000000000000000000000000000..ae1522233bc3582773cced7527c6d20320a8bff5 --- /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 d3ecfe64d5da79c4afd45e9bba1414718d1f21ac..567b089d53875bcf246984f73be901a415f34f46 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