Add AuthClient protocol and expose authClient on CrossmintSDK#97
Merged
Conversation
Exposes CrossmintSDK.shared.authClient with sendOTP(to:)/verifyOTP(code:requestId:)/logout() as the public-facing auth API. DefaultAuthClient orchestrates the OTP flow by delegating JWT/session management to CrossmintAuthManager, keeping a single source of truth for the token. normalizeEmail moved to Utils so both auth types share one implementation.
…ion, tighten init visibility
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
Sources/CrossmintClient/SwiftUI/CrossmintSDK.swift:98-105
**`CrossmintSDK.logout()` leaves `authClient` state dirty**
`CrossmintSDK.logout()` calls `authManager.logout()` and resets the TEE, but never calls `authClient.logout()`. This means `DefaultAuthClient.pendingEmailsByRequestId` retains its entries after the primary logout path runs. While requestIds are server-scoped UUIDs that won't collide across sessions, the larger concern is that these two logout paths are now out of sync: `CrossmintSDK.shared.logout()` (the documented public API) leaves the auth client in a state where old pending OTP mappings still exist, while `authClient.logout()` clears the dictionary but skips `crossmintTEE.resetState()`.
Reviews (1): Last reviewed commit: "Polish AuthClient: rename otp vars, retu..." | Re-trigger Greptile |
…out warning, align mocks with conventions
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
Sources/CrossmintClient/SwiftUI/CrossmintSDK.swift:98-105
**SDK-level logout doesn't clear `DefaultAuthClient` state**
`CrossmintSDK.shared.logout()` calls `authManager.logout()` directly and then `crossmintTEE.resetState()`, but it never touches `authClient`. This means `DefaultAuthClient.pendingEmailsByRequestId` is left with stale entries after the full-SDK logout path runs. The inverse is also true: `authClient.logout()` clears `pendingEmailsByRequestId` but skips `crossmintTEE.resetState()`.
If the PR's goal is for `authClient` to be the single entry point for auth, `CrossmintSDK.shared.logout()` should delegate to `authClient.logout()` (and add the TEE reset on top), or `authClient.logout()` should be updated to also call `crossmintTEE.resetState()` if there is a way to reach it. As it stands, neither logout path fully owns all the state.
### Issue 2 of 2
Sources/CrossmintClient/CrossmintClientSDK.swift:28-32
**`DefaultAuthClient` uses a divergent `AuthService` when an external `authManager` is injected**
When `CrossmintClientSDK.init(apiKey:authManager:)` is called with a non-nil `authManager`, the injected manager retains its own `authService` instance while `DefaultAuthClient` is always constructed with the locally-created `authService`. So `validateEmail`/`validateToken` calls flow through the local service, while the subsequent `establishSession` call (which runs `refreshJWT`) flows through the injected manager's service.
In practice both services wrap the same Crossmint API with the same key, so functional correctness is preserved — but this split makes the injected-manager path harder to mock and test correctly, because a test would need to configure two separate service instances to control behavior end-to-end.
Reviews (2): Last reviewed commit: "Use .sheet(item:) in OTPSignInView, drop..." | Re-trigger Greptile |
afeight
reviewed
Jun 8, 2026
Comment on lines
+20
to
+24
| @Test func sendsOTPAndReturnsRequestId() async throws { | ||
| let client = makeClient() | ||
| let request = try await client.sendOTP(to: "user@example.com") | ||
| #expect(request.requestId == "test-email-id") | ||
| } |
Contributor
There was a problem hiding this comment.
missing cases: normalize email, invalid email throws
afeight
reviewed
Jun 8, 2026
Comment on lines
+26
to
+32
| @Test func verifiesOTPWithValidCodeAndReturnsSession() async throws { | ||
| let client = makeClient() | ||
| let request = try await client.sendOTP(to: "user@example.com") | ||
| let session = try await client.verifyOTP(code: "123456", requestId: request.requestId) | ||
| #expect(session.jwt == "test-jwt") | ||
| #expect(session.user.email == "user@example.com") | ||
| } |
Contributor
There was a problem hiding this comment.
this isn't the test I expected, I'm looking at contents of verifyOTP and I'd expect:
#expect(authService.validateToken).toBeCalledWith(ValidateTokenRequest(...))
#expect(authService.establishSession).toBeCalledWith(oneTimeSecret...)
and then missing an error for attempting to double-verify the same OTP
afeight
reviewed
Jun 8, 2026
|
|
||
| private func normalizeEmail(_ email: String) -> String { | ||
| email.lowercased().trimmingCharacters(in: .whitespacesAndNewlines) | ||
| internal func establishSession(oneTimeSecret: String) async throws(AuthError) -> (jwt: String, email: String) { |
afeight
reviewed
Jun 8, 2026
| return EmailSyntaxValidator.correctlyFormatted(email) | ||
| } | ||
|
|
||
| public func normalizeEmail(_ email: String) -> String { |
afeight
approved these changes
Jun 8, 2026
afeight
left a comment
afeight
left a comment
Contributor
There was a problem hiding this comment.
still some missing tests, ok otherwise
Contributor
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
Sources/CrossmintClient/SwiftUI/CrossmintSDK.swift:98-106
`authManager.logout()` is called twice on every `CrossmintSDK.logout()` invocation. `authClient.logout()` internally calls `authManager.logout()` (line 43 of `DefaultAuthClient.swift`), so the direct call above is redundant. After the first call succeeds, `otpAuthenticationStatus` is `.nonAuthenticated`, and the second call just hits the early-return guard and logs "User is not authenticated. Nothing to logout". The unique work `authClient.logout()` adds is clearing `pendingEmailsByRequestId` — routing entirely through `authClient.logout()` is sufficient.
```suggestion
public func logout() async {
do {
_ = try await authClient.logout()
} catch {
Logger.sdk.warn("Logout request failed: \(error) — clearing local state anyway")
}
crossmintTEE.resetState()
}
```
Reviews (3): Last reviewed commit: "Add tests for normalizeEmail, establishS..." | Re-trigger Greptile |
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
2 participants
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.
This PR introduces
AuthClient, a protocol that exposes the OTP authentication flow as a clean public API. Previously the only way to do email OTP was to callsendEmailOtpandconfirmEmailOtpdirectly onCrossmintAuthManager, which wasn't on any public protocol and forced callers to reach through implementation details.CrossmintSDK.shared.authClientis now the intended entry point for authentication.Changes
AuthClientprotocol withsendOTP(to:),verifyOTP(code:requestId:), andlogout(). All methods use typed throws (throws(AuthError)) to match the existing convention in the SDK.DefaultAuthClientas a Swiftactor. It owns the pending OTP state ([requestId: email]) and delegates all JWT and keychain management toCrossmintAuthManagervia a new internalestablishSession(oneTimeSecret:)method, so there's only one source of truth for session state.normalizeEmailmoved fromCrossmintAuthManagertoUtils/EmailValidation.swiftso bothDefaultAuthClientandCrossmintAuthManagershare the same implementation.authClientonCrossmintClientSDKandCrossmintSDK.shared. TheDefaultAuthClientshares the sameCrossmintAuthManagerinstance as the rest of the SDK, so JWT refresh and logout stay in sync.OTPSignInViewandVerificationViewin the demo app to useCrossmintSDK.shared.authClient.VerificationViewnow accepts arequestIdat init and handles resend by updating it in place.DefaultAuthClientTestsand extracted shared mocks (MockAuthService,MockSecureStorage) toTests/WebTests/Mocks/for reuse across test suites.