fix(mfa): remove three MFA routes the API does not have - #115
Merged
Merged
Conversation
Every no-content reply — deleting a user, a factor, an organization — went out as `Content-Type: text/plain; charset=UTF-8`, which production never sends. The header was not ours: @hono/node-server 1.x stamped a text/plain default onto any response lacking one, empty body included. 2.x skips the default when the body is null, so the bump fixes all 27 routes at once with no adapter-level workaround. Refs #110
Every authentication_challenge on the wire — from the challenge routes, the password grant's mfa_challenge step and the legacy MFA API — carried the store's join columns, `factor_id` and `user_id`, where the spec's AuthenticationChallenge has `authentication_factor_id` and no user. Every generated SDK reads the spec's name: Kotlin, Rust, Swift, Python and PHP refuse to deserialize without it, and Go, Ruby, .NET and Elixir hand back an empty factor id. The store keeps its columns; only the formatter, where every route already meets, changes. Refs #110
`POST /user_management/users/:id/auth_factors` answered with a bare
factor, where the spec's UserlandUserAuthenticationFactorEnrollResponse
wraps it as `{ authentication_factor, authentication_challenge }`.
Every backend SDK reads that envelope: Node throws a TypeError before
returning, Python, Kotlin, Rust, Swift and PHP refuse to deserialize,
and Go, Ruby, .NET and Elixir hand back nil for both halves — so no
SDK could drive TOTP enrollment through the emulator.
The wrapper alone would not have been enough. The spec's enrolled
factor requires `totp.secret`, `totp.qr_code` and `totp.uri`, and the
same SDKs fail one level down without them; the emulator stored only a
hex secret buried in the URI. Enrollment now mints a Base32 secret
(honoring a caller-supplied `totp_secret`, validated as production
does) and is the only response that shows it — GET and LIST return
the spec's secretless AuthenticationFactor, as production does.
The `qr_code` is a valid 1x1 PNG rather than a scannable code: the
field is required and typed as a string, the emulator never verifies
a real TOTP code, and `uri` already carries everything the code would.
Fixes #110
Issue #110 shipped in several releases because neither conformance loop looked at MFA: the enrollment envelope, the factor and the challenge were all outside the curated catalogs, so the bare factor and the challenge's `factor_id` never met the spec. Both loops already do exactly the check that was needed — the gap was coverage, not design. The six spec operations with a JSON body and the two resources join the catalogs; run against main without the fix, the new cases fail six times, each naming one of the drifts. The loops diff top-level keys, so they cannot see the enrolled factor's `totp.secret`, `qr_code` and `uri`; the route tests added with the fix pin those. Refs #110
`RegExp.test` coerces its argument, and the digits 2–7 are Base32, so a JSON number like 234567 passed validation and was stored and returned as a number — a `totp.secret` the strictly typed SDKs would refuse to deserialize. Refs #110
`POST /user_management/auth_factors/:id/challenges`, `POST /user_management/auth_challenges/:id/verify` and `DELETE /user_management/auth_factors/:id` date from the first commit and exist in neither the OpenAPI spec (checked through 0.95.0), the production controllers, nor any of the twelve backend SDKs. WorkOS challenges and verifies factors through `/auth/factors/:id/challenge` and `/auth/challenges/:id/verify`, and deletes them through `DELETE /auth/factors/:id` — all of which the emulator already serves with the same logic, so the removed handlers were a second copy of it behind paths no client calls. Their tests move to the real routes; the coverage they gave (correct code, expired challenge, wire shape) is kept.
 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #114 (GitHub retargets to
mainwhen it merges; the two touch the same files).POST /user_management/auth_factors/:id/challenges,POST /user_management/auth_challenges/:id/verifyandDELETE /user_management/auth_factors/:id. They date from the emulator's first commit and exist in none of: the OpenAPI spec (checked at the pinned 0.80.0 and the newest published 0.95.0), the production controllers (the only challenge controller isauth/challenges; the only factor delete isDELETE /auth/factors/:id), or any of the twelve backend SDKs (Node, Python, Go, .NET, Kotlin, Android, PHP, Ruby, Elixir, Rust, Swift; php-laravel wraps php).POST /auth/factors/:id/challenge→POST /auth/challenges/:id/verify(or themfa-totpgrant on/user_management/authenticate), and deletes them throughDELETE /auth/factors/:id. The emulator already serves all of those with the same logic, so the removed handlers were a duplicate copy of it behind paths no client calls —auth-challenges.tsis deleted outright.auth-challenges.spec.ts(correct-code success, wrong code, expired challenge, challenge wire shape, 404) is repointed at the real routes, and the twoe2e.spec.tscleanups now useDELETE /auth/factors/:id.SUPPORTED.mdis unchanged — the invented routes never counted toward spec coverage.Notes
SUPPORTED.md, the README, the spec or an SDK, so the only possible caller is a hand-written request against a guessed path. Happy to retitle with!if you'd rather bump major.