Review PR Comments on Cross-Jurisdiction Routing · Entire

for PR #1457, let's have a look at the comments posted on the trail #599, use entire trail to find the comments, once you've switched to the right branch

Bottom line: On branch fix/coreapi-cross-juris-transport (trail 599). 5 open review findings, all in internal/coreapi/cross_juris_transport.go — 3 medium, 2 low. The two highest-value ones are the same bug (default-port normalization) flagged in two places.

Medium

1. Default-port host comparison — isSafeOrigin/validateExchangeURL (line 365) Exact string equality on url.URL.Host. Go omits default ports (example.com) but keeps explicit ones (example.com:443). A server returning token_exchange_url with :443 won't match a request to the bare host → legitimate cross-juris auth fails. Fix: normalize default ports / use origin-equality. (conf 0.8)

2. Same bug in followMisdirected (line 371) homeCore.Host vs federation-manifest hosts, exact equality, same :443-vs-bare-host failure → legitimate 421 redirect rejected. Same fix. (conf 0.85)

3. Brittle path coupling in exchangeSubjectToken (line 515) strings.TrimSuffix(hint.TokenExchangeURL, oauthTokenPath) strips /oauth/token, then PostOAuthToken re-appends it — but validateExchangeURL only checks scheme+host, not path. If a hint ever supplies a non-/oauth/token path, the trim is a no-op and the POST goes to a wrong URL. Not exploitable today (same-origin enforced, server controls its path). Fix: assert path == oauthTokenPath, or pass the validated origin.

Low

4. expires_in discarded (line 519) PostOAuthToken returns expiresIn but it's dropped (token, _, err); cache always uses fixed cachedTokenTTL (4m). If server issues a token shorter than 4m, cache could serve a stale token → unexpected 401. Suggest min(expiresIn - buffer, cachedTokenTTL).

5. Doc/const contradiction (lines 67–68) Struct doc says TTL "matches the foreign-session token's lifetime (5 minutes)" but cachedTokenTTL is 4m (with a correct buffer comment at line 103). Just fix/drop the misleading sentence.

Findings 1 and 2 are clearly the most worth fixing (real functional failure, not just hardening). Want me to implement any/all of these?

Bottom line: Addressed findings 3, 4, 5 in internal/coreapi/cross_juris_transport.go; left 1 & 2 alone per your call. Not committed.

What changed:

• #3 (path coupling): validateExchangeURL now asserts exchange.Path == oauthTokenPath. The strip-then-re-append in exchangeSubjectToken is no longer a hidden assumption — it's enforced. Added a refusal test case.
• #4 (expires_in discarded): exchangeSubjectToken now returns expiresIn; new effectiveTokenTTL(expiresIn) = min(expiresIn − 1m buffer, cachedTokenTTL), falling back to the 4m cap when the server advertises nothing. storeToken takes an explicit TTL and declines <=0 (the live retry still succeeds, we just don't cache a too-short token). New tokenExpiryBuffer const. Added TestEffectiveTokenTTL + TestStoreTokenDeclinesNonPositiveTTL.
• #5 (doc/const contradiction): Rewrote the lines 67–68 comment to describe the real TTL behavior instead of the wrong "5 minutes".

Note: I ran the package tests, not the full mise run test:ci suite. Want me to run the full suite, and/or dismiss findings 1 & 2 on the trail with a rationale ("server-controlled strings, not worth normalization machinery")?

Three commits, each its own fix:

• 4a937e7 #3 — assert token_exchange_url path in validateExchangeURL
• c41ab4c #4 — honor exchange expires_in when caching tokens
• e1ed824 #5 — correct the cache-TTL doc comment

Commit messages:

• 4a937e7fix(coreapi): assert token_exchange_url path in validateExchangeURL

exchangeSubjectToken strips oauthTokenPath and PostOAuthToken re-appends
it, but validateExchangeURL only gated scheme and host. A hint whose path
wasn't /oauth/token would make the strip a no-op and POST to a
server-chosen path. Validate the path so the round-trip is honest.

• c41ab4cfix(coreapi): honor exchange expires_in when caching exchanged tokens

storeToken always used the fixed 4m cachedTokenTTL and discarded the
expires_in PostOAuthToken returns. A server token shorter than 4m could
be served stale, producing a 401 the cache assumed impossible. Thread
expires_in through and cache for min(expires_in - buffer, cachedTokenTTL),
declining to cache lifetimes shorter than the buffer.

• e1ed824docs(coreapi): correct cache-TTL comment to match cachedTokenTTL

The struct doc claimed the cache TTL "matches the foreign-session token's
lifetime (5 minutes)" while cachedTokenTTL is 4m. Describe the actual
expires_in-honoring, buffered behavior.