Commit f7786e56 authored by Stan Hu's avatar Stan Hu

Fix logging of user in /jwt/auth

Previously the `user_id` and `username` would be logged as `null` in
`production_json.log` for any user that accessed the container registry.

Unlike other descendants of `ApplicationController` that use Devise,
`JwtController` authenticates users via `@authentication_result`.  We
now override `auth_user` to get this information in the logs.
parent 1f2ef687
...@@ -75,4 +75,9 @@ class JwtController < ApplicationController ...@@ -75,4 +75,9 @@ class JwtController < ApplicationController
Array(Rack::Utils.parse_query(request.query_string)['scope']) Array(Rack::Utils.parse_query(request.query_string)['scope'])
end end
def auth_user
actor = @authentication_result&.actor
actor.is_a?(User) ? actor : nil
end
end end
---
title: Fix logging of username in /jwt/auth
merge_request:
author:
type: fixed
...@@ -7,11 +7,25 @@ describe JwtController do ...@@ -7,11 +7,25 @@ describe JwtController do
let(:service_class) { double(new: service) } let(:service_class) { double(new: service) }
let(:service_name) { 'test' } let(:service_name) { 'test' }
let(:parameters) { { service: service_name } } let(:parameters) { { service: service_name } }
let(:log_output) { StringIO.new }
let(:logger) do
Logger.new(log_output).tap { |logger| logger.formatter = ->(_, _, _, msg) { msg } }
end
let(:log_data) { Gitlab::Json.parse(log_output.string) }
before do before do
Lograge.logger = logger
stub_const('JwtController::SERVICES', service_name => service_class) stub_const('JwtController::SERVICES', service_name => service_class)
end end
shared_examples 'user logging' do
it 'logs username and ID' do
expect(log_data['username']).to eq(user.username)
expect(log_data['user_id']).to eq(user.id)
end
end
context 'existing service' do context 'existing service' do
subject! { get '/jwt/auth', params: parameters } subject! { get '/jwt/auth', params: parameters }
...@@ -37,14 +51,17 @@ describe JwtController do ...@@ -37,14 +51,17 @@ describe JwtController do
end end
context 'using CI token' do context 'using CI token' do
let(:build) { create(:ci_build, :running) } let(:user) { create(:user) }
let(:build) { create(:ci_build, :running, user: user) }
let(:project) { build.project } let(:project) { build.project }
let(:headers) { { authorization: credentials('gitlab-ci-token', build.token) } } let(:headers) { { authorization: credentials('gitlab-ci-token', build.token) } }
context 'project with enabled CI' do context 'project with enabled CI' do
subject! { get '/jwt/auth', params: parameters, headers: headers } subject! { get '/jwt/auth', params: parameters, headers: headers }
it { expect(service_class).to have_received(:new).with(project, nil, ActionController::Parameters.new(parameters).permit!) } it { expect(service_class).to have_received(:new).with(project, user, ActionController::Parameters.new(parameters).permit!) }
it_behaves_like 'user logging'
end end
context 'project with disabled CI' do context 'project with disabled CI' do
...@@ -57,8 +74,23 @@ describe JwtController do ...@@ -57,8 +74,23 @@ describe JwtController do
it { expect(response).to have_gitlab_http_status(:unauthorized) } it { expect(response).to have_gitlab_http_status(:unauthorized) }
end end
context 'using deploy tokens' do
let(:deploy_token) { create(:deploy_token, read_registry: true, projects: [project]) }
let(:headers) { { authorization: credentials(deploy_token.username, deploy_token.token) } }
subject! { get '/jwt/auth', params: parameters, headers: headers }
it 'authenticates correctly' do
expect(response).to have_gitlab_http_status(:ok)
expect(service_class).to have_received(:new).with(nil, deploy_token, ActionController::Parameters.new(parameters).permit!)
end
it 'does not log a user' do
expect(log_data.keys).not_to include(%w(username user_id))
end
end
context 'using personal access tokens' do context 'using personal access tokens' do
let(:user) { create(:user) }
let(:pat) { create(:personal_access_token, user: user, scopes: ['read_registry']) } let(:pat) { create(:personal_access_token, user: user, scopes: ['read_registry']) }
let(:headers) { { authorization: credentials('personal_access_token', pat.token) } } let(:headers) { { authorization: credentials('personal_access_token', pat.token) } }
...@@ -74,6 +106,7 @@ describe JwtController do ...@@ -74,6 +106,7 @@ describe JwtController do
end end
it_behaves_like 'rejecting a blocked user' it_behaves_like 'rejecting a blocked user'
it_behaves_like 'user logging'
end end
end end
...@@ -104,6 +137,8 @@ describe JwtController do ...@@ -104,6 +137,8 @@ describe JwtController do
end end
it { expect(service_class).to have_received(:new).with(nil, user, service_parameters) } it { expect(service_class).to have_received(:new).with(nil, user, service_parameters) }
it_behaves_like 'user logging'
end end
context 'when user has 2FA enabled' do context 'when user has 2FA enabled' do
......
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