Merge pull request #29 from idiap/prefix_normalization

fix: ensure the prefix follows the same convention as the username

.github/workflows/workflow.yml

@@ -33,11 +33,14 @@ jobs:
       fail-fast: false
       matrix:
         python-version:
-          - "3.7"
           - "3.8"
           - "3.9"
           - "3.10"
           - "3.11"
+        jupyterhub-version:
+          - "3"
+          - "4"
+          - "5"
 
     steps:
       - uses: actions/checkout@v4
@@ -48,6 +51,7 @@ jobs:
       - name: Install Python dependencies
         run: |
           pip install --upgrade pip
+          pip install jupyterhub==${{ matrix.jupyterhub-version }}
           pip install -e ".[test]"
 
       - name: List packages

multiauthenticator/multiauthenticator.py

@@ -81,7 +81,8 @@ class WrapperAuthenticator(URLScopeMixin, authenticator_klass):
 
                 @property
                 def username_prefix(self):
-                    return f"{getattr(self, 'service_name', self.login_service)}{PREFIX_SEPARATOR}"
+                    prefix = f"{getattr(self, 'service_name', self.login_service)}{PREFIX_SEPARATOR}"
+                    return self.normalize_username(prefix)
 
                 async def authenticate(self, handler, data=None, **kwargs):
                     response = await super().authenticate(handler, data, **kwargs)

multiauthenticator/tests/test_multiauthenticator.py

@@ -2,6 +2,7 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 """Test module for the MultiAuthenticator class"""
+import jupyterhub
 import pytest
 
 from jinja2 import Template
@@ -11,11 +12,17 @@
 from oauthenticator.github import GitHubOAuthenticator
 from oauthenticator.gitlab import GitLabOAuthenticator
 from oauthenticator.google import GoogleOAuthenticator
+from packaging.version import Version
 
 from ..multiauthenticator import PREFIX_SEPARATOR
 from ..multiauthenticator import MultiAuthenticator
 
 
+class CustomDummyAuthenticator(DummyAuthenticator):
+    def normalize_username(self, username):
+        return username.upper()
+
+
 def test_different_authenticators():
     MultiAuthenticator.authenticators = [
         (
@@ -200,62 +207,111 @@ def test_username_prefix():
             },
         ),
         (PAMAuthenticator, "/pam", {"service_name": "PAM"}),
+        (CustomDummyAuthenticator, "/dummy", {"service_name": "Dummy"}),
     ]
 
     multi_authenticator = MultiAuthenticator()
-    assert len(multi_authenticator._authenticators) == 2
+    assert len(multi_authenticator._authenticators) == 3
     assert (
         multi_authenticator._authenticators[0].username_prefix
-        == f"GitLab{PREFIX_SEPARATOR}"
+        == f"gitlab{PREFIX_SEPARATOR}"
     )
     assert (
         multi_authenticator._authenticators[1].username_prefix
-        == f"PAM{PREFIX_SEPARATOR}"
+        == f"pam{PREFIX_SEPARATOR}"
+    )
+    assert (
+        multi_authenticator._authenticators[2].username_prefix
+        == f"DUMMY{PREFIX_SEPARATOR}"
     )
 
 
 @pytest.mark.asyncio
 async def test_authenticated_username_prefix():
     MultiAuthenticator.authenticators = [
-        (DummyAuthenticator, "/pam", {"service_name": "Dummy"}),
+        (CustomDummyAuthenticator, "/dummy", {"service_name": "Dummy"}),
     ]
 
     multi_authenticator = MultiAuthenticator()
     assert len(multi_authenticator._authenticators) == 1
-    username = await multi_authenticator._authenticators[0].authenticate(
+    user = await multi_authenticator._authenticators[0].get_authenticated_user(
         None, {"username": "test"}
     )
-    assert username == f"Dummy{PREFIX_SEPARATOR}test"
+    assert user["name"] == f"DUMMY{PREFIX_SEPARATOR}TEST"
 
 
 def test_username_prefix_checks():
     MultiAuthenticator.authenticators = [
         (PAMAuthenticator, "/pam", {"service_name": "PAM", "allowed_users": {"test"}}),
         (
             PAMAuthenticator,
-            "/pam",
+            "/pam2",
             {"service_name": "PAM2", "blocked_users": {"test2"}},
         ),
+        (
+            CustomDummyAuthenticator,
+            "/dummy",
+            {"service_name": "Dummy", "allowed_users": {"TEST3"}},
+        ),
+        (
+            CustomDummyAuthenticator,
+            "/dummy2",
+            {
+                "service_name": "Dummy",
+                "allowed_users": {"TEST3"},
+                "blocked_users": {"TEST4"},
+            },
+        ),
     ]
 
     multi_authenticator = MultiAuthenticator()
-    assert len(multi_authenticator._authenticators) == 2
+    assert len(multi_authenticator._authenticators) == 4
     authenticator = multi_authenticator._authenticators[0]
 
     assert authenticator.check_allowed("test") == False
-    assert authenticator.check_allowed("PAM:test") == True
+    assert authenticator.check_allowed("pam:test") == True
     assert (
         authenticator.check_blocked_users("test") == False
     )  # Even if no block list, it does not have the correct prefix
-    assert authenticator.check_blocked_users("PAM:test") == True
+    assert authenticator.check_blocked_users("pam:test") == True
 
     authenticator = multi_authenticator._authenticators[1]
     assert authenticator.check_allowed("test2") == False
+    if Version(jupyterhub.__version__) < Version("5"):
+        assert (
+            authenticator.check_allowed("pam2:test2") == True
+        )  # Because allowed_users is empty
+    else:
+        assert authenticator.check_allowed("pam2:test2") == False
+    assert (
+        authenticator.check_blocked_users("test2") == False
+    )  # Because of missing prefix
+    assert (
+        authenticator.check_blocked_users("pam2:test2") == False
+    )  # Because user is in blocked list
+
+    authenticator = multi_authenticator._authenticators[2]
+    assert authenticator.check_allowed("TEST3") == False
+    assert authenticator.check_allowed("DUMMY:TEST3") == True
+    assert (
+        authenticator.check_blocked_users("TEST3") == False
+    )  # Because of missing prefix
+    assert (
+        authenticator.check_blocked_users("DUMMY:TEST3") == True
+    )  # Because blocked_users is empty thus allowed
+
+    authenticator = multi_authenticator._authenticators[3]
+    assert authenticator.check_allowed("TEST3") == False
+    assert authenticator.check_allowed("DUMMY:TEST3") == True
+    assert (
+        authenticator.check_blocked_users("TEST3") == False
+    )  # Because of missing prefix
+    assert (
+        authenticator.check_blocked_users("DUMMY:TEST3") == True
+    )  # Because user is not in blocked list
     assert (
-        authenticator.check_allowed("PAM2:test2") == True
-    )  # Because allowed_users is empty
-    assert authenticator.check_blocked_users("test2") == False
-    assert authenticator.check_blocked_users("PAM2:test2") == False
+        authenticator.check_blocked_users("DUMMY:TEST4") == False
+    )  # Because user is in blocked list
 
 
 @pytest.fixture(params=[f"test me{PREFIX_SEPARATOR}", f"second{PREFIX_SEPARATOR} test"])