feat(cli): add --api-key option to auth login#4472
Conversation
`superset auth login --api-key sk_live_…` validates the key, stores it at `~/.superset/config.json`, and clears any prior OAuth session. Subsequent commands resolve auth from the stored key with a new `config` auth source. `auth logout` now clears the stored key alongside the OAuth session, and `auth whoami` surfaces the new source.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds API-key authentication to the Superset CLI: config now stores ChangesAPI Key Authentication Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
Greptile SummaryThis PR adds
Confidence Score: 3/5The API-key login path writes credentials to disk and deletes the OAuth session before org selection completes; a bad --organization value leaves the user's auth state changed without a clear recovery message. The core resolveAuth logic and test coverage are solid, but the login command commits irreversible credential changes mid-flow before org-selection can fail, making the command non-atomic in a way that silently corrupts stored auth on a simple typo in --organization. packages/cli/src/commands/auth/login/command.ts — specifically the runApiKeyLogin function where config is written before org selection resolves.
|
| Filename | Overview |
|---|---|
| packages/cli/src/commands/auth/login/command.ts | Adds --api-key login path; config is committed to disk (OAuth session deleted, API key stored) before org selection succeeds, leaving credentials in a partial state on error. |
| packages/cli/src/lib/resolve-auth.ts | Adds config as a new auth source; falls back to config.apiKey between explicit override and OAuth, logic is correct and well-structured. |
| packages/cli/src/lib/resolve-auth.test.ts | New test file covering all nine auth-resolution branches including flag, env, config, oauth, expired session, and priority ordering. |
| packages/cli/src/lib/config.ts | Adds optional apiKey field to SupersetConfig; straightforward schema change, permissions (0o600) already enforced. |
| packages/cli/src/commands/auth/logout/command.ts | Adds delete config.apiKey alongside the existing delete config.auth; correctly clears both credential types on logout. |
| packages/cli/src/commands/auth/whoami/command.ts | Adds explicit config branch to the authSource switch; now each source is handled explicitly rather than relying on the else fallback. |
Sequence Diagram
sequenceDiagram
participant User
participant CLI as auth login command
participant API as Superset API
participant FS as ~/.superset/config.json
alt --api-key flag provided
User->>CLI: superset auth login --api-key sk_live_…
CLI->>API: user.me (x-api-key header) [validate]
API-->>CLI: user info
CLI->>FS: "write {apiKey, delete auth} ⚠️ before org selection"
CLI->>API: user.myOrganizations / myOrganization
API-->>CLI: org list
CLI->>API: user.me (again, for userId output)
API-->>CLI: user info
CLI->>FS: "write {apiKey, organizationId}"
CLI-->>User: Logged in successfully
else OAuth flow (default)
User->>CLI: superset auth login
CLI->>API: OAuth loopback exchange
API-->>CLI: accessToken / refreshToken
CLI->>FS: "write {auth, delete apiKey}"
CLI->>API: user.myOrganizations
API-->>CLI: org list
CLI->>FS: "write {auth, organizationId}"
CLI-->>User: Logged in successfully
end
note over CLI,FS: resolveAuth priority: flag → env → config.apiKey → oauth
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/cli/src/commands/auth/login/command.ts:126-129
**Config clobbered before org selection succeeds**
`config.apiKey` is written to disk and `config.auth` is deleted at lines 127–129 — before `selectOrganization` is called. If org selection then throws (e.g., `--organization nonexistent-slug` which triggers `CLIError` inside `selectOrganization`), the user is left in a state where their previous OAuth session is permanently gone and the new API key is stored without an organization ID. The error the user sees ("Organization not found") gives no indication that their auth credentials were modified. Moving the `writeConfig` call to after `selectOrganization` returns successfully (or passing the org as part of the same write) would prevent this partial-write state.
### Issue 2 of 3
packages/cli/src/commands/auth/login/command.ts:117-118
**Redundant `user.me` call**
`api.user.me.query()` is called here to validate the key and print the user's name/email, but `user.id` is never used from this result. A second `api.user.me.query()` is made inside `selectOrganization` (to populate `output.userId`). The `user` object fetched here could be forwarded to `selectOrganization` to avoid the extra round-trip.
### Issue 3 of 3
packages/cli/src/commands/auth/login/command.ts:25-29
**Duplicated argv-scanning logic**
`apiKeyFlagInArgv()` here is byte-for-byte identical to `flagApiKeyFromArgv()` in `resolve-auth.ts` — both scan `process.argv` for the `--api-key` flag name. Extracting this to a single shared utility (e.g., `lib/argv.ts`) would remove the duplication and ensure both call-sites stay in sync if the flag name ever changes.
Reviews (1): Last reviewed commit: "feat(cli): add --api-key option to auth ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const config = readConfig(); | ||
| config.apiKey = apiKey; | ||
| delete config.auth; | ||
| writeConfig(config); |
There was a problem hiding this comment.
config.apiKey is written to disk and config.auth is deleted at lines 127–129 — before selectOrganization is called. If org selection then throws (e.g., --organization nonexistent-slug which triggers CLIError inside selectOrganization), the user is left in a state where their previous OAuth session is permanently gone and the new API key is stored without an organization ID. The error the user sees ("Organization not found") gives no indication that their auth credentials were modified. Moving the writeConfig call to after selectOrganization returns successfully (or passing the org as part of the same write) would prevent this partial-write state.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 126-129
Comment:
**Config clobbered before org selection succeeds**
`config.apiKey` is written to disk and `config.auth` is deleted at lines 127–129 — before `selectOrganization` is called. If org selection then throws (e.g., `--organization nonexistent-slug` which triggers `CLIError` inside `selectOrganization`), the user is left in a state where their previous OAuth session is permanently gone and the new API key is stored without an organization ID. The error the user sees ("Organization not found") gives no indication that their auth credentials were modified. Moving the `writeConfig` call to after `selectOrganization` returns successfully (or passing the org as part of the same write) would prevent this partial-write state.
How can I resolve this? If you propose a fix, please make it concise.| try { | ||
| user = await api.user.me.query(); |
There was a problem hiding this comment.
user.me call
api.user.me.query() is called here to validate the key and print the user's name/email, but user.id is never used from this result. A second api.user.me.query() is made inside selectOrganization (to populate output.userId). The user object fetched here could be forwarded to selectOrganization to avoid the extra round-trip.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 117-118
Comment:
**Redundant `user.me` call**
`api.user.me.query()` is called here to validate the key and print the user's name/email, but `user.id` is never used from this result. A second `api.user.me.query()` is made inside `selectOrganization` (to populate `output.userId`). The `user` object fetched here could be forwarded to `selectOrganization` to avoid the extra round-trip.
How can I resolve this? If you propose a fix, please make it concise.| function apiKeyFlagInArgv(): boolean { | ||
| return process.argv.some( | ||
| (arg) => arg === "--api-key" || arg.startsWith("--api-key="), | ||
| ); | ||
| } |
There was a problem hiding this comment.
apiKeyFlagInArgv() here is byte-for-byte identical to flagApiKeyFromArgv() in resolve-auth.ts — both scan process.argv for the --api-key flag name. Extracting this to a single shared utility (e.g., lib/argv.ts) would remove the duplication and ensure both call-sites stay in sync if the flag name ever changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/commands/auth/login/command.ts
Line: 25-29
Comment:
**Duplicated argv-scanning logic**
`apiKeyFlagInArgv()` here is byte-for-byte identical to `flagApiKeyFromArgv()` in `resolve-auth.ts` — both scan `process.argv` for the `--api-key` flag name. Extracting this to a single shared utility (e.g., `lib/argv.ts`) would remove the duplication and ensure both call-sites stay in sync if the flag name ever changes.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/docs/content/docs/cli/getting-started.mdx (1)
80-90: ⚡ Quick winConsider showing example output for API key authentication.
The "Sign in" section now documents API key authentication (lines 62-76), but the example output here only shows OAuth session-based auth. Users who follow the API key path will see different output. Consider adding a second example or note showing what
whoamireports when authenticated via API key (e.g.,API key (stored via auth login --api-key)).📝 Suggested addition
After line 90, you could add:
Auth: Session (expires in 32 min)
+When authenticated with an API key via
auth login --api-key:
+
+text +Signed in as Satya Patel (you@example.com) +Organization: Acme +Auth: API key (stored via auth login --api-key) +</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@apps/docs/content/docs/cli/getting-started.mdxaround lines 80 - 90, Add a
second example output under the "Verify your session" section (the block showing
the result of running the superset auth whoami command) to show what the command
prints when authenticated via API key; specifically, after the existing OAuth
session example add a note or code block demonstrating the API-key case (e.g.,
"Auth: API key (stored via auth login --api-key)") so readers who used the auth
login --api-key flow see the correct whoami output.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@packages/cli/src/lib/resolve-auth.test.ts:
- Around line 9-10: Save the original process.env.SUPERSET_HOME_DIR before
overriding it with tempHome in the test suite (e.g., const originalHome =
process.env.SUPERSET_HOME_DIR), set process.env.SUPERSET_HOME_DIR = tempHome for
the test, and then restore it in an afterEach or afterAll hook
(process.env.SUPERSET_HOME_DIR = originalHome) so the global env var is returned
to its prior value; update the same pattern wherever
process.env.SUPERSET_HOME_DIR is changed (see occurrences around the tempHome
assignment and the cleanup that removes tempHome).
Nitpick comments:
In@apps/docs/content/docs/cli/getting-started.mdx:
- Around line 80-90: Add a second example output under the "Verify your session"
section (the block showing the result of running the superset auth whoami
command) to show what the command prints when authenticated via API key;
specifically, after the existing OAuth session example add a note or code block
demonstrating the API-key case (e.g., "Auth: API key (stored via auth login
--api-key)") so readers who used the auth login --api-key flow see the correct
whoami output.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `47038996-b9ae-4623-a681-e6088cd94868` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6735f924ec58ea498b3aa554679289ad09dd791f and b8411a4e455b34ec1f3d82ee59703d67eea52b9b. </details> <details> <summary>📒 Files selected for processing (9)</summary> * `apps/docs/content/docs/cli/cli-reference.mdx` * `apps/docs/content/docs/cli/getting-started.mdx` * `packages/cli/CLI_SPEC_TARGET.md` * `packages/cli/src/commands/auth/login/command.ts` * `packages/cli/src/commands/auth/logout/command.ts` * `packages/cli/src/commands/auth/whoami/command.ts` * `packages/cli/src/lib/config.ts` * `packages/cli/src/lib/resolve-auth.test.ts` * `packages/cli/src/lib/resolve-auth.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/CLI_SPEC_TARGET.md">
<violation number="1" location="packages/cli/CLI_SPEC_TARGET.md:229">
P2: The `auth login` side-effects text is now inconsistent with the new `--api-key` flow: it still says login writes `auth`, but API-key login writes `apiKey` and clears `auth`. Update this to describe both modes.</violation>
</file>
<file name="packages/cli/src/lib/resolve-auth.test.ts">
<violation number="1" location="packages/cli/src/lib/resolve-auth.test.ts:9">
P2: Restore `SUPERSET_HOME_DIR` after the suite to avoid leaking global env state into other tests.</violation>
</file>
<file name="packages/cli/src/commands/auth/login/command.ts">
<violation number="1" location="packages/cli/src/commands/auth/login/command.ts:160">
P2: `--api-key=` (or whitespace-only values) is treated as "no API key", so the command unexpectedly falls back to OAuth instead of failing fast on invalid input.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
`--api-key` and `SUPERSET_API_KEY` are resolved at the same step already; tracking them as separate `authSource` values added a process.argv sniff in resolveAuth purely to label the same key differently in whoami. Collapse to one value.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/CLI_SPEC_TARGET.md`:
- Around line 229-237: Update the "login side-effects" wording to explicitly
describe both flows: state that the API-key flow (--api-key) validates via
user.me with x-api-key and on success writes apiKey to ~/.superset/config.json
(removing any stored OAuth auth), the OAuth flow stores auth in
~/.superset/config.json, and both flows persist organizationId; replace the
OAuth-only phrasing in the side-effects paragraph with this consolidated
contract mentioning auth, apiKey, and organizationId by name and preserving the
validation/exit behavior for API-key mode.
In `@packages/cli/src/lib/resolve-auth.ts`:
- Around line 29-31: Trim config.apiKey before using it: in the resolve-auth
logic where you handle the config path (the branch that sets bearer =
config.apiKey and authSource = "config"), normalize by calling .trim() on
config.apiKey (like apiKeyOption is normalized) and assign the trimmed value to
bearer and set authSource; also ensure you only set authSource to "config" when
the trimmed value is non-empty so OAuth fallback is not blocked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6c6c3960-1ab9-4e04-b6dc-e7459557079e
📒 Files selected for processing (5)
apps/docs/content/docs/cli/cli-reference.mdxpackages/cli/CLI_SPEC_TARGET.mdpackages/cli/src/commands/auth/whoami/command.tspackages/cli/src/lib/resolve-auth.test.tspackages/cli/src/lib/resolve-auth.ts
✅ Files skipped from review due to trivial changes (1)
- apps/docs/content/docs/cli/cli-reference.mdx
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/commands/auth/whoami/command.ts">
<violation number="1" location="packages/cli/src/commands/auth/whoami/command.ts:15">
P2: `auth whoami` no longer reports the exact API key source (flag vs env); it always emits a combined override label.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } else if (ctx.authSource === "flag") { | ||
| authLine = "API key (from --api-key flag)"; | ||
| } else if (ctx.authSource === "override") { | ||
| authLine = "API key (from --api-key flag or SUPERSET_API_KEY env)"; |
There was a problem hiding this comment.
P2: auth whoami no longer reports the exact API key source (flag vs env); it always emits a combined override label.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/auth/whoami/command.ts, line 15:
<comment>`auth whoami` no longer reports the exact API key source (flag vs env); it always emits a combined override label.</comment>
<file context>
@@ -11,10 +11,8 @@ export default command({
- } else if (ctx.authSource === "env") {
- authLine = "API key (from SUPERSET_API_KEY env)";
+ } else if (ctx.authSource === "override") {
+ authLine = "API key (from --api-key flag or SUPERSET_API_KEY env)";
} else {
authLine = "API key (stored via auth login --api-key)";
</file context>
- Defer the apiKey write until pickOrganization() validates the requested org, so `--api-key X --organization bad-slug` no longer clobbers the user's OAuth session before throwing. - Reuse the user fetched for validation so we don't call user.me twice on each login. - Fail fast on `--api-key=` (empty/whitespace) instead of silently falling back to OAuth. - Restore SUPERSET_HOME_DIR after resolve-auth tests so the env doesn't leak between test files. - Update CLI_SPEC_TARGET.md side-effects to describe both write paths.
If config.apiKey was hand-edited and ends up with surrounding whitespace, we'd send a junk key and the request would fail without falling back to OAuth. Treat whitespace-only as empty so we skip to the OAuth branch.
Changes since v0.2.15: - workspaces: `superset workspaces list` table now shows the workspace ID column. (#4463) - docs: VHS demo walkthrough recording added under `packages/cli/demo/`. (#4461) - auth: `superset auth login --api-key sk_live_…` stores an API key at `~/.superset/config.json` instead of running the OAuth flow; `whoami` and `logout` updated to recognize stored keys. (#4472) - hosts: `superset hosts list` shows `local` for the host machine you're invoking from, distinct from `online`/`no`. (#4476) - automations: `--agent` is now the host agent instance/preset id (e.g. `claude`, `codex`, `superset`) and is resolved live from the host on every run. The `--agent-config-file` flag is removed; create/update rename `agentConfig` -> `agent` end-to-end. (#4481) Push cli-v0.2.16 after this lands to fire the release pipeline.
1 participant
Summary
Adds
--api-keytosuperset auth login, storing the key at~/.superset/config.jsonso subsequent commands authenticate without re-supplying it.superset auth login --api-key sk_live_…validates the key viauser.meand stores it. Mutually exclusive with OAuth — passing this clears any storedauthsession, and a fresh OAuth login clears any storedapiKey.resolveAuthfalls back to the stored key when no--api-keyflag orSUPERSET_API_KEYenv override is set. NewauthSourceis"override" | "config" | "oauth"(the previousflag/envsplit was cosmetic and required argv-sniffing — collapsed tooverride).auth logoutclears both the stored API key and the OAuth session.auth whoamireports the new source.--api-key X --organization bad-slugno longer clobbers an existing OAuth session before erroring (greptile P1).--api-key=(empty/whitespace value) now fails fast instead of silently falling through to OAuth.Docs updated:
apps/docs/content/docs/cli/{cli-reference,getting-started}.mdx,packages/cli/CLI_SPEC_TARGET.md.Test plan
bun test packages/cli/src/lib/resolve-auth.test.ts— 7 tests covering precedence: no creds → error, override →overridesource, stored key →configsource, unexpired OAuth →oauthsource, expired OAuth without refresh → "Session expired", override beats stored key, stored key beats stored OAuth.bun run typecheck(full monorepo, 29 packages).bun run lintclean.localhost:6801withSUPERSET_HOME_DIR=$(mktemp -d):apiKey+organizationId, auto-selects single org.auth whoami→authSource: "config".--api-key=→ "Option --api-key requires a non-empty value".--organization bad-slugover existing OAuth → errors after listing valid slugs; existing OAuth preserved, noapiKeywritten.--organization <slug>→ no prompt, writes both fields.auth logout→ clearsauthandapiKey, preservesorganizationId(per spec).SUPERSET_API_KEY=… auth whoami→authSource: "override",config.jsonuntouched.Review responses
--organization.user.mecall) → fixed; user object is fetched once and forwarded.resolve-auth.ts+login/command.ts) → resolved by collapsingflag/envintooverridein commit 2; theresolve-auth.tscopy is gone, onlylogin/command.tskeeps the sniff for the storage-vs-ephemeral distinction.--api-key=falls through to OAuth) → fixed; throws "Option --api-key requires a non-empty value".SUPERSET_HOME_DIR) → fixed inafterAll.Summary by CodeRabbit
Release Notes
New Features
superset auth login --api-keyto authenticate with stored credentials, or setSUPERSET_API_KEYenvironment variable for per-invocation accesssuperset auth logoutto clear both OAuth sessions and stored API keyssuperset auth whoamito display authentication source (API key or OAuth)Documentation