Skip to content

Commit 7906e46

Browse files
rustyconoverclaude
andcommitted
Add return_to support in OAuth PKCE for external frontend redirects and fix session cookie cleanup path (v0.6.6)
Session cookie v4 adds a return_to field so external frontends can receive auth tokens via URL fragment redirect. Fixed cookie cleanup path mismatch in the external redirect branch that would prevent the session cookie from being deleted by the browser. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8771094 commit 7906e46

File tree

5 files changed

+99
-35
lines changed

5 files changed

+99
-35
lines changed

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[project]
22
name = "vgi-rpc"
3-
version = "0.6.5"
3+
version = "0.6.6"
44
description = "Vector Gateway Interface - RPC framework based on Apache Arrow"
55
readme = "README.md"
66
requires-python = ">=3.13"

tests/test_http.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -266,15 +266,15 @@ def test_stream_exchange_server_error_returns_200_with_error_header(self, client
266266
pytest.raises(RpcError, match="bidi boom") as exc_info,
267267
http_connect(RpcFixtureService, client=client) as proxy,
268268
):
269-
session = proxy.fail_bidi_mid(factor=2.0)
270-
assert isinstance(session, HttpStreamSession)
271-
schema = pa.schema([pa.field("value", pa.float64())])
272-
batch = pa.RecordBatch.from_pydict({"value": [1.0]}, schema=schema)
273-
ab = AnnotatedBatch(batch=batch)
274-
# First exchange succeeds
275-
session.exchange(ab)
276-
# Second exchange triggers the error
277-
session.exchange(ab)
269+
session = proxy.fail_bidi_mid(factor=2.0)
270+
assert isinstance(session, HttpStreamSession)
271+
schema = pa.schema([pa.field("value", pa.float64())])
272+
batch = pa.RecordBatch.from_pydict({"value": [1.0]}, schema=schema)
273+
ab = AnnotatedBatch(batch=batch)
274+
# First exchange succeeds
275+
session.exchange(ab)
276+
# Second exchange triggers the error
277+
session.exchange(ab)
278278
assert exc_info.value.error_type == "RuntimeError"
279279

280280
def test_400_errors_do_not_get_error_header(self, client: _SyncTestClient) -> None:

tests/test_oauth_pkce.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@
2323
from vgi_rpc.http._oauth_pkce import (
2424
_AUTH_COOKIE_NAME,
2525
_SESSION_COOKIE_NAME,
26-
_SESSION_COOKIE_VERSION,
2726
_derive_session_key,
28-
_exchange_code_for_token,
2927
_generate_code_challenge,
3028
_generate_code_verifier,
3129
_generate_state_nonce,
@@ -180,10 +178,23 @@ def test_roundtrip(self) -> None:
180178
state = "test-state"
181179
url = "/vgi/"
182180
cookie = _pack_oauth_cookie(cv, state, url, self.session_key)
183-
got_cv, got_state, got_url = _unpack_oauth_cookie(cookie, self.session_key)
181+
got_cv, got_state, got_url, got_rt = _unpack_oauth_cookie(cookie, self.session_key)
184182
assert got_cv == cv
185183
assert got_state == state
186184
assert got_url == url
185+
assert got_rt == ""
186+
187+
def test_roundtrip_with_return_to(self) -> None:
188+
cv = "test-code-verifier"
189+
state = "test-state"
190+
url = "/vgi/"
191+
rt = "http://localhost:4321/?service=https://example.com"
192+
cookie = _pack_oauth_cookie(cv, state, url, self.session_key, return_to=rt)
193+
got_cv, got_state, got_url, got_rt = _unpack_oauth_cookie(cookie, self.session_key)
194+
assert got_cv == cv
195+
assert got_state == state
196+
assert got_url == url
197+
assert got_rt == rt
187198

188199
def test_tampered_rejected(self) -> None:
189200
cookie = _pack_oauth_cookie("cv", "st", "/", self.session_key)

vgi_rpc/http/_oauth_pkce.py

Lines changed: 72 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343

4444
_SESSION_COOKIE_NAME = "_vgi_oauth_session"
4545
_AUTH_COOKIE_NAME = "_vgi_auth"
46-
_SESSION_COOKIE_VERSION = 3 # distinct from _TOKEN_VERSION=2 in _server.py
46+
_SESSION_COOKIE_VERSION = 4 # v4 adds return_to field for external frontends
4747
_SESSION_MAX_AGE = 600 # 10 minutes
4848
_AUTH_COOKIE_DEFAULT_MAX_AGE = 3600 # 1 hour fallback
4949
_MAX_ORIGINAL_URL_LEN = 2048
@@ -91,19 +91,22 @@ def _pack_oauth_cookie(
9191
original_url: str,
9292
session_key: bytes,
9393
created_at: int | None = None,
94+
return_to: str = "",
9495
) -> str:
9596
"""Pack PKCE session data into a signed, base64-encoded cookie value.
9697
9798
Wire format::
9899
99-
[1 byte: version=3 (uint8)]
100+
[1 byte: version=4 (uint8)]
100101
[8 bytes: created_at (uint64 LE, seconds since epoch)]
101102
[2 bytes: cv_len (uint16 LE)]
102103
[cv_len bytes: code_verifier (UTF-8)]
103104
[2 bytes: state_len (uint16 LE)]
104105
[state_len bytes: state_nonce (UTF-8)]
105106
[2 bytes: url_len (uint16 LE)]
106107
[url_len bytes: original_url (UTF-8)]
108+
[2 bytes: rt_len (uint16 LE)]
109+
[rt_len bytes: return_to (UTF-8)]
107110
[32 bytes: HMAC-SHA256(session_key, all above)]
108111
109112
"""
@@ -112,6 +115,7 @@ def _pack_oauth_cookie(
112115
cv_bytes = code_verifier.encode("utf-8")
113116
state_bytes = state_nonce.encode("utf-8")
114117
url_bytes = original_url.encode("utf-8")
118+
rt_bytes = return_to.encode("utf-8")
115119
payload = (
116120
struct.pack("B", _SESSION_COOKIE_VERSION)
117121
+ struct.pack("<Q", created_at)
@@ -121,6 +125,8 @@ def _pack_oauth_cookie(
121125
+ state_bytes
122126
+ struct.pack("<H", len(url_bytes))
123127
+ url_bytes
128+
+ struct.pack("<H", len(rt_bytes))
129+
+ rt_bytes
124130
)
125131
mac = hmac.new(session_key, payload, hashlib.sha256).digest()
126132
return base64.urlsafe_b64encode(payload + mac).decode("ascii")
@@ -130,11 +136,11 @@ def _unpack_oauth_cookie(
130136
cookie_value: str,
131137
session_key: bytes,
132138
max_age: int = _SESSION_MAX_AGE,
133-
) -> tuple[str, str, str]:
139+
) -> tuple[str, str, str, str]:
134140
"""Unpack and verify a signed OAuth session cookie.
135141
136142
Returns:
137-
``(code_verifier, state_nonce, original_url)``
143+
``(code_verifier, state_nonce, original_url, return_to)``
138144
139145
Raises:
140146
ValueError: On tampered, expired, or malformed cookies.
@@ -145,8 +151,8 @@ def _unpack_oauth_cookie(
145151
except Exception as exc:
146152
raise ValueError("Malformed session cookie") from exc
147153

148-
# Minimum: version(1) + timestamp(8) + 3 x length(2) + HMAC(32) = 47
149-
if len(raw) < 47:
154+
# Minimum: version(1) + timestamp(8) + 4 x length(2) + HMAC(32) = 49
155+
if len(raw) < 49:
150156
raise ValueError("Session cookie too short")
151157

152158
# Verify HMAC before inspecting payload
@@ -181,8 +187,13 @@ def _unpack_oauth_cookie(
181187
url_len = struct.unpack_from("<H", payload, pos)[0]
182188
pos += 2
183189
original_url = payload[pos : pos + url_len].decode("utf-8")
190+
pos += url_len
184191

185-
return code_verifier, state_nonce, original_url
192+
rt_len = struct.unpack_from("<H", payload, pos)[0]
193+
pos += 2
194+
return_to = payload[pos : pos + rt_len].decode("utf-8")
195+
196+
return code_verifier, state_nonce, original_url, return_to
186197

187198

188199
# ---------------------------------------------------------------------------
@@ -310,6 +321,18 @@ def _validate_original_url(url: str, prefix: str) -> str:
310321
return url
311322

312323

324+
def _validate_return_to(url: str) -> str:
325+
"""Validate an external return-to URL. Returns empty string if invalid."""
326+
if not url or len(url) > 2048:
327+
return ""
328+
parsed = urlparse(url)
329+
if parsed.scheme not in ("http", "https"):
330+
return ""
331+
if not parsed.netloc:
332+
return ""
333+
return url
334+
335+
313336
# ---------------------------------------------------------------------------
314337
# Error HTML page
315338
# ---------------------------------------------------------------------------
@@ -365,15 +388,15 @@ class _OAuthCallbackResource:
365388
"""
366389

367390
__slots__ = (
368-
"_session_key",
369-
"_oidc_discovery",
370391
"_client_id",
371392
"_client_secret",
372-
"_use_id_token",
373-
"_prefix",
374-
"_secure_cookie",
375393
"_cookie_path",
394+
"_oidc_discovery",
395+
"_prefix",
376396
"_redirect_uri",
397+
"_secure_cookie",
398+
"_session_key",
399+
"_use_id_token",
377400
)
378401

379402
def __init__(
@@ -441,7 +464,9 @@ def on_get(self, req: falcon.Request, resp: falcon.Response) -> None:
441464
return
442465

443466
try:
444-
code_verifier, expected_state, original_url = _unpack_oauth_cookie(session_cookie, self._session_key)
467+
code_verifier, expected_state, original_url, return_to = _unpack_oauth_cookie(
468+
session_cookie, self._session_key
469+
)
445470
except ValueError as exc:
446471
logger.warning("OAuth session cookie invalid: %s", exc)
447472
resp.status = "400 Bad Request"
@@ -503,7 +528,30 @@ def on_get(self, req: falcon.Request, resp: falcon.Response) -> None:
503528

504529
logger.info("OAuth PKCE authentication successful")
505530

506-
# Redirect to original page with cookies
531+
# External frontend: redirect with token in URL fragment (no cookie needed)
532+
if return_to:
533+
separator = "#" if "#" not in return_to else "&"
534+
redirect_url = f"{return_to}{separator}token={token}"
535+
logger.info("OAuth redirecting to external frontend: %s", return_to.split("?")[0])
536+
resp.status = "302 Found"
537+
resp.set_header("Location", redirect_url)
538+
resp.set_header("Cache-Control", "no-cache, no-store, must-revalidate")
539+
resp.content_type = "text/html; charset=utf-8"
540+
resp.data = None
541+
resp.text = None
542+
# Clear session cookie (path must match where it was set)
543+
resp.set_cookie(
544+
_SESSION_COOKIE_NAME,
545+
"",
546+
max_age=0,
547+
path=f"{self._prefix}/_oauth/",
548+
secure=self._secure_cookie,
549+
http_only=True,
550+
same_site="Lax",
551+
)
552+
return
553+
554+
# Same-origin: redirect to original page with cookies
507555
original_url = _validate_original_url(original_url, self._prefix)
508556
resp.status = "302 Found"
509557
resp.set_header("Location", original_url)
@@ -545,7 +593,7 @@ class _OAuthLogoutResource:
545593
Clears the auth cookie and redirects to the landing page.
546594
"""
547595

548-
__slots__ = ("_prefix", "_secure_cookie", "_cookie_path")
596+
__slots__ = ("_cookie_path", "_prefix", "_secure_cookie")
549597

550598
def __init__(self, prefix: str, secure_cookie: bool) -> None:
551599
self._prefix = prefix
@@ -579,13 +627,13 @@ class _OAuthPkceMiddleware:
579627
"""
580628

581629
__slots__ = (
582-
"_session_key",
583-
"_oidc_discovery",
584630
"_client_id",
631+
"_oidc_discovery",
585632
"_prefix",
586-
"_secure_cookie",
587633
"_redirect_uri",
588634
"_scope",
635+
"_secure_cookie",
636+
"_session_key",
589637
)
590638

591639
def __init__(
@@ -651,8 +699,13 @@ def process_response(
651699
original_url = f"{original_url}?{req.query_string}"
652700
original_url = _validate_original_url(original_url, self._prefix)
653701

702+
# Check for external frontend return URL
703+
return_to = _validate_return_to(req.get_param("_vgi_return_to") or "")
704+
654705
# Pack session cookie
655-
cookie_value = _pack_oauth_cookie(code_verifier, state_nonce, original_url, self._session_key)
706+
cookie_value = _pack_oauth_cookie(
707+
code_verifier, state_nonce, original_url, self._session_key, return_to=return_to
708+
)
656709

657710
# Build authorization URL
658711
params = {

vgi_rpc/http/_server.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1819,7 +1819,7 @@ class _AuthMiddleware:
18191819
silently swallowed as 401s.
18201820
"""
18211821

1822-
__slots__ = ("_authenticate", "_on_auth_failure", "_www_authenticate", "_exempt_prefixes")
1822+
__slots__ = ("_authenticate", "_exempt_prefixes", "_on_auth_failure", "_www_authenticate")
18231823

18241824
def __init__(
18251825
self,
@@ -2234,11 +2234,11 @@ def make_wsgi_app(
22342234

22352235
from vgi_rpc.http._bearer import chain_authenticate
22362236
from vgi_rpc.http._oauth_pkce import (
2237+
_create_oidc_discovery,
2238+
_derive_session_key,
22372239
_OAuthCallbackResource,
22382240
_OAuthLogoutResource,
22392241
_OAuthPkceMiddleware,
2240-
_create_oidc_discovery,
2241-
_derive_session_key,
22422242
build_user_info_html,
22432243
make_cookie_authenticate,
22442244
)

0 commit comments

Comments
 (0)