diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb
index 8b36646810d37670459f5641a9e1968de566a01e..bbd8c4111a5427bb77a47d1543ae529ae90277fa 100644
--- a/app/channels/application_cable/channel.rb
+++ b/app/channels/application_cable/channel.rb
@@ -9,7 +9,7 @@ class Channel < ActionCable::Channel::Base
def validate_token_scope
validate_and_save_access_token!(scopes: authorization_scopes)
- rescue Gitlab::Auth::InsufficientScopeError
+ rescue Gitlab::Auth::AuthenticationError
reject
end
diff --git a/app/channels/application_cable/connection.rb b/app/channels/application_cable/connection.rb
index 7b9988379afd485fa6cba1138b6618b637652bb0..7316b1ce0e5319aa171173b5806113b6fe7cd41d 100644
--- a/app/channels/application_cable/connection.rb
+++ b/app/channels/application_cable/connection.rb
@@ -11,7 +11,7 @@ class Connection < ActionCable::Connection::Base
def connect
self.current_user = find_user_from_bearer_token || find_user_from_session_store
- rescue Gitlab::Auth::UnauthorizedError
+ rescue Gitlab::Auth::AuthenticationError
reject_unauthorized_connection
end
diff --git a/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb b/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb
index 071eb6c967eaff0bf79d8688b77f22e1863b3631..4d52caa9441f3becda6cd6790e5f92963849d91e 100644
--- a/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb
+++ b/app/graphql/mutations/admin/sidekiq_queues/delete_jobs.rb
@@ -8,7 +8,7 @@ class DeleteJobs < BaseMutation
ADMIN_MESSAGE = 'You must be an admin to use this mutation'
- ::Gitlab::ApplicationContext.known_keys.each do |key|
+ ::Gitlab::ApplicationContext.allowed_job_keys.each do |key|
argument key,
GraphQL::Types::String,
required: false,
diff --git a/lib/gitlab/application_context.rb b/lib/gitlab/application_context.rb
index d616c2e037ddac1fea8200914f50873b7b7980a9..7b600ec45c0f7fdd9abedcc437b63d56bb910b15 100644
--- a/lib/gitlab/application_context.rb
+++ b/lib/gitlab/application_context.rb
@@ -28,10 +28,18 @@ class ApplicationContext
:root_caller_id,
:merge_action_status,
:bulk_import_entity_id,
- :sidekiq_destination_shard_redis
+ :sidekiq_destination_shard_redis,
+ :auth_fail_reason,
+ :auth_fail_token_id
].freeze
private_constant :KNOWN_KEYS
+ WEB_ONLY_KEYS = [
+ :auth_fail_reason,
+ :auth_fail_token_id
+ ].freeze
+ private_constant :WEB_ONLY_KEYS
+
APPLICATION_ATTRIBUTES = [
Attribute.new(:project, Project),
Attribute.new(:namespace, Namespace),
@@ -49,7 +57,9 @@ class ApplicationContext
Attribute.new(:root_caller_id, String),
Attribute.new(:merge_action_status, String),
Attribute.new(:bulk_import_entity_id, Integer),
- Attribute.new(:sidekiq_destination_shard_redis, String)
+ Attribute.new(:sidekiq_destination_shard_redis, String),
+ Attribute.new(:auth_fail_reason, String),
+ Attribute.new(:auth_fail_token_id, String)
].freeze
private_constant :APPLICATION_ATTRIBUTES
@@ -57,6 +67,12 @@ def self.known_keys
KNOWN_KEYS
end
+ # Sidekiq jobs may be deleted by matching keys in ApplicationContext.
+ # Filter out keys that aren't available in Sidekiq jobs.
+ def self.allowed_job_keys
+ known_keys - WEB_ONLY_KEYS
+ end
+
def self.application_attributes
APPLICATION_ATTRIBUTES
end
@@ -113,6 +129,8 @@ def to_lazy_hash
assign_hash_if_value(hash, :merge_action_status)
assign_hash_if_value(hash, :bulk_import_entity_id)
assign_hash_if_value(hash, :sidekiq_destination_shard_redis)
+ assign_hash_if_value(hash, :auth_fail_reason)
+ assign_hash_if_value(hash, :auth_fail_token_id)
hash[:user] = -> { username } if include_user?
hash[:user_id] = -> { user_id } if include_user?
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index 9b052fe1882704562eaa283e394573ab7a593a5c..a15abd4ec9bb94c0d28b504a33057e1f653f9a3c 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -174,7 +174,7 @@ def find_runner_from_token
::Ci::Runner.find_by_token(token.to_s) || raise(UnauthorizedError)
end
- def validate_and_save_access_token!(scopes: [])
+ def validate_and_save_access_token!(scopes: [], save_auth_context: true)
# return early if we've already authenticated via a job token
return if @current_authenticated_job.present? # rubocop:disable Gitlab/ModuleWithInstanceVariables
@@ -185,14 +185,18 @@ def validate_and_save_access_token!(scopes: [])
case AccessTokenValidationService.new(access_token, request: request).validate(scopes: scopes)
when AccessTokenValidationService::INSUFFICIENT_SCOPE
+ save_auth_failure_in_application_context(access_token, :insufficient_scope) if save_auth_context
raise InsufficientScopeError, scopes
when AccessTokenValidationService::EXPIRED
+ save_auth_failure_in_application_context(access_token, :token_expired) if save_auth_context
raise ExpiredError
when AccessTokenValidationService::REVOKED
+ save_auth_failure_in_application_context(access_token, :token_revoked) if save_auth_context
revoke_token_family(access_token)
raise RevokedError
when AccessTokenValidationService::IMPERSONATION_DISABLED
+ save_auth_failure_in_application_context(access_token, :impersonation_disabled) if save_auth_context
raise ImpersonationDisabled
end
@@ -211,6 +215,12 @@ def save_current_token_in_env
request.env[API_TOKEN_ENV] = { token_id: access_token.id, token_type: access_token.class.to_s }
end
+ def save_auth_failure_in_application_context(access_token, cause)
+ Gitlab::ApplicationContext.push(
+ auth_fail_reason: cause.to_s,
+ auth_fail_token_id: "#{access_token.class}/#{access_token.id}")
+ end
+
def find_user_from_job_bearer_token
return unless route_authentication_setting[:job_token_allowed]
diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb
index 0b3b3badcdc694859c15841e8f65c051b6bc33a5..d288943483abcb8fcb2c8b173c7e662f59a8fe92 100644
--- a/lib/gitlab/auth/request_authenticator.rb
+++ b/lib/gitlab/auth/request_authenticator.rb
@@ -70,7 +70,9 @@ def find_user_from_personal_access_token_for_api_or_git
end
def valid_access_token?(scopes: [])
- validate_and_save_access_token!(scopes: scopes)
+ # We may just be checking whether the user has :admin_mode access, so
+ # don't construe an auth failure as a real failure.
+ validate_and_save_access_token!(scopes: scopes, save_auth_context: false)
true
rescue Gitlab::Auth::AuthenticationError
diff --git a/lib/gitlab/sidekiq_queue.rb b/lib/gitlab/sidekiq_queue.rb
index df2dd7b5c0767ffd66ca097422c21f43433b75bf..b7c4a7c28f1cf62c629f8289cd5b186bc8eb14d6 100644
--- a/lib/gitlab/sidekiq_queue.rb
+++ b/lib/gitlab/sidekiq_queue.rb
@@ -8,7 +8,7 @@ class SidekiqQueue
InvalidQueueError = Class.new(StandardError)
WORKER_KEY = 'worker_class'
- ALLOWED_KEYS = Gitlab::ApplicationContext.known_keys.map(&:to_s) + [WORKER_KEY]
+ ALLOWED_KEYS = Gitlab::ApplicationContext.allowed_job_keys.map(&:to_s) + [WORKER_KEY]
attr_reader :queue_name
diff --git a/spec/channels/application_cable/connection_spec.rb b/spec/channels/application_cable/connection_spec.rb
index 961568608e787b1b392d4b2463e68adce0b8f871..ba0a5aeecc1aaf6ce03382f780120c5669260a71 100644
--- a/spec/channels/application_cable/connection_spec.rb
+++ b/spec/channels/application_cable/connection_spec.rb
@@ -46,12 +46,47 @@
context 'when bearer header is provided' do
context 'when it is a personal_access_token' do
let(:user_pat) { create(:personal_access_token) }
+ let(:app_context) { Gitlab::ApplicationContext.current }
+ let_it_be(:expired_token) { create(:personal_access_token, :expired, scopes: %w[read_api]) }
+ let_it_be(:revoked_token) { create(:personal_access_token, :revoked, scopes: %w[read_api]) }
it 'finds user by PAT' do
connect(ActionCable.server.config.mount_path, headers: { Authorization: "Bearer #{user_pat.token}" })
expect(connection.current_user).to eq(user_pat.user)
end
+
+ context 'when an expired personal_access_token' do
+ let_it_be(:user_pat) { expired_token }
+
+ it 'sets the current_user as `nil`, and rejects the connection' do
+ expect do
+ connect(ActionCable.server.config.mount_path,
+ headers: { Authorization: "Bearer #{user_pat.token}" }
+ )
+ end.to have_rejected_connection
+
+ expect(connection.current_user).to be_nil
+ expect(app_context['meta.auth_fail_reason']).to eq('token_expired')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{user_pat.id}")
+ end
+ end
+
+ context 'when a revoked personal_access_token' do
+ let_it_be(:user_pat) { revoked_token }
+
+ it 'sets the current_user as `nil`, and rejects the connection' do
+ expect do
+ connect(ActionCable.server.config.mount_path,
+ headers: { Authorization: "Bearer #{user_pat.token}" }
+ )
+ end.to have_rejected_connection
+
+ expect(connection.current_user).to be_nil
+ expect(app_context['meta.auth_fail_reason']).to eq('token_revoked')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{user_pat.id}")
+ end
+ end
end
context 'when it is an OAuth access token' do
diff --git a/spec/channels/graphql_channel_spec.rb b/spec/channels/graphql_channel_spec.rb
index e02c58f30f98ea04058779ca81cea8ee25690f17..97e7bf392facf171e9f96b9a4029ee5392376cd8 100644
--- a/spec/channels/graphql_channel_spec.rb
+++ b/spec/channels/graphql_channel_spec.rb
@@ -11,6 +11,9 @@
create(:personal_access_token, scopes: %w[read_user read_api], user: user)
end
+ let_it_be(:expired_token) { create(:personal_access_token, :expired, scopes: %w[read_api], user: user) }
+ let_it_be(:revoked_token) { create(:personal_access_token, :revoked, scopes: %w[read_api], user: user) }
+
describe '#subscribed' do
let(:query) do
<<~GRAPHQL
@@ -41,6 +44,8 @@
end
context 'with a personal access token' do
+ let(:app_context) { Gitlab::ApplicationContext.current }
+
before do
stub_action_cable_connection current_user: user, access_token: access_token
end
@@ -53,6 +58,7 @@
expect(subscription).to be_confirmed
expect(subscription.streams).to include(/graphql-event::mergeRequestReviewersUpdated:issuableId/)
+ expect(app_context.keys).not_to include('meta.auth_fail_reason', 'meta.auth_fail_token_id')
end
end
@@ -63,6 +69,8 @@
subscribe(subscribe_params)
expect(subscription).not_to be_confirmed
+ expect(app_context['meta.auth_fail_reason']).to eq('insufficient_scope')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{access_token.id}")
end
end
@@ -74,6 +82,31 @@
expect(subscription).to be_confirmed
expect(subscription.streams).to include(/graphql-event::mergeRequestReviewersUpdated:issuableId/)
+ expect(app_context.keys).not_to include('meta.auth_fail_reason', 'meta.auth_fail_token_id')
+ end
+ end
+
+ context 'with an expired read_user personal access token' do
+ let(:access_token) { expired_token }
+
+ it 'does not subscribe to the given graphql subscription' do
+ subscribe(subscribe_params)
+
+ expect(subscription).not_to be_confirmed
+ expect(app_context['meta.auth_fail_reason']).to eq('token_expired')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{access_token.id}")
+ end
+ end
+
+ context 'with a revoked read_user personal access token' do
+ let(:access_token) { revoked_token }
+
+ it 'does not subscribe to the given graphql subscription' do
+ subscribe(subscribe_params)
+
+ expect(subscription).not_to be_confirmed
+ expect(app_context['meta.auth_fail_reason']).to eq('token_revoked')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{access_token.id}")
end
end
end
diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb
index c35374e8bb417839eca0a8a1ccdd3d1f3121f689..958a2b5eee2661501aac8d37fd5d0cf5957f6cd2 100644
--- a/spec/controllers/graphql_controller_spec.rb
+++ b/spec/controllers/graphql_controller_spec.rb
@@ -8,6 +8,8 @@
# two days is enough to make timezones irrelevant
let_it_be(:last_activity_on) { 2.days.ago.to_date }
+ let(:app_context) { Gitlab::ApplicationContext.current }
+
describe 'rescue_from' do
let_it_be(:message) { 'green ideas sleep furiously' }
@@ -326,6 +328,32 @@
end
end
+ context 'with an expired token' do
+ let(:token) { create(:personal_access_token, :expired, user: user, scopes: [:api]) }
+
+ it_behaves_like 'invalid token'
+
+ it 'registers token_expire in application context' do
+ subject
+
+ expect(app_context['meta.auth_fail_reason']).to eq('token_expired')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{token.id}")
+ end
+ end
+
+ context 'with a revoked token' do
+ let(:token) { create(:personal_access_token, :revoked, user: user, scopes: [:api]) }
+
+ it_behaves_like 'invalid token'
+
+ it 'registers token_expire in application context' do
+ subject
+
+ expect(app_context['meta.auth_fail_reason']).to eq('token_revoked')
+ expect(app_context['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{token.id}")
+ end
+ end
+
context 'with an invalid token' do
context 'with auth header' do
subject do
@@ -433,7 +461,8 @@
it "assigns username in ApplicationContext" do
subject
- expect(Gitlab::ApplicationContext.current).to include('meta.user' => user.username)
+ expect(app_context).to include('meta.user' => user.username)
+ expect(app_context.keys).not_to include('meta.auth_fail_reason', 'meta.auth_fail_token_id')
end
it 'calls the track api when trackable method' do
@@ -513,7 +542,8 @@
it "does not assign a username in ApplicationContext" do
subject
- expect(Gitlab::ApplicationContext.current.key?('meta.user')).to be false
+ expect(app_context.key?('meta.user')).to be false
+ expect(app_context.keys).not_to include('meta.auth_fail_reason', 'meta.auth_fail_token_id')
end
end
diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb
index b1a1fc7f62d5e92e80ff54af07de54913934f0f4..24dcf4722f60ccd375e39107c1d8dc5a9be795ce 100644
--- a/spec/lib/gitlab/application_context_spec.rb
+++ b/spec/lib/gitlab/application_context_spec.rb
@@ -2,7 +2,14 @@
require 'spec_helper'
-RSpec.describe Gitlab::ApplicationContext do
+RSpec.describe Gitlab::ApplicationContext, feature_category: :shared do
+ describe '.allowed_job_keys' do
+ it 'includes known keys but omits Web-specific keys' do
+ expect(described_class.allowed_job_keys).to include(:user, :user_id, :project, :root_namespace, :client_id)
+ expect(described_class.allowed_job_keys).not_to include(:auth_fail_reason, :auth_fail_token_id)
+ end
+ end
+
describe '.with_context' do
it 'yields the block' do
expect { |b| described_class.with_context({}, &b) }.to yield_control
diff --git a/spec/lib/gitlab/auth/auth_finders_spec.rb b/spec/lib/gitlab/auth/auth_finders_spec.rb
index f96906cad01d3a1992ef2ccc082ece16be86417f..5a08ef7c164485d52f775fb9cfcedca870647c5d 100644
--- a/spec/lib/gitlab/auth/auth_finders_spec.rb
+++ b/spec/lib/gitlab/auth/auth_finders_spec.rb
@@ -985,6 +985,8 @@ def auth_header_with(token)
expect { validate_and_save_access_token! }.to raise_error(Gitlab::Auth::ExpiredError)
expect(request.env).not_to have_key(described_class::API_TOKEN_ENV)
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_reason']).to eq('token_expired')
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{personal_access_token.id}")
end
it 'returns Gitlab::Auth::RevokedError if token revoked', :aggregate_failures do
@@ -992,11 +994,15 @@ def auth_header_with(token)
expect { validate_and_save_access_token! }.to raise_error(Gitlab::Auth::RevokedError)
expect(request.env).not_to have_key(described_class::API_TOKEN_ENV)
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_reason']).to eq('token_revoked')
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{personal_access_token.id}")
end
it 'returns Gitlab::Auth::InsufficientScopeError if invalid token scope', :aggregate_failures do
expect { validate_and_save_access_token!(scopes: [:sudo]) }.to raise_error(Gitlab::Auth::InsufficientScopeError)
expect(request.env).not_to have_key(described_class::API_TOKEN_ENV)
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_reason']).to eq('insufficient_scope')
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{personal_access_token.id}")
end
end
end
@@ -1012,6 +1018,8 @@ def auth_header_with(token)
it 'returns Gitlab::Auth::ImpersonationDisabled' do
expect { validate_and_save_access_token! }.to raise_error(Gitlab::Auth::ImpersonationDisabled)
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_reason']).to eq('impersonation_disabled')
+ expect(Gitlab::ApplicationContext.current['meta.auth_fail_token_id']).to eq("PersonalAccessToken/#{personal_access_token.id}")
end
end
end
diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb
index 23511c57d5ee2ce70d7947194936f9eb8cba119e..a1f551528dbe581b30b0145bc23ac6092f50e694 100644
--- a/spec/requests/api/api_spec.rb
+++ b/spec/requests/api/api_spec.rb
@@ -141,6 +141,30 @@
end
end
+ context 'with an expired token' do
+ let_it_be(:private_project) { create(:project) }
+ let_it_be(:token) { create(:personal_access_token, :expired, user: user) }
+
+ it 'logs all application context fields and the route' do
+ expect(described_class::LOG_FORMATTER).to receive(:call) do |_severity, _datetime, _, data|
+ expect(data.stringify_keys).to include(
+ 'correlation_id' => an_instance_of(String),
+ 'meta.caller_id' => 'GET /api/:version/projects/:id/issues',
+ 'meta.remote_ip' => an_instance_of(String),
+ 'meta.client_id' => a_string_matching(%r{\Aip/.+}),
+ 'meta.auth_fail_reason' => "token_expired",
+ 'meta.auth_fail_token_id' => "PersonalAccessToken/#{token.id}",
+ 'meta.feature_category' => 'team_planning',
+ 'route' => '/api/:version/projects/:id/issues'
+ )
+ end
+
+ get(api("/projects/#{private_project.id}/issues", personal_access_token: token))
+
+ expect(response).to have_gitlab_http_status(:unauthorized)
+ end
+ end
+
it 'skips context fields that do not apply' do
expect(described_class::LOG_FORMATTER).to receive(:call) do |_severity, _datetime, _, data|
expect(data.stringify_keys).to include(