diff --git a/app/services/project_access_tokens/rotate_service.rb b/app/services/resource_access_tokens/rotate_service.rb
similarity index 56%
rename from app/services/project_access_tokens/rotate_service.rb
rename to app/services/resource_access_tokens/rotate_service.rb
index 63d8d2a82cc69d57b717d0979dc4c47f78b3738c..a55ddcb795f4a34f43788db21cb6fc0c5aaee2f7 100644
--- a/app/services/project_access_tokens/rotate_service.rb
+++ b/app/services/resource_access_tokens/rotate_service.rb
@@ -1,20 +1,20 @@
# frozen_string_literal: true
-module ProjectAccessTokens
+module ResourceAccessTokens
class RotateService < ::PersonalAccessTokens::RotateService
extend ::Gitlab::Utils::Override
def initialize(current_user, token, resource = nil)
@current_user = current_user
@token = token
- @project = resource
+ @resource = resource
end
def execute(params = {})
super
end
- attr_reader :project
+ attr_reader :resource
private
@@ -44,15 +44,34 @@ def update_bot_membership(target_user, expires_at)
end
def valid_access_level?
- return true if current_user.can_admin_all_resources?
- return false unless current_user.can?(:manage_resource_access_tokens, project)
+ return true if admin_all_resources?
+ return false unless can_manage_tokens?
- token_access_level = project.team.max_member_access(token.user.id).to_i
- current_user_access_level = project.team.max_member_access(current_user.id).to_i
+ token_access_level <= current_user_access_level
+ end
+
+ def admin_all_resources?
+ current_user.can_admin_all_resources?
+ end
+
+ def can_manage_tokens?
+ current_user.can?(:manage_resource_access_tokens, resource)
+ end
- return true if token_access_level.to_i <= current_user_access_level
+ def token_access_level
+ if resource.is_a? Project
+ resource.team.max_member_access(token.user.id).to_i
+ else
+ resource.max_member_access_for_user(token.user).to_i
+ end
+ end
- false
+ def current_user_access_level
+ if resource.is_a? Project
+ resource.team.max_member_access(current_user.id).to_i
+ else
+ resource.max_member_access_for_user(current_user).to_i
+ end
end
end
end
diff --git a/lib/api/resource_access_tokens.rb b/lib/api/resource_access_tokens.rb
index 1e1b5d77cfdab4a2f0f54903df065299dfbc6ce0..7a477c6fed559546c2f0003bad027c8b26638de1 100644
--- a/lib/api/resource_access_tokens.rb
+++ b/lib/api/resource_access_tokens.rb
@@ -153,13 +153,8 @@ class ResourceAccessTokens < ::API::Base
token = find_token(resource, params[:token_id]) if resource_accessible
if token
- response = if source_type == "project"
- ::ProjectAccessTokens::RotateService.new(current_user, token, resource)
+ response = ::ResourceAccessTokens::RotateService.new(current_user, token, resource)
.execute(declared_params)
- else
- ::PersonalAccessTokens::RotateService.new(current_user, token)
- .execute(declared_params)
- end
if response.success?
status :ok
diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb
index 70d665383758b07366d2d9569dc29dcdcc8fe59c..7497add0adc6662cb21c674f91955299c4656039 100644
--- a/spec/requests/api/resource_access_tokens_spec.rb
+++ b/spec/requests/api/resource_access_tokens_spec.rb
@@ -575,21 +575,14 @@
context 'when service raises an error' do
let(:error_message) { 'boom!' }
- let(:personal_token_service) { PersonalAccessTokens::RotateService }
- let(:project_token_service) { ProjectAccessTokens::RotateService }
+ let(:resource_token_service) { ResourceAccessTokens::RotateService }
before do
resource.add_maintainer(project_bot)
resource.add_owner(user)
- if source_type == 'project'
- allow_next_instance_of(project_token_service) do |service|
- allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
- end
- else
- allow_next_instance_of(personal_token_service) do |service|
- allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
- end
+ allow_next_instance_of(resource_token_service) do |service|
+ allow(service).to receive(:execute).and_return(ServiceResponse.error(message: error_message))
end
end
diff --git a/spec/services/project_access_tokens/rotate_service_spec.rb b/spec/services/project_access_tokens/rotate_service_spec.rb
deleted file mode 100644
index 10e29be4979f109ffdb89f6d99af89edd32a8cb6..0000000000000000000000000000000000000000
--- a/spec/services/project_access_tokens/rotate_service_spec.rb
+++ /dev/null
@@ -1,189 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-RSpec.describe ProjectAccessTokens::RotateService, feature_category: :system_access do
- describe '#execute' do
- let_it_be(:token, reload: true) { create(:personal_access_token) }
- let(:current_user) { create(:user) }
- let(:project) { create(:project, group: create(:group)) }
- let(:error_message) { 'Not eligible to rotate token with access level higher than the user' }
-
- subject(:response) { described_class.new(current_user, token, project).execute }
-
- shared_examples_for 'rotates token succesfully' do
- it "rotates user's own token", :freeze_time do
- expect(response).to be_success
-
- new_token = response.payload[:personal_access_token]
-
- expect(new_token.token).not_to eq(token.token)
- expect(new_token.expires_at).to eq(Date.today + 1.week)
- expect(new_token.user).to eq(token.user)
- end
- end
-
- context 'when user tries to rotate token with different access level' do
- before do
- project.add_guest(token.user)
- end
-
- context 'when current user is an owner' do
- before do
- project.add_owner(current_user)
- end
-
- it_behaves_like "rotates token succesfully"
-
- context 'when creating the new token fails' do
- let(:error_message) { 'boom!' }
-
- before do
- allow_next_instance_of(PersonalAccessToken) do |token|
- allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message)
- allow(token).to receive_message_chain(:errors, :clear)
- allow(token).to receive_message_chain(:errors, :empty?).and_return(false)
- end
- end
-
- it 'returns an error' do
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
-
- it 'reverts the changes' do
- expect { response }.not_to change { token.reload.revoked? }.from(false)
- end
- end
- end
-
- context 'when current user is not an owner' do
- context 'when current user is maintainer' do
- before do
- project.add_maintainer(current_user)
- end
-
- context 'when access level is not owner' do
- it_behaves_like "rotates token succesfully"
- end
-
- context 'when access level is owner' do
- before do
- project.add_owner(token.user)
- end
-
- it "does not rotate token with higher priviledge" do
- response
-
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
- end
- end
-
- context 'when current user is not maintainer' do
- before do
- project.add_developer(current_user)
- end
-
- it 'does not rotate the token' do
- response
-
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
- end
- end
-
- context 'when current user is admin' do
- let(:current_user) { create(:admin) }
-
- context 'when admin mode enabled', :enable_admin_mode do
- it_behaves_like "rotates token succesfully"
- end
-
- context 'when admin mode not enabled' do
- it 'does not rotate the token' do
- response
-
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
- end
- end
-
- context 'when nested membership' do
- let_it_be(:project_bot) { create(:user, :project_bot) }
- let(:token) { create(:personal_access_token, user: project_bot) }
- let(:top_level_group) { create(:group) }
- let(:sub_group) { create(:group, parent: top_level_group) }
- let(:project) { create(:project, group: sub_group) }
-
- before do
- project.add_maintainer(project_bot)
- end
-
- context 'when current user is an owner' do
- before do
- project.add_owner(current_user)
- end
-
- it_behaves_like "rotates token succesfully"
-
- context 'when its a bot user' do
- let_it_be(:bot_user) { create(:user, :project_bot) }
- let_it_be(:bot_user_membership) do
- create(:project_member, :developer, user: bot_user, project: create(:project))
- end
-
- let_it_be(:token, reload: true) { create(:personal_access_token, user: bot_user) }
-
- it 'updates membership expires at' do
- response
-
- new_token = response.payload[:personal_access_token]
- expect(bot_user_membership.reload.expires_at).to eq(new_token.expires_at)
- end
- end
- end
-
- context 'when current user is not an owner' do
- context 'when current user is maintainer' do
- before do
- project.add_maintainer(current_user)
- end
-
- context 'when access level is not owner' do
- it_behaves_like "rotates token succesfully"
- end
-
- context 'when access level is owner' do
- before do
- project.add_owner(token.user)
- end
-
- it "does not rotate token with higher priviledge" do
- response
-
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
- end
- end
-
- context 'when current user is not maintainer' do
- before do
- project.add_developer(current_user)
- end
-
- it 'does not rotate the token' do
- response
-
- expect(response).to be_error
- expect(response.message).to eq(error_message)
- end
- end
- end
- end
- end
- end
-end
diff --git a/spec/services/resource_access_tokens/rotate_service_spec.rb b/spec/services/resource_access_tokens/rotate_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..67ec8816c9c5caeb70b161a273168113d9b3b679
--- /dev/null
+++ b/spec/services/resource_access_tokens/rotate_service_spec.rb
@@ -0,0 +1,157 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ResourceAccessTokens::RotateService, feature_category: :system_access do
+ shared_examples_for 'rotates token successfully' do
+ it "rotates user's own token", :freeze_time do
+ expect(response).to be_success
+
+ new_token = response.payload[:personal_access_token]
+
+ expect(new_token.token).not_to eq(token.token)
+ expect(new_token.expires_at).to eq(Date.today + 1.week)
+ expect(new_token.user).to eq(token.user)
+ end
+
+ it 'updates membership expires_at' do
+ response
+
+ new_token = response.payload[:personal_access_token]
+ expect(bot_user.members.first.reload.expires_at).to eq(new_token.expires_at)
+ end
+ end
+
+ shared_examples 'token rotation access level check' do |source_type|
+ before do
+ resource.add_guest(token.user)
+ end
+
+ context 'when current user is an owner' do
+ before do
+ resource.add_owner(current_user)
+ end
+
+ it_behaves_like "rotates token successfully"
+
+ context 'when creating the new token fails' do
+ let(:error_message) { 'boom!' }
+
+ before do
+ allow_next_instance_of(PersonalAccessToken) do |token|
+ allow(token).to receive_message_chain(:errors, :full_messages, :to_sentence).and_return(error_message)
+ allow(token).to receive_message_chain(:errors, :clear)
+ allow(token).to receive_message_chain(:errors, :empty?).and_return(false)
+ end
+ end
+
+ it 'returns an error' do
+ expect(response).to be_error
+ expect(response.message).to eq(error_message)
+ end
+
+ it 'reverts the changes' do
+ expect { response }.not_to change { token.reload.revoked? }.from(false)
+ end
+ end
+ end
+
+ context 'when current user is maintainer' do
+ before do
+ resource.add_maintainer(current_user)
+ end
+
+ context 'and token user is not owner' do
+ if source_type == 'project'
+ it_behaves_like "rotates token successfully"
+ elsif source_type == 'group'
+ it 'cannot rotate the token' do
+ response
+
+ expect(response).to be_error
+ expect(response.message).to eq(error_message)
+ end
+ end
+ end
+
+ context 'and token user is owner' do
+ before do
+ resource.add_owner(token.user)
+ end
+
+ it "cannot rotate token with higher privilege" do
+ response
+
+ expect(response).to be_error
+ expect(response.message).to eq(error_message)
+ end
+ end
+ end
+
+ context 'when current user is neither owner or maintainer' do
+ before do
+ resource.add_developer(current_user)
+ end
+
+ it 'cannot rotate the token' do
+ response
+
+ expect(response).to be_error
+ expect(response.message).to eq(error_message)
+ end
+ end
+
+ context 'when current user is admin' do
+ let(:current_user) { create(:admin) }
+
+ context 'with admin mode', :enable_admin_mode do
+ it_behaves_like "rotates token successfully"
+ end
+
+ context 'without admin mode' do
+ it 'cannot rotate the token' do
+ response
+
+ expect(response).to be_error
+ expect(response.message).to eq(error_message)
+ end
+ end
+ end
+ end
+
+ describe '#execute' do
+ subject(:response) { described_class.new(current_user, token, resource).execute }
+
+ let(:current_user) { create(:user) }
+ let(:error_message) { 'Not eligible to rotate token with access level higher than the user' }
+ let(:bot_user) { create(:user, :project_bot) }
+ let(:token) { create(:personal_access_token, user: bot_user) }
+
+ context 'for project' do
+ let_it_be(:resource) { create(:project, group: create(:group)) }
+
+ it_behaves_like 'token rotation access level check', 'project'
+
+ context 'with a nested membership' do
+ let(:top_level_group) { create(:group) }
+ let(:sub_group) { create(:group, parent: top_level_group) }
+ let(:resource) { create(:project, group: sub_group) }
+
+ it_behaves_like 'token rotation access level check', 'project'
+ end
+ end
+
+ context 'for group' do
+ let(:resource) { create(:group) }
+
+ it_behaves_like 'token rotation access level check', 'group'
+
+ context 'with a nested membership' do
+ let(:top_level_group) { create(:group) }
+ let(:resource) { create(:group, parent: top_level_group) }
+
+ it_behaves_like 'token rotation access level check', 'group'
+ end
+ end
+ end
+end