Skip to content

Commit 391e648

Browse files
committed
fix: ensure the prefix follows the same convention as the username
Authenticators have a normalization function that should also be used on the prefix otherwise there might be issues when dealing with checks as the get_authenticated_user method returns a normalized user while authenticate returns the "raw" user.
1 parent b16ef09 commit 391e648

File tree

2 files changed

+66
-14
lines changed

2 files changed

+66
-14
lines changed

multiauthenticator/multiauthenticator.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ class WrapperAuthenticator(URLScopeMixin, authenticator_klass):
8181

8282
@property
8383
def username_prefix(self):
84-
return f"{getattr(self, 'service_name', self.login_service)}{PREFIX_SEPARATOR}"
84+
prefix = f"{getattr(self, 'service_name', self.login_service)}{PREFIX_SEPARATOR}"
85+
return self.normalize_username(prefix)
8586

8687
async def authenticate(self, handler, data=None, **kwargs):
8788
response = await super().authenticate(handler, data, **kwargs)

multiauthenticator/tests/test_multiauthenticator.py

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
from ..multiauthenticator import MultiAuthenticator
1717

1818

19+
class CustomDummyAuthenticator(DummyAuthenticator):
20+
def normalize_username(self, username):
21+
return username.upper()
22+
23+
1924
def test_different_authenticators():
2025
MultiAuthenticator.authenticators = [
2126
(
@@ -200,62 +205,108 @@ def test_username_prefix():
200205
},
201206
),
202207
(PAMAuthenticator, "/pam", {"service_name": "PAM"}),
208+
(CustomDummyAuthenticator, "/dummy", {"service_name": "Dummy"}),
203209
]
204210

205211
multi_authenticator = MultiAuthenticator()
206-
assert len(multi_authenticator._authenticators) == 2
212+
assert len(multi_authenticator._authenticators) == 3
207213
assert (
208214
multi_authenticator._authenticators[0].username_prefix
209-
== f"GitLab{PREFIX_SEPARATOR}"
215+
== f"gitlab{PREFIX_SEPARATOR}"
210216
)
211217
assert (
212218
multi_authenticator._authenticators[1].username_prefix
213-
== f"PAM{PREFIX_SEPARATOR}"
219+
== f"pam{PREFIX_SEPARATOR}"
220+
)
221+
assert (
222+
multi_authenticator._authenticators[2].username_prefix
223+
== f"DUMMY{PREFIX_SEPARATOR}"
214224
)
215225

216226

217227
@pytest.mark.asyncio
218228
async def test_authenticated_username_prefix():
219229
MultiAuthenticator.authenticators = [
220-
(DummyAuthenticator, "/pam", {"service_name": "Dummy"}),
230+
(CustomDummyAuthenticator, "/dummy", {"service_name": "Dummy"}),
221231
]
222232

223233
multi_authenticator = MultiAuthenticator()
224234
assert len(multi_authenticator._authenticators) == 1
225-
username = await multi_authenticator._authenticators[0].authenticate(
235+
user = await multi_authenticator._authenticators[0].get_authenticated_user(
226236
None, {"username": "test"}
227237
)
228-
assert username == f"Dummy{PREFIX_SEPARATOR}test"
238+
assert user["name"] == f"DUMMY{PREFIX_SEPARATOR}TEST"
229239

230240

231241
def test_username_prefix_checks():
232242
MultiAuthenticator.authenticators = [
233243
(PAMAuthenticator, "/pam", {"service_name": "PAM", "allowed_users": {"test"}}),
234244
(
235245
PAMAuthenticator,
236-
"/pam",
246+
"/pam2",
237247
{"service_name": "PAM2", "blocked_users": {"test2"}},
238248
),
249+
(
250+
CustomDummyAuthenticator,
251+
"/dummy",
252+
{"service_name": "Dummy", "allowed_users": {"TEST3"}},
253+
),
254+
(
255+
CustomDummyAuthenticator,
256+
"/dummy2",
257+
{
258+
"service_name": "Dummy",
259+
"allowed_users": {"TEST3"},
260+
"blocked_users": {"TEST4"},
261+
},
262+
),
239263
]
240264

241265
multi_authenticator = MultiAuthenticator()
242-
assert len(multi_authenticator._authenticators) == 2
266+
assert len(multi_authenticator._authenticators) == 4
243267
authenticator = multi_authenticator._authenticators[0]
244268

245269
assert authenticator.check_allowed("test") == False
246-
assert authenticator.check_allowed("PAM:test") == True
270+
assert authenticator.check_allowed("pam:test") == True
247271
assert (
248272
authenticator.check_blocked_users("test") == False
249273
) # Even if no block list, it does not have the correct prefix
250-
assert authenticator.check_blocked_users("PAM:test") == True
274+
assert authenticator.check_blocked_users("pam:test") == True
251275

252276
authenticator = multi_authenticator._authenticators[1]
253277
assert authenticator.check_allowed("test2") == False
254278
assert (
255-
authenticator.check_allowed("PAM2:test2") == True
279+
authenticator.check_allowed("pam2:test2") == True
256280
) # Because allowed_users is empty
257-
assert authenticator.check_blocked_users("test2") == False
258-
assert authenticator.check_blocked_users("PAM2:test2") == False
281+
assert (
282+
authenticator.check_blocked_users("test2") == False
283+
) # Because of missing prefix
284+
assert (
285+
authenticator.check_blocked_users("pam2:test2") == False
286+
) # Because user is in blocked list
287+
288+
authenticator = multi_authenticator._authenticators[2]
289+
assert authenticator.check_allowed("TEST3") == False
290+
assert authenticator.check_allowed("DUMMY:TEST3") == True
291+
assert (
292+
authenticator.check_blocked_users("TEST3") == False
293+
) # Because of missing prefix
294+
assert (
295+
authenticator.check_blocked_users("DUMMY:TEST3") == True
296+
) # Because blocked_users is empty thus allowed
297+
298+
authenticator = multi_authenticator._authenticators[3]
299+
assert authenticator.check_allowed("TEST3") == False
300+
assert authenticator.check_allowed("DUMMY:TEST3") == True
301+
assert (
302+
authenticator.check_blocked_users("TEST3") == False
303+
) # Because of missing prefix
304+
assert (
305+
authenticator.check_blocked_users("DUMMY:TEST3") == True
306+
) # Because user is not in blocked list
307+
assert (
308+
authenticator.check_blocked_users("DUMMY:TEST4") == False
309+
) # Because user is in blocked list
259310

260311

261312
@pytest.fixture(params=[f"test me{PREFIX_SEPARATOR}", f"second{PREFIX_SEPARATOR} test"])

0 commit comments

Comments
 (0)