diff --git a/app/models/group.rb b/app/models/group.rb index b18da3591e03102573ec6132b03cbbbcf68aade0..fb9e6bcee790097a746ef3b6f22bbf96e66c0033 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -628,11 +628,16 @@ def group_member(user) group_members.find_by(user_id: user) end end + alias_method :resource_member, :group_member def highest_group_member(user) GroupMember.where(source_id: self_and_ancestors_ids, user_id: user.id).order(:access_level).last end + def bots + users.project_bot + end + def related_group_ids [id, *ancestors.pluck(:id), diff --git a/app/models/project.rb b/app/models/project.rb index 0b33f28f82cf94cc80a53b81cd3cc0298e26e0b2..5c4ffd083043ca53df10b383975011527355e0f2 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1667,6 +1667,7 @@ def project_member(user) project_members.find_by(user_id: user) end end + alias_method :resource_member, :project_member def membership_locked? false diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 5c4990ffd9bd41acfab232bd4714893782d2f3bf..391da5b8a81b7d3f0eaaa31b43671dd2b1337c79 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -23,6 +23,9 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy condition(:parent_share_with_group_locked, scope: :subject) { @subject.parent&.share_with_group_lock? } condition(:can_change_parent_share_with_group_lock) { can?(:change_share_with_group_lock, @subject.parent) } + desc "User is a project bot" + condition(:project_bot) { user.project_bot? && access_level >= GroupMember::GUEST } + condition(:has_projects) do group_projects_for(user: @user, group: @subject).any? end @@ -250,6 +253,8 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :admin_dependency_proxy end + rule { project_bot }.enable :project_bot_access + rule { can?(:admin_group) & resource_access_token_feature_available }.policy do enable :read_resource_access_tokens enable :destroy_resource_access_tokens @@ -260,6 +265,10 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :create_resource_access_tokens end + rule { can?(:project_bot_access) }.policy do + prevent :create_resource_access_tokens + end + rule { support_bot & has_project_with_service_desk_enabled }.policy do enable :read_label end diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index e0371e5d80f6410bee9d39af2d9e36cc0b56ef85..af0feef4eaa9a7763edf55704a18b62c674886ac 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -63,7 +63,7 @@ def default_user_params name: params[:name] || "#{resource.name.to_s.humanize} bot", email: generate_email, username: generate_username, - user_type: "#{resource_type}_bot".to_sym, + user_type: :project_bot, skip_confirmation: true # Bot users should always have their emails confirmed. } end diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index b5fac5019d9a2b28055232918dad3bfa69f4c464..783823f80fbe24c49054f6e92e6fc032cda75107 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -25,7 +25,7 @@ The following API resources are available in the project context: | Resource | Available endpoints | |:------------------------------------------------------------------------|:--------------------| | [Access requests](access_requests.md) | `/projects/:id/access_requests` (also available for groups) | -| [Access tokens](resource_access_tokens.md) | `/projects/:id/access_tokens` | +| [Access tokens](resource_access_tokens.md) | `/projects/:id/access_tokens` (also available for groups) | | [Award emoji](award_emoji.md) | `/projects/:id/issues/.../award_emoji`, `/projects/:id/merge_requests/.../award_emoji`, `/projects/:id/snippets/.../award_emoji` | | [Branches](branches.md) | `/projects/:id/repository/branches/`, `/projects/:id/repository/merged_branches` | | [Commits](commits.md) | `/projects/:id/repository/commits`, `/projects/:id/statuses` | @@ -100,6 +100,7 @@ The following API resources are available in the group context: | Resource | Available endpoints | |:-----------------------------------------------------------------|:--------------------| | [Access requests](access_requests.md) | `/groups/:id/access_requests/` (also available for projects) | +| [Access tokens](group_access_tokens.md) | `/groups/:id/access_tokens` (also available for projects) | | [Custom attributes](custom_attributes.md) | `/groups/:id/custom_attributes` (also available for projects and users) | | [Debian distributions](packages/debian_group_distributions.md) | `/groups/:id/-/packages/debian` (also available for projects) | | [Deploy tokens](deploy_tokens.md) | `/groups/:id/deploy_tokens` (also available for projects and standalone) | diff --git a/doc/api/group_access_tokens.md b/doc/api/group_access_tokens.md new file mode 100644 index 0000000000000000000000000000000000000000..71c6828de496368095e24aa37837da8fd33b197e --- /dev/null +++ b/doc/api/group_access_tokens.md @@ -0,0 +1,112 @@ +--- +stage: Manage +group: Authentication & Authorization +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Group access tokens API **(FREE)** + +You can read more about [group access tokens](../user/project/settings/project_access_tokens.md#group-access-tokens). + +## List group access tokens + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77236) in GitLab 14.7. + +Get a list of [group access tokens](../user/project/settings/project_access_tokens.md#group-access-tokens). + +```plaintext +GET groups/:id/access_tokens +``` + +| Attribute | Type | required | Description | +|-----------|---------|----------|---------------------| +| `id` | integer or string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) | + +```shell +curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups//access_tokens" +``` + +```json +[ + { + "user_id" : 141, + "scopes" : [ + "api" + ], + "name" : "token", + "expires_at" : "2021-01-31", + "id" : 42, + "active" : true, + "created_at" : "2021-01-20T22:11:48.151Z", + "revoked" : false, + "access_level": 40 + } +] +``` + +## Create a group access token + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77236) in GitLab 14.7. + +Create a [group access token](../user/project/settings/project_access_tokens.md#group-access-tokens). + +```plaintext +POST groups/:id/access_tokens +``` + +| Attribute | Type | required | Description | +|-----------|---------|----------|---------------------| +| `id` | integer or string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) | +| `name` | String | yes | The name of the group access token | +| `scopes` | `Array[String]` | yes | [List of scopes](../user/project/settings/project_access_tokens.md#scopes-for-a-project-access-token) | +| `access_level` | Integer | no | A valid access level. Default value is 40 (Maintainer). Other allowed values are 10 (Guest), 20 (Reporter), and 30 (Developer). | +| `expires_at` | Date | no | The token expires at midnight UTC on that date | + +```shell +curl --request POST --header "PRIVATE-TOKEN: " \ +--header "Content-Type:application/json" \ +--data '{ "name":"test_token", "scopes":["api", "read_repository"], "expires_at":"2021-01-31", "access_level": 30 }' \ +"https://gitlab.example.com/api/v4/groups//access_tokens" +``` + +```json +{ + "scopes" : [ + "api", + "read_repository" + ], + "active" : true, + "name" : "test", + "revoked" : false, + "created_at" : "2021-01-21T19:35:37.921Z", + "user_id" : 166, + "id" : 58, + "expires_at" : "2021-01-31", + "token" : "D4y...Wzr", + "access_level": 30 +} +``` + +## Revoke a group access token + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77236) in GitLab 14.7. + +Revoke a [group access token](../user/project/settings/project_access_tokens.md#group-access-tokens). + +```plaintext +DELETE groups/:id/access_tokens/:token_id +``` + +| Attribute | Type | required | Description | +|-----------|---------|----------|---------------------| +| `id` | integer or string | yes | The ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) | +| `token_id` | integer or string | yes | The ID of the group access token | + +```shell +curl --request DELETE --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/groups//access_tokens/" +``` + +### Responses + +- `204: No Content` if successfully revoked. +- `400 Bad Request` or `404 Not Found` if not revoked successfully. diff --git a/lib/api/entities/resource_access_token.rb b/lib/api/entities/resource_access_token.rb index a1c7b28af45436da4eb8394da7c568be7bffdd0c..d16baed38f0507358553ac6fc2ad1a09f5186970 100644 --- a/lib/api/entities/resource_access_token.rb +++ b/lib/api/entities/resource_access_token.rb @@ -4,7 +4,7 @@ module API module Entities class ResourceAccessToken < Entities::PersonalAccessToken expose :access_level do |token, options| - options[:project].project_member(token.user).access_level + options[:resource].resource_member(token.user).access_level end end end diff --git a/lib/api/resource_access_tokens.rb b/lib/api/resource_access_tokens.rb index f42acc6b2ebcf5d5ceeec3976b963c01f0fdf21e..e52f8fd911166c813121ffdd2d79491c06c77a0a 100644 --- a/lib/api/resource_access_tokens.rb +++ b/lib/api/resource_access_tokens.rb @@ -8,7 +8,7 @@ class ResourceAccessTokens < ::API::Base feature_category :authentication_and_authorization - %w[project].each do |source_type| + %w[project group].each do |source_type| resource source_type.pluralize, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do desc 'Get list of all access tokens for the specified resource' do detail 'This feature was introduced in GitLab 13.9.' @@ -23,8 +23,8 @@ class ResourceAccessTokens < ::API::Base tokens = PersonalAccessTokensFinder.new({ user: resource.bots, impersonation: false }).execute.preload_users - resource.project_members.load - present paginate(tokens), with: Entities::ResourceAccessToken, project: resource + resource.members.load + present paginate(tokens), with: Entities::ResourceAccessToken, resource: resource end desc 'Revoke a resource access token' do @@ -58,7 +58,7 @@ class ResourceAccessTokens < ::API::Base requires :id, type: String, desc: "The #{source_type} ID" requires :name, type: String, desc: "Resource access token name" requires :scopes, type: Array[String], desc: "The permissions of the token" - optional :access_level, type: Integer, desc: "The access level of the token in the project" + optional :access_level, type: Integer, desc: "The access level of the token in the #{source_type}" optional :expires_at, type: Date, desc: "The expiration date of the token" end post ':id/access_tokens' do @@ -71,7 +71,7 @@ class ResourceAccessTokens < ::API::Base ).execute if token_response.success? - present token_response.payload[:access_token], with: Entities::ResourceAccessTokenWithToken, project: resource + present token_response.payload[:access_token], with: Entities::ResourceAccessTokenWithToken, resource: resource else bad_request!(token_response.message) end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 27aa9192579df457b517af3581f1444020a3a400..f1a752fa434cfa0785e67033801ba2dd8f99f071 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -2066,6 +2066,23 @@ def setup_group_members(group) end end + describe '#bots' do + subject { group.bots } + + let_it_be(:group) { create(:group) } + let_it_be(:project_bot) { create(:user, :project_bot) } + let_it_be(:user) { create(:user) } + + before_all do + [project_bot, user].each do |member| + group.add_maintainer(member) + end + end + + it { is_expected.to contain_exactly(project_bot) } + it { is_expected.not_to include(user) } + end + describe '#related_group_ids' do let(:nested_group) { create(:group, parent: group) } let(:shared_with_group) { create(:group, parent: group) } diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 7822ee2b92e27756453c474ecadf6aa219b61fa6..7cd8d514c85d438f244c9d951861f116c8ccd3ce 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -975,7 +975,7 @@ it { expect_disallowed(:read_label) } context 'when group hierarchy has a project with service desk enabled' do - let_it_be(:subgroup) { create(:group, :private, parent: group)} + let_it_be(:subgroup) { create(:group, :private, parent: group) } let_it_be(:project) { create(:project, group: subgroup, service_desk_enabled: true) } it { expect_allowed(:read_label) } @@ -983,6 +983,49 @@ end end + context "project bots" do + let(:project_bot) { create(:user, :project_bot) } + let(:user) { create(:user) } + + context "project_bot_access" do + context "when regular user and part of the group" do + let(:current_user) { user } + + before do + group.add_developer(user) + end + + it { is_expected.not_to be_allowed(:project_bot_access) } + end + + context "when project bot and not part of the project" do + let(:current_user) { project_bot } + + it { is_expected.not_to be_allowed(:project_bot_access) } + end + + context "when project bot and part of the project" do + let(:current_user) { project_bot } + + before do + group.add_developer(project_bot) + end + + it { is_expected.to be_allowed(:project_bot_access) } + end + end + + context 'with resource access tokens' do + let(:current_user) { project_bot } + + before do + group.add_maintainer(project_bot) + end + + it { is_expected.not_to be_allowed(:create_resource_access_tokens) } + end + end + describe 'update_runners_registration_token' do context 'admin' do let(:current_user) { admin } diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index 23061ab4bf0336cb7b37990619997d9d8f0c25e7..7e3e682767f3aa9bc44cc3b40ab1b56de58cb1a5 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -3,25 +3,27 @@ require "spec_helper" RSpec.describe API::ResourceAccessTokens do - context "when the resource is a project" do - let_it_be(:project) { create(:project) } - let_it_be(:other_project) { create(:project) } - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:user_non_priviledged) { create(:user) } - describe "GET projects/:id/access_tokens" do - subject(:get_tokens) { get api("/projects/#{project_id}/access_tokens", user) } + shared_examples 'resource access token API' do |source_type| + context "GET #{source_type}s/:id/access_tokens" do + subject(:get_tokens) { get api("/#{source_type}s/#{resource_id}/access_tokens", user) } - context "when the user has maintainer permissions" do + context "when the user has valid permissions" do let_it_be(:project_bot) { create(:user, :project_bot) } let_it_be(:access_tokens) { create_list(:personal_access_token, 3, user: project_bot) } - let_it_be(:project_id) { project.id } + let_it_be(:resource_id) { resource.id } before do - project.add_maintainer(user) - project.add_maintainer(project_bot) + if source_type == 'project' + resource.add_maintainer(project_bot) + else + resource.add_owner(project_bot) + end end - it "gets a list of access tokens for the specified project" do + it "gets a list of access tokens for the specified #{source_type}" do get_tokens token_ids = json_response.map { |token| token['id'] } @@ -38,16 +40,22 @@ expect(api_get_token["name"]).to eq(token.name) expect(api_get_token["scopes"]).to eq(token.scopes) - expect(api_get_token["access_level"]).to eq(project.team.max_member_access(token.user.id)) + + if source_type == 'project' + expect(api_get_token["access_level"]).to eq(resource.team.max_member_access(token.user.id)) + else + expect(api_get_token["access_level"]).to eq(resource.max_member_access_for_user(token.user)) + end + expect(api_get_token["expires_at"]).to eq(token.expires_at.to_date.iso8601) expect(api_get_token).not_to have_key('token') end - context "when using a project access token to GET other project access tokens" do + context "when using a #{source_type} access token to GET other #{source_type} access tokens" do let_it_be(:token) { access_tokens.first } - it "gets a list of access tokens for the specified project" do - get api("/projects/#{project_id}/access_tokens", personal_access_token: token) + it "gets a list of access tokens for the specified #{source_type}" do + get api("/#{source_type}s/#{resource_id}/access_tokens", personal_access_token: token) token_ids = json_response.map { |token| token['id'] } @@ -56,16 +64,15 @@ end end - context "when tokens belong to a different project" do + context "when tokens belong to a different #{source_type}" do let_it_be(:bot) { create(:user, :project_bot) } let_it_be(:token) { create(:personal_access_token, user: bot) } before do - other_project.add_maintainer(bot) - other_project.add_maintainer(user) + other_resource.add_maintainer(bot) end - it "does not return tokens from a different project" do + it "does not return tokens from a different #{source_type}" do get_tokens token_ids = json_response.map { |token| token['id'] } @@ -74,12 +81,8 @@ end end - context "when the project has no access tokens" do - let(:project_id) { other_project.id } - - before do - other_project.add_maintainer(user) - end + context "when the #{source_type} has no access tokens" do + let(:resource_id) { other_resource.id } it 'returns an empty array' do get_tokens @@ -89,8 +92,8 @@ end end - context "when trying to get the tokens of a different project" do - let_it_be(:project_id) { other_project.id } + context "when trying to get the tokens of a different #{source_type}" do + let_it_be(:resource_id) { unknown_resource.id } it "returns 404" do get_tokens @@ -99,8 +102,8 @@ end end - context "when the project does not exist" do - let(:project_id) { non_existing_record_id } + context "when the #{source_type} does not exist" do + let(:resource_id) { non_existing_record_id } it "returns 404" do get_tokens @@ -111,13 +114,13 @@ end context "when the user does not have valid permissions" do + let_it_be(:user) { user_non_priviledged } let_it_be(:project_bot) { create(:user, :project_bot) } let_it_be(:access_tokens) { create_list(:personal_access_token, 3, user: project_bot) } - let_it_be(:project_id) { project.id } + let_it_be(:resource_id) { resource.id } before do - project.add_developer(user) - project.add_maintainer(project_bot) + resource.add_maintainer(project_bot) end it "returns 401" do @@ -128,40 +131,36 @@ end end - describe "DELETE projects/:id/access_tokens/:token_id", :sidekiq_inline do - subject(:delete_token) { delete api("/projects/#{project_id}/access_tokens/#{token_id}", user) } + context "DELETE #{source_type}s/:id/access_tokens/:token_id", :sidekiq_inline do + subject(:delete_token) { delete api("/#{source_type}s/#{resource_id}/access_tokens/#{token_id}", user) } let_it_be(:project_bot) { create(:user, :project_bot) } let_it_be(:token) { create(:personal_access_token, user: project_bot) } - let_it_be(:project_id) { project.id } + let_it_be(:resource_id) { resource.id } let_it_be(:token_id) { token.id } before do - project.add_maintainer(project_bot) + resource.add_maintainer(project_bot) end - context "when the user has maintainer permissions" do - before do - project.add_maintainer(user) - end - - it "deletes the project access token from the project" do + context "when the user has valid permissions" do + it "deletes the #{source_type} access token from the #{source_type}" do delete_token expect(response).to have_gitlab_http_status(:no_content) expect(User.exists?(project_bot.id)).to be_falsy end - context "when using project access token to DELETE other project access token" do + context "when using #{source_type} access token to DELETE other #{source_type} access token" do let_it_be(:other_project_bot) { create(:user, :project_bot) } let_it_be(:other_token) { create(:personal_access_token, user: other_project_bot) } let_it_be(:token_id) { other_token.id } before do - project.add_maintainer(other_project_bot) + resource.add_maintainer(other_project_bot) end - it "deletes the project access token from the project" do + it "deletes the #{source_type} access token from the #{source_type}" do delete_token expect(response).to have_gitlab_http_status(:no_content) @@ -169,37 +168,31 @@ end end - context "when attempting to delete a non-existent project access token" do + context "when attempting to delete a non-existent #{source_type} access token" do let_it_be(:token_id) { non_existing_record_id } it "does not delete the token, and returns 404" do delete_token expect(response).to have_gitlab_http_status(:not_found) - expect(response.body).to include("Could not find project access token with token_id: #{token_id}") + expect(response.body).to include("Could not find #{source_type} access token with token_id: #{token_id}") end end - context "when attempting to delete a token that does not belong to the specified project" do - let_it_be(:project_id) { other_project.id } - - before do - other_project.add_maintainer(user) - end + context "when attempting to delete a token that does not belong to the specified #{source_type}" do + let_it_be(:resource_id) { other_resource.id } it "does not delete the token, and returns 404" do delete_token expect(response).to have_gitlab_http_status(:not_found) - expect(response.body).to include("Could not find project access token with token_id: #{token_id}") + expect(response.body).to include("Could not find #{source_type} access token with token_id: #{token_id}") end end end context "when the user does not have valid permissions" do - before do - project.add_developer(user) - end + let_it_be(:user) { user_non_priviledged } it "does not delete the token, and returns 400", :aggregate_failures do delete_token @@ -211,23 +204,19 @@ end end - describe "POST projects/:id/access_tokens" do + context "POST #{source_type}s/:id/access_tokens" do let(:params) { { name: "test", scopes: ["api"], expires_at: expires_at, access_level: access_level } } let(:expires_at) { 1.month.from_now } let(:access_level) { 20 } - subject(:create_token) { post api("/projects/#{project_id}/access_tokens", user), params: params } + subject(:create_token) { post api("/#{source_type}s/#{resource_id}/access_tokens", user), params: params } - context "when the user has maintainer permissions" do - let_it_be(:project_id) { project.id } - - before do - project.add_maintainer(user) - end + context "when the user has valid permissions" do + let_it_be(:resource_id) { resource.id } context "with valid params" do context "with full params" do - it "creates a project access token with the params", :aggregate_failures do + it "creates a #{source_type} access token with the params", :aggregate_failures do create_token expect(response).to have_gitlab_http_status(:created) @@ -242,7 +231,7 @@ context "when 'expires_at' is not set" do let(:expires_at) { nil } - it "creates a project access token with the params", :aggregate_failures do + it "creates a #{source_type} access token with the params", :aggregate_failures do create_token expect(response).to have_gitlab_http_status(:created) @@ -255,7 +244,7 @@ context "when 'access_level' is not set" do let(:access_level) { nil } - it 'creates a project access token with the default access level', :aggregate_failures do + it "creates a #{source_type} access token with the default access level", :aggregate_failures do create_token expect(response).to have_gitlab_http_status(:created) @@ -272,7 +261,7 @@ context "when missing the 'name' param" do let_it_be(:params) { { scopes: ["api"], expires_at: 5.days.from_now } } - it "does not create a project access token without 'name'" do + it "does not create a #{source_type} access token without 'name'" do create_token expect(response).to have_gitlab_http_status(:bad_request) @@ -283,7 +272,7 @@ context "when missing the 'scopes' param" do let_it_be(:params) { { name: "test", expires_at: 5.days.from_now } } - it "does not create a project access token without 'scopes'" do + it "does not create a #{source_type} access token without 'scopes'" do create_token expect(response).to have_gitlab_http_status(:bad_request) @@ -292,50 +281,80 @@ end end - context "when trying to create a token in a different project" do - let_it_be(:project_id) { other_project.id } + context "when trying to create a token in a different #{source_type}" do + let_it_be(:resource_id) { unknown_resource.id } - it "does not create the token, and returns the project not found error" do + it "does not create the token, and returns the #{source_type} not found error" do create_token expect(response).to have_gitlab_http_status(:not_found) - expect(response.body).to include("Project Not Found") + expect(response.body).to include("#{source_type.capitalize} Not Found") end end end context "when the user does not have valid permissions" do - let_it_be(:project_id) { project.id } + let_it_be(:resource_id) { resource.id } - context "when the user is a developer" do - before do - project.add_developer(user) - end + context "when the user role is too low" do + let_it_be(:user) { user_non_priviledged } it "does not create the token, and returns the permission error" do create_token expect(response).to have_gitlab_http_status(:bad_request) - expect(response.body).to include("User does not have permission to create project access token") + expect(response.body).to include("User does not have permission to create #{source_type} access token") end end - context "when a project access token tries to create another project access token" do + context "when a #{source_type} access token tries to create another #{source_type} access token" do let_it_be(:project_bot) { create(:user, :project_bot) } let_it_be(:user) { project_bot } before do - project.add_maintainer(user) + if source_type == 'project' + resource.add_maintainer(project_bot) + else + resource.add_owner(project_bot) + end end - it "does not allow a project access token to create another project access token" do + it "does not allow a #{source_type} access token to create another #{source_type} access token" do create_token expect(response).to have_gitlab_http_status(:bad_request) - expect(response.body).to include("User does not have permission to create project access token") + expect(response.body).to include("User does not have permission to create #{source_type} access token") end end end end end + + context 'when the resource is a project' do + let_it_be(:resource) { create(:project) } + let_it_be(:other_resource) { create(:project) } + let_it_be(:unknown_resource) { create(:project) } + + before_all do + resource.add_maintainer(user) + other_resource.add_maintainer(user) + resource.add_developer(user_non_priviledged) + end + + it_behaves_like 'resource access token API', 'project' + end + + context 'when the resource is a group' do + let_it_be(:resource) { create(:group) } + let_it_be(:other_resource) { create(:group) } + let_it_be(:unknown_resource) { create(:project) } + + before_all do + resource.add_owner(user) + other_resource.add_owner(user) + resource.add_maintainer(user_non_priviledged) + end + + it_behaves_like 'resource access token API', 'group' + end end diff --git a/spec/services/resource_access_tokens/create_service_spec.rb b/spec/services/resource_access_tokens/create_service_spec.rb index 42520ea26b240363238337e173dc078c807bcb93..218bff7ed0453886ace26dd75c816859c7a5d080 100644 --- a/spec/services/resource_access_tokens/create_service_spec.rb +++ b/spec/services/resource_access_tokens/create_service_spec.rb @@ -7,10 +7,10 @@ let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :private) } + let_it_be(:group) { create(:group, :private) } let_it_be(:params) { {} } describe '#execute' do - # Created shared_examples as it will easy to include specs for group bots in https://gitlab.com/gitlab-org/gitlab/-/issues/214046 shared_examples 'token creation fails' do let(:resource) { create(:project)} @@ -31,7 +31,7 @@ access_token = response.payload[:access_token] - expect(access_token.user.reload.user_type).to eq("#{resource_type}_bot") + expect(access_token.user.reload.user_type).to eq("project_bot") expect(access_token.user.created_by_id).to eq(user.id) end @@ -112,10 +112,8 @@ end context 'when user is external' do - let(:user) { create(:user, :external) } - before do - project.add_maintainer(user) + user.update!(external: true) end it 'creates resource bot user with external status' do @@ -162,7 +160,7 @@ access_token = response.payload[:access_token] project_bot = access_token.user - expect(project.members.find_by(user_id: project_bot.id).expires_at).to eq(nil) + expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(nil) end end end @@ -183,7 +181,7 @@ access_token = response.payload[:access_token] project_bot = access_token.user - expect(project.members.find_by(user_id: project_bot.id).expires_at).to eq(params[:expires_at]) + expect(resource.members.find_by(user_id: project_bot.id).expires_at).to eq(params[:expires_at]) end end end @@ -234,24 +232,41 @@ end end + shared_examples 'when user does not have permission to create a resource bot' do + it_behaves_like 'token creation fails' + + it 'returns the permission error message' do + response = subject + + expect(response.error?).to be true + expect(response.errors).to include("User does not have permission to create #{resource_type} access token") + end + end + context 'when resource is a project' do let_it_be(:resource_type) { 'project' } let_it_be(:resource) { project } - context 'when user does not have permission to create a resource bot' do - it_behaves_like 'token creation fails' + it_behaves_like 'when user does not have permission to create a resource bot' - it 'returns the permission error message' do - response = subject - - expect(response.error?).to be true - expect(response.errors).to include("User does not have permission to create #{resource_type} access token") + context 'user with valid permission' do + before_all do + resource.add_maintainer(user) end + + it_behaves_like 'allows creation of bot with valid params' end + end + + context 'when resource is a project' do + let_it_be(:resource_type) { 'group' } + let_it_be(:resource) { group } + + it_behaves_like 'when user does not have permission to create a resource bot' context 'user with valid permission' do before_all do - resource.add_maintainer(user) + resource.add_owner(user) end it_behaves_like 'allows creation of bot with valid params' diff --git a/spec/services/resource_access_tokens/revoke_service_spec.rb b/spec/services/resource_access_tokens/revoke_service_spec.rb index 4f4e2ab0c9935e325451850970b4e14887e73276..3d724a79feffa877ac8f5f76bd49685bb3761a0f 100644 --- a/spec/services/resource_access_tokens/revoke_service_spec.rb +++ b/spec/services/resource_access_tokens/revoke_service_spec.rb @@ -6,11 +6,12 @@ subject { described_class.new(user, resource, access_token).execute } let_it_be(:user) { create(:user) } + let_it_be(:user_non_priviledged) { create(:user) } + let_it_be(:resource_bot) { create(:user, :project_bot) } let(:access_token) { create(:personal_access_token, user: resource_bot) } describe '#execute', :sidekiq_inline do - # Created shared_examples as it will easy to include specs for group bots in https://gitlab.com/gitlab-org/gitlab/-/issues/214046 shared_examples 'revokes access token' do it { expect(subject.success?).to be true } @@ -79,71 +80,80 @@ end end - context 'when resource is a project' do - let_it_be(:resource) { create(:project, :private) } + shared_examples 'revoke fails' do |resource_type| + let_it_be(:other_user) { create(:user) } - let(:resource_bot) { create(:user, :project_bot) } + context "when access token does not belong to this #{resource_type}" do + it 'does not find the bot' do + other_access_token = create(:personal_access_token, user: other_user) - before do - resource.add_maintainer(user) - resource.add_maintainer(resource_bot) - end + response = described_class.new(user, resource, other_access_token).execute - it_behaves_like 'revokes access token' + expect(response.success?).to be false + expect(response.message).to eq("Failed to find bot user") + expect(access_token.reload.revoked?).to be false + end + end - context 'revoke fails' do - let_it_be(:other_user) { create(:user) } + context 'when user does not have permission to destroy bot' do + context "when non-#{resource_type} member tries to delete project bot" do + it 'does not allow other user to delete bot' do + response = described_class.new(other_user, resource, access_token).execute - context 'when access token does not belong to this project' do - it 'does not find the bot' do - other_access_token = create(:personal_access_token, user: other_user) + expect(response.success?).to be false + expect(response.message).to eq("#{other_user.name} cannot delete #{access_token.user.name}") + expect(access_token.reload.revoked?).to be false + end + end - response = described_class.new(user, resource, other_access_token).execute + context "when non-priviledged #{resource_type} member tries to delete project bot" do + it 'does not allow developer to delete bot' do + response = described_class.new(user_non_priviledged, resource, access_token).execute expect(response.success?).to be false - expect(response.message).to eq("Failed to find bot user") + expect(response.message).to eq("#{user_non_priviledged.name} cannot delete #{access_token.user.name}") expect(access_token.reload.revoked?).to be false end end + end - context 'when user does not have permission to destroy bot' do - context 'when non-project member tries to delete project bot' do - it 'does not allow other user to delete bot' do - response = described_class.new(other_user, resource, access_token).execute - - expect(response.success?).to be false - expect(response.message).to eq("#{other_user.name} cannot delete #{access_token.user.name}") - expect(access_token.reload.revoked?).to be false - end + context 'when deletion of bot user fails' do + before do + allow_next_instance_of(::ResourceAccessTokens::RevokeService) do |service| + allow(service).to receive(:execute).and_return(false) end + end + + it_behaves_like 'rollback revoke steps' + end + end - context 'when non-maintainer project member tries to delete project bot' do - let(:developer) { create(:user) } + context 'when resource is a project' do + let_it_be(:resource) { create(:project, :private) } - before do - resource.add_developer(developer) - end + before do + resource.add_maintainer(user) + resource.add_developer(user_non_priviledged) + resource.add_maintainer(resource_bot) + end - it 'does not allow developer to delete bot' do - response = described_class.new(developer, resource, access_token).execute + it_behaves_like 'revokes access token' - expect(response.success?).to be false - expect(response.message).to eq("#{developer.name} cannot delete #{access_token.user.name}") - expect(access_token.reload.revoked?).to be false - end - end - end + it_behaves_like 'revoke fails', 'project' + end - context 'when deletion of bot user fails' do - before do - allow_next_instance_of(::ResourceAccessTokens::RevokeService) do |service| - allow(service).to receive(:execute).and_return(false) - end - end + context 'when resource is a group' do + let_it_be(:resource) { create(:group, :private) } - it_behaves_like 'rollback revoke steps' - end + before do + resource.add_owner(user) + resource.add_maintainer(user_non_priviledged) + resource.add_maintainer(resource_bot) end + + it_behaves_like 'revokes access token' + + it_behaves_like 'revoke fails', 'group' end end end