Merge branch 'hide-read-registry-scope-when-registry-disabled' into 'master'

Hide read_registry scope when registry is disabled on instance

See merge request !13314

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

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

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

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

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

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

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)
 

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