feat(cli): add oauth login - #703
Conversation
60985b5 to
a186fd9
Compare
@storyblok/angular
@storyblok/astro
@storyblok/api-client
storyblok
@storyblok/experiments
@storyblok/js
storyblok-js-client
@storyblok/lint-config
@storyblok/live-preview
@storyblok/management-api-client
@storyblok/migrations
@storyblok/nuxt
@storyblok/react
@storyblok/region-helper
@storyblok/richtext
@storyblok/schema
@storyblok/svelte
@storyblok/vue
commit: |
a186fd9 to
e30f50a
Compare
|
Wait for the product feature to go live before merging! |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 5 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e30f50a. Configure here.
alexjoverm
left a comment
There was a problem hiding this comment.
@maoberlehner added some comments. Adding also here some minor Claude findings:
Minor (4):
- test/setup.ts session mock does not include clearOauthSession or authType: 'pat', which is fragile for future OAuth test paths.
- performInteractiveLogin returns { token: '', region } for OAuth -- the empty string token is a latent footgun.
- grant.ts uses raw fetch while actions.ts uses customFetch -- inconsistent but defensible; needs a clarifying comment.
- DEFAULT_LOGIN_SCOPES hard-codes a restrictive fallback for BYO-client path without warning the user.
Nit (1): colorPalette.OAUTH ?? colorPalette.PRIMARY fallback is dead code.
alexjoverm
left a comment
There was a problem hiding this comment.
BUG 1 — oauth setup --token hangs after API errors
The spinner at setup/index.ts:64 is never stopped when findOrCreateCliClient() throws. the process hangs indefinitely instead of exiting. Fix: add spinner.failed() in the catch block.
BUG 2 — PAT logout destroys OAuth client credentials
logout/index.ts:30 calls removeAllCredentials() which writes {} to the entire credentials file. This wipes out OAuth client credentials stored via oauth setup. Fix: only remove the PAT-specific entry (api.storyblok.com), not the whole file.
…f process.exit The OAuth space guard's process.exit(1) was a workaround for program.parse() being synchronous, which let async preAction hook rejections escape as unhandled rejections. Switch index.ts to program.parseAsync().catch() with a single top-level handleError + process.exitCode, and let the guard throw normally.
…eAction refresh path The preAction hook's OAuth refresh catch logged via getLogger(), but the logger has zero transports and getUI() is disabled at that point in the hook (both are only configured in Step 2, later in the same hook). The "run `storyblok login` again" re-login message was silently swallowed. Switch to konsola, which always prints, without changing the non-throwing behavior. Add program.oauth.test.ts covering the end-to-end preAction path: a successful proactive refresh initializes the mapi client with the new token, and a failed refresh (invalid_grant) surfaces the re-login message without throwing out of the hook.
The GET /v1/oauth/grant response nests its payload under a `grant` root key (storyrails renders `root: "grant", adapter: :json`), but introspectGrant read scopes and spaces at the top level, so both always resolved to empty. Empty spaces silently disabled the space-scoping guard, letting an OAuth session target any space, and the login summary reported no granted scopes. Unwrap the `grant` root before reading the fields, and fix the grant and login integration tests that mocked the un-nested shape. Fixes DX-490
requireAuthentication gated on state.password, which OAuth sessions never set (they populate oauthAccessToken), so every command using it rejected OAuth logins with "You are currently not logged in" even though the mapi client was already configured with the OAuth token in the preAction hook. Accept OAuth sessions in requireAuthentication (credential is a PAT password or an OAuth access token) and add a sessionCredential() helper that resolves the API credential. Make getUser credential-aware (PAT or OAuth token) and update the user and create commands to pass the resolved credential instead of assuming a password. Fixes DX-490
Mirror the assets push README layout (intro, Basic Usage examples, Options tables) and drop the redundant "(manual path)" wording from the setup client flag help. Fixes DX-490
- Introspect the grant before persisting tokens so a failed introspection cannot leave tokens on disk without a spaces list (which the space guard would treat as unrestricted). - Preserve provisioned client credentials on logout: clear tokens and spaces only, so users need not re-run `oauth setup` before the next login. - Bind the OAuth callback server to loopback (127.0.0.1) only. - Trim the oauth command README to the terse option-table style. - Remove the unused defaultGrantSpace export. Fixes DX-490
Rename `Oauth*` symbols to `OAuth*` to match the canonical OAuth spelling used in the OAuth 2.0 spec, as raised in review. Pure rename; no behavior change. The storyrails `OauthGrantIntrospectionSerializer` reference in a comment is left as-is since it names a backend class. Fixes DX-490
- login 401 error now distinguishes OAuth sessions from PATs and points to `storyblok login --oauth` for re-authentication. - performInteractiveLogin returns a discriminated union instead of a sentinel empty-string token for OAuth logins. - introspectGrant uses customFetch (retries + uniform errors) like the other oauth requests instead of raw fetch. - warn when an OAuth login falls back to the default scope set for a bring-your-own client with no stored scope catalog. - document the deterministic eu-first region precedence when loading an oauth session across regions. - add authType and clearOAuthSession to the session test mock. - drop the unreachable colorPalette.PRIMARY fallback in oauth setup. Fixes DX-490
Store an `activeRegion` pointer in the oauth credentials section and set it on each successful `login --oauth`. When multiple regions are authenticated, the session now loads the most recently used region instead of a hardcoded eu-first order. Resolution falls back to the fixed order when the pointer is unset (pre-existing sessions) or stale, and logout of the active region drops the pointer so the next session resolves to a remaining region. An explicit `--region` still overrides. Fixes DX-490
- resolve login scopes from the resolved OAuth client so credentials and scopes share a source; env-var clients fall back to the default set - serve a non-200 callback page on authorization errors instead of the success page - validate non-empty client secret and PAT setup prompts - route the token endpoint through customFetch for retry and consistency - make GrantIntrospection.expires_at optional and drop its fallback - drop a duplicate blank line on logout Fixes DX-490
Revoke the OAuth grant server-side on logout so tokens can no longer mint new tokens after the session is cleared. Revocation is best-effort: a network/API failure warns but never blocks the local logout. The revoke call uses a raw fetch because /oauth/revoke returns 200 with an empty body (RFC 7009 / storyrails `head :ok`), which customFetch would reject. Also stop PAT logout from wiping the entire credentials file: it now clears only the PAT machine entries and preserves the `oauth` section (provisioned client credentials and any other-region OAuth sessions). Fixes DX-490
The provisioning spinner was created inside the try block, so a failure in findOrCreateCliClient left it spinning (keeping the event loop alive) while the catch reported the error. Hoist it and call spinner.failed() on error. Also render the command title as "OAuth setup" rather than "Oauth setup". Fixes DX-490
initializeSession() resolves the OAuth region purely from the activeRegion pointer and fixed-order fallback; a command's --region does not override it. Reword the comment to say re-login to switch the active region. Fixes DX-490
Post-rebase alignment: `konsola` and `src/utils/ui` were removed on main in favor of `src/lib/ui`, so the oauth command, login/logout output, and their tests now use `getUI()` from `lib/ui`.
The OAuth app registers http://localhost:4900/oauth/callback as its only redirect URI, so the CLI cannot retry on a free port. A busy port surfaced a bare EADDRINUSE with nothing actionable in it. Look up the listening process (lsof on Unix, netstat plus tasklist on Windows) and report its name, PID, and the command to stop it. The lookup is best-effort: a missing tool, denied permission, or timeout degrades to a generic message plus the lookup command, so a diagnostic never becomes a second failure. Fixes DX-490
`storyblok oauth setup` existed only because the CLI had no first-party OAuth client: users had to provision one with a PAT (org manager role plus an OAuth-enabled org) or paste a client id and secret. Product confirmed that hardcoding the client id and secret of a public integration app in the CLI is approved, and that one app covers every region, so the whole command goes away and `storyblok login --oauth` needs no configuration, matching gh and gcloud. The client is public: the secret is not a security boundary, PKCE protects the code exchange. Both values are placeholders until the "Storyblok CLI" app is registered; while they are, `--oauth` fails fast with an explanation instead of opening a broken authorization page. Scopes become one hardcoded constant covering the full catalog (storyrails GROUPED_SCOPES plus offline_access) so a single consent covers every command, replacing the catalog previously fetched at setup time. This must stay a subset of the registered app's allowed_scopes. The credential store drops its per-region `client` entry, so logout clears a region outright. STORYBLOK_OAUTH_CLIENT_ID/_SECRET still override the baked-in client for development and self-hosted instances. Fixes DX-490
The description only named explicit test requests, so any task that had to run for real against a live space, seeding, credentials, driving a built CLI, could skip the skill. Name those cases too.
A local build refuses `login --oauth` until the client env vars are set, and the flow leaves state on disk that later tests inherit, so the manual test guide needs the setup, the credential file, and the cases unit tests cannot reach. The QA env template gains the matching STORYBLOK_OAUTH_CLIENT_ID/_SECRET keys. Fixes DX-490
7d0b27b to
668ff70
Compare

Adds OAuth login (Authorization Code grant with PKCE) to the Storyblok CLI, alongside the existing PAT and email/password flows.
storyblok login --oauth(also selectable in interactivestoryblok login) opens the browser for consent, receives the callback onhttp://localhost:4900/oauth/callback, exchanges the code for tokens, and introspects the grant for scopes and authorized spaces.ghandgclouduse. It is a public client, so the secret is not a security boundary; PKCE protects the code exchange. One integration app covers every region.STORYBLOK_OAUTH_CLIENT_ID/STORYBLOK_OAUTH_CLIENT_SECRETstill override it for development or self-hosted instances.TODO(DX-490)inoauth/constants.ts). Until then--oauthfails fast with guidance to use a PAT instead of opening a broken authorization page.GROUPED_SCOPESplusoffline_access) in one consent, so every command is covered. This must stay a subset of the registered app'sallowed_scopes.oauthkey in~/.storyblok/credentials.json(mode0600), kept separate from PAT entries. AnactiveRegionpointer records the region most recently logged into, so the right session loads when several regions are authenticated.storyblok logoutclears the OAuth session for the region, including theactiveRegionpointer.Fixes DX-490