Merge branch 'feat/group-access-token-api' into 'master'

Add new group access token REST API

See merge request gitlab-org/gitlab!77236

app/models/group.rb

@@ -634,11 +634,16 @@ class Group < Namespace
       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),

app/models/project.rb

@@ -1667,6 +1667,7 @@ class Project < ApplicationRecord
       project_members.find_by(user_id: user)
     end
   end
+  alias_method :resource_member, :project_member
 
   def membership_locked?
     false

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

app/services/resource_access_tokens/create_service.rb

@@ -63,7 +63,7 @@ module ResourceAccessTokens
         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

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) |

doc/api/group_access_tokens.md

+---
+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: <your_access_token>" "https://gitlab.example.com/api/v4/groups/<group_id>/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: <your_access_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/<group_id>/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: <your_access_token>" "https://gitlab.example.com/api/v4/groups/<group_id>/access_tokens/<token_id>"
+```
+
+### Responses
+
+- `204: No Content` if successfully revoked.
+- `400 Bad Request` or `404 Not Found` if not revoked successfully.

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

lib/api/resource_access_tokens.rb

@@ -8,7 +8,7 @@ module API
 
     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 @@ module API
 
           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 @@ module API
           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 @@ module API
           ).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

spec/models/group_spec.rb

@@ -2086,6 +2086,23 @@ RSpec.describe Group do
     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) }

spec/policies/group_policy_spec.rb

@@ -975,7 +975,7 @@ RSpec.describe GroupPolicy do
     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 @@ RSpec.describe GroupPolicy do
     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 }

spec/services/resource_access_tokens/create_service_spec.rb

@@ -7,10 +7,10 @@ RSpec.describe ResourceAccessTokens::CreateService do
 
   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 @@ RSpec.describe ResourceAccessTokens::CreateService do
 
         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 @@ RSpec.describe ResourceAccessTokens::CreateService do
         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 @@ RSpec.describe ResourceAccessTokens::CreateService do
                 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 @@ RSpec.describe ResourceAccessTokens::CreateService do
                 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 @@ RSpec.describe ResourceAccessTokens::CreateService do
       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'

spec/services/resource_access_tokens/revoke_service_spec.rb

@@ -6,11 +6,12 @@ RSpec.describe ResourceAccessTokens::RevokeService do
   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 @@ RSpec.describe ResourceAccessTokens::RevokeService do
       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