feat(login): add user browser-based login for all commands - #50
Open
aistackdev wants to merge 26 commits into
Open
feat(login): add user browser-based login for all commands#50aistackdev wants to merge 26 commits into
aistackdev wants to merge 26 commits into
Conversation
GreenNode is the company/brand (the compound form is "greennode"), but much of the repo used the single-n "greenode". Rename consistently across code, docs, and scripts. - Go identifiers: GreenodeClient / NewGreenodeClient -> Greennode... (internal/client, internal/cli, internal/vserverclient, cmd/vks, cmd/vserver/*, templates). Pure internal rename; nothing external imports these. - Domain: greenode.ai -> greennode.ai in the trusted-endpoint list, its tests (incl. the notgreennode.ai negative case), and the --allow-untrusted-endpoint flag help. - Config dir: ~/.greenode -> ~/.greennode, with a legacy read-fallback (effectiveConfigDir prefers ~/.greennode, falls back to ~/.greenode) so existing users keep loading until they re-run `grn configure`, which migrates them onto the new path. New tests cover legacy-only fallback and new-dir-takes-precedence. - Install dir: ~/.local/lib/greenode -> ~/.local/lib/greennode in the install scripts. - Prose "Greenode CLI" -> "GreenNode CLI"; docs URLs greenode-cli -> greennode-cli (matches mkdocs.yml); ~/.greenode -> ~/.greennode in README, docs/configuration.md, CLAUDE.md. CHANGELOG.md is intentionally untouched (release-please managed). Co-Authored-By: Claude <noreply@anthropic.com>
Copy seven self-contained packages from greennode-agentbase-cli into internal/agentbase/ (auth, client, config, cliinput, jsonslice, identity, output), rewriting their import paths to the greennode-cli module. Bump go.mod directive to 1.25 and add required deps (tablewriter, oauth2, term; fatih/color pulled transitively). No cmd/ wiring yet; default grn build unchanged. - 23 .go files vendored (auth 3, client 3, config 4, cliinput 2, jsonslice 3, identity 5, output 3) - All 105 ported tests pass; go build ./... and go vet ./... clean - viper absent; cobra stays v1.10.2; term pinned to v0.41.0 Co-Authored-By: Claude <noreply@anthropic.com>
Task 2 of agentbase->grn migration: - cmd/agentbase/agentbase.go: 'grn agentbase' subcommand root with self-registration via cli.RegisterService, persistent flags (--interactive/-i, --env, --output/-o), and ASCII banner gating. - cmd/agentbase/helpers.go: pruned helpers (mustLoadConfig, mustLoadConfigWithCreds, newAuthProvider) used by the identity group. - cmd/agentbase/context.go: ported context command group (switch/current/headers/decorators) from greennode-agentbase-cli with import paths and rootCmd->AgentbaseCmd rewritten. - cmd/agentbase/agentbase_test.go: command-tree and persistent-flag guard tests. - go.mod: github.com/fatih/color promoted from indirect to direct (cmd/agentbase/agentbase.go imports it directly for the banner). Constraint clean: package imports only internal/agentbase/* and internal/cli (RegisterService); no greennode-cli core imports. Verified: gofmt clean, go build ./..., go test ./... all green. Co-Authored-By: Claude <noreply@anthropic.com>
Add cmd/register_agentbase.go with a `//go:build agentbase` constraint that blank-imports cmd/agentbase so its init() self-registers via cli.RegisterService. The default grn binary and the public release build (-tags vks_only) both exclude agentbase while it remains under development. Verified: - `grn agentbase --help|current|headers` + `context switch prod` all work under -tags agentbase. - Default and -tags vks_only builds: `grn agentbase` => "unknown command". - `go test -tags agentbase ./...` all packages PASS. Co-Authored-By: Claude <noreply@anthropic.com>
Port the agentbase identity command group from greennode-agentbase-cli into grn under `grn agentbase identity ...`. The file is copied verbatim with the standard sed transform: - package cmd -> package agentbase - greennode-agentbase-cli/internal/* -> greennode-cli/internal/agentbase/* - greennode-agentbase-cli/pkg/output -> greennode-cli/internal/agentbase/output - rootCmd -> AgentbaseCmd The local `func max(a, b int) int` is dropped (shadows the Go 1.21+ builtin; the module is now on go 1.25). Local helpers str, joinStrings, and formatTime port with the file. Extend agentbase_test.go with two new tests: - TestAgentbaseCmd_HasIdentitySubtree verifies the identity group and its workload CRUD subtree mounted under grn agentbase. - TestJoinStrings_jsonsliceArray ports the agentbase helper test for joinStrings (now defined in identity.go). All 4 tests in cmd/agentbase PASS under `go test -tags agentbase`; the identity, workload, and outbound-auth oauth2 --help trees all render. Co-Authored-By: Claude <noreply@anthropic.com>
The verbatim agentbase port carries commands whose UX (identity config show, delete without --dry-run/--force) violates the cross-product conventions enforced in cmd/conventions_test.go. The grn agentbase subtree is gated (-tags agentbase only), self-contained (own v2 OAuth2 auth + .greennode.json config), and not yet default-on. Mirror the existing isConfigureSub exemption by adding isAgentbaseSub and skipping agentbase leaf commands in TestNoDeniedVerbs and TestDestructiveCommandsHaveDryRunAndForce. TestEveryCommandHasShortHelp still applies. Conformity will be revisited when agentbase flips to default-on. Co-Authored-By: Claude <noreply@anthropic.com>
Document the agentbase subtree migration in CLAUDE.md (project structure, build tags, gating rationale) and docs/development/architecture.md. The agentbase command is gated behind the opt-in agentbase build tag and excluded from default and release builds while still in development. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Add StoredToken, Save, Load, Clear, and ErrNoStoredToken; persist only refresh token + expiry, never the access token. Co-Authored-By: Claude <noreply@anthropic.com>
Add Listener: a one-shot loopback HTTP server (127.0.0.1:0) that captures IAM's OAuth callback (?code=...&appState=... or RFC 6749 ?error=...), serves a static result page, then shuts down. Serve selects on the received signal, ctx cancellation, or a serve error; Close is idempotent. Co-Authored-By: Claude <noreply@anthropic.com>
Wire the prior task outputs (pkce, iamidp, tokencx, store, listen) into the package's public entry point Login(ctx, cfg, storePath) (Token, error): generate verifier/challenge + nonce, bind loopback listener, build camelCase authorize URL, open browser best-effort, Serve callback, state check, secret-optional Basic token exchange, fail-loud access_token extraction, decode refresh/expiry, persist refresh 0600 (or warn on partial-success when no refresh_token). Access token is never written to disk. Co-Authored-By: Claude <noreply@anthropic.com>
The listener was nonce-agnostic: it served "Login complete" for any callback with a code, then Login detected the appState/nonce mismatch afterward — so a user saw a success page while grn login returned an error. Spec §5 requires a failure page on mismatch. Add an opt-in wantState field on Listener (armed by Login via setExpectedState); handle compares the callback's appState against it, serves the failure page and reports ErrStateMismatch on mismatch. Empty wantState leaves the listener nonce-agnostic (the standalone unit tests stay green). Login now arms the check and drops its redundant post-hoc comparison. Tighten the e2e state-mismatch assertion to errors.Is (was a loose strings.Contains). Add a listener-level test pinning both the sentinel and the failure-page status on mismatch. Co-Authored-By: Claude <noreply@anthropic.com>
Apply the final review's "batch-tidy-now" verdicts to internal/login: - tokencx: rename Error prefix `idpoauth:` -> `login:` (package-correct). - store: make Save atomic (temp file in same dir + Chmod 0600 + Rename) so a crash mid-write cannot truncate an existing token and the rename re-asserts 0600 even when the target already exists with looser perms (os.WriteFile preserves an existing file's mode bits on overwrite). - store: drop the redundant `dir != ""` guard (filepath.Dir never returns ""). No behavior change to the public flow; vet + go test + go test -race clean. Co-Authored-By: Claude <noreply@anthropic.com>
Bolt the cobra steering wheel onto the internal/login PKCE library. The library landed in prior commits but had no command, so `grn login` was "unknown command". Default build, no tag — login is shared root infra. - cmd/login: LoginCmd resolves a login.Config from flags + env (--client-id/--client-secret + GRN_LOGIN_CLIENT_ID/GRN_LOGIN_CLIENT_SECRET, flag > env), picks prod/dev IAM endpoint presets via --iam-env (overridable piecewise with --authorize-url/--token-url), and calls login.Login with a --timeout-bounded ctx. On success prints store path + access-token expiry only — the access token and client secret are never echoed, logged, or persisted (refresh token stays 0600 via the library's atomic Save). LogoutCmd calls login.Clear (idempotent). - cmd/root.go: register login/logout alongside configure. - cmd/login tests: table-driven resolveConfig (flag>env, presets, piecewise overrides, scope split, missing-client-id error, public-client path, timeout fallback) + logout idempotency via a storePathFn seam. Non-parallel (shared env/seam). go build/vet/test/-race clean; conventions_test passes; `grn login --help` lists the command; `grn login` with no client-id fails fast (exit 1). Per spec §7, client identity is flags+env only (never baked in the binary). Subcommand Bearer attachment and id_token/JWKS remain deferred (spec §1). Co-Authored-By: Claude <noreply@anthropic.com>
… URL The dev IAM signin page is a distinct host, not the shared prod signin (301→greennode.ai). Set the dev preset AuthorizeURL to https://dev-signin.vngcloud.tech/ap/auth so `grn login --iam-env dev` points the browser at the dev signin page; the dev token endpoint (pub-iamapis.api-dev.vngcloud.tech) is unchanged. Prod preset untouched. Verified reachable (HTTP/2, TLS on vngcloud.tech; bare GET 400s as expected since IAM rejects an authorize URL missing the camelCase params the CLI appends). go build/vet/test clean; the dev-preset test reads the map so it auto-tracks the new value. Co-Authored-By: Claude <noreply@anthropic.com>
Diagnosing a real IAM rejection (AUTHENTICATION_FAILED on the token POST) requires seeing what the POST actually carries. Gate a stderr trace on the existing global --debug flag via a library seam (SetDebug/dbg writing to the package's noisyStderr). The trace prints, BEFORE the browser opens: resolved authorize/token endpoints, client_id, redirect_uri, has_basic, secret_len (never the secret value), and the full authorize URL (safe — it carries the PKCE challenge, not the verifier). At the token POST it prints grant_type/client_id/redirect_uri/code_len/verifier_len/has_basic, then the result: transport error, or non-2xx status + capped (512 B) body, or http_ok + decoded access_token_len/token_type/refresh_present/expires_at. Security: every secret-shaped value (auth code, PKCE verifier, client secret, access token, refresh token) is redacted to LENGTH or PRESENCE only — never its value. A dedicated test (TestLogin_DebugTrace_RedactsSecretValues) asserts the access token / refresh token / auth code values do not appear in the trace, while non-secret context (client_id) and the redacted length form do. cmd/login runLogin enables the seam when --debug is set and restores it on return. go build/vet/test/-race clean. Co-Authored-By: Claude <noreply@anthropic.com>
…public clients) Real dev-IAM testing showed the public (no-secret) client rejects a textbook PKCE-only token POST with AUTHENTICATION_FAILED. Curl probes against dev's /v2 token endpoint isolated the cause: - no Basic (correct PKCE, bogus code) -> AUTHENTICATION_FAILED (auth gate) - Basic(client_id:) + bogus code -> AUTH_CODE_INVALID (past auth) - Basic(client_id:client_id) + bogus -> AUTH_CODE_INVALID (past auth) - client_secret in body -> AUTHENTICATION_FAILED (not honored) - fully anonymous -> AUTHENTICATION_FAILED VNG IAM's /v2 endpoint requires client_secret_basic for ALL clients, including "public" (no-secret) ones: the client_id travels as the Basic username and the password is empty (only the username is validated). Body secrets are NOT honored. The spec's §7 assumption (public client = PKCE-only, no Basic) does not match VNG IAM's actual behavior. Fix: tokencx.post now sends Basic unconditionally — SetBasicAuth(QueryEscape(clientID), QueryEscape(clientSecret)) — so confidential clients send the real secret (unchanged) and public clients send client_id with an empty password. PKCE (code_verifier) is still always sent alongside. Tests updated to match: tokencx_test decodes the Basic header and asserts username/password for both the public (empty) and confidential (secret) cases across ExchangeCode and Refresh; login_test's public-client case now asserts Basic IS sent. go build/vet/test/-race clean. Co-Authored-By: Claude <noreply@anthropic.com>
Fold the PKCE login refresh_token + non-secret refresh context (auth_mode, login_client_id, iam_env, token_expires_at) into the existing per-profile ~/.greennode/credentials INI, alongside machine client_id/client_secret — one auth-identity file per profile instead of a separate login_token.json. Fixes the dev/prod last-write-wins overwrite and closes the refresh-context gap the deferred usage-wiring slice needs. internal/login is mint-only now: Login(ctx, cfg) returns a Token and no longer persists (the library stays stdlib-only / profile-agnostic); the JSON store (store.go) is removed entirely. Persistence moves to the config layer: WriteLoginToken / ClearLoginToken write idempotently into a profile's section (preserving machine creds and other sections), and save() is hardened to atomic (temp + Chmod 0600 + same-dir Rename) so a crash mid-write can never truncate credentials+refresh and perms are re-asserted on rewrite. cmd/login resolves the profile (flag -> GRN_PROFILE -> default) like configure, persists on success (skips on partial / no refresh_token), and logout clears only the login keys. configure list/get mask refresh_token (secret-at-rest) and show auth_mode/login_client_id/iam_env/token_expires_at as-is. The access token is NEVER persisted — only the refresh token is (0600, masked in list/get). Greenfield: no migration of the old login_token.json; users re-login to land in the new location. Wiring the token into grn vks/vserver + agentbase usage stays deferred (next slice; this is its prerequisite). Verified: go build/vet/test -short ./... green; go test -race on cmd/login, cmd/configure, internal/login, internal/config green; isolated-HOME smoke test confirms per-profile isolation, masked refresh_token (no plaintext), non-secret context visible, logout clears login keys while keeping machine creds. Co-Authored-By: Claude <noreply@anthropic.com>
Bake a per-env public client_id into the binary so `grn login` needs no --client-id for dev: dev uses the real registered public client (23d6e219-...) and prod uses a zero-UUID placeholder (not a real client; prod login fails at IAM until it is replaced). No client_secret is ever baked — dev is public/no-secret, and the placeholder is not a credential. Rotating a baked id needs a rebuild, so it stays overridable via --client-id / GRN_LOGIN_CLIENT_ID (precedence: flag > env > embedded default). Add GRN_IAM_ENV so `GRN_IAM_ENV=dev grn login` selects dev without the flag (precedence: --iam-env > GRN_IAM_ENV > prod default). The --iam-env flag default moves to "" so the env var can apply; resolveIamEnv still falls back to prod when neither is set. runLogin now resolves iamEnv once and threads the SAME resolved value to resolveConfig AND WriteLoginToken — fixing a latent bug where a GRN_IAM_ENV=dev login would persist iam_env=prod (the raw flag default). When the resolved client_id is the prod placeholder, warn on stderr before hitting IAM so prod login fails with an explanation, not an opaque AUTHENTICATION_FAILED. Verified: build/vet/test -short green; -race on cmd/login/internal/login/ internal/config green; isolated-HOME smoke test confirms placeholder+warning on prod default, embedded dev id on --iam-env dev, and embedded dev id on GRN_IAM_ENV=dev (verified via the authorize URL the CLI builds). Co-Authored-By: Claude <noreply@anthropic.com>
The per-env IAM endpoint presets and baked-in public client_id constants move from cmd/login (local, unexported) to internal/login/iamenv.go (exported: IamEndpoints, DevClientID, ProdClientID, DefaultIamEnv, TokenURLForEnv, EndpointForEnv) so the login authorize flow and the forthcoming subcommand refresh-token path resolve the SAME /v2 token URL for a given iam_env. Replace the prod client_id placeholder (zero-UUID stand-in) with the real registered public client 09b427e9-3b45-437b-a3a6-bf9f7fd24185. dev's id is unchanged (70a17ade-b887-4354-9ecb-cfcfd06150b0). Both are PUBLIC no-secret OAuth client identifiers (RFC 6749) — safe to bake in; the client_secret is NEVER baked in. With a real prod id, drop the pre-IAM placeholder warning. cmd/login is repointed to loginpkg.* (no behavior change beyond the prod id swap); tests updated to the moved symbols. Co-Authored-By: Claude <noreply@anthropic.com>
Close the gap that a `grn login`-only profile could not call vks/vserver:
both errored on missing machine client_id/client_secret. A profile whose
auth_mode=user now mints short-lived access tokens from its persisted
refresh token via the IAM /v2 refresh_token grant and uses them as the
Bearer source; profiles without auth_mode=user keep today's machine
client_credentials behavior byte-for-byte. One auth type per profile,
profile-driven (no per-call flag/env override); a user profile that fails
refresh hard-errors pointing to `grn login` — no silent fallback to
machine creds.
Pieces:
- internal/client: TokenProvider interface in the consumer package;
*auth.TokenManager satisfies it structurally. The field/param moves
from the concrete type to the interface; the two GetToken/RefreshToken
call sites keep their shape.
- internal/auth/login_token.go (new): LoginTokenProvider — refresh_token
grant, 60s pre-expiry cache skew, best-effort rotation persist (new
refresh_token + expiry written back when IAM rotates), hard-error on
transport/non-2xx. Access token in-memory only, NEVER persisted.
- internal/cli/token_provider.go (new): NewTokenProvider branches on
cfg.AuthMode (user -> LoginTokenProvider, else -> TokenManager). Shared
by cli.NewClient (vks) and vserverclient.BuildClient (vserver).
- internal/login/iamenv.go: ClientIDForEnv + TokenURLForEnv shared by the
login authorize flow and the refresh path so a profile's iam_env
resolves the SAME /v2 token URL and the SAME baked-in public client_id
at login and at refresh.
- internal/login/login.go: export DecodeTokenBody for the refresh path.
- client_id NOT persisted: drop login_client_id from the Config struct,
the credentials write/read, and configure get/list. The refresh path
resolves the client_id from iam_env (ClientIDForEnv) — it is a public
id already baked into source. ClearLoginToken still scrubs a legacy
login_client_id key from older CLI versions; LoadConfig ignores it.
- rotation fix: the persist callback closes over cfg.Profile (the
RESOLVED profile from LoadConfig), not the raw --profile flag (" when
unset) — pre-fix rotation wrote an empty section name and the next
invocation refreshed against a STALE token.
- `grn login` prompts for the default region (same UX as `grn configure`)
so a login-only profile is immediately usable for vks/vserver without a
separate configure; existing output/project_id are preserved.
- internal/login/listen.go: the localhost callback page is static and
XSS-safe — only package-constant strings reach the template, never
query params.
Security: refresh_token is secret-at-rest (atomic 0600 write, masked in
configure list/get); access token is in-memory only; client_secret is
never baked in or persisted. Machine path unchanged.
Verified: go build ./... && go vet ./... && go test ./... (full) and
go test -race on the affected packages.
Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Brief description of what this PR does.
Type of change
Checklist
cd go && go test ./...)feat:/fix:/feat!:) — drives the releaseRelated issues
Closes #