Conversation
There was a problem hiding this comment.
Pull request overview
This PR ensures the Auth0-Client (user-agent) header is attached to requests made by MyAccountAPIClient and MfaApiClient, aligning them with other API clients in the SDK that already send this metadata.
Changes:
- Configure
MyAccountAPIClient’sRequestFactoryto always setAuth0-Clientfromauth0.auth0UserAgent.value. - Configure
MfaApiClient’s request factories to setAuth0-Clientfromauth0.auth0UserAgent.value. - Add a unit test in
MyAccountAPIClientTestintended to verify the header configuration.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| auth0/src/test/java/com/auth0/android/myaccount/MyAccountAPIClientTest.kt | Adds a test intended to verify Auth0-Client header configuration (but currently mocks a final class). |
| auth0/src/main/java/com/auth0/android/myaccount/MyAccountAPIClient.kt | Sets Auth0-Client header on the My Account request factory during initialization. |
| auth0/src/main/java/com/auth0/android/authentication/mfa/MfaApiClient.kt | Sets Auth0-Client header for MFA request factories during initialization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| init { | ||
| factory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| } |
There was a problem hiding this comment.
The factory is now configured with setAuth0ClientInfo(...), but the test suite doesn’t currently assert that Auth0-Client is actually sent on the wire for My Account requests. Consider adding a request-level assertion in MyAccountAPIClientTest (e.g., for passkeyEnrollmentChallenge) that the recorded request includes the Auth0-Client header with auth0.auth0UserAgent.value to prevent regressions.
| val auth0UserAgent: Auth0UserAgent = mock() | ||
| val factory: RequestFactory<MyAccountException> = mock() | ||
| val account = Auth0.getInstance(CLIENT_ID, DOMAIN) | ||
|
|
||
| whenever(auth0UserAgent.value).thenReturn("the-user-agent-data") | ||
| account.auth0UserAgent = auth0UserAgent | ||
| MyAccountAPIClient(account, ACCESS_TOKEN, factory, gson) | ||
|
|
||
| verify(factory).setAuth0ClientInfo("the-user-agent-data") |
There was a problem hiding this comment.
Auth0UserAgent is a final Kotlin class and this project only depends on mockito-core (no mockito-inline / inline mock maker). Mocking it with mock() is likely to fail at runtime. Prefer using a real Auth0UserAgent instance and verifying setAuth0ClientInfo(account.auth0UserAgent.value) (or add inline mock maker support if you intentionally want to mock finals).
| init { | ||
| listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| } | ||
| } |
There was a problem hiding this comment.
These RequestFactory properties are declared by lazy, but the new init block eagerly touches all of them, forcing creation of 4 factories (and error adapters) even if the caller only uses one MFA operation. To preserve the intended laziness, set the Auth0-Client header inside each lazy { ... } initializer (e.g., create the factory then call setAuth0ClientInfo(...) before returning it), or drop lazy if eager creation is acceptable.
| init { | |
| listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | |
| enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | |
| challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | |
| verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | |
| } | |
| } | |
| } |
| init { | ||
| listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) |
There was a problem hiding this comment.
This change adds the Auth0-Client header via setAuth0ClientInfo, but there’s no test asserting that MFA requests now include the Auth0-Client header. Since MfaApiClientTest already validates request headers for these endpoints, add at least one assertion (e.g., in shouldIncludeAuthorizationHeaderInGetAuthenticators / enroll / verify) that request.getHeader("Auth0-Client") is present and matches auth0.auth0UserAgent.value.
There was a problem hiding this comment.
All four factories in MfaApiClient are declared with by lazy:
private val listAuthenticatorsFactory: RequestFactory<...> by lazy { ... }
private val enrollmentFactory: RequestFactory<...> by lazy { ... }
private val challengeFactory: RequestFactory<...> by lazy { ... }
private val verifyFactory: RequestFactory<...> by lazy { ... }
But the new init block accesses all of them immediately:
init {
listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
}
This forces eager instantiation of all four factories (and their error adapters) on every construction, even if only one operation is ever called. It defeats the stated reason for using lazy in the first place.
Can we Set the header inside each lazy initializer like belwo:
private val listAuthenticatorsFactory by lazy {
RequestFactory(auth0.networkingClient, createListAuthenticatorsErrorAdapter())
.also { it.setAuth0ClientInfo(auth0.auth0UserAgent.value) }
}
Sam for other factors
| } | ||
|
|
||
| init { | ||
| listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) |
There was a problem hiding this comment.
All four factories in MfaApiClient are declared with by lazy:
private val listAuthenticatorsFactory: RequestFactory<...> by lazy { ... }
private val enrollmentFactory: RequestFactory<...> by lazy { ... }
private val challengeFactory: RequestFactory<...> by lazy { ... }
private val verifyFactory: RequestFactory<...> by lazy { ... }
But the new init block accesses all of them immediately:
init {
listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
}
This forces eager instantiation of all four factories (and their error adapters) on every construction, even if only one operation is ever called. It defeats the stated reason for using lazy in the first place.
Can we set the header inside each lazy initializer:
e.g
private val listAuthenticatorsFactory by lazy {
RequestFactory(auth0.networkingClient, createListAuthenticatorsErrorAdapter())
.also { it.setAuth0ClientInfo(auth0.auth0UserAgent.value) }
}
| init { | ||
| listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value) |
There was a problem hiding this comment.
All four factories in MfaApiClient are declared with by lazy:
private val listAuthenticatorsFactory: RequestFactory<...> by lazy { ... }
private val enrollmentFactory: RequestFactory<...> by lazy { ... }
private val challengeFactory: RequestFactory<...> by lazy { ... }
private val verifyFactory: RequestFactory<...> by lazy { ... }
But the new init block accesses all of them immediately:
init {
listAuthenticatorsFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
enrollmentFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
challengeFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
verifyFactory.setAuth0ClientInfo(auth0.auth0UserAgent.value)
}
This forces eager instantiation of all four factories (and their error adapters) on every construction, even if only one operation is ever called. It defeats the stated reason for using lazy in the first place.
Can we Set the header inside each lazy initializer like belwo:
private val listAuthenticatorsFactory by lazy {
RequestFactory(auth0.networkingClient, createListAuthenticatorsErrorAdapter())
.also { it.setAuth0ClientInfo(auth0.auth0UserAgent.value) }
}
Sam for other factors
| init { | ||
| factory.setAuth0ClientInfo(auth0.auth0UserAgent.value) | ||
| } |
| val auth0UserAgent: Auth0UserAgent = mock() | ||
| val factory: RequestFactory<MyAccountException> = mock() | ||
| val account = Auth0.getInstance(CLIENT_ID, DOMAIN) | ||
|
|
||
| whenever(auth0UserAgent.value).thenReturn("the-user-agent-data") | ||
| account.auth0UserAgent = auth0UserAgent | ||
| MyAccountAPIClient(account, ACCESS_TOKEN, factory, gson) | ||
|
|
||
| verify(factory).setAuth0ClientInfo("the-user-agent-data") |
Changes
This PR adds user-agent info to
MyAccountAPIClientandMfaApiClientAPI requests.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