Skip to content

remote/oidc: discriminated credential record + session-aware resolve (T4) - #203

Merged
platypii merged 2 commits into
oidc-client-loginfrom
chunk3-credentials-kind
Jun 29, 2026
Merged

remote/oidc: discriminated credential record + session-aware resolve (T4)#203
platypii merged 2 commits into
oidc-client-loginfrom
chunk3-credentials-kind

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Chunk 3 of the OIDC client login (LLP 0046-0048). Recreated against the integration branch (the original #199 was auto-closed when its stacked base branch was deleted during merge; its review is at #199).

What (LLP 0046 D4/D5)

  • Per-target records gain a kind: 'static' | 'oidc' discriminator; a legacy token-only record reads as static (read-implicit migration). writeToken stamps static; writeSession writes oidc.
  • resolveAccessJwt: env override wins; static returns its token; oidc returns a fresh JWT, refreshing + persisting within a 60s skew of expiry, propagating invalid_grant. resolveToken stays for the stdio proxy.
  • removeToken drops either kind. types.d.ts gains the record interfaces.

Review + fixes applied

Independent review (approve-with-nits); fix applied here: normalizeRecord now keeps a usable static token when a corrupt record carries an incomplete oidc shape. Added boundary tests (skew-window edge, malformed-oidc drop, resolveToken on oidc). Full credential suite green (21), typecheck clean.

…(T4)
Extends the 0600 credential store to carry OIDC sessions alongside the
existing static tokens (LLP 0046 D4), and adds the attach-path resolver
(LLP 0046 D5):
- Records gain a kind discriminator. readCredentials normalizes each
record; a legacy token-only record (no kind) reads as static, so
existing files keep working without a rewrite. writeToken now stamps
kind: 'static'.
- writeSession(stateDir, target, { refreshToken, accessJwt, expiresAt,
org }) writes a kind: 'oidc' record through the same atomic 0600 path.
- resolveAccessJwt({ target, env, stateDir, identityBase, now, fetchImpl
}): env override wins; a static record returns its token; an oidc
record returns a fresh access JWT, refreshing + persisting when the
cached one is within a 60s skew of expiry, and propagating a refresh
failure (typed invalid_grant). resolveToken stays for the stdio proxy.
- removeToken drops either kind (whole-record delete, unchanged).
- types.d.ts: RemoteStaticRecord / RemoteOidcRecord / the union.
Unit tests cover the round-trip, legacy normalization, env override,
fresh vs stale refresh + persistence, and failure propagation. Full
suite green.
… record
Review follow-up (PR #199): an incomplete oidc shape (refreshToken but no
accessJwt) on a hand-edited record no longer returns null and drop a
working static `token`; it falls through to the static branch. Added
boundary tests: the static fallthrough, a malformed-oidc drop, resolveToken
on an oidc record, and a skew-window-boundary refresh.
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review (independent agent review) — carried from #199

Verdict: approve-with-nits. Read-implicit migration, env-override precedence, isFresh skew + NaN handling, atomic 0600 write, and invalid_grant propagation all check out.

Findings: (minor) normalizeRecord could drop a usable static token on a corrupt record carrying an incomplete oidc shape; (minor, intentional per D5) refresh failures throw while other failures return {ok:false}; (follow-up) resolveToken hands the proxy an oidc cached JWT without a freshness check; (nits) boundary test gaps.

Addressed here: normalizeRecord falls through to the static token; added skew-window-boundary, malformed-oidc-drop, and resolveToken-on-oidc tests. 21 tests green, typecheck clean.

@platypii
platypii merged commit 44d88fc into oidc-client-loginJun 29, 2026
6 checks passed
@platypii
platypii deleted the chunk3-credentials-kind branch June 29, 2026 18:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@platypii