Skip to content

feat: support keyless PKCE public clients - #127

Open
nicknisi wants to merge 6 commits into
mainfrom
riker/98-bring-keyless-pkce-public-clients-to-wor
Open

nicknisi wants to merge 6 commits into
mainfrom
riker/98-bring-keyless-pkce-public-clients-to-wor

Conversation

@nicknisi

@nicknisi nicknisi commented Oct 7, 2026

Copy link
Copy Markdown
Member

Bring keyless PKCE public clients to @workos/authkit-tanstack-react-start, the same experience as workos/authkit-react-router#96 (workos/authkit-react-router#96). It builds on the authkit-session change from the preceding job, whose branch and PR it names.

authkit-tanstack-start delegates to @workos/authkit-session (src/server/authkit-loader.ts calls createAuthService and getConfig). So:

  • Develop against the new authkit-session. Use the authkit-session job's packed build (its worktree under ~/.riker/worktrees; find it from the preceding job's branch riker/<n>-let-workos-authkit-session-run-as-a-keyle… or ask), via a pnpm override in this worktree only while developing. Never commit a file:, link: or override dependency.
  • Make the package's own surface keyless-ready: validateConfig, middleware options, any configure/options types re-exported from authkit-session, and anything here that assumes an API key. Key-only features must surface authkit-session's clear error.
  • README: a public-client (keyless) section matching fix: update to authkit-session@0.5.3 #96's.
  • Unit tests here: config validation in both modes, middleware/sign-in working without a key, and the clear error for key-only features.
  • End to end with example/: copy it to a scratch directory, point it at both packed builds (authkit-session and this package), run it with no WORKOS_API_KEY, and show the sign-in route redirects with code_challenge and S256. Leave the dev server running and give Nick the URL and steps to sign in, see the session load, and see a refresh succeed.
  • The dependency: the committed package.json should keep a published @workos/authkit-session range. Say exactly which authkit-session release this needs, and whether the PR can merge before that release (e.g. if nothing here type-depends on the new union, it can; otherwise note that it must wait and bump the range when it's out).

Riker's check pnpm install --frozen-lockfile >/dev/null && git diff --quiet origin/main...HEAD -- pnpm-lock.yaml && ! grep -qE '"(file|link):' package.json example/package.json && pnpm typecheck && pnpm lint && pnpm format:check && pnpm test && pnpm build passed.

Opened as a draft by Riker (job 98).

Rethrow the WorkOS SDK's ApiKeyRequiredException instead of swallowing it to null, and let the impersonation banner degrade when the action rejects.
Wrap the WorkOS SDK's ApiKeyRequiredException in an error naming WORKOS_API_KEY. The impersonation banner logs the rejection and renders without the organization name instead of leaving an unhandled rejection.
@nicknisi
nicknisi marked this pull request as ready for review October 7, 2026 17:31
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Adds keyless PKCE public-client mode to the auth flow.

The PR appears safe to merge, with the previous organization-name issue fixed.

What we checked:

  • Old lookups change the name: Cleanup marks the old lookup stale. Both result handlers check that flag before changing the name.

Summary

This PR documents keyless sign-in, adds tests for the package’s keyless behavior, and gives organization lookups a clear missing-key error.

  • The latest changes stop old organization lookups from overwriting or clearing the current name.
  • Added tests cover both late success and late failure.
  • The previous finding is fixed. No new actionable issues were found.

Reviews (2) · Last reviewed commit: "fix: ignore stale organization lookups i..." · Reviewed by Greptile

Comment thread src/client/components/impersonation.tsx Outdated
A lookup for a previous organization that settles after the organization changed no longer overwrites the current name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant