Fix test failureszj-read-registry-pat

5 files changed, 38 insertions, 30 deletions

diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb
index 2648b901f01..c585d26df77 100644
--- a/app/controllers/jwt_controller.rb
+++ b/app/controllers/jwt_controller.rb
@@ -20,7 +20,7 @@ class JwtController < ApplicationController
   private
 
   def authenticate_project_or_user
-    @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_api_abilities)
+    @authentication_result = Gitlab::Auth::Result.new(nil, nil, :none, Gitlab::Auth.read_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/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index f461d0f97f1..da07ba2f2a3 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -107,7 +107,7 @@ module Gitlab
 
         raise Gitlab::Auth::MissingPersonalTokenError if user.two_factor_enabled?
 
-        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_api_abilities)
+        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end
 
       def oauth_access_token_check(login, password)
@@ -116,7 +116,7 @@ module Gitlab
 
           if valid_oauth_token?(token)
             user = User.find_by(id: token.resource_owner_id)
-            Gitlab::Auth::Result.new(user, nil, :oauth, full_api_abilities)
+            Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
           end
         end
       end
@@ -126,26 +126,23 @@ module Gitlab
 
         token = PersonalAccessTokensFinder.new(state: 'active').find_by(token: password)
 
-        if token && valid_scoped_token?(token, scopes: AVAILABLE_SCOPES.map(&:to_s))
+        if token && valid_scoped_token?(token, AVAILABLE_SCOPES.map(&:to_s))
           Gitlab::Auth::Result.new(token.user, nil, :personal_token, abilities_for_scope(token.scopes))
         end
       end
 
       def valid_oauth_token?(token)
-        token && token.accessible? && valid_scoped_token?(token)
+        token && token.accessible? && valid_scoped_token?(token, ["api"])
       end
 
-      def valid_scoped_token?(token, scopes: %w[api])
+      def valid_scoped_token?(token, scopes)
         AccessTokenValidationService.new(token).include_any_scope?(scopes)
       end
 
       def abilities_for_scope(scopes)
-        abilities = Set.new
-
-        abilities.merge(full_api_abilities) if scopes.include?("api")
-        abilities << :read_container_image if scopes.include?("read_registry")
-
-        abilities.to_a
+        scopes.map do |scope|
+          self.public_send(:"#{scope}_scope_authentication_abilities")
+        end.flatten.uniq
       end
 
       def lfs_token_check(login, password)
@@ -164,9 +161,9 @@ module Gitlab
 
         authentication_abilities =
           if token_handler.user?
-            full_api_abilities
+            full_authentication_abilities
           else
-            read_api_abilities
+            read_authentication_abilities
           end
 
         if Devise.secure_compare(token_handler.token, password)
@@ -202,7 +199,7 @@ module Gitlab
         ]
       end
 
-      def read_api_abilities
+      def read_authentication_abilities
         [
           :read_project,
           :download_code,
@@ -210,12 +207,22 @@ module Gitlab
         ]
       end
 
-      def full_api_abilities
-        read_api_abilities + [
+      def full_authentication_abilities
+        read_authentication_abilities + [
           :push_code,
           :create_container_image
         ]
       end
+      alias_method :api_scope_authentication_abilities, :full_authentication_abilities
+
+      def read_registry_scope_authentication_abilities
+        [:read_container_image]
+      end
+
+      # The currently used auth method doesn't allow any actions for this scope
+      def read_user_scope_authentication_abilities
+        []
+      end
     end
   end
 end
diff --git a/spec/features/profiles/personal_access_tokens_spec.rb b/spec/features/profiles/personal_access_tokens_spec.rb
index 27a20e78a43..7e2e685df26 100644
--- a/spec/features/profiles/personal_access_tokens_spec.rb
+++ b/spec/features/profiles/personal_access_tokens_spec.rb
@@ -17,6 +17,7 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
 
   def disallow_personal_access_token_saves!
     allow_any_instance_of(PersonalAccessToken).to receive(:save).and_return(false)
+
     errors = ActiveModel::Errors.new(PersonalAccessToken.new).tap { |e| e.add(:name, "cannot be nil") }
     allow_any_instance_of(PersonalAccessToken).to receive(:errors).and_return(errors)
   end
@@ -91,8 +92,11 @@ describe 'Profile > Personal Access Tokens', feature: true, js: true do
 
     context "when revocation fails" do
       it "displays an error message" do
-        disallow_personal_access_token_saves!
         visit profile_personal_access_tokens_path
+        allow_any_instance_of(PersonalAccessToken).to receive(:update!).and_return(false)
+
+        errors = ActiveModel::Errors.new(PersonalAccessToken.new).tap { |e| e.add(:name, "cannot be nil") }
+        allow_any_instance_of(PersonalAccessToken).to receive(:errors).and_return(errors)
 
         click_on "Revoke"
         expect(active_personal_access_tokens).to have_text(personal_access_token.name)
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 6574e6d0087..d6006eab0c9 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -17,7 +17,11 @@ describe Gitlab::Auth, lib: true do
     end
 
     it 'OPTIONAL_SCOPES contains all non-default scopes' do
-      expect(subject::OPTIONAL_SCOPES).to eq [:read_user, :openid]
+      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]
     end
   end
 
@@ -157,18 +161,11 @@ describe Gitlab::Auth, lib: true do
         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_token, full_authentication_abilities))
       end
 
-      it 'fails for personal access tokens with other scopes' do
+      it 'limits abilities based on scope' do
         personal_access_token = create(:personal_access_token, scopes: ['read_user'])
 
-        expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-        expect(gl_auth.find_for_git_client('', personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
-      end
-
-      it 'fails for impersonation token with other scopes' do
-        impersonation_token = create(:personal_access_token, scopes: ['read_user'])
-
-        expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: '')
-        expect(gl_auth.find_for_git_client('', impersonation_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
+        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, []))
       end
 
       it 'fails if password is nil' do
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index 8ddae9f6b89..e056353fa6f 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -102,7 +102,7 @@ describe JwtController do
     end
 
     it 'allows read access' do
-      expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_api_abilities)
+      expect(service).to receive(:execute).with(authentication_abilities: Gitlab::Auth.read_authentication_abilities)
 
       get '/jwt/auth', parameters
     end