feat: provide an end-to-end login flow story with spec-accurate webhooks#2
feat: provide an end-to-end login flow story with spec-accurate webhooks#2gjtorikian wants to merge 8 commits into
Conversation
Hand-written event constants had drifted from the real API (authentication.magicauth_succeeded vs the spec's authentication.magic_auth_succeeded), and the public docs pages are themselves incomplete — authentication.mfa_failed exists only in the spec. Generating the catalog from the published spec package makes drift impossible to reintroduce: regenerating is `npm run gen:events`, and picking up a newer spec is an ordinary dependency bump. The generated file is committed so package consumers never need the spec itself.
The emulator exists so apps can test their documented login flow locally, but whole flows (SSO, magic auth, email verification, password reset) emitted no events at all, failed logins emitted nothing, and some emitted names (connection.created, directory_user.*) don't exist in the real catalog — webhook handlers verified against the emulator could pass on names production never sends. Every emit site now references the spec-generated catalog, so an invalid event name no longer compiles. Sessions gained the spec-required auth_method/status/expires_at/ended_at fields, and update hooks now receive the previous value so connection.activated/deactivated fire only on real state transitions. Renames for consumers asserting old names: authentication.magicauth_succeeded → authentication.magic_auth_succeeded (same for email_verification), connection.created/updated → connection.activated/deactivated, directory*.* → dsync.*, and the payload's `method` field → spec `type`/`status` fields.
Route specs run in-process and webhook delivery was only covered with a mocked fetch, so nothing verified the actual story: a registered endpoint receiving signed, spec-shaped payloads as a user walks the documented flows. The e2e spec boots the real server plus a local receiver and asserts payloads against the spec-generated required-field metadata, so future catalog drift fails CI here instead of surfacing in user apps.
The webhook system existed but the README never mentioned it, so users had no way to discover the emulator's core story: driving an entire documented login flow locally, with codes WorkOS would email delivered in webhook payloads instead.
Greptile SummaryThis PR delivers end-to-end spec-faithful webhook coverage for the WorkOS emulator: event names and payload shapes are now generated from
Confidence Score: 5/5Safe to merge — no functional regressions identified across the authentication, session, and webhook emission paths. Every grant type now has matching success and failure events, the token-refresh path correctly avoids spurious events, the session lifecycle fields are correctly populated before the delete hook fires, and the MFA primary-factor recording is tested end-to-end. The generated catalog eliminates the class of hand-maintained name drift that motivated the PR. Test coverage spans unit, integration, and a new real-HTTP e2e story. No files require special attention. Important Files Changed
|
|
|
||
| const WEBHOOK_SECRET = 'whsec_e2e_test_secret'; | ||
|
|
||
| interface ReceivedWebhook { |
There was a problem hiding this comment.
Hardcoded webhook secret in source code
WEBHOOK_SECRET is committed as a literal string in a test file. The engineering rule for this repository prohibits hardcoded secrets in source code. While this value is clearly a test fixture (not a production credential), keeping it as a constant in source could trip secret-scanning tools and creates a pattern others might follow for real values. Consider moving it to a test-scoped env variable or generating it via crypto.randomBytes in beforeAll.
Rule Used: No hardcoded secrets or API keys in source code (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The repo had the oxfmt formatter but no linter and no CI gate enforcing either linting or formatting. oxlint (oxc's linter, pairing with the oxfmt already in use) now runs alongside the format check on every push and PR, so correctness regressions like unused code can't merge unnoticed. The dead-code removals in the spec and source files are the findings oxlint surfaced on its first run; they ship in this commit so the new workflow lands green instead of red.
| if (eventBus) { | ||
| const authEventType = `authentication.${authMethod.toLowerCase()}_succeeded`; | ||
| const succeededEvent = AUTH_EVENTS[authMethod]?.succeeded; | ||
| if (eventBus && succeededEvent) { |
There was a problem hiding this comment.
Confirmed — and it's slightly broader than the auth event alone.
The refresh_token grant sets authMethod = 'OAuth', so this success path emits authentication.oauth_succeeded on every rotation. A refresh isn't a fresh login, so agreed — it's a false positive for anything counting or alerting on login events.
One thing to add: the handler also runs ws.sessions.insert(...) unconditionally for every grant, and sessions.onInsert emits session.created. So a refresh currently fires a spurious session.created and mints a brand-new session + refresh token, instead of rotating within the existing session (refreshToken.session_id). The misleading auth event is really the most visible symptom of "refresh is being treated as a fresh login."
Planned fix: gate the authentication-event emission behind an explicit "fresh login" check so the refresh_token grant is excluded — your first suggestion (skip emission), rather than mapping to a value with no AUTH_EVENTS entry, since that would also wipe out the legitimate session auth_method. The deeper "don't create a new session on refresh" behavior is a larger change we'll address separately.
There was a problem hiding this comment.
Landed in f6c8309.
The refresh_token grant now clears an isFreshLogin flag that gates the success-event emission, so a token refresh no longer fires authentication.oauth_succeeded. Added a regression test asserting a refresh emits no authentication.* event (it fails without the gate, passes with it).
The deeper issue — a refresh still runs ws.sessions.insert(...) and so emits a spurious session.created while minting a brand-new session instead of rotating within the existing one — is left as a separate follow-up.
There was a problem hiding this comment.
Follow-up landed in 55f1d48: refresh no longer creates a new session.
A refresh_token rotation now reuses the session referenced by the old token (issuing fresh access + refresh tokens for it), so it emits no session.created and no authentication.* event. A revoked session can no longer be refreshed either — it returns invalid_grant rather than resurrecting itself. Regression test asserts the session.created count and total session count both stay at 1 across a rotation.
The refresh_token grant set authMethod = 'OAuth' and fell through to the shared success path, so every silent token rotation emitted an authentication.oauth_succeeded webhook. A refresh is not a login, so this was a false positive for any consumer counting authentication events. Gate the emission behind a fresh-login flag that the refresh grant clears. Also pin MFA and email-verification sessions to auth_method 'unknown' explicitly. The spec's session auth_method enum has no value for either (MFA is a second factor, not a primary method), so the prior silent fallthrough was correct but undocumented — and the obvious "fix" of adding 'mfa'/'email_verification' would emit out-of-enum values. The comment and explicit entries guard against that regression. Both address Greptile review feedback on #2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Refresh reused the shared session-creation path, so every token rotation inserted a new session (firing session.created) and minted a fresh session/refresh token instead of rotating within the existing one. Refresh now reuses the session referenced by the old token and emits no session.created; a revoked session can no longer be refreshed (it returns invalid_grant rather than resurrecting itself). For MFA, the session auth_method now records the primary factor the pending token was issued for (e.g. password) rather than the second factor, while the event stays authentication.mfa_succeeded. To make the flow reachable, a password login for a user with enrolled factors now returns a pending token + challenge (mfa_challenge) instead of a session, so completing mfa-totp yields a session keyed to the primary factor. The spec documents the mfa_challenge code but not the response body that carries pending_authentication_token; that shape mirrors WorkOS. Follow-up to Greptile review on #2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
@workos/openapi-specdevDependency (npm run gen:events) instead of hand-maintained constants, which had drifted (authentication.magicauth_succeededvs the realauthentication.magic_auth_succeeded). Emit sites reference generated constants, so an invalid event name no longer compiles.authentication.*_failedwith the spec-requirederror: {code, message}. Codes WorkOS would email arrive in webhook payloads (magic_auth.created,password_reset.created,email_verification.created), so tests can drive the whole flow from webhooks alone.connection.created/updated→connection.activated/deactivated(on state transitions),directory*.*→dsync.*, and the auth payload'smethodfield → spectype/statusfields. Sessions gained spec-requiredauth_method/status/expires_at/ended_at.src/e2e.spec.ts, which boots the real server plus a local webhook receiver and walks the documented flows, verifying HMAC signatures and spec-required payload fields; documents the story in the README. Also migrates formatting from prettier to oxfmt.Test plan
npm test— 376 tests across 40 files, including the new e2e story specnpm run typecheckandnpm run buildnpm run gen:eventsproduces no diff (generated catalog is current and idempotent)npm run fmt:checknode dist/cli.js, register a webhook endpoint, walk authorize → authenticate via curl, observe signeduser.created,session.created,authentication.oauth_succeededwebhooks🤖 Generated with Claude Code