Added support to enroll passkeys with My Account API #837
Conversation
| private val gson: Gson | ||
| ) { | ||
|
|
||
| public val clientId: String |
There was a problem hiding this comment.
Not required. removed
| * ``` | ||
| * | ||
| * Then, call ``enroll()`` with the created passkey credential and the challenge to complete | ||
| * the enrollment |
There was a problem hiding this comment.
| * the enrollment | |
| * the enrollment. |
| */ | ||
| @Throws(IOException::class) | ||
| public fun fromJson(reader: Reader): T | ||
| public fun fromJson(reader: Reader, headers: Map<String, List<String>>): T |
There was a problem hiding this comment.
Would this be a breaking change?
There was a problem hiding this comment.
Maybe an overload with a default implementation (so it doesn't break current conformances) would be enough?
That is, leaving the current interface method as-is –and adding a new one that takes both a reader and a headers map, and that has a default implementation.
There was a problem hiding this comment.
Made this an optional parameter ,so this wouldn't break any existing usage .
| @SerializedName("type") | ||
| val type: String, | ||
| @SerializedName("usage") | ||
| val usage: String, |
There was a problem hiding this comment.
This doesn't seem to be a field present in the OAS scheme.
There was a problem hiding this comment.
Nevermind, this was added recently. I'll have to add it to the iOS implementation.
There was a problem hiding this comment.
I'd suggest not including it as of now, given how the comment in the PR that added it says "This will probably change in future PRs but I'm fine merging as-is.".
| } | ||
|
|
||
| @Test | ||
| public fun `passkeyEnrollmentChallenge should include correct parameters`() { |
There was a problem hiding this comment.
Should we also include a test case that asserts that, by default, no params are being sent other than "type": "passkey"?
| } | ||
| mockAPI.takeRequest() | ||
| assertThat(error, Matchers.notNullValue()) | ||
| assertThat(error?.message, Matchers.`is`("Authentication ID not found")) |
There was a problem hiding this comment.
This should be Authentication method ID not found. This is relevant because the error message is developer-facing.
|
|
||
| @RunWith(RobolectricTestRunner::class) | ||
| @Config(manifest = Config.NONE) | ||
| public class MyAccountAPIClientTest { |
There was a problem hiding this comment.
Should we include test cases for when the My Account API returns an error?
There was a problem hiding this comment.
Added cases returning error
Changes
This PR adds:
Testing
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