-
Notifications
You must be signed in to change notification settings - Fork 166
feat: Add support for organization to custom token exchange
#885
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
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 support for an optional organization parameter to the customTokenExchange() method in the AuthenticationAPIClient class. It also introduces a new validation framework with a RequestValidator interface and implements client-side validation for custom token exchange to reject reserved namespaces and validate HTTP/HTTPS URIs.
Key changes:
- Added optional
organizationparameter tocustomTokenExchange()method with default value ofnull - Implemented
RequestValidatorinterface for pre-request validation - Created
CustomTokenExchangeValidatorto validate subject token types (rejects reserved namespaces, validates HTTP/HTTPS URIs) - Added validator support to
Request,AuthenticationRequest, and their implementations
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| auth0/src/main/java/com/auth0/android/authentication/AuthenticationAPIClient.kt | Added organization parameter to customTokenExchange method and integrated validator; expanded wildcard imports to explicit imports |
| auth0/src/main/java/com/auth0/android/request/internal/validator/CustomTokenExchangeValidator.kt | New validator class to validate subject_token_type parameter, rejecting reserved namespaces and validating HTTP/HTTPS URIs |
| auth0/src/main/java/com/auth0/android/request/RequestValidator.kt | New interface defining the validator contract for pre-request validation |
| auth0/src/main/java/com/auth0/android/request/Request.kt | Added addValidator() method with default no-op implementation |
| auth0/src/main/java/com/auth0/android/request/AuthenticationRequest.kt | Added addValidator() method override; removed unused Callback import |
| auth0/src/main/java/com/auth0/android/request/internal/BaseRequest.kt | Implemented validator storage and execution before request execution |
| auth0/src/main/java/com/auth0/android/request/internal/BaseAuthenticationRequest.kt | Implemented addValidator() to delegate to underlying request; expanded wildcard imports |
| auth0/src/main/java/com/auth0/android/request/SignUpRequest.kt | Implemented addValidator() to delegate to authentication request |
| auth0/src/test/java/com/auth0/android/authentication/AuthenticationAPIClientTest.kt | Added comprehensive tests for organization parameter and validator behavior; includes tests for reserved namespaces and invalid URIs; fixed formatting issues |
| auth0/src/test/java/com/auth0/android/authentication/request/RequestMock.java | Added addValidator() no-op implementation to test mock |
| auth0/src/test/java/com/auth0/android/authentication/request/AuthenticationRequestMock.java | Added addValidator() no-op implementation to test mock |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
auth0/src/main/java/com/auth0/android/authentication/AuthenticationAPIClient.kt
Show resolved
Hide resolved
...0/src/main/java/com/auth0/android/request/internal/validator/CustomTokenExchangeValidator.kt
Outdated
Show resolved
Hide resolved
...0/src/main/java/com/auth0/android/request/internal/validator/CustomTokenExchangeValidator.kt
Outdated
Show resolved
Hide resolved
...0/src/main/java/com/auth0/android/request/internal/validator/CustomTokenExchangeValidator.kt
Outdated
Show resolved
Hide resolved
...0/src/main/java/com/auth0/android/request/internal/validator/CustomTokenExchangeValidator.kt
Outdated
Show resolved
Hide resolved
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
|
Claude finished @NandanPrabhu's task —— View job Code Review Complete
Overall Assessment❌ This pull request requires changes before it can be merged. While the feature implementation is solid and well-tested, there are several issues that need to be addressed: a critical type safety vulnerability in the validator that could cause runtime crashes, incomplete validation logic for reserved namespace URLs, and minor code style inconsistencies with the existing codebase patterns. |
|
@pmathew92 do we still need to add the client side validation? I am assuming it we dont have as confirmed in the CTE channel. profile api alrready does validation for reserve keyword |
Changes
This PR adds support for organization support in
customTokenExchangein theAuthenticationAPIClientclassTesting
Please describe how this can be tested by reviewers. Be specific about anything not tested and reasons why. Since this library has unit testing, tests should be added for new functionality and existing tests should complete without errors.
This change adds unit test coverage
This change adds integration test coverage
This change has been tested on the latest version of the platform/language or why not
Checklist
I have read the Auth0 general contribution guidelines
I have read the Auth0 Code of Conduct
All existing and new tests complete without errors