Commit 22360f45 authored by Sean McGivern's avatar Sean McGivern

Add user information to context metadata for Devise controllers

In the ApplicationController, we use Warden to get the current user for
the context metadata. These Devise-derived controllers don't necessarily
use Warden, so we override the user details in that case to provide
something instead of nothing.
parent 7945d196
...@@ -457,9 +457,7 @@ class ApplicationController < ActionController::Base ...@@ -457,9 +457,7 @@ class ApplicationController < ActionController::Base
def set_current_context(&block) def set_current_context(&block)
Gitlab::ApplicationContext.with_context( Gitlab::ApplicationContext.with_context(
# Avoid loading the auth_user again after the request. Otherwise calling user: -> { context_user },
# `auth_user` again would also trigger the Warden callbacks again
user: -> { auth_user if strong_memoized?(:auth_user) },
project: -> { @project if @project&.persisted? }, project: -> { @project if @project&.persisted? },
namespace: -> { @group if @group&.persisted? }, namespace: -> { @group if @group&.persisted? },
caller_id: caller_id, caller_id: caller_id,
...@@ -542,6 +540,12 @@ class ApplicationController < ActionController::Base ...@@ -542,6 +540,12 @@ class ApplicationController < ActionController::Base
end end
end end
# Avoid loading the auth_user again after the request. Otherwise calling
# `auth_user` again would also trigger the Warden callbacks again
def context_user
auth_user if strong_memoized?(:auth_user)
end
def caller_id def caller_id
"#{self.class.name}##{action_name}" "#{self.class.name}##{action_name}"
end end
......
...@@ -34,6 +34,10 @@ class ConfirmationsController < Devise::ConfirmationsController ...@@ -34,6 +34,10 @@ class ConfirmationsController < Devise::ConfirmationsController
def after_sign_in(resource) def after_sign_in(resource)
after_sign_in_path_for(resource) after_sign_in_path_for(resource)
end end
def context_user
resource
end
end end
ConfirmationsController.prepend_mod_with('ConfirmationsController') ConfirmationsController.prepend_mod_with('ConfirmationsController')
...@@ -287,6 +287,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -287,6 +287,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
def fail_admin_mode_invalid_credentials def fail_admin_mode_invalid_credentials
redirect_to new_admin_session_path, alert: _('Invalid login or password') redirect_to new_admin_session_path, alert: _('Invalid login or password')
end end
def context_user
current_user
end
end end
OmniauthCallbacksController.prepend_mod_with('OmniauthCallbacksController') OmniauthCallbacksController.prepend_mod_with('OmniauthCallbacksController')
...@@ -67,6 +67,10 @@ class PasswordsController < Devise::PasswordsController ...@@ -67,6 +67,10 @@ class PasswordsController < Devise::PasswordsController
redirect_to new_user_session_path, redirect_to new_user_session_path,
notice: I18n.t('devise.passwords.send_paranoid_instructions') notice: I18n.t('devise.passwords.send_paranoid_instructions')
end end
def context_user
resource
end
end end
PasswordsController.prepend_mod_with('PasswordsController') PasswordsController.prepend_mod_with('PasswordsController')
...@@ -202,6 +202,10 @@ class RegistrationsController < Devise::RegistrationsController ...@@ -202,6 +202,10 @@ class RegistrationsController < Devise::RegistrationsController
experiment(:invite_signup_page_interaction, actor: member).track(:form_submission) experiment(:invite_signup_page_interaction, actor: member).track(:form_submission)
experiment('members/invite_email', actor: member).track(:accepted) experiment('members/invite_email', actor: member).track(:accepted)
end end
def context_user
current_user
end
end end
RegistrationsController.prepend_mod_with('RegistrationsController') RegistrationsController.prepend_mod_with('RegistrationsController')
...@@ -12,7 +12,9 @@ RSpec.describe ConfirmationsController do ...@@ -12,7 +12,9 @@ RSpec.describe ConfirmationsController do
describe '#show' do describe '#show' do
render_views render_views
subject { get :show, params: { confirmation_token: confirmation_token } } def perform_request
get :show, params: { confirmation_token: confirmation_token }
end
context 'user is already confirmed' do context 'user is already confirmed' do
let_it_be_with_reload(:user) { create(:user, :unconfirmed) } let_it_be_with_reload(:user) { create(:user, :unconfirmed) }
...@@ -20,20 +22,37 @@ RSpec.describe ConfirmationsController do ...@@ -20,20 +22,37 @@ RSpec.describe ConfirmationsController do
before do before do
user.confirm user.confirm
subject
end end
it 'renders `new`' do it 'renders `new`' do
perform_request
expect(response).to render_template(:new) expect(response).to render_template(:new)
end end
it 'displays an error message' do it 'displays an error message' do
perform_request
expect(response.body).to include('Email was already confirmed, please try signing in') expect(response.body).to include('Email was already confirmed, please try signing in')
end end
it 'does not display the email of the user' do it 'does not display the email of the user' do
perform_request
expect(response.body).not_to include(user.email) expect(response.body).not_to include(user.email)
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:show).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'ConfirmationsController#show')
end
perform_request
end
end end
context 'user accesses the link after the expiry of confirmation token has passed' do context 'user accesses the link after the expiry of confirmation token has passed' do
...@@ -42,39 +61,64 @@ RSpec.describe ConfirmationsController do ...@@ -42,39 +61,64 @@ RSpec.describe ConfirmationsController do
before do before do
allow(Devise).to receive(:confirm_within).and_return(1.day) allow(Devise).to receive(:confirm_within).and_return(1.day)
travel_to(3.days.from_now) do
subject
end
end end
it 'renders `new`' do it 'renders `new`' do
travel_to(3.days.from_now) { perform_request }
expect(response).to render_template(:new) expect(response).to render_template(:new)
end end
it 'displays an error message' do it 'displays an error message' do
travel_to(3.days.from_now) { perform_request }
expect(response.body).to include('Email needs to be confirmed within 1 day, please request a new one below') expect(response.body).to include('Email needs to be confirmed within 1 day, please request a new one below')
end end
it 'does not display the email of the user' do it 'does not display the email of the user' do
travel_to(3.days.from_now) { perform_request }
expect(response.body).not_to include(user.email) expect(response.body).not_to include(user.email)
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:show).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'ConfirmationsController#show')
end
travel_to(3.days.from_now) { perform_request }
end
end end
context 'with an invalid confirmation token' do context 'with an invalid confirmation token' do
let(:confirmation_token) { 'invalid_confirmation_token' } let(:confirmation_token) { 'invalid_confirmation_token' }
before do
subject
end
it 'renders `new`' do it 'renders `new`' do
perform_request
expect(response).to render_template(:new) expect(response).to render_template(:new)
end end
it 'displays an error message' do it 'displays an error message' do
perform_request
expect(response.body).to include('Confirmation token is invalid') expect(response.body).to include('Confirmation token is invalid')
end end
it 'sets the the caller_id in the context' do
expect(controller).to receive(:show).and_wrap_original do |m, *args|
expect(Gitlab::ApplicationContext.current)
.to include('meta.caller_id' => 'ConfirmationsController#show')
m.call(*args)
end
perform_request
end
end end
end end
end end
...@@ -293,6 +293,18 @@ RSpec.describe OmniauthCallbacksController, type: :controller do ...@@ -293,6 +293,18 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
expect(request.env['warden']).to be_authenticated expect(request.env['warden']).to be_authenticated
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:atlassian_oauth2).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'OmniauthCallbacksController#atlassian_oauth2')
end
post :atlassian_oauth2
end
end end
context 'for a new user' do context 'for a new user' do
...@@ -454,6 +466,18 @@ RSpec.describe OmniauthCallbacksController, type: :controller do ...@@ -454,6 +466,18 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
it 'doesn\'t link a new identity to the user' do it 'doesn\'t link a new identity to the user' do
expect { post :saml, params: { SAMLResponse: mock_saml_response } }.not_to change { user.identities.count } expect { post :saml, params: { SAMLResponse: mock_saml_response } }.not_to change { user.identities.count }
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:saml).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'OmniauthCallbacksController#saml')
end
post :saml, params: { SAMLResponse: mock_saml_response }
end
end end
end end
......
...@@ -77,6 +77,18 @@ RSpec.describe PasswordsController do ...@@ -77,6 +77,18 @@ RSpec.describe PasswordsController do
expect(user.password_expires_at).not_to be_nil expect(user.password_expires_at).not_to be_nil
end end
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:update).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'PasswordsController#update')
end
subject
end
end end
end end
end end
...@@ -434,6 +434,18 @@ RSpec.describe RegistrationsController do ...@@ -434,6 +434,18 @@ RSpec.describe RegistrationsController do
expect(User.last.last_name).to eq(base_user_params[:last_name]) expect(User.last.last_name).to eq(base_user_params[:last_name])
expect(User.last.name).to eq("#{base_user_params[:first_name]} #{base_user_params[:last_name]}") expect(User.last.name).to eq("#{base_user_params[:first_name]} #{base_user_params[:last_name]}")
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:create).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => base_user_params[:username],
'meta.caller_id' => 'RegistrationsController#create')
end
subject
end
end end
describe '#destroy' do describe '#destroy' do
...@@ -525,5 +537,17 @@ RSpec.describe RegistrationsController do ...@@ -525,5 +537,17 @@ RSpec.describe RegistrationsController do
end end
end end
end end
it 'sets the username and caller_id in the context' do
expect(controller).to receive(:destroy).and_wrap_original do |m, *args|
m.call(*args)
expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username,
'meta.caller_id' => 'RegistrationsController#destroy')
end
post :destroy
end
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