Skip to content

feat(cli): add hyperframes auth OAuth (PKCE + loopback + refresh)#1084

Open
jrusso1020 wants to merge 1 commit into
05-26-feat_cli_add_hyperframes_auth_login_--api-key_status_logoutfrom
05-26-feat_cli_add_hyperframes_auth_oauth_pkce_loopback_refresh_
Open

feat(cli): add hyperframes auth OAuth (PKCE + loopback + refresh)#1084
jrusso1020 wants to merge 1 commit into
05-26-feat_cli_add_hyperframes_auth_login_--api-key_status_logoutfrom
05-26-feat_cli_add_hyperframes_auth_oauth_pkce_loopback_refresh_

Conversation

@jrusso1020
Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 commented May 26, 2026

What

Adds OAuth 2.0 + PKCE login as the default for hyperframes auth login,
plus refresh-token + 401 auto-retry + auth refresh. Stacks on top of
PR #1081 (the API-key + shared store work).

  • hyperframes auth login (no flags) — opens the user's browser to
    /v1/oauth/authorize, captures the code on an ephemeral
    127.0.0.1:<port>/oauth/callback, exchanges it for tokens with
    PKCE S256, and persists. --api-key opts back into the legacy
    long-lived-key path from PR feat(cli): add hyperframes auth login --api-key, status, logout #1081.
  • hyperframes auth refresh — force-refresh the OAuth access token
    using the stored refresh_token. Mostly useful for testing the path.
  • hyperframes auth logout — best-effort revokes via
    POST /v1/oauth/revoke (RFC 7009) before wiping local state.
  • AuthClient now refreshes-and-retries once on a 401 when the
    caller wires onUnauthenticatedRefresh. auth status wires it.

Internals added in packages/cli/src/auth/:

  • pkce.ts — RFC 7636 code_verifier + S256 code_challenge.
  • loopback.ts — ephemeral 127.0.0.1 HTTP server; state validation,
    120s timeout, styled success/error page.
  • browser.ts — wraps open with a BROWSER=none /
    HF_NO_BROWSER=1 fallback that prints the URL.
  • oauth.tsstartAuthorizationCodeFlow, refreshTokens,
    revokeTokens, requireOAuthConfigured, parseTokenResponse.

Why

This is the foundation OAuth flow that lets free-tier users authenticate
without managing a long-lived key. Refresh + auto-retry means CLI
commands keep working past the access_token lifetime without bugging
the user.

The OAuth client_id (q2A2QRSke2LrFTPJhoDbHtXh) is the one James
created in the oauth2_client table. Baked in as a build-time default;
override via HYPERFRAMES_OAUTH_CLIENT_ID for dev/test.

How

  • Public client: PKCE only, no client_secret. Backend already
    requires PKCE (movio/logic/oauth2.py:638).
  • Loopback port is ephemeral (server.listen(0)) — the backend
    wildcards localhost ports for public clients
    (movio/model/oauth2.py:check_redirect_uri), so the registered
    redirect URI's port is a placeholder.
  • State parameter is generated per-flow + validated on callback to
    prevent CSRF.
  • Token-response parsing is permissive on expires_in type (some
    servers return it as a string) but strict on access_token presence.
  • 401 retry happens at the AuthClient.fetchUser layer, not the
    command layer — so future endpoints inherit it for free.
  • persistOAuth merges into the existing store (preserves co-located
    api_key). auth login (API-key path) does the symmetric thing.

Test plan

  • 80 unit tests, all green. vitest run src/auth/.
  • PKCE: verifier within 43-128 chars, challenge = SHA-256, S256
    method, distinct outputs each call.
  • Loopback: state mismatch / IdP error / missing-code / timeout /
    404 non-callback paths all rejected; success path captures code.
  • OAuth: refreshTokens posts correct body, persists, throws
    REFRESH_FAILED on 400/401 and API_ERROR on 5xx. Existing
    api_key preserved on refresh.
  • AuthClient: 401 retries with refreshed bearer on OAuth, does
    NOT retry for api_key, returns 401 if refresh hook fails.
  • bunx oxlint / bunx oxfmt --check / bunx tsc clean.
  • bunx fallow audit --base origin/main --fail-on-issues — only
    inherited help.ts:showUsage finding (from main, not this PR).
  • Smoke test against dev API:
    HEYGEN_API_URL=https://api.dev.heygen.com hyperframes auth login
    then hyperframes auth status then hyperframes auth refresh.

Out of scope

  • Cloud render commands — separate plan.
  • PR 4 (heygen-cli read-side JSON support) — independent, ships after.

Copy link
Copy Markdown
Collaborator Author

jrusso1020 commented May 26, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@jrusso1020 jrusso1020 force-pushed the 05-26-feat_cli_add_hyperframes_auth_oauth_pkce_loopback_refresh_ branch from 2605d59 to 733ea2e Compare May 26, 2026 18:16
@jrusso1020
Copy link
Copy Markdown
Collaborator Author

Self-review pass on the OAuth work — addressed 12 of 15 findings. Force-push above includes the fixes.

Fixed (high severity):

  1. persistOAuth wiped refresh_token on no-rotation refresh — RFC 6749 §6 lets the server omit refresh_token in a refresh response, and we were silently dropping the previous one. Now merges the new tokens into the existing oauth block. New test covers this.
  2. redirectUri was rebuilt from req.socket.localAddress — could drift on dual-stack hosts and fail the byte-identical match at /v1/oauth/token. Now the loopback passes the original redirectUri (captured once from server.address().port) through to the token-exchange callsite.
  3. refresh.ts double-persisted with a stale snapshot — dropped the second writeStore; refreshTokens already persists via persistOAuth with merge semantics.
  4. revokeTokens had no timeout — added AbortController with 5s default. A flaky network can no longer hang logout for the OS-level TCP timeout.
  5. revokeTokens ignored response status and only revoked one token — now logs HTTP ≥400 to stderr; logout revokes both refresh_token and access_token with the proper RFC 7009 token_type_hint.

Fixed (medium severity):
6. expires_in ≤ 0 → immediate-refresh loop — clamped to a 30s minimum.
7. parseTokenResponse threw ErrApi(200, …) for shape failures (self-contradictory message, wrong routing through tryRefresh). Now throws ErrRefreshFailed so callers consistently route to "log in again".
8. OAuth access_token / refresh_token not isHeaderSafe-validated — added the same isHeaderSafe check used by the store's JSON read path, so an IdP can't smuggle CRLF into headers via the token-exchange response.
9. runOAuthLogin no rollback on verify failure — wired the same refresh hook as auth status so a transient 401 right after sign-in transparently refreshes-and-retries; verify failure now downgraded to a warning (tokens are valid on disk, force-rolling-back would be worse).
10. revokeTokens called resolveClientId() outside the try — now returns silently when OAuth isn't configured. Best-effort contract holds for any caller.
11. State leaked in console.log of authorize URL — now logs origin + pathname only; the random state stays out of scrollback / CI logs.
12. state !== expectedState — replaced with crypto.timingSafeEqual. Low real risk on loopback but a gratuitous deviation. Also added GET-only method check on the loopback handler.
13. requireOAuthConfigured() exited inline — renamed and split: assertOAuthConfiguredOrExit() (command-entry helper that exits) vs resolveClientId() (throws). Pattern matches the rest of the file.
14. Dynamic-import of same module already statically imported — removed.

Deferred (won't fix in this PR):

  • HEYGEN_API_URL accepts arbitrary hosts — flagged in the prior review too. By design today for dev testing (api.dev.heygen.com), and removing it would block that workflow. Reasonable follow-up: warn on stderr when the URL is non-default, and possibly require --allow-insecure-base-url for non-HTTPS schemes. Tracked separately.
  • process.exit after console.error can drop stderr when piped — would need to await stderr drain across every call site. Real but small-impact; deferring to a focused follow-up.

Tests: 89 unit tests, all green. Lint/format/typecheck/fallow clean.

@jrusso1020 jrusso1020 force-pushed the 05-26-feat_cli_add_hyperframes_auth_login_--api-key_status_logout branch from a72b6ac to 5c81d96 Compare May 26, 2026 20:30
@jrusso1020 jrusso1020 force-pushed the 05-26-feat_cli_add_hyperframes_auth_oauth_pkce_loopback_refresh_ branch from 733ea2e to fa489bf Compare May 26, 2026 20:30
Sign up for free to 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