ID token validation for the API that return id token as part of Credentials and SSOCredentials#1091
Conversation
6dcfa75 to
bcca3b1
Compare
17f7333 to
c386dec
Compare
c386dec to
b089ff5
Compare
There was a problem hiding this comment.
Pull request overview
This pull request moves ID token validation from a separate post-processing step to request-level response handlers for all Authentication and MFAClient APIs that return credentials. The changes ensure ID tokens are validated immediately when received, improving security and developer experience by eliminating the need for manual validation steps.
Changes:
- Introduced
IDTokenProtocolto identify credential types carrying ID tokens (CredentialsandSSOCredentials) - Added ID token validation parameters (issuer, leeway, maxAge, nonce, organization) to all credential-returning methods in
AuthenticationandMFAClientprotocols - Implemented request-level validation handlers (
authenticationDecodableWithIDTokenValidationandmfaVerifyDecodableWithIDTokenValidation) that validate tokens before invoking success callbacks - Updated
CredentialsManagerto accept validation parameters and skip retry logic for deterministic validation failures - Modified PKCE flow in
OAuth2Grantto use new validation integration and improve error mapping - Added
isIDTokenValidationError()helper to accurately identify validation errors without false positives - Updated GitHub Actions workflow files (note: contains incorrect Xcode version numbers)
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Auth0/IDTokenProtocol.swift | New protocol defining types with ID tokens (Credentials, SSOCredentials) |
| Auth0/IDTokenValidator.swift | Added helper function to identify specific ID token validation error types |
| Auth0/AuthenticationHandlers.swift | New handler integrating ID token validation for Authentication API responses |
| Auth0/MFA/MFAHandlers.swift | New handler integrating ID token validation for MFA API responses |
| Auth0/MFA/MFAClient.swift | Updated protocol with validation parameters; added backward-compatible extension |
| Auth0/MFA/Auth0MFAClient.swift | Implementation of validation parameter support in MFA verify methods |
| Auth0/OAuth2Grant.swift | Refactored PKCE flow to use request-level validation with improved error mapping |
| Auth0/Authentication.swift | Updated protocol methods with validation parameters; added backward-compatible extension |
| Auth0/Auth0Authentication.swift | Implementation of validation parameters across all credential-returning methods |
| Auth0/CredentialsManager.swift | Added validation parameters and logic to bypass retry for validation failures |
| Auth0/Request.swift | Added @escaping annotation to callback closure for correctness |
| Auth0/Credentials.swift | Minor whitespace cleanup |
| Auth0Tests/MFA/Auth0MFAClientTests.swift | Updated tests with authentication parameter for MFA verify methods |
| Auth0.xcodeproj/project.pbxproj | Added IDTokenProtocol.swift to build targets |
| App/ContentViewModel.swift | Removed WEB_AUTH_PLATFORM conditional compilation directives |
| App/ContentView.swift | Removed WEB_AUTH_PLATFORM conditional compilation directives |
| .github/workflows/rl-scanner.yml | Updated Xcode version (contains error: "26.2" is invalid) |
| .github/workflows/main.yml | Updated Xcode version and macOS runner (contains error: "26.2" is invalid) |
Comments suppressed due to low confidence (4)
Auth0/MFA/Auth0MFAClient.swift:205
- There's redundant error wrapping logic here. The mfaVerifyDecodableWithIDTokenValidation handler (line 74 in MFAHandlers.swift) already wraps ID token validation errors in MFAVerifyError(cause: error). Then this code checks if error.cause is an ID token validation error and wraps it again in MFAVerifyError(cause: cause), creating a double-wrapped error structure. This logic should be simplified - either remove this check entirely since the handler already handles it correctly, or if unwrapping is needed, just pass through the cause directly without re-wrapping.
switch verifyResult {
case .failure(let error):
if let cause = error.cause, isIDTokenValidationError(cause) {
return callback(.failure(MFAVerifyError(cause: cause)))
}
callback(.failure(error))
App/ContentViewModel.swift:68
- The removal of WEB_AUTH_PLATFORM conditional compilation directives from the App files (ContentViewModel.swift and ContentView.swift) is not mentioned in the PR description. This appears to be an unrelated change that should either be documented in the PR description or moved to a separate PR. If this is intentional, please explain why the conditional compilation is no longer needed for these web authentication functions.
func webLogin(presentationWindow window: Auth0WindowRepresentable? = nil) async {
isLoading = true
errorMessage = nil
#if !os(tvOS) && !os(watchOS)
do {
let credentials = try await Auth0
.webAuth()
.scope("openid profile email offline_access")
.start()
let stored = credentialsManager.store(credentials: credentials)
if stored {
isAuthenticated = true
} else {
errorMessage = "Failed to store credentials"
}
} catch let error as Auth0Error {
errorMessage = "Login failed: \(error.localizedDescription)"
} catch {
errorMessage = "Unexpected error: \(error.localizedDescription)"
}
#endif
isLoading = false
}
func logout(presentationWindow window: Auth0WindowRepresentable? = nil) async {
isLoading = true
errorMessage = nil
#if !os(tvOS) && !os(watchOS)
do {
var webAuth = Auth0.webAuth()
if let window = window {
webAuth = webAuth.presentationWindow(window)
}
try await webAuth.clearSession()
let cleared = credentialsManager.clear()
if cleared {
isAuthenticated = false
print("Logout successful")
}
} catch let error as Auth0Error {
errorMessage = "Logout failed: \(error.localizedDescription)"
print("Logout failed with error: \(error)")
} catch {
errorMessage = "Unexpected error: \(error.localizedDescription)"
}
#endif
isLoading = false
}
App/ContentView.swift:50
- The removal of WEB_AUTH_PLATFORM conditional compilation directives is not mentioned in the PR description. This appears to be an unrelated change that should either be documented in the PR description or moved to a separate PR. If this is intentional, please explain why the conditional compilation is no longer needed.
Button {
Task {
#if os(macOS)
await viewModel.webLogin(presentationWindow: currentWindow)
#else
await viewModel.webLogin(presentationWindow: window)
#endif
}
} label: {
VStack(spacing: 4) {
Text("Login")
}
}
.buttonStyle(PrimaryButtonStyle())
.disabled(viewModel.isLoading)
Divider()
.padding(.vertical)
Button {
Task {
#if os(macOS)
await viewModel.logout(presentationWindow: currentWindow)
#else
await viewModel.logout(presentationWindow: window)
#endif
}
Auth0/Credentials.swift:16
- This whitespace-only change (removing a blank line after the class declaration) appears unrelated to the PR's stated purpose of adding ID token validation. Consider either reverting this cosmetic change or explaining why it's included.
/// Token that can be used to make authenticated requests to the specified API (the **audience** value used on login).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
eb54193 to
bfe4efb
Compare
bfe4efb to
3f25b5c
Compare
3f25b5c to
736c750
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b9a0380 to
5d5f673
Compare
…g underlying properties of the Auth0MFAClient
📋 Changes
Introduces opt-in, chainable ID token validation for credential-returning
AuthenticationmethodsNew
TokenRequestableprotocol — extendsRequestableand is returned by all credential-producing methods (login,codeExchange,renew,ssoExchange, and MFA verify variants). It exposes a fluent validation builder:Key design decisions:
.validateClaims()opts in; omitting it preserves existing behaviour unchanged.IDTokenDecodingError.missingIDTokenrather than silently succeeding.parameters(),headers(), andrequestValidators()preserve the concrete return type so.validateClaims()can be chained in any order.ValidationErrortypes now conform to a sharedIDTokenValidationErrormarker protocol, replacing the previous exhaustive type-check inisIDTokenValidationError.Auth0MFAClientpropagatesdpop,logger, andauth0ClientInfoupdates to its underlyingAuthenticationinstance viadidSetobservers.withLeewaynow takes seconds (consistent withwithMaxAge); default is60.No migration required for call sites. Protocol conformances (mocks, custom
Authenticationimplementations) must update method return types fromany Requestabletoany TokenRequestablefor credential-returning methods.🎯 Testing
All existing unit tests pass. New coverage added for:
validateClaims()with valid, invalid, and missing ID tokensdidSetpropagation onAuth0MFAClientfordpop,logger, andauth0ClientInfoTokenRequestorRequesttype sincedpopis not part of theRequestable/TokenRequestableprotocol surface