From 3ba831bba7ed69077c7e88664d4e2941295e9ee0 Mon Sep 17 00:00:00 2001 From: JS Ng Date: Fri, 2 Oct 2026 15:04:11 +0800 Subject: [PATCH] =?UTF-8?q?fix(auth):=20token()=20passes=20a=20relative=20?= =?UTF-8?q?path=20=E2=80=94=20absolute=20URL=20was=20double-prefixed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit token() built an absolute URL (base_url + url_prefix + "/oauth/token") and passed it to client.post(), but CampusRequest._build_url() always prepends the client's base_url (it expects a relative path, like every other resource method). Every token() call therefore hit https://host/https://host/auth/v1/oauth/token and 404ed against real deployments — verified live: curl to the same endpoint returns 200 while the client returns NotFoundError. This predates #61 (the original "/token" construction had the same shape) and is the actual root cause of the long-standing "with_app_session 404s" in campus-classroom#24. It survived all in-repo tests because campus's Flask test transport routes absolute URLs leniently. Pass a relative path (matching oauth.poll_for_token's "/oauth/token") and pin it: the path tests now use a deliberately non-empty mock base_url, and a regression test asserts the path stays relative (#62). Verified live against the dev deployment with this fix: grant 200 (scope campus.profile), and with_app_session() read reaches the campus.api resource lookup. --- campus_python/auth/v1/__init__.py | 10 ++++++++-- tests/unit/test_oauth_token_contract.py | 20 +++++++++++++++++++- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/campus_python/auth/v1/__init__.py b/campus_python/auth/v1/__init__.py index daae0eb..bdad825 100644 --- a/campus_python/auth/v1/__init__.py +++ b/campus_python/auth/v1/__init__.py @@ -331,6 +331,12 @@ def token( authorization-code session endpoint at /auth/v1/token (campus/auth/provider.py), whose contract requires code and redirect_uri and rejects every other grant type (#60). + + The path passed to the client is relative (like every other + resource method): CampusRequest._build_url() prepends the + client's base_url itself, so an absolute URL here would be + double-prefixed into `https://host/https://host/...` and 404 + against real deployments. """ json_body: dict[str, str] = { "grant_type": grant_type, @@ -347,7 +353,7 @@ def token( ) json_body["refresh_token"] = refresh_token - base_url = self.base_url + self.url_prefix + "/oauth/token" - resp = self.client.post(base_url, json=json_body) + token_path = self.url_prefix + "/oauth/token" + resp = self.client.post(token_path, json=json_body) resp.raise_for_status() return campus.model.OAuthToken.from_resource(resp.json()) diff --git a/tests/unit/test_oauth_token_contract.py b/tests/unit/test_oauth_token_contract.py index 8f97145..a4b7fa7 100644 --- a/tests/unit/test_oauth_token_contract.py +++ b/tests/unit/test_oauth_token_contract.py @@ -178,11 +178,17 @@ class TestTokenEndpointPath(unittest.TestCase): authorization-code session endpoint at /auth/v1/token (campus/auth/provider.py) — whose contract requires code and redirect_uri and rejects every other grant type (client issue #60). + + The path must also be RELATIVE: CampusRequest._build_url() prepends + the client's base_url itself, so an absolute URL here gets + double-prefixed into `https://host/https://host/...` and 404s + against real deployments (client issue #62). The mock client's + base_url is therefore deliberately non-empty in these tests. """ def setUp(self): self.auth, self.client = make_auth() - self.client.base_url = "" + self.client.base_url = "https://campusauth.example.com" self.client.post.return_value.json.return_value = dict( RFC_TOKEN_PAYLOAD ) @@ -206,6 +212,18 @@ def test_refresh_token_targets_oauth_token_endpoint(self): self.assertEqual(kwargs["json"]["grant_type"], "refresh_token") self.assertEqual(kwargs["json"]["refresh_token"], "rt123") + def test_token_path_is_relative_not_double_prefixed(self): + """Regression (client issue #62): with a non-empty client + base_url, the path handed to the client must stay relative — + building `base_url + url_prefix + "/oauth/token"` produced + `https://host/https://host/auth/v1/oauth/token` (404 live).""" + with patch.dict(os.environ, {"CLIENT_ID": "cid123", "CLIENT_SECRET": "sec123"}): + self.auth.token(grant_type="client_credentials") + + args, _ = self.client.post.call_args + self.assertFalse(args[0].startswith(("http://", "https://"))) + self.assertEqual(args[0], "/auth/v1/oauth/token") + class TestExpirySecondsAliasRemoved(unittest.TestCase): """The legacy `expiry_seconds` alias is gone (campus #659, closing