-
Notifications
You must be signed in to change notification settings - Fork 232
feat: add optional scope parameter to clearApiCredentials #1412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…port audience and scope parameters
…dle audience and scope parameters
sanchitmehta94
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds optional audience and scope parameters to the clearCredentials and clearApiCredentials methods across all platforms, enabling more granular credential management. When an audience is provided to clearCredentials, it delegates to clearApiCredentials for consistency.
- Added optional
audienceandscopeparameters toclearCredentialsmethod - Added optional
scopeparameter toclearApiCredentialsmethod - Updated TypeScript interfaces, native bridge implementations (iOS/Android), and web platform adapter
- Added comprehensive test coverage for new parameter combinations
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/specs/NativeA0Auth0.ts | Updated TurboModule spec with optional parameters for clearing credentials |
| src/core/interfaces/ICredentialsManager.ts | Updated interface documentation to reflect optional audience/scope filtering |
| src/platforms/native/bridge/INativeBridge.ts | Added JSDoc and optional parameters to bridge interface methods |
| src/platforms/native/bridge/NativeBridgeManager.ts | Updated to pass optional parameters through to native module |
| src/platforms/native/adapters/NativeCredentialsManager.ts | Updated to accept and forward optional parameters to bridge |
| src/platforms/native/adapters/tests/NativeCredentialsManager.spec.ts | Added tests for clearing credentials with various parameter combinations |
| src/platforms/web/adapters/WebCredentialsManager.ts | Delegates to clearApiCredentials when audience is provided; updates warning messages with scope info |
| src/platforms/web/adapters/tests/WebCredentialsManager.spec.ts | Added tests for delegation behavior and scope handling in warning messages |
| ios/NativeBridge.swift | Implements conditional clearing logic based on audience parameter |
| ios/A0Auth0.mm | Updated method signature to accept nullable audience and scope parameters |
| android/src/main/oldarch/com/auth0/react/A0Auth0Spec.kt | Updated abstract methods with nullable parameters |
| android/src/main/java/com/auth0/react/A0Auth0Module.kt | Implements conditional clearing logic with null-safe parameter handling |
| EXAMPLES.md | Added examples demonstrating new optional parameters usage |
Comments suppressed due to low confidence (1)
ios/NativeBridge.swift:257
- The DPoP key is being cleared even when clearing credentials for a specific audience. Since the DPoP key is a global resource (not specific to any audience), it should only be cleared when clearing all credentials (i.e., when audience is null/undefined). Clearing it when removing credentials for a specific audience could affect other API credentials that still need the DPoP key. Consider moving the DPoP clearing logic inside the else block where all credentials are cleared.
// Also clear DPoP key if DPoP is enabled
if self.useDPoP {
do {
try DPoP.clearKeypair()
} catch {
// Log error but don't fail the operation
print("Warning: Failed to clear DPoP key: \(error.localizedDescription)")
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| await this.bridge.clearDPoPKey(); | ||
| } catch { | ||
| // Silently ignore DPoP key clearing errors | ||
| // The main credentials are already cleared at this point | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to call this. As we are calling it in the native module
…ssing scope when clearing credentials
API Changes:
clearApiCredentials(audience, scope?)- Now accepts optional scope parameter