diff options
author | James Lopez <james@gitlab.com> | 2019-04-15 13:05:55 +0000 |
---|---|---|
committer | James Lopez <james@gitlab.com> | 2019-04-15 13:05:55 +0000 |
commit | b15a78dfdd497848a0fd763185e79b0361da1dbb (patch) | |
tree | f97e1d6d289af84b5838db149a9208ae0f70b267 | |
parent | 922fae29ca100e7f7f30fcb62541305994430779 (diff) | |
parent | 0aa56d895dba21d3a01b78d35c445107e224ed0c (diff) | |
download | gitlab-ce-b15a78dfdd497848a0fd763185e79b0361da1dbb.tar.gz |
Merge branch 'feature/add_write_repository_scope' into 'master'
Added write_repository scope for personal access token
Closes #58847 and #51709
See merge request gitlab-org/gitlab-ce!26021
-rw-r--r-- | app/controllers/admin/impersonation_tokens_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/jwt_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/profiles/personal_access_tokens_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 2 | ||||
-rw-r--r-- | app/models/personal_access_token.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/26021-added-write_repository-scope-for-personal-access-token.yml | 5 | ||||
-rw-r--r-- | config/locales/doorkeeper.en.yml | 5 | ||||
-rw-r--r-- | doc/user/profile/personal_access_tokens.md | 5 | ||||
-rw-r--r-- | lib/gitlab/auth.rb | 40 | ||||
-rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 75 | ||||
-rw-r--r-- | spec/requests/git_http_spec.rb | 51 | ||||
-rw-r--r-- | spec/requests/jwt_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/openid_connect_spec.rb | 2 |
13 files changed, 152 insertions, 43 deletions
diff --git a/app/controllers/admin/impersonation_tokens_controller.rb b/app/controllers/admin/impersonation_tokens_controller.rb index cfe29d734b7..c35619a944e 100644 --- a/app/controllers/admin/impersonation_tokens_controller.rb +++ b/app/controllers/admin/impersonation_tokens_controller.rb @@ -49,7 +49,7 @@ class Admin::ImpersonationTokensController < Admin::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def set_index_vars - @scopes = Gitlab::Auth.available_scopes(current_user) + @scopes = Gitlab::Auth.available_scopes_for(current_user) @impersonation_token ||= finder.build @inactive_impersonation_tokens = finder(state: 'inactive').execute diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index f9008a5b67e..49d21456f8e 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -22,7 +22,7 @@ class JwtController < ApplicationController private def authenticate_project_or_user - @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_authentication_abilities) + @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_only_authentication_abilities) authenticate_with_http_basic do |login, password| @authentication_result = Gitlab::Auth.find_for_git_client(login, password, project: nil, ip: request.ip) diff --git a/app/controllers/profiles/personal_access_tokens_controller.rb b/app/controllers/profiles/personal_access_tokens_controller.rb index 213d900a563..f1c07cd9a1d 100644 --- a/app/controllers/profiles/personal_access_tokens_controller.rb +++ b/app/controllers/profiles/personal_access_tokens_controller.rb @@ -42,7 +42,7 @@ class Profiles::PersonalAccessTokensController < Profiles::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def set_index_vars - @scopes = Gitlab::Auth.available_scopes(current_user) + @scopes = Gitlab::Auth.available_scopes_for(current_user) @inactive_personal_access_tokens = finder(state: 'inactive').execute @active_personal_access_tokens = finder(state: 'active').execute.order(:expires_at) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 85628dd32d8..7a80da53025 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -83,7 +83,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController def render_missing_personal_access_token render plain: "HTTP Basic: Access denied\n" \ - "You must use a personal access token with 'api' scope for Git over HTTP.\n" \ + "You must use a personal access token with 'read_repository' or 'write_repository' scope for Git over HTTP.\n" \ "You can generate one at #{profile_personal_access_tokens_url}", status: :unauthorized end diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index d5770387872..f69f0e2dccb 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -56,7 +56,7 @@ class PersonalAccessToken < ApplicationRecord protected def validate_scopes - unless revoked || scopes.all? { |scope| Gitlab::Auth.available_scopes.include?(scope.to_sym) } + unless revoked || scopes.all? { |scope| Gitlab::Auth.all_available_scopes.include?(scope.to_sym) } errors.add :scopes, "can only contain available scopes" end end diff --git a/changelogs/unreleased/26021-added-write_repository-scope-for-personal-access-token.yml b/changelogs/unreleased/26021-added-write_repository-scope-for-personal-access-token.yml new file mode 100644 index 00000000000..da550d35f12 --- /dev/null +++ b/changelogs/unreleased/26021-added-write_repository-scope-for-personal-access-token.yml @@ -0,0 +1,5 @@ +--- +title: Added write_repository scope for personal access token +merge_request: 26021 +author: Horatiu Eugen Vlad +type: added diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index a2dff92908e..23bbc9f4035 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -60,7 +60,8 @@ en: scopes: api: Access the authenticated user's API read_user: Read the authenticated user's personal information - read_repository: Allows read-access to the repository + read_repository: Allows read-only access to the repository + write_repository: Allows read-write access to the repository read_registry: Grants permission to read container registry images openid: Authenticate using OpenID Connect sudo: Perform API actions as any user in the system @@ -73,6 +74,8 @@ en: 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: Grants read-only access to repositories on private projects using Git-over-HTTP (not using the API). + write_repository: + Grants read-write access to repositories on private projects using Git-over-HTTP (not using the API). read_registry: Grants read-only access to container registry images on private projects. openid: diff --git a/doc/user/profile/personal_access_tokens.md b/doc/user/profile/personal_access_tokens.md index 52b4a72e688..4085f3b678c 100644 --- a/doc/user/profile/personal_access_tokens.md +++ b/doc/user/profile/personal_access_tokens.md @@ -40,10 +40,11 @@ the following table. | Scope | Description | | ----- | ----------- | |`read_user` | Allows access to the read-only endpoints under `/users`. Essentially, any of the `GET` requests in the [Users API][users] are allowed ([introduced][ce-5951] in GitLab 8.15). | -| `api` | Grants complete access to the API and Container Registry (read/write) ([introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951) in GitLab 8.15). Required for accessing Git repositories over HTTP when 2FA is enabled. | +| `api` | Grants complete access to the API and Container Registry (read/write) ([introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5951) in GitLab 8.15). | | `read_registry` | Allows to read (pull) [container registry] images if a project is private and authorization is required ([introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11845) in GitLab 9.3). | | `sudo` | Allows performing API actions as any user in the system (if the authenticated user is an admin) ([introduced][ce-14838] in GitLab 10.2). | -| `read_repository` | Allows read-access (pull) to the repository through git clone. | +| `read_repository` | Allows read-only access (pull) to the repository through git clone. | +| `write_repository` | Allows read-write access (pull, push) to the repository through git clone. Required for accessing Git repositories over HTTP when 2FA is enabled. | [2fa]: ../account/two_factor_authentication.md [api]: ../../api/README.md diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index b2ef04d23d7..4317992d933 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -4,10 +4,17 @@ module Gitlab module Auth MissingPersonalAccessTokenError = Class.new(StandardError) + # Scopes used for GitLab API access + API_SCOPES = [:api, :read_user].freeze + + # Scopes used for GitLab Repository access + REPOSITORY_SCOPES = [:read_repository, :write_repository].freeze + + # Scopes used for GitLab Docker Registry access REGISTRY_SCOPES = [:read_registry].freeze - # Scopes used for GitLab API access - API_SCOPES = [:api, :read_user, :sudo, :read_repository].freeze + # Scopes used for GitLab as admin + ADMIN_SCOPES = [:sudo].freeze # Scopes used for OpenID Connect OPENID_SCOPES = [:openid].freeze @@ -159,7 +166,7 @@ module Gitlab token = PersonalAccessTokensFinder.new(state: 'active').find_by_token(password) - if token && valid_scoped_token?(token, available_scopes) + if token && valid_scoped_token?(token, all_available_scopes) Gitlab::Auth::Result.new(token.user, nil, :personal_access_token, abilities_for_scopes(token.scopes)) end end @@ -176,7 +183,8 @@ module Gitlab abilities_by_scope = { api: full_authentication_abilities, read_registry: [:read_container_image], - read_repository: [:download_code] + read_repository: [:download_code], + write_repository: [:download_code, :push_code] } scopes.flat_map do |scope| @@ -196,7 +204,7 @@ module Gitlab scopes = abilities_for_scopes(token.scopes) - if valid_scoped_token?(token, available_scopes) + if valid_scoped_token?(token, all_available_scopes) Gitlab::Auth::Result.new(token, token.project, :deploy_token, scopes) end end @@ -222,7 +230,7 @@ module Gitlab elsif token_handler.deploy_key_pushable?(project) read_write_authentication_abilities else - read_authentication_abilities + read_only_authentication_abilities end if token_handler.token_valid?(encoded_token) @@ -258,7 +266,7 @@ module Gitlab ] end - def read_authentication_abilities + def read_only_authentication_abilities [ :read_project, :download_code, @@ -267,7 +275,7 @@ module Gitlab end def read_write_authentication_abilities - read_authentication_abilities + [ + read_only_authentication_abilities + [ :push_code, :create_container_image ] @@ -279,15 +287,19 @@ module Gitlab ] end - def available_scopes(current_user = nil) - scopes = API_SCOPES + registry_scopes - scopes.delete(:sudo) if current_user && !current_user.admin? + def available_scopes_for(current_user) + scopes = non_admin_available_scopes + scopes += ADMIN_SCOPES if current_user.admin? scopes end + def all_available_scopes + non_admin_available_scopes + ADMIN_SCOPES + end + # Other available scopes def optional_scopes - available_scopes + OPENID_SCOPES + PROFILE_SCOPES - DEFAULT_SCOPES + all_available_scopes + OPENID_SCOPES + PROFILE_SCOPES - DEFAULT_SCOPES end def registry_scopes @@ -298,6 +310,10 @@ module Gitlab private + def non_admin_available_scopes + API_SCOPES + REPOSITORY_SCOPES + registry_scopes + end + def find_build_by_token(token) ::Ci::Build.running.find_by_token(token) end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index a4a6338961e..3b5ca7c950c 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -5,7 +5,15 @@ describe Gitlab::Auth do describe 'constants' do it 'API_SCOPES contains all scopes for API access' do - expect(subject::API_SCOPES).to eq %i[api read_user sudo read_repository] + expect(subject::API_SCOPES).to eq %i[api read_user] + end + + it 'ADMIN_SCOPES contains all scopes for ADMIN access' do + expect(subject::ADMIN_SCOPES).to eq %i[sudo] + end + + it 'REPOSITORY_SCOPES contains all scopes for REPOSITORY access' do + expect(subject::REPOSITORY_SCOPES).to eq %i[read_repository write_repository] end it 'OPENID_SCOPES contains all scopes for OpenID Connect' do @@ -19,7 +27,29 @@ describe Gitlab::Auth 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 sudo read_repository read_registry openid profile email] + expect(subject.optional_scopes).to eq %i[read_user read_repository write_repository read_registry sudo openid profile email] + end + end + + context 'available_scopes' 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] + 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] + 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] end context 'registry_scopes' do @@ -122,7 +152,7 @@ describe Gitlab::Auth do token = Gitlab::LfsToken.new(key).token expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") - expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) + expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_only_authentication_abilities)) end it 'does not try password auth before oauth' do @@ -150,7 +180,7 @@ describe Gitlab::Auth do token = Gitlab::LfsToken.new(key).token expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") - expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) + expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_only_authentication_abilities)) end end @@ -182,8 +212,19 @@ describe Gitlab::Auth do it 'succeeds for personal access tokens with the `api` scope' do personal_access_token = create(:personal_access_token, scopes: ['api']) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') - expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, full_authentication_abilities)) + expect_results_with_abilities(personal_access_token, full_authentication_abilities) + end + + it 'succeeds for personal access tokens with the `read_repository` scope' do + personal_access_token = create(:personal_access_token, scopes: ['read_repository']) + + expect_results_with_abilities(personal_access_token, [:download_code]) + end + + it 'succeeds for personal access tokens with the `write_repository` scope' do + personal_access_token = create(:personal_access_token, scopes: ['write_repository']) + + expect_results_with_abilities(personal_access_token, [:download_code, :push_code]) end context 'when registry is enabled' do @@ -194,28 +235,24 @@ describe Gitlab::Auth do it 'succeeds for personal access tokens with the `read_registry` scope' do personal_access_token = create(:personal_access_token, scopes: ['read_registry']) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') - expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [:read_container_image])) + expect_results_with_abilities(personal_access_token, [:read_container_image]) end end it 'succeeds if it is an impersonation token' do impersonation_token = create(:personal_access_token, :impersonation, scopes: ['api']) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') - expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(impersonation_token.user, nil, :personal_access_token, full_authentication_abilities)) + expect_results_with_abilities(impersonation_token, full_authentication_abilities) end it 'limits abilities based on scope' do personal_access_token = create(:personal_access_token, scopes: %w[read_user sudo]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: '') - expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(personal_access_token.user, nil, :personal_access_token, [])) + expect_results_with_abilities(personal_access_token, []) end it 'fails if password is nil' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '') - expect(gl_auth.find_for_git_client('', nil, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) + expect_results_with_abilities(nil, nil, false) end end @@ -479,7 +516,7 @@ describe Gitlab::Auth do ] end - def read_authentication_abilities + def read_only_authentication_abilities [ :read_project, :download_code, @@ -488,7 +525,7 @@ describe Gitlab::Auth do end def read_write_authentication_abilities - read_authentication_abilities + [ + read_only_authentication_abilities + [ :push_code, :create_container_image ] @@ -499,4 +536,10 @@ describe Gitlab::Auth do :admin_container_image ] end + + def expect_results_with_abilities(personal_access_token, abilities, success = true) + expect(gl_auth).to receive(:rate_limit!).with('ip', success: success, login: '') + expect(gl_auth.find_for_git_client('', personal_access_token&.token, project: nil, ip: 'ip')) + .to eq(Gitlab::Auth::Result.new(personal_access_token&.user, nil, personal_access_token.nil? ? nil : :personal_access_token, abilities)) + end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index bfa178f5cae..5c9a5b73ee5 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -549,14 +549,14 @@ describe 'Git HTTP requests' do it 'rejects pulls with personal access token error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') + expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') end end it 'rejects the push attempt with personal access token error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') + expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') end end end @@ -566,6 +566,47 @@ describe 'Git HTTP requests' do it_behaves_like 'pulls are allowed' it_behaves_like 'pushes are allowed' + + it 'rejects the push attempt for read_repository scope' do + read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository]) + + upload(path, user: user.username, password: read_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:forbidden) + expect(response.body).to include('You are not allowed to upload code') + end + end + + it 'accepts the push attempt for write_repository scope' do + write_access_token = create(:personal_access_token, user: user, scopes: [:write_repository]) + + upload(path, user: user.username, password: write_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:ok) + end + end + + it 'accepts the pull attempt for read_repository scope' do + read_access_token = create(:personal_access_token, user: user, scopes: [:read_repository]) + + download(path, user: user.username, password: read_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:ok) + end + end + + it 'accepts the pull attempt for api scope' do + read_access_token = create(:personal_access_token, user: user, scopes: [:api]) + + download(path, user: user.username, password: read_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:ok) + end + end + + it 'accepts the push attempt for api scope' do + write_access_token = create(:personal_access_token, user: user, scopes: [:api]) + + upload(path, user: user.username, password: write_access_token.token) do |response| + expect(response).to have_gitlab_http_status(:ok) + end + end end end @@ -577,14 +618,14 @@ describe 'Git HTTP requests' do it 'rejects pulls with personal access token error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') + expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') end end it 'rejects pushes with personal access token error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') + expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') end end @@ -598,7 +639,7 @@ describe 'Git HTTP requests' do it 'does not display the personal access token error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP') + expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 4bb3b848e17..bba473f1c20 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -142,7 +142,7 @@ describe JwtController do end it 'allows read access' do - expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_authentication_abilities) + expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_only_authentication_abilities) get '/jwt/auth', params: parameters end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 2a455523e2c..86e41cbdf00 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -187,7 +187,7 @@ describe 'OpenID Connect requests' do expect(response).to have_gitlab_http_status(200) 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 sudo read_repository openid profile email]) + expect(json_response['scopes_supported']).to eq(%w[api read_user read_repository write_repository sudo openid profile email]) end end |