Repository navigation
feat(api): add typed Strategy and migrate strategy comparisons - #1025
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 6 reviews per hour. 📝 WalkthroughWalkthroughThe API adds a typed ChangesTyped authentication strategies
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~30 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to The reviewed changes preserve the checked strategy and authentication-flow behavior; no introduced issue requiring remediation was established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
3c38a6f to
9279665
Compare
Add com.clerk.api.auth.types.Strategy, a sealed type with the known strategies, the OAuth/OAuthToken families and Unknown(raw), serialized as the raw wire string. Expose typed accessors next to the existing String fields (Factor.strategyType, Verification.strategyType, Client.lastAuthenticationStrategyType, SignIn.identifyingFirstFactor(Strategy)). Collapse the four strategy constant copies onto Strategy: the public Constants.Strategy, Constants.Passkey.PASSKEY_STRATEGY_VALUE and SignIn.PrepareSecondFactorParams constants are deprecated (without ReplaceWith, since Strategy.X.value is not a compile-time constant), the internal copies (UI StrategyKeys, PasskeyHelper.passkeyStrategy, SignUp.CreateParams companions) are removed, and api/ui strategy comparisons use the typed form, including the Clerk.auth facade paths (AuthFlows, firstFactorFor/noMatchingFactor, prepareSecondFactor and the sendMfa* helpers).
9279665 to
851854a
Compare
Why
Architecture review finding #11: strategies are plain strings, compared as strings dozens of times across api and ui, with the constants duplicated in four places (
Constants.Strategy,Constants.Passkey.PASSKEY_STRATEGY_VALUE, theSignIn.PrepareSecondFactorParamscompanion, and the UI's internalStrategyKeys). This PR adds the typedStrategythe review asked for and moves the SDK's own comparisons onto it.This is PR 1 of 3 for #11:
Strategy.OAuthResult(stacked on 2).Scope
com.clerk.api.auth.types.Strategy, a sealed class:PhoneCode,EmailCode,EmailLink,Totp,BackupCode,Password,Passkey,ResetPasswordEmailCode,ResetPasswordPhoneCode,Ticket,Transfer,EnterpriseSso,Saml,TrustedDevice,GoogleOneTap)OAuth(oauth_*, exposesprovider) andOAuthToken(oauth_token_*)Unknown(raw)for anything elseStrategy.from(raw)never fails. Equality andtoString()use the wirevalue.StrategySerializerround-trips the raw string.Stringfields, which are unchanged:Factor.strategyType,Verification.strategyType,Client.lastAuthenticationStrategyType, and an overloadSignIn.identifyingFirstFactor(Strategy).Constants.Strategy.*,Constants.Passkey.PASSKEY_STRATEGY_VALUE, andSignIn.PrepareSecondFactorParams.{PHONE_CODE, EMAIL_CODE, PASSKEY}. They have noReplaceWith, becauseStrategy.X.valueis not a compile-time constant and an automatic swap would breakconst valand annotation uses. The message names the replacement and says this.StrategyKeys,PasskeyHelper.passkeyStrategy, and theSignUp.CreateParams.{Ticket,GoogleOneTap}companionSTRATEGYconsts.==,when,in, sort orders,LastUsedAuth) now usestrategyType. Request parameter defaults useStrategy.X.value.AuthFlows.kt(createSignIn/createSignUp): transfer, ticket and Google One Tap strategies come fromStrategy.SignInExtensions.kt:firstFactorFortakesList<Strategy>andnoMatchingFactortakes aStrategy. The identifier-masking logic is unchanged.SignIn.kt:prepareFirstFactorImpl,prepareSecondFactorandsendMfaPhoneCode/sendMfaEmailCodecomparestrategyTypeinstead of the now-deprecatedPrepareSecondFactorParamsconstants.Clerk.ktidentifyingFirstFactorand the UISignInPrepareHandler.SignInFactorCodeViewModelkeeps main's removal ofprefersEmailLinkOverEmailCode/shouldPreferEmailLinkfrom fix(ui): keep the email code screen when email link is supported #1043.StrategyTestcovers known, OAuth, OAuth-token and unknown mapping, serializer round-trip, decoding aFactorwith an unrecognized strategy, and the typed accessors.Blast Radius
alternativeFirstFactors()usedcontains("oauth"). It now excludes theoauth_*andoauth_token_*families explicitly. No real strategy is affected.SignInFactorCodeViewModel.preparenow has an explicitStrategy.Totp -> Unitbranch (TOTP has nothing to prepare) pluselse -> Unit, becausewhenover a sealed type must be exhaustive. Both are no-ops, matching the old fall-through.Strategywill needpublicmodifiers.Testing
After the rebase onto main (7ffc6d3):
./gradlew spotlessApply :source:api:detekt :source:ui:detekt :source:api:testDebugUnitTest :source:ui:testDebugUnitTest :samples:quickstart:compileDebugKotlin :samples:custom-flows:compileDebugKotlin :e2e:compileDebugKotlin: all pass.Before the rebase:
./gradlew :source:api:spotlessApply :source:ui:spotlessApply :source:api:detekt :source:ui:detekt :source:api:testDebugUnitTest :source:ui:testDebugUnitTest :source:ui:verifyPaparazziDebug --continueorigin/main(a2efe4f) in this environment. I ranverifyPaparazziDebugthere and diffed the failing-test lists: they are identical. Several goldens are missing from the repo, and the rest are sub-0.1% font-rendering diffs. No snapshot changed because of this PR.🤖 Generated with Claude Code
Summary by CodeRabbit