Skip to content

remote/oidc: browser mode for hyp remote login (T6) - #200

Merged
platypii merged 2 commits into
oidc-client-loginfrom
chunk4-login-command
Jun 29, 2026
Merged

remote/oidc: browser mode for hyp remote login (T6)#200
platypii merged 2 commits into
oidc-client-loginfrom
chunk4-login-command

Conversation

@platypii

Copy link
Copy Markdown
Contributor

Chunk 4 of the OIDC client login (LLP 0046-0048), milestone 2. Stacked on #199 (base chunk3-credentials-kind); review/merge after #197-#199.

What

runRemoteLogin gains an interactive browser authorization-code mode (LLP 0046 D1), the default when no static token is supplied:

  • --token-file/stdin keep the static path unchanged (D8); --org <name> selects an org; --browser forces the flow; --no-browser prints the URL.
  • Identity base derived from the target URL origin as <origin>/v1/identity (D6).
  • Success prints the resolved org and stores the session via writeSession.
  • Callback errors (access_denied, no_membership, org_selection_required, org_not_permitted) translated to clear messages; org_selection_required instructs a --org re-run rather than enumerating orgs (D7).

Tests

remote-login-command.test.js (8 tests) - --org/identity forwarding, --no-browser, each error mapping, unconfigured-target refusal, unchanged static + piped-stdin paths. Typecheck clean; remote/CLI suite green (69).

🤖 Generated with Claude Code

@platypii
platypiiforce-pushed the chunk3-credentials-kind branch from 280281f to a6bb8cbCompareJune 29, 2026 18:37
@platypii
platypii changed the base branch from chunk3-credentials-kind to oidc-client-loginJune 29, 2026 18:38
runRemoteLogin gains an interactive browser authorization-code mode
(LLP 0046 D1), selected by default when no static token is supplied:
- Flag parsing: --token-file / stdin keep the static path unchanged
(kind: 'static', the headless escape hatch, D8); --org <name> selects
an org; --browser forces the flow even with stdin piped; --no-browser
prints the URL instead of opening it.
- The identity base is derived from the configured target URL's origin as
<origin>/v1/identity, so no second URL is configured (D6).
- On success the resolved org is printed and the OIDC session is stored
via writeSession.
- A server-surfaced callback error (access_denied, no_membership,
org_selection_required, org_not_permitted) is translated to a clear
message; org_selection_required instructs a --org re-run rather than
enumerating the user's orgs (D7).
A small `deps.login` seam keeps the browser path unit-testable. Tests
cover --org + identity-base forwarding, --no-browser, each error mapping,
the unconfigured-target refusal, and the unchanged static token-file and
piped-stdin paths.
@platypii

Copy link
Copy Markdown
ContributorAuthor

Review (independent agent review)

Verdict: approve-with-nits. Solid split of static vs browser modes, faithful to D1/D6/D7/D8, with complete error mapping (no enumeration of the user's orgs) and no secret leakage. Mode decision (TTY→browser, piped→static, --browser overrides piped, --token-file validated) is correct; config/target resolution reads the on-disk config consistently with the rest of the file.

Findings

  • minor — flag values could leak into positional name detection.argv.filter(a => !a.startsWith('-')) let a flag's value fall through, so hyp remote login --org acme (target omitted) read acme as the target, and --token-file tok.txt prod could pick tok.txt.
  • minor — --org silently ignored on the static path when a token file / piped token forces static mode.
  • nits — test gaps:--browser over piped stdin; flag-validation exits.

Addressed in this PR

  • Target name now comes from a firstPositional() helper that skips the value slot of --token-file/--org, so login --org acme is a usage error rather than a misread target.
  • A static path with --org now prints note: --org is ignored with a static token ....
  • Added tests: --browser-over-piped-stdin, only-flags usage error, --org-missing-value exit, and the static --org-ignored note.

12 tests green, typecheck clean.

Review follow-ups (PR #200):
- the target name now skips the value slot of --token-file/--org via a
firstPositional() helper, so `login --org acme` (name omitted) is a
usage error instead of misreading 'acme' as the target.
- a static path with --org now prints a note that --org applies only to
the browser flow, instead of silently dropping it.
- tests: --browser overriding piped stdin, only-flags usage error,
--org-missing-value exit, and the static --org-ignored note.
@platypii
platypiiforce-pushed the chunk4-login-command branch from 3f3841e to 36ab1feCompareJune 29, 2026 18:41
@platypii
platypii merged commit b25785c into oidc-client-loginJun 29, 2026
6 checks passed
@platypii
platypii deleted the chunk4-login-command branch June 29, 2026 18:43
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