Commit 76ddd135 authored by James Edwards-Jones's avatar James Edwards-Jones

Avoid incorrectly unblocking users on SAML login

Previously, our `required_groups` check was indiscriminately unblocking
users whenever the condition was met. This was problematic because it
interfered with manually blocked users, as well as users blocked
because they were automatically set up.

Now we only unblock `ldap_blocked`, since those are the ones that could
have been blocked due to `required_groups`.

In the future we might introduce `saml_blocked` users for clarity, but
that might require a data migration.
parent 357573ba
---
title: Avoid SAML required_groups indiscriminately unblocking users on login
merge_request: 9489
author:
type: fixed
...@@ -12,7 +12,7 @@ module EE ...@@ -12,7 +12,7 @@ module EE
user = super user = super
if user_in_required_group? if user_in_required_group?
unblock_user(user, "in required group") if user.persisted? && user.blocked? unblock_user(user, "in required group") if user.persisted? && user.ldap_blocked?
elsif user.persisted? elsif user.persisted?
block_user(user, "not in required group") unless user.blocked? block_user(user, "not in required group") unless user.blocked?
else else
......
...@@ -119,6 +119,13 @@ describe Gitlab::Auth::Saml::User do ...@@ -119,6 +119,13 @@ describe Gitlab::Auth::Saml::User do
expect(saml_user.find_user).to be_active expect(saml_user.find_user).to be_active
end end
it 'does not unblock manually blocked members' do
stub_saml_required_group_config(%w(Developers))
saml_user.save.block!
expect(saml_user.find_user).not_to be_active
end
it 'does not allow non-members' do it 'does not allow non-members' do
stub_saml_required_group_config(%w(ArchitectureAstronauts)) stub_saml_required_group_config(%w(ArchitectureAstronauts))
......
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