Skip to content

refactor: delegate auth flow to @doist/cli-core/auth#215

Merged
scottlovegrove merged 3 commits into
mainfrom
refactor/cli-core-auth
May 11, 2026
Merged

refactor: delegate auth flow to @doist/cli-core/auth#215
scottlovegrove merged 3 commits into
mainfrom
refactor/cli-core-auth

Conversation

@scottlovegrove
Copy link
Copy Markdown
Collaborator

Summary

  • Drops the local OAuth coordination (src/lib/pkce.ts, oauth.ts, oauth-server.ts, commands/auth/login.ts body) and delegates the flow to @doist/cli-core/auth's runOAuthFlow + attachLoginCommand.
  • New TwistAuthProvider (src/lib/auth-provider.ts) owns the twist-specific behaviour:
    • prepare() — Dynamic Client Registration via POST /oauth/register.
    • authorize() — builds the twist authorize URL using cli-core's generateVerifier / deriveChallenge.
    • exchangeCode()client_secret_basic token exchange with the PKCE verifier.
    • validateToken()users.getSessionUser to populate the TwistAccount.
  • New TwistTokenStore adapter wraps the existing saveApiToken / probeApiToken / clearApiToken so cli-core writes the token through the same dual-storage (keyring + config fallback) twist already uses elsewhere.
  • Branded success / error HTML (logo, terminal mockup, 30-second auto-close) preserved byte-for-byte in src/lib/auth-pages.ts and passed to attachLoginCommand via renderSuccess / renderError.
  • tw auth login gains --callback-port, --json, and --ndjson for free from attachLoginCommand (was --read-only only).
  • Net: 9 files changed, +618 / −764. 580/580 tests pass. Login-flow tests retargeted to provider + store units in src/lib/auth-provider.test.ts.

Test plan

  • npm run type-check
  • npm run lint:check (oxlint + oxfmt)
  • npm test (580 tests pass)
  • npm run build
  • Smoke: tw auth logout && tw auth login against twist.com — branded success page renders, token persisted, green checkmark printed
  • tw auth login --help lists --read-only, --callback-port, --json, --ndjson

Replaces the local OAuth coordination (pkce, oauth, oauth-server, login)
with @doist/cli-core/auth's runOAuthFlow + attachLoginCommand. A new
TwistAuthProvider handles the twist-specific bits — Dynamic Client
Registration, client_secret_basic token exchange, and getSessionUser
validation — while cli-core owns the PKCE primitives, callback server,
browser open, and timeout. Branded success/error HTML preserved verbatim
in src/lib/auth-pages.ts and passed through renderSuccess / renderError.

`tw auth login` gains --callback-port, --json, and --ndjson for free
from attachLoginCommand. Net: −146 LOC, 580/580 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove self-assigned this May 11, 2026
@doistbot doistbot requested a review from pawelgrimm May 11, 2026 16:07
Copy link
Copy Markdown
Member

@doistbot doistbot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR successfully delegates the OAuth flow to @doist/cli-core/auth by introducing a custom Twist provider and token store while preserving the existing branded auth pages. The changes cleanly reduce local orchestration boilerplate and standardize the login command's behavior across the CLI. A few areas need adjustment, primarily restoring structured error handling for network failures, refining the token store's account metadata handling, updating the skill content for the newly added command flags, and addressing a couple of minor test wiring gaps.

Share FeedbackReview Logs

Comment thread src/commands/auth/login.ts
Comment thread src/commands/auth/login.ts Outdated
Comment thread src/lib/auth-provider.ts
Comment thread src/lib/auth-provider.test.ts Outdated
Comment thread src/lib/auth-provider.ts Outdated
Comment thread src/commands/auth/auth.test.ts
Comment thread src/lib/auth-provider.test.ts Outdated
scottlovegrove and others added 2 commits May 11, 2026 17:28
- [P1] Rewrap raw fetch rejections + JSON-parse failures in `prepare()` and
  `exchangeCode()` as `AUTH_FAILED` CliError with hints, restoring the
  catch-and-convert behavior the old `oauth.ts` helpers had.
- [P2] Narrow `TwistAccount` to the fields that round-trip through the local
  token store ({ id, label, authMode, authScope }). Persist `authUserId` /
  `authUserName` alongside the token so `TokenStore.active()` rebuilds the
  same identity that `validateToken()` and `set()` work with — no more
  placeholder 'twist' account. `active()` returns null when the stored
  token predates this adapter (env var or pre-upgrade config).
- [P2] Suppress `console.log` in `onSuccess` when `--json` or `--ndjson` is
  active so the cli-core machine-output envelope on stdout stays clean.
- [P2] Document `--callback-port`, `--json`, `--ndjson` on `tw auth login`
  in `SKILL_CONTENT` and regenerate the published skill file.
- [P2] Switch the provider test mock to `importOriginal` for the real
  `NoTokenError` instead of a handcrafted stub.
- [P2] Add `attachLoginCommand` wiring assertions to `auth.test.ts` —
  provider/store/preferredPort/renderers and scope resolution.
- [P2] Add rejected-fetch (network failure) cases to provider tests for
  both `prepare()` and `exchangeCode()`, and consolidate the existing
  error-path tests into single shared cases per method.

Tests: 579 pass; type-check, lint, and oxfmt all clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The local declarations in auth.ts (TokenStorageResult, SaveApiTokenOptions,
AuthMetadata, AuthProbeMetadata, AuthProbeResult) and auth-provider.ts
(TwistAccount, TwistHandshake, TwistTokenStore) are only used as types —
no implements, no declaration merging — so type aliases express intent
more directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@scottlovegrove scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 11, 2026
@scottlovegrove scottlovegrove merged commit f281e9b into main May 11, 2026
4 checks passed
@scottlovegrove scottlovegrove deleted the refactor/cli-core-auth branch May 11, 2026 16:36
Copy link
Copy Markdown

@pawelgrimm pawelgrimm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

scottlovegrove added a commit to Doist/outline-cli that referenced this pull request May 11, 2026
## Summary

- Drops the bespoke OAuth stack (`src/lib/pkce.ts`, `oauth.ts`,
`oauth-server.ts`, the entire login action in `commands/auth.ts`) and
delegates the flow to `@doist/cli-core/auth`'s `runOAuthFlow` +
`attachLoginCommand`. Mirrors
[twist-cli#215](Doist/twist-cli#215).
- New `OutlineAuthProvider` (`src/lib/auth-provider.ts`) owns the
outline-specific behaviour:
- `authorize()` — builds the `/oauth/authorize` URL with cli-core's
`generateVerifier` / `deriveChallenge`. Resolves `--base-url` /
`--client-id` from flags → env → config → interactive prompt.
- `exchangeCode()` — form-urlencoded POST to `/oauth/token`. Public
client — no `client_secret`, no `Authorization: Basic` header (unlike
twist).
- `validateToken()` — hits `auth.info` with the unsaved token via a new
`apiRequest(path, body, { token, baseUrl })` override so we can identify
the user before persisting.
- New `OutlineTokenStore` adapter maps the cli-core account onto the
existing config file (adds `auth_user_id` / `auth_user_name` to
round-trip the identity through `store.active()`).
- Branded success / error HTML preserved byte-for-byte in
`src/lib/auth-pages.ts` and passed via `renderSuccess` / `renderError`.
- `ol auth login` gains `--read-only`, `--callback-port`, `--json`,
`--ndjson` for free from `attachLoginCommand`; `--base-url` /
`--client-id` chained onto the returned command.
- **Breaking:** `ol auth login --token <token>` flag is gone. Login is
OAuth-only now. Set `OUTLINE_API_TOKEN` env var if you need to script
with a personal token.
- Net: 14 files changed, +625 / −688. 115 tests pass.

## Test plan

- [x] `npm run type-check`
- [x] `npm run lint:check`
- [x] `npm test` (115 tests pass)
- [x] `npm run build`
- [x] `npm run check:skill-sync`
- [x] `ol auth login --help` lists `--read-only`, `--callback-port`,
`--json`, `--ndjson`, `--base-url`, `--client-id`
- [ ] Smoke: `ol auth logout && ol auth login --base-url https://…
--client-id …` against a real Outline workspace — branded success page
renders, token persisted, terminal prints green "Authenticated to … as
…"
- [ ] `ol auth status` reports team + user
- [ ] `ol auth login --json` emits machine-readable success envelope

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@doist-release-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 2.36.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released 👀 Show PR PR must be reviewed before or after merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants