Merge branch '20440-limit-the-api-scope-of-personal-access-tokens' into 'master'

Limit the API scope of Personal Access Tokens

Closes #20440

See merge request gitlab-org/gitlab!28944

changelogs/unreleased/20440-limit-the-api-scope-of-personal-access-tokens.yml

+---
+title: Add read_api scope to personal access tokens for granting read only API access
+merge_request: 28944
+author:
+type: added

config/locales/doorkeeper.en.yml

@@ -70,6 +70,8 @@ en:
     scope_desc:
       api:
         Grants complete read/write access to the API, including all groups and projects, the container registry, and the package registry.
+      read_api:
+        Grants read access to the API, including all groups and projects, the container registry, and the package registry.
       read_user:
         Grants read-only access to the authenticated user's profile through the /user API endpoint, which includes username, public email, and full name. Also grants access to read-only API endpoints under /users.
       read_repository:

doc/user/profile/personal_access_tokens.md

@@ -43,6 +43,7 @@ the following table.
 | ------------------ | ------------- | ----------- |
 | `read_user`        | [GitLab 8.15](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/5951)   | Allows access to the read-only endpoints under `/users`. Essentially, any of the `GET` requests in the [Users API][users] are allowed. |
 | `api`              | [GitLab 8.15](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/5951)   | Grants complete read/write access to the API, including all groups and projects, the container registry, and the package registry. |
+| `read_api`           | [GitLab 12.10](https://https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28944)  | Grants read access to the API, including all groups and projects, the container registry, and the package registry. |
 | `read_registry`    | [GitLab 9.3](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11845)   | Allows to read (pull) [container registry] images if a project is private and authorization is required. |
 | `sudo`             | [GitLab 10.2](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14838)  | Allows performing API actions as any user in the system (if the authenticated user is an admin). |
 | `read_repository`  | [GitLab 10.7](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/17894)  | Allows read-only access (pull) to the repository through `git clone`. |

lib/api/api.rb

@@ -28,6 +28,7 @@ module API
                   ]
 
     allow_access_with_scope :api
+    allow_access_with_scope :read_api, if: -> (request) { request.get? }
     prefix :api
 
     version 'v3', using: :path do

lib/gitlab/auth.rb

@@ -6,7 +6,7 @@ module Gitlab
     IpBlacklisted = Class.new(StandardError)
 
     # Scopes used for GitLab API access
-    API_SCOPES = [:api, :read_user].freeze
+    API_SCOPES = [:api, :read_user, :read_api].freeze
 
     # Scopes used for GitLab Repository access
     REPOSITORY_SCOPES = [:read_repository, :write_repository].freeze
@@ -198,6 +198,7 @@ module Gitlab
       def abilities_for_scopes(scopes)
         abilities_by_scope = {
           api: full_authentication_abilities,
+          read_api: read_only_authentication_abilities,
           read_registry: [:read_container_image],
           read_repository: [:download_code],
           write_repository: [:download_code, :push_code]

spec/lib/gitlab/auth_spec.rb

@@ -8,7 +8,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
   describe 'constants' do
     it 'API_SCOPES contains all scopes for API access' do
-      expect(subject::API_SCOPES).to eq %i[api read_user]
+      expect(subject::API_SCOPES).to eq %i[api read_user read_api]
     end
 
     it 'ADMIN_SCOPES contains all scopes for ADMIN access' do
@@ -30,7 +30,7 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'optional_scopes contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.optional_scopes).to eq %i[read_user read_repository write_repository read_registry sudo openid profile email]
+      expect(subject.optional_scopes).to eq %i[read_user read_api read_repository write_repository read_registry sudo openid profile email]
     end
   end
 
@@ -38,21 +38,21 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'contains all non-default scopes' do
       stub_container_registry_config(enabled: true)
 
-      expect(subject.all_available_scopes).to eq %i[api read_user read_repository write_repository read_registry sudo]
+      expect(subject.all_available_scopes).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
     end
 
     it 'contains for non-admin user all non-default scopes without ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: false)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_repository write_repository read_registry]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry]
     end
 
     it 'contains for admin user all non-default scopes with ADMIN access' do
       stub_container_registry_config(enabled: true)
       user = create(:user, admin: true)
 
-      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_repository write_repository read_registry sudo]
+      expect(subject.available_scopes_for(user)).to eq %i[api read_user read_api read_repository write_repository read_registry sudo]
     end
 
     context 'registry_scopes' do

spec/requests/api/api_spec.rb

@@ -3,15 +3,60 @@
 require 'spec_helper'
 
 describe API::API do
-  let(:user) { create(:user, last_activity_on: Date.yesterday) }
+  include GroupAPIHelpers
 
   describe 'Record user last activity in after hook' do
     # It does not matter which endpoint is used because last_activity_on should
     # be updated on every request. `/groups` is used as an example
     # to represent any API endpoint
+    let(:user) { create(:user, last_activity_on: Date.yesterday) }
 
-    it 'updates the users last_activity_on date' do
+    it 'updates the users last_activity_on to the current date' do
       expect { get api('/groups', user) }.to change { user.reload.last_activity_on }.to(Date.today)
     end
   end
+
+  describe 'User with only read_api scope personal access token' do
+    # It does not matter which endpoint is used because this should behave
+    # in the same way for every request. `/groups` is used as an example
+    # to represent any API endpoint
+
+    context 'when personal access token has only read_api scope' do
+      let_it_be(:user) { create(:user) }
+      let_it_be(:group) { create(:group) }
+      let_it_be(:token) { create(:personal_access_token, user: user, scopes: [:read_api]) }
+
+      before_all do
+        group.add_owner(user)
+      end
+
+      it 'does authorize user for get request' do
+        get api('/groups', personal_access_token: token)
+
+        expect(response).to have_gitlab_http_status(:ok)
+      end
+
+      it 'does not authorize user for post request' do
+        params = attributes_for_group_api
+
+        post api("/groups", personal_access_token: token), params: params
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      it 'does not authorize user for put request' do
+        group_param = { name: 'Test' }
+
+        put api("/groups/#{group.id}", personal_access_token: token), params: group_param
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      it 'does not authorize user for delete request' do
+        delete api("/groups/#{group.id}", personal_access_token: token)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+    end
+  end
 end

spec/requests/openid_connect_spec.rb

@@ -180,7 +180,7 @@ describe 'OpenID Connect requests' do
       expect(response).to have_gitlab_http_status(:ok)
       expect(json_response['issuer']).to eq('http://localhost')
       expect(json_response['jwks_uri']).to eq('http://www.example.com/oauth/discovery/keys')
-      expect(json_response['scopes_supported']).to eq(%w[api read_user read_repository write_repository sudo openid profile email])
+      expect(json_response['scopes_supported']).to eq(%w[api read_user read_api read_repository write_repository sudo openid profile email])
     end
   end