From 9fcc3e5982311a380681c822df72fe470a5ea1ca Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Tue, 6 Jun 2017 13:18:01 +0200 Subject: [PATCH] Fix test failures --- app/controllers/jwt_controller.rb | 2 +- lib/gitlab/auth.rb | 39 +++++++++++-------- .../profiles/personal_access_tokens_spec.rb | 6 ++- spec/lib/gitlab/auth_spec.rb | 19 ++++----- spec/requests/jwt_controller_spec.rb | 2 +- 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 -- GitLab