Merge pull request #1022 from lsst-sqre/tickets/DM-44136

DM-44136: Refactor OIDC tests

src/gafaelfawr/storage/ldap.py

@@ -197,9 +197,6 @@ def _build_group_member_search(self, username: str) -> str:
         group_class = self._config.group_object_class
         member_attr = self._config.group_member_attr
         if self._config.group_search_by_dn:
-            if not self._config.user_base_dn:
-                # Checked in Settings model so should be impossible.
-                raise ValueError("user_base_dn not set")
             base_dn = self._config.user_base_dn
             attr = self._config.user_search_attr
             target = f"{attr}={username},{base_dn}"

tests/handlers/api_tokens_test.py

@@ -17,7 +17,7 @@
 from gafaelfawr.factory import Factory
 from gafaelfawr.models.state import State
 from gafaelfawr.models.token import Token, TokenUserInfo
-from gafaelfawr.models.userinfo import Group
+from gafaelfawr.models.userinfo import Group, UserInfo
 
 from ..support.config import reconfigure
 from ..support.constants import TEST_HOSTNAME
@@ -1171,29 +1171,19 @@ async def test_create_admin_ldap(
     tmp_path: Path, client: AsyncClient, factory: Factory, mock_ldap: MockLDAP
 ) -> None:
     """Create a token through the admin interface with LDAP user data."""
-    config = await reconfigure(tmp_path, "oidc", factory)
+    await reconfigure(tmp_path, "oidc", factory)
     token_data = await create_session_token(factory, scopes=["admin:token"])
     csrf = await set_session_cookie(client, token_data.token)
-
-    assert config.ldap
-    assert config.ldap.user_base_dn
-    mock_ldap.add_entries_for_test(
-        config.ldap.user_base_dn,
-        config.ldap.user_search_attr,
-        "some-user",
-        [
-            {
-                "displayName": ["Some User"],
-                "mail": ["user@example.com"],
-                "uidNumber": ["1234"],
-            }
-        ],
+    mock_ldap.add_test_user(
+        UserInfo(
+            username="some-user",
+            name="Some User",
+            email="user@example.com",
+            uid=1234,
+        )
     )
-    mock_ldap.add_entries_for_test(
-        config.ldap.group_base_dn,
-        "member",
-        "some-user",
-        [{"cn": ["some-group"], "gidNumber": ["12381"]}],
+    mock_ldap.add_test_group_membership(
+        "some-user", [Group(name="some-group", id=12381)]
     )
 
     # Create a new service token with no user metadata.
@@ -1297,7 +1287,7 @@ async def test_create_admin_firestore(
     mock_firestore: MockFirestore,
     mock_ldap: MockLDAP,
 ) -> None:
-    """Create a token through the admin interface with LDAP user data."""
+    """Create a token through the admin interface with Firestore enabled."""
     await reconfigure(tmp_path, "oidc-firestore", factory)
     firestore_storage = factory.create_firestore_storage()
     await firestore_storage.initialize()
@@ -1495,18 +1485,11 @@ async def test_ldap_error(
 ) -> None:
     config = await reconfigure(tmp_path, "oidc", factory)
     assert config.ldap
-    assert config.ldap.user_base_dn
     mock_ldap.add_entries_for_test(
         config.ldap.user_base_dn,
         config.ldap.user_search_attr,
         "ldap-user",
-        [
-            {
-                "displayName": ["LDAP User"],
-                "mail": ["ldap-user@example.com"],
-                "uidNumber": ["bogus"],
-            }
-        ],
+        [{"uidNumber": ["bogus"]}],
     )
     token_data = await create_session_token(
         factory, username="ldap-user", scopes=["read:all"], minimal=True

tests/handlers/auth_test.py

@@ -1012,18 +1012,11 @@ async def test_ldap_error(
 ) -> None:
     config = await reconfigure(tmp_path, "oidc", factory)
     assert config.ldap
-    assert config.ldap.user_base_dn
     mock_ldap.add_entries_for_test(
         config.ldap.user_base_dn,
         config.ldap.user_search_attr,
         "ldap-user",
-        [
-            {
-                "displayName": ["LDAP User"],
-                "mail": ["ldap-user@example.com"],
-                "uidNumber": ["bogus"],
-            }
-        ],
+        [{"uidNumber": ["bogus"]}],
     )
     token_data = await create_session_token(
         factory, username="ldap-user", scopes=["read:all"], minimal=True

tests/handlers/internal_test.py

@@ -11,6 +11,7 @@
 
 from gafaelfawr.config import Config
 from gafaelfawr.factory import Factory
+from gafaelfawr.models.userinfo import Group, UserInfo
 
 from ..support.config import reconfigure
 from ..support.constants import TEST_HOSTNAME
@@ -51,42 +52,23 @@ async def test_health(
     # Configure LDAP so that we'll also do LDAP lookups. The test should still
     # pass because successful LDAP lookups are optional as long as the LDAP
     # server is responding.
-    config = await reconfigure(tmp_path, "oidc")
+    await reconfigure(tmp_path, "oidc")
     token_service = factory.create_token_service()
     async with factory.session.begin():
         await token_service.delete_token(
             token.token.key, token, token.username, ip_address="127.0.0.1"
         )
     token = await create_session_token(factory)
-    assert config.ldap
-    assert config.ldap.user_base_dn
     r = await client.get("/health")
     assert r.status_code == 200
     assert r.json() == {"status": "healthy"}
 
     # Add the entries for the test user and try again.
-    mock_ldap.add_entries_for_test(
-        config.ldap.user_base_dn,
-        config.ldap.user_search_attr,
-        token.username,
-        [
-            {
-                "displayName": ["LDAP User"],
-                "mail": ["ldap-user@example.com"],
-                "uidNumber": ["2000"],
-                "gidNumber": ["1222"],
-            }
-        ],
+    mock_ldap.add_test_user(
+        UserInfo(username=token.username, uid=2000, gid=1222)
     )
-    mock_ldap.add_entries_for_test(
-        config.ldap.group_base_dn,
-        "member",
-        token.username,
-        [
-            {"cn": ["foo"], "gidNumber": ["1222"]},
-            {"cn": ["group-1"], "gidNumber": ["123123"]},
-            {"cn": ["group-2"], "gidNumber": ["123442"]},
-        ],
+    mock_ldap.add_test_group_membership(
+        token.username, [Group(name="foo", id=1222)]
     )
     r = await client.get("/health")
     assert r.status_code == 200

tests/handlers/login_github_test.py

@@ -60,11 +60,11 @@ async def simulate_github_login(
     httpx.Response
         The response from the return to the ``/login`` handler.
     """
-    config = await config_dependency()
+    config = config_dependency.config()
     assert config.github
     if not headers:
         headers = {}
-    await mock_github(
+    mock_github(
         respx_mock,
         "some-code",
         user_info,
@@ -164,8 +164,7 @@ async def test_login(
         },
     ]
 
-    # Examine the resulting cookie and ensure that it has the proper metadata
-    # set.
+    # Ensure the resulting cookie has the proper metadata set.
     cookie = next(c for c in r.cookies.jar if c.name == "gafaelfawr")
     assert cookie.secure
     assert cookie.discard
@@ -210,7 +209,7 @@ async def test_redirect_header(
         teams=[GitHubTeam(slug="a-team", gid=1000, organization="ORG")],
     )
     return_url = "https://example.com/foo?a=bar&b=baz"
-    await mock_github(respx_mock, "some-code", user_info)
+    mock_github(respx_mock, "some-code", user_info)
 
     # Simulate the initial authentication request.
     r = await client.get(