-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Handle integrity fails if groups map to same roles (#1605)
If AUTH_USER_REGISTRATION is enabled and AUTH_ROLE_MAPPING is set such that multiple LDAP groups map to the same role, then upon attempting to register the user, specifically inserting that user's roles into the database, then these inserts will fail due to the contraint check. To ensure this does not occur, the roles that are to be inserted for the user needs to be de-dupicated. Fixes issue #1603. Co-authored-by: Daniel Vaz Gaspar <[email protected]>
- Loading branch information
1 parent
46b7050
commit 8c3989c
Showing
2 changed files
with
106 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,21 @@ def tearDown(self): | |
| "email": [b"[email protected]"], | ||
| }, | ||
| ) | ||
| user_natalie = ( | ||
| "uid=natalie,ou=users,o=test", | ||
| { | ||
| "uid": ["natalie"], | ||
| "userPassword": ["natalie_password"], | ||
| "memberOf": [ | ||
| b"cn=staff,ou=groups,o=test", | ||
| b"cn=admin,ou=groups,o=test", | ||
| b"cn=exec,ou=groups,o=test", | ||
| ], | ||
| "givenName": [b"Natalie"], | ||
| "sn": [b"Smith"], | ||
| "email": [b"[email protected]"], | ||
| }, | ||
| ) | ||
| group_admins = ( | ||
| "cn=admins,ou=groups,o=test", | ||
| {"cn": ["admins"], "member": [user_admin[0]]}, | ||
|
|
@@ -92,7 +107,16 @@ def tearDown(self): | |
| {"cn": ["staff"], "member": [user_alice[0]]}, | ||
| ) | ||
| directory = dict( | ||
| [top, ou_users, ou_groups, user_admin, user_alice, group_admins, group_staff] | ||
| [ | ||
| top, | ||
| ou_users, | ||
| ou_groups, | ||
| user_admin, | ||
| user_alice, | ||
| user_natalie, | ||
| group_admins, | ||
| group_staff, | ||
| ] | ||
| ) | ||
|
|
||
| # ---------------- | ||
|
|
@@ -111,6 +135,11 @@ def tearDown(self): | |
| tuple(["uid=alice,ou=users,o=test", "alice_password"]), | ||
| {}, | ||
| ) | ||
| call_bind_natalie = ( | ||
| "simple_bind_s", | ||
| tuple(["uid=natalie,ou=users,o=test", "natalie_password"]), | ||
| {}, | ||
| ) | ||
| call_search_alice = ( | ||
| "search_s", | ||
| tuple(["ou=users,o=test", 2, "(uid=alice)", ["givenName", "sn", "email"]]), | ||
|
|
@@ -128,6 +157,18 @@ def tearDown(self): | |
| ), | ||
| {}, | ||
| ) | ||
| call_search_natalie_memberof = ( | ||
| "search_s", | ||
| tuple( | ||
| [ | ||
| "ou=users,o=test", | ||
| 2, | ||
| "(uid=natalie)", | ||
| ["givenName", "sn", "email", "memberOf"], | ||
| ] | ||
| ), | ||
| {}, | ||
| ) | ||
| call_search_alice_filter = ( | ||
| "search_s", | ||
| tuple( | ||
|
|
@@ -320,6 +361,59 @@ def test__inactive_user(self): | |
| # validate - expected LDAP methods were called | ||
| self.assertEquals(self.ldapobj.methods_called(with_args=True), []) | ||
|
|
||
| def test__multi_group_user_mapping_to_same_role(self): | ||
| """ | ||
| LDAP: test login flow for - user in multiple groups mapping to same role | ||
| """ | ||
| self.app.config["AUTH_ROLES_MAPPING"] = { | ||
| "cn=staff,ou=groups,o=test": ["Admin"], | ||
| "cn=admin,ou=groups,o=test": ["Admin", "User"], | ||
| "cn=exec,ou=groups,o=test": ["Public"], | ||
| } | ||
| self.app.config["AUTH_LDAP_SEARCH"] = "ou=users,o=test" | ||
| self.app.config["AUTH_LDAP_USERNAME_FORMAT"] = "uid=%s,ou=users,o=test" | ||
| self.app.config["AUTH_USER_REGISTRATION"] = True | ||
| self.app.config["AUTH_USER_REGISTRATION_ROLE"] = "Public" | ||
| self.appbuilder = AppBuilder(self.app, self.db.session) | ||
| sm = self.appbuilder.sm | ||
|
|
||
| # add User role | ||
| sm.add_role("User") | ||
|
|
||
| # validate - no users are registered | ||
| self.assertEqual(sm.get_all_users(), []) | ||
|
|
||
| # attempt login | ||
| user = sm.auth_user_ldap("natalie", "natalie_password") | ||
|
|
||
| # validate - user was allowed to log in | ||
| self.assertIsInstance(user, sm.user_model) | ||
|
|
||
| # validate - user was registered | ||
| self.assertEqual(len(sm.get_all_users()), 1) | ||
|
|
||
| # validate - user was given the correct roles | ||
| self.assertListEqual( | ||
| user.roles, | ||
| [sm.find_role("Admin"), sm.find_role("Public"), sm.find_role("User")], | ||
| ) | ||
|
|
||
| # validate - user was given the correct attributes (read from LDAP) | ||
| self.assertEqual(user.first_name, "Natalie") | ||
| self.assertEqual(user.last_name, "Smith") | ||
| self.assertEqual(user.email, "[email protected]") | ||
|
|
||
| # validate - expected LDAP methods were called | ||
| self.assertEqual( | ||
| self.ldapobj.methods_called(with_args=True), | ||
| [ | ||
| self.call_initialize, | ||
| self.call_set_option, | ||
| self.call_bind_natalie, | ||
| self.call_search_natalie_memberof, | ||
| ], | ||
| ) | ||
|
|
||
| def test__direct_bind__unregistered(self): | ||
| """ | ||
| LDAP: test login flow for - direct bind - unregistered user | ||
|
|
||