Commit 6fd7b260 authored by Thong Kuah's avatar Thong Kuah

Merge branch 'sh-fix-graphql-username-logging' into 'master'

Fix GraphQlController not logging sessionless user

See merge request gitlab-org/gitlab!83144
parents 18cbbc26 39cee0ce
......@@ -196,6 +196,27 @@ class ApplicationController < ActionController::Base
end
end
# Devise defines current_user to be:
#
# def current_user
# @current_user ||= warden.authenticate(scope: mapping)
# end
#
# That means whenever current_user is called and `@current_user` is
# nil, Warden will attempt to authenticate a user. To avoid
# reauthenticating anonymous users, we may need to invalidate
# the user.
def reset_auth_user!
return if strong_memoized?(:auth_user) && auth_user
# Controllers usually call auth_user first, but for some controllers
# authenticate_sessionless_user! is called after that. If we relied
# on the memoized auth_user, the value would always be nil for
# sessionless users.
clear_memoization(:auth_user)
auth_user
end
def log_exception(exception)
# At this point, the controller already exits set_current_context around
# block. To maintain the context while handling error exception, we need to
......
......@@ -20,16 +20,21 @@ module SessionlessAuthentication
end
def sessionless_sign_in(user)
if user.can_log_in_with_non_expired_password?
# Notice we are passing store false, so the user is not
# actually stored in the session and a token is needed
# for every request. If you want the token to work as a
# sign in token, you can simply remove store: false.
sign_in(user, store: false, message: :sessionless_sign_in)
elsif request_authenticator.can_sign_in_bot?(user)
# we suppress callbacks to avoid redirecting the bot
sign_in(user, store: false, message: :sessionless_sign_in, run_callbacks: false)
end
signed_in_user =
if user.can_log_in_with_non_expired_password?
# Notice we are passing store false, so the user is not
# actually stored in the session and a token is needed
# for every request. If you want the token to work as a
# sign in token, you can simply remove store: false.
sign_in(user, store: false, message: :sessionless_sign_in)
elsif request_authenticator.can_sign_in_bot?(user)
# we suppress callbacks to avoid redirecting the bot
sign_in(user, store: false, message: :sessionless_sign_in, run_callbacks: false)
end
reset_auth_user! if respond_to?(:reset_auth_user!, true)
signed_in_user
end
def sessionless_bypass_admin_mode!(&block)
......
......@@ -134,6 +134,12 @@ RSpec.describe GraphqlController do
post :execute
end
it "assigns username in ApplicationContext" do
post :execute
expect(Gitlab::ApplicationContext.current).to include('meta.user' => user.username)
end
end
context 'when user uses an API token' do
......@@ -189,6 +195,12 @@ RSpec.describe GraphqlController do
expect(assigns(:context)[:is_sessionless_user]).to be true
end
it "assigns username in ApplicationContext" do
subject
expect(Gitlab::ApplicationContext.current).to include('meta.user' => user.username)
end
it 'calls the track api when trackable method' do
agent = 'vs-code-gitlab-workflow/3.11.1 VSCode/1.52.1 Node.js/12.14.1 (darwin; x64)'
request.env['HTTP_USER_AGENT'] = agent
......@@ -222,6 +234,12 @@ RSpec.describe GraphqlController do
expect(assigns(:context)[:is_sessionless_user]).to be false
end
it "does not assign a username in ApplicationContext" do
subject
expect(Gitlab::ApplicationContext.current.key?('meta.user')).to be false
end
end
it 'includes request object in context' 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