fix(auth): verify credentials and classify authentication failures - #50
Open
jzeng-bw wants to merge 1 commit into
Open
fix(auth): verify credentials and classify authentication failures#50jzeng-bw wants to merge 1 commit into
jzeng-bw wants to merge 1 commit into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
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
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.
Fixes #24.
What was wrong
band auth status --plainreportedauthenticated: truewhenever the keychain lookup succeeded, even if the client secret had been revoked. The first real API command then failed during token exchange with exit 1, raw OAuth response text, and a full usage block. Scripts could neither trust the status check nor reliably recognize the re-authentication failure.What this changes
Verified status.
band auth statusperforms a fresh token exchange by default. Verification bypasses the token cache, because a token can outlive the secret that minted it. On success, the command reports fresh JWT accounts, roles, capabilities, and Build type without updating saved configuration or changing the selected account. Verification does not require an account ID and follows the same environment/endpoint selection as API commands.The existing JSON fields remain, with two additions:
{ "authenticated": true, "credentials_stored": true, "token": { "status": "valid", "expires_in": 3599 } }credentials_storedmeans the configured client ID and a non-empty keychain secret are available. It does not assert that they work.authenticatedbecomes true only after successful verification and JWT parsing.valid, withexpires_ininvalid_clientrejected/ recognized OAuth code (otherwisetoken_endpoint_error)unknown/probe_failedunknown/probe_failedunknown/not_logged_inunknown/credentials_unavailableunknown/not_verifiedMigration / offline mode. Status previously meant stored state and performed no token exchange. Scripts that need that check should use
band auth status --no-verify --plainand inspectcredentials_stored. In offline mode,authenticatedremains false; missing credentials are reported with the corresponding reason but exit 0. Config/output errors still fail. The background update checker is also skipped, so this mode does not make network requests. Default verification prints its structured result before returning a verification error; human-readable status and remediation go to stderr.Typed auth errors throughout the CLI. Token exchange now preserves a typed error carrying HTTP status, a recognized OAuth code, and the profile name. Rejected credentials map to exit 2 through the existing API wrapping chain, with guidance such as
band auth login --profile admin. Missing stored credentials also map to exit 2. Raw OAuth error bodies and unrecognized error strings are discarded, preventing server-echoed secrets from reaching diagnostics. Token 429 and 5xx remain distinguishable from rejection. Existing resource-API 403 behavior is unchanged.Runtime error output. The execution boundary renders errors once, suppresses usage for runtime failures, and unwraps auth diagnostics for readable remediation. Usage remains available for argument, unknown-flag, required-flag, and typed
FlagErrorvalidation failures. Entry intoRunEis tracked because Cobra checks required flags after pre-run hooks. Wrappers and rendering settings are restored after execution so repeated invocations do not retain suppression state.Cancellation. Login, status verification, and API token acquisition now pass the command context into the token request. This closes the token-exchange context gap left by #48 while retaining the 15-second HTTP timeout and cached-token behavior for ordinary API requests.
Scope and compatibility
shellescapewas already in the module graph; it is promoted to a direct dependency at the same version to quote profile names in remediation commands.mainand is independent of the open SIP fix in fix(sip): resolve short realm names through realm list #49.Tests
TestStatusVerification: valid and rejected credentials, OAuth 400 rejection, 403/429/500 distinctions, malformed responses/JWTs, offline inspection, missing credentials, cancellation, and connection failure. Checks stdout JSON, absence of secrets, fresh claims, request counts, and unchanged saved config.TestTokenVerificationRejectsRevokedSecretDespiteCachedToken: a cached token does not turn fresh credential rejection into success.TestTokenContextCancelsExchange: cancellation aborts an in-flight token request.TestTokenErrorDropsUnknownServerText: unrecognized provider text is not printed.TestExecuteOAuthFailure: token failures travel through a real API client and CLI execution boundary; asserts exit codes, profile remediation, no resource request, no raw secret, and no usage dump.TestExecuteUsageAndRepeatedInvocations: validation still prints usage after a prior runtime error, and execution settings are restored.Verified with Go 1.26.6: full
go test ./...,go vet ./..., CLI build, focused race tests for auth/commands/API/cmdutil,golangci-lint2.13.2 (zero issues), formatting, andgit diff --check. Live credentials were not used.This PR was substantially AI-generated with Codex.