Commit a9b05ffc authored by Peter Leitzen's avatar Peter Leitzen

Merge branch '212526-oauth-orphan-check' into 'master'

Fix oauth check to verify user exists

See merge request gitlab-org/gitlab!28159
parents badfc1d1 eeef12d4
---
title: Fix Gitlab::Auth to handle orphaned oauth tokens
merge_request: 28159
author:
type: fixed
...@@ -164,20 +164,18 @@ module Gitlab ...@@ -164,20 +164,18 @@ module Gitlab
Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities) Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
end end
# rubocop: disable CodeReuse/ActiveRecord
def oauth_access_token_check(login, password) def oauth_access_token_check(login, password)
if login == "oauth2" && password.present? if login == "oauth2" && password.present?
token = Doorkeeper::AccessToken.by_token(password) token = Doorkeeper::AccessToken.by_token(password)
if valid_oauth_token?(token) if valid_oauth_token?(token)
user = User.find_by(id: token.resource_owner_id) user = User.id_in(token.resource_owner_id).first
return unless user.can?(:log_in) return unless user&.can?(:log_in)
Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities) Gitlab::Auth::Result.new(user, nil, :oauth, full_authentication_abilities)
end end
end end
end end
# rubocop: enable CodeReuse/ActiveRecord
def personal_access_token_check(password) def personal_access_token_check(password)
return unless password.present? return unless password.present?
......
...@@ -250,6 +250,13 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do ...@@ -250,6 +250,13 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
let(:token_w_api_scope) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') } let(:token_w_api_scope) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') }
let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
shared_examples 'an oauth failure' do
it 'fails' do
expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip'))
.to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
end
end
it 'succeeds for OAuth tokens with the `api` scope' do it 'succeeds for OAuth tokens with the `api` scope' do
expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, described_class.full_authentication_abilities)) expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, described_class.full_authentication_abilities))
end end
...@@ -269,10 +276,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do ...@@ -269,10 +276,15 @@ describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
context 'blocked user' do context 'blocked user' do
let(:user) { create(:user, :blocked) } let(:user) { create(:user, :blocked) }
it 'fails' do it_behaves_like 'an oauth failure'
expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')) end
.to eq(Gitlab::Auth::Result.new(nil, nil, nil, nil))
context 'orphaned token' do
before do
user.destroy
end end
it_behaves_like 'an oauth failure'
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment