fix: bug sweep — timetable envelopes, session cleanup, oauth device flow

campus_python/__init__.py

@@ -159,6 +159,9 @@ def _get_token_from_session(
             self.auth.credentials["campus"][login_session.user_id].update(
                 token=token
             )
+            # Return the refreshed token — user_creds still holds the
+            # pre-refresh (now rotated) access token.
+            return token
         return user_creds.token
 
     def revoke_session(self) -> None:

campus_python/api/v1/timetable.py

@@ -114,7 +114,8 @@ def new(self, metadata: dict, data: dict) -> dict:
             }
         )
         resp.raise_for_status()
-        return resp.json()
+        # POST /timetable/ wraps the created resource in a data envelope
+        return resp.json()["data"]
 
     def list(self, **filters: typing.Any) -> "list[campus.model.TimetableMetadata]":
         """List timetables matching the provided filters.
@@ -129,7 +130,7 @@ def list(self, **filters: typing.Any) -> "list[campus.model.TimetableMetadata]":
         resp.raise_for_status()
         return [
             campus.model.TimetableMetadata.from_resource(item)
-            for item in resp.json()
+            for item in resp.json()["data"]
         ]
 
     class Timetable(Resource):
@@ -146,13 +147,18 @@ def metadata(self) -> "Timetables.Timetable.Metadata":
             return Timetables.Timetable.Metadata("metadata", parent=self)
 
         def get(self) -> campus.model.Timetable:
-            """Get the metadata for this timetable."""
+            """Get this timetable (metadata and labeled entries).
+
+            GET /timetable/<id>/ wraps the resource in a
+            ``{"timetable": ...}`` envelope.
+            """
             resp = self.client.get(
                 self.make_path(end_slash=True)
-                # json={},
             )
             resp.raise_for_status()
-            return resp.json()
+            return campus.model.Timetable.from_resource(
+                resp.json()["timetable"]
+            )
 
         class Entries(Resource):
             """Entries for a single timetable."""
@@ -178,15 +184,20 @@ def get(self) -> campus.model.TimetableMetadata:
                 """
                 resp = self.client.get(self.make_path())
                 resp.raise_for_status()
-                return campus.model.Timetable.from_resource(resp.json())
+                return campus.model.TimetableMetadata.from_resource(resp.json())
 
-            def update(self, **kwargs) -> None:
+            def update(self, *, start_date: str, end_date: str) -> None:
                 """Update the metadata for this timetable.
 
-                Args:
-                    **kwargs: Fields to update (e.g., start_date, end_date)
+                The API replaces both dates together: each is required.
 
-                Raises:
-                    NotImplementedError: Not yet implemented
+                Args:
+                    start_date: New start date (schema.DateTime-compatible)
+                    end_date: New end date (schema.DateTime-compatible)
                 """
-                raise NotImplementedError("TODO: Student to implement")
+                resp = self.client.patch(self.make_path(), json={
+                    "start_date": start_date,
+                    "end_date": end_date,
+                })
+                resp.raise_for_status()
+                return None

campus_python/auth/v1/__init__.py

@@ -169,11 +169,12 @@ def finalize(
         # 3. Finalize session and get target
         target = self.sessions[auth_session.id].finalize()
 
-        # 4. Ensure user exists
-        self.users[auth_session.user_id]
+        # 4. Ensure user exists (the server provisions the user record
+        # during verify_login; a missing user is a hard error here)
+        self.users[auth_session.user_id].get()
 
         # 5. Create login session
-        ls = self.logins.new(
+        self.logins.new(
             user_id=auth_session.user_id,
             device_id=uid.generate_category_uid("device", length=16),
             agent_string=flask.request.headers.get("User-Agent", ""),

campus_python/auth/v1/clients.py

@@ -145,13 +145,15 @@ def get(
                     self,
                     vault: str | None = None
             ) -> JsonDict:
+                # GET rule is /<client_id>/access/ (trailing slash) and the
+                # auth app sets strict_slashes — the slash is required.
                 if vault:
                     resp = self.client.get(
-                        self.make_path(),
+                        self.make_path(end_slash=True),
                         query={"vault": vault}
                     )
                 else:
-                    resp = self.client.get(self.make_path())
+                    resp = self.client.get(self.make_path(end_slash=True))
                 # Raise error if status code is not 2XX or 3XX
                 resp.raise_for_status()
                 return resp.json()

campus_python/auth/v1/logins.py

@@ -77,7 +77,7 @@ def new(
         resp.raise_for_status()
         loginsession = campus.model.LoginSession.from_resource(resp.json())
         flask.session[self._session_key] = loginsession.id
-        return campus.model.LoginSession.from_resource(resp.json())
+        return loginsession
 
     class Login(Resource):
         """A single login session resource."""
@@ -92,7 +92,9 @@ def revoke(self) -> None:
             )
             # Raise error if status code is not 2XX or 3XX
             resp.raise_for_status()
-            del flask.session[self.parent._session_key]  # type: ignore
+            # The key may be absent (expired cookie, new worker) —
+            # revocation must not fail on local cleanup.
+            flask.session.pop(self.parent._session_key, None)  # type: ignore
 
         def get(self) -> campus.model.LoginSession:
             resp = self.client.get(

campus_python/auth/v1/oauth.py

@@ -21,6 +21,11 @@ class OAuth(ResourceRoot):
     Reference: https://datatracker.ietf.org/doc/html/rfc8628
     """
 
+    # All OAuth endpoints live under /auth/v1/oauth (campus/auth/routes/
+    # oauth.py); the auth app enables strict_slashes, so the slash-less
+    # relative paths built here must match the route rules exactly.
+    url_prefix = "/auth/v1/oauth"
+
     def __init__(self, root: ResourceRoot):
         super().__init__(json_client=root.client)
         self._root = root
@@ -49,9 +54,7 @@ def request_device_code(
         json_body = {
             "client_id": client_id,
         }
-        # Use /oauth prefix (not /auth/v1) for device authorization endpoints
-        device_code_path = "/oauth/device_authorize"
-        resp = self.client.post(device_code_path, json=json_body)
+        resp = self.client.post(self.make_path("device_authorize"), json=json_body)
         resp.raise_for_status()
         return resp.json()
 
@@ -86,39 +89,40 @@ def poll_for_token(
             "client_id": client_id,
             "device_code": device_code,
         }
-        token_path = "/oauth/token"
-        resp = self.client.post(token_path, json=json_body)
+        resp = self.client.post(self.make_path("token"), json=json_body)
 
         # Handle OAuth error responses
         if resp.status_code == 400:
             error_data = resp.json()
             error = error_data.get("error", "")
 
-            # Map RFC 8628 errors to AuthenticationError
+            # Map RFC 8628 errors to AuthenticationError; the OAuth error
+            # code travels in details so callers can read it back via the
+            # APIError.oauth_error property.
             if error == "authorization_pending":
                 raise errors.AuthenticationError(
                     error_description="Authorization pending",
-                    error_code="authorization_pending"
+                    details={"oauth_error": "authorization_pending"}
                 )
             elif error == "slow_down":
                 raise errors.AuthenticationError(
                     error_description="Slow down",
-                    error_code="slow_down"
+                    details={"oauth_error": "slow_down"}
                 )
             elif error == "expired_token":
                 raise errors.AuthenticationError(
                     error_description="Device code has expired",
-                    error_code="expired_token"
+                    details={"oauth_error": "expired_token"}
                 )
             elif error == "access_denied":
                 raise errors.AuthenticationError(
                     error_description="Access denied by user",
-                    error_code="access_denied"
+                    details={"oauth_error": "access_denied"}
                 )
             else:
                 raise errors.AuthenticationError(
                     error_description=error_data.get("error_description", "Unknown error"),
-                    error_code=error
+                    details={"oauth_error": error}
                 )
 
         resp.raise_for_status()
@@ -149,7 +153,8 @@ def authorize_device(
             "user_code": user_code,
             "user_id": user_id,
         }
-        authorize_path = "/oauth/device/authorize"
-        resp = self.client.post(authorize_path, json=json_body)
+        resp = self.client.post(
+            self.make_path("device/authorize"), json=json_body
+        )
         resp.raise_for_status()
         return resp.json()

campus_python/auth/v1/sessions.py

@@ -22,7 +22,7 @@ class CampusSessions(ResourceCollection):
 
     @property
     def _session_key(self) -> str:
-        provider = self.path.split("/")[-1]
+        provider = self.path.rstrip("/").split("/")[-1]
         return f"{provider}_session_id"
 
     def __getitem__(self, session_id: str) -> "CampusSessions.Session":
@@ -102,8 +102,12 @@ def finalize(self) -> str:
                 self.make_path(end_slash=True)
             )
             resp.raise_for_status()
-            # Remove session ID from Flask session using correct key
-            del flask.session[cast(CampusSessions, self.parent)._session_key]
+            # Remove session ID from Flask session using correct key.
+            # The key may be absent (expired cookie, new worker, lost
+            # session) — clearing it must not fail the finalization (#59).
+            flask.session.pop(
+                cast(CampusSessions, self.parent)._session_key, None
+            )
             body = resp.json()
             return body["target"]
 

tests/unit/test_auth_clients.py

@@ -158,5 +158,29 @@ def test_update_empty_allowed_scopes_is_sent(self):
         )
 
 
+class TestClientAccessTrailingSlash(unittest.TestCase):
+    """ClientAccess.get() must request /access/ with the trailing slash:
+    the auth app sets strict_slashes, so the slash-less path 404s."""
+
+    def setUp(self):
+        self.auth, self.client = make_auth()
+        response = Mock()
+        response.json.return_value = {"access": []}
+        self.client.get.return_value = response
+
+    def test_get_requests_trailing_slash(self):
+        self.auth.clients["cid123"].access.get()
+        self.client.get.assert_called_once_with(
+            "/auth/v1/clients/cid123/access/"
+        )
+
+    def test_get_with_vault_requests_trailing_slash(self):
+        self.auth.clients["cid123"].access.get(vault="google")
+        self.client.get.assert_called_once_with(
+            "/auth/v1/clients/cid123/access/",
+            query={"vault": "google"},
+        )
+
+
 if __name__ == "__main__":
     unittest.main()

tests/unit/test_nested_resources.py

@@ -133,7 +133,7 @@ def test_entries_list_gets_entries_endpoint(self):
 
     def test_metadata_get_gets_metadata_endpoint(self):
         with patch.object(
-            campus.model.Timetable, "from_resource", return_value=Mock()
+            campus.model.TimetableMetadata, "from_resource", return_value=Mock()
         ):
             self.api.timetable["tt-1"].metadata.get()
         self.client.get.assert_called_once_with(