diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb index 654be927ed8..ec0ebe4d353 100644 --- a/app/models/personal_access_token.rb +++ b/app/models/personal_access_token.rb @@ -28,7 +28,7 @@ class PersonalAccessToken < ActiveRecord::Base protected def validate_scopes - unless scopes.all? { |scope| Gitlab::Auth::AVAILABLE_SCOPES.include?(scope.to_sym) } + unless revoked || scopes.all? { |scope| Gitlab::Auth::AVAILABLE_SCOPES.include?(scope.to_sym) } errors.add :scopes, "can only contain available scopes" end end diff --git a/changelogs/unreleased/hide-read-registry-scope-when-registry-disabled.yml b/changelogs/unreleased/hide-read-registry-scope-when-registry-disabled.yml new file mode 100644 index 00000000000..22ac9b9073f --- /dev/null +++ b/changelogs/unreleased/hide-read-registry-scope-when-registry-disabled.yml @@ -0,0 +1,4 @@ +--- +title: Hide read_registry scope when registry is disabled on instance +merge_request: 13314 +author: Robin Bobbitt diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 3fd81759d25..11ace83c15c 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -2,7 +2,7 @@ module Gitlab module Auth MissingPersonalTokenError = Class.new(StandardError) - REGISTRY_SCOPES = [:read_registry].freeze + REGISTRY_SCOPES = Gitlab.config.registry.enabled ? [:read_registry].freeze : [].freeze # Scopes used for GitLab API access API_SCOPES = [:api, :read_user].freeze diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index f685bb83d0d..4f4a27e4c41 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -17,11 +17,31 @@ describe Gitlab::Auth do end 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_registry openid] end - it 'REGISTRY_SCOPES contains all registry related scopes' do - expect(subject::REGISTRY_SCOPES).to eq %i[read_registry] + context 'REGISTRY_SCOPES' do + context 'when registry is disabled' do + before do + stub_container_registry_config(enabled: false) + end + + it 'is empty' do + expect(subject::REGISTRY_SCOPES).to eq [] + end + end + + context 'when registry is enabled' do + before do + stub_container_registry_config(enabled: true) + end + + it 'contains all registry related scopes' do + expect(subject::REGISTRY_SCOPES).to eq %i[read_registry] + end + end end end @@ -147,11 +167,17 @@ describe Gitlab::Auth do 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_token, full_authentication_abilities)) end - it 'succeeds for personal access tokens with the `read_registry` scope' do - personal_access_token = create(:personal_access_token, scopes: ['read_registry']) + context 'when registry is enabled' do + before do + stub_container_registry_config(enabled: true) + end - 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_token, [:read_container_image])) + 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_token, [:read_container_image])) + end end it 'succeeds if it is an impersonation token' do diff --git a/spec/models/personal_access_token_spec.rb b/spec/models/personal_access_token_spec.rb index b2f2a3ce914..01440b15674 100644 --- a/spec/models/personal_access_token_spec.rb +++ b/spec/models/personal_access_token_spec.rb @@ -41,7 +41,7 @@ describe PersonalAccessToken do it 'revokes the token' do active_personal_access_token.revoke! - expect(active_personal_access_token.revoked?).to be true + expect(active_personal_access_token).to be_revoked end end @@ -61,10 +61,37 @@ describe PersonalAccessToken do expect(personal_access_token).to be_valid end - it "allows creating a token with read_registry scope" do - personal_access_token.scopes = [:read_registry] + context 'when registry is disabled' do + before do + stub_container_registry_config(enabled: false) + end - expect(personal_access_token).to be_valid + it "rejects creating a token with read_registry scope" do + personal_access_token.scopes = [:read_registry] + + expect(personal_access_token).not_to be_valid + expect(personal_access_token.errors[:scopes].first).to eq "can only contain available scopes" + end + + it "allows revoking a token with read_registry scope" do + personal_access_token.scopes = [:read_registry] + + personal_access_token.revoke! + + expect(personal_access_token).to be_revoked + end + end + + context 'when registry is enabled' do + before do + stub_container_registry_config(enabled: true) + end + + it "allows creating a token with read_registry scope" do + personal_access_token.scopes = [:read_registry] + + expect(personal_access_token).to be_valid + end end it "rejects creating a token with unavailable scopes" do diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 8d79ea3dd40..41bf43a9bce 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -49,6 +49,10 @@ describe JwtController do let(:pat) { create(:personal_access_token, user: user, scopes: ['read_registry']) } let(:headers) { { authorization: credentials('personal_access_token', pat.token) } } + before do + stub_container_registry_config(enabled: true) + end + subject! { get '/jwt/auth', parameters, headers } it 'authenticates correctly' do diff --git a/spec/support/api/scopes/read_user_shared_examples.rb b/spec/support/api/scopes/read_user_shared_examples.rb index 3bd589d64b9..57e28e040d7 100644 --- a/spec/support/api/scopes/read_user_shared_examples.rb +++ b/spec/support/api/scopes/read_user_shared_examples.rb @@ -23,6 +23,10 @@ shared_examples_for 'allows the "read_user" scope' do context 'when the requesting token does not have any required scope' do let(:token) { create(:personal_access_token, scopes: ['read_registry'], user: user) } + before do + stub_container_registry_config(enabled: true) + end + it 'returns a "401" response' do get api_call.call(path, user, personal_access_token: token) diff --git a/spec/support/stub_gitlab_calls.rb b/spec/support/stub_gitlab_calls.rb index 78a2ff73746..9695f35bd25 100644 --- a/spec/support/stub_gitlab_calls.rb +++ b/spec/support/stub_gitlab_calls.rb @@ -26,9 +26,11 @@ module StubGitlabCalls end def stub_container_registry_config(registry_settings) - allow(Gitlab.config.registry).to receive_messages(registry_settings) allow(Auth::ContainerRegistryAuthenticationService) .to receive(:full_access_token).and_return('token') + + allow(Gitlab.config.registry).to receive_messages(registry_settings) + load 'lib/gitlab/auth.rb' end def stub_container_registry_tags(repository: :any, tags:)