Commit 00a4831d authored by Bob Van Landuyt's avatar Bob Van Landuyt

Fix infinite loop on saml login of a blocked user

When a blocked user tries to log in they are immediately signed out
again.

This in turn calls the `before_logout` callback in Warden. In that
callback we try to log the activity for the blocked user in the
`BlockedUserTracker`:
https://gitlab.com/gitlab-org/gitlab/blob/e30d909d3f19511742d218da0b4bd2cecca8b7a2/config/initializers/warden.rb#L60

If a `SystemHook` was configured, this would also mean scheduling a
job. Scheduling a job tries to get the current user from the
context. For the `OmniAuthCallbacksController` this would call
`current_user` which would try to sign the user in again.

The `OmniauthCallbacksController` is an `ApplicationController`, which has
a `#context_user` method that uses `#auth_user` which also exposes
`#current_user`, but it has a safeguard to not call current user
multiple times.

Instead of trying to fetch it again for the omniauth sign in
request. We push it into the context when the sign in was valid.

Changelog: fixed
parent e30d909d
......@@ -162,6 +162,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
user = auth_user.find_and_update!
if auth_user.valid_sign_in?
# In this case the `#current_user` would not be set. So we can't fetch it
# from that in `#context_user`. Pushing it manually here makes the information
# available in the logs for this request.
Gitlab::ApplicationContext.push(user: user)
log_audit_event(user, with: oauth['provider'])
set_remember_me(user)
......@@ -287,10 +291,6 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
def fail_admin_mode_invalid_credentials
redirect_to new_admin_session_path, alert: _('Invalid login or password')
end
def context_user
current_user
end
end
OmniauthCallbacksController.prepend_mod_with('OmniauthCallbacksController')
......@@ -479,6 +479,19 @@ RSpec.describe OmniauthCallbacksController, type: :controller do
post :saml, params: { SAMLResponse: mock_saml_response }
end
end
context 'with a blocked user trying to log in when there are hooks set up' do
let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml') }
subject(:post_action) { post :saml, params: { SAMLResponse: mock_saml_response } }
before do
create(:system_hook)
user.block!
end
it { expect { post_action }.not_to raise_error }
end
end
describe 'enable admin mode' 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