fix(types)!: remove is_multi_factor_auth_enabled from SignUpRequest#48
Merged
Conversation
Server (authorizerdev/authorizer PR #710, release 2.4.0-rc.6) removed this field from SignupRequest/input SignUpRequest — it let an unauthenticated signup caller bypass the server's MFA-on-by-default policy for their own new account. Field stays valid on UpdateProfileRequest and admin UpdateUserRequest; only the signup path lost it. BREAKING CHANGE: SignUpRequest/SignupRequest no longer has is_multi_factor_auth_enabled. Passing it to signup() is now ignored by the server.
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.
Why
Server PR authorizerdev/authorizer#710 (release
2.4.0-rc.6) removedis_multi_factor_auth_enabledfromSignupRequest/input SignUpRequestin both the GraphQL schema and the proto message. It was a security fix: the field let an unauthenticated signup caller bypass the server's MFA-on-by-default policy for their own new account.What changed
src/types.ts: removedis_multi_factor_auth_enabledfrom theSignUpRequestinterface (and itsSignupRequestalias). Thesignup()mutation insrc/index.tssends the whole typed object as GraphQL variables, so no separate query-string change was needed.Scope
Narrowly scoped to this one breaking field removal on the signup path only:
UpdateProfileRequest/updateProfile()— untouched. The field remains valid and required there.UpdateUserRequest/updateUser()(src/admin.ts) — untouched. Same, unaffected by the server change.src/admin.ts'sadminSignup/AdminSignupRequestnever had this field — unaffected.This is a breaking change for TS consumers who set
is_multi_factor_auth_enabledonsignup()— expected, matching the server's break. It is additive-safe (removing an optional field is source-compatible for anyone who wasn't already setting it).Verification
npm run build— passes, no type errors.npx tsc --noEmit— clean, no other references to the field on the signup path.npx jest— 7/9 suites pass. The 2 failing suites (index.test.ts,admin.test.ts) fail identically onmainbefore this change too — they spin up a real Authorizer server via Docker pinned toquay.io/authorizer/authorizer:2.4.0-rc.1, which is a 404 in the registry (pre-existing environment/pinned-tag issue, unrelated to this diff).Refs authorizerdev/authorizer#710