-
Notifications
You must be signed in to change notification settings - Fork 165
SDK-6103 Added support for My Account API. #847
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
SDK-6103 Added support for My Account API. #847
Conversation
| internal class Deserializer : JsonDeserializer<AuthenticationMethod> { | ||
| override fun deserialize( | ||
| json: JsonElement, | ||
| typeOfT: Type, | ||
| context: JsonDeserializationContext | ||
| ): AuthenticationMethod? { | ||
| val jsonObject = json.asJsonObject | ||
| val type = jsonObject.get("type")?.asString ?: return null | ||
|
|
||
| val targetClass = when (type) { | ||
| "password" -> PasswordAuthenticationMethod::class.java | ||
| "passkey" -> PasskeyAuthenticationMethod::class.java | ||
| "recovery-code" -> MfaRecoveryCodeAuthenticationMethod::class.java | ||
| "push-notification" -> MfaPushNotificationAuthenticationMethod::class.java | ||
| "totp" -> MfaTotpAuthenticationMethod::class.java | ||
| "webauthn-platform" -> WebAuthnPlatformAuthenticationMethod::class.java | ||
| "webauthn-roaming" -> WebAuthnRoamingAuthenticationMethod::class.java | ||
| "phone" -> PhoneAuthenticationMethod::class.java | ||
| "email" -> EmailAuthenticationMethod::class.java | ||
| else -> UnknownAuthenticationMethod::class.java | ||
| } | ||
| return context.deserialize(jsonObject, targetClass) | ||
| } | ||
| } |
Check notice
Code scanning / CodeQL
Unused classes and interfaces Note
Copilot Autofix
AI 3 months ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
auth0/src/main/java/com/auth0/android/result/EnrollmentChallenge.kt
Dismissed
Show dismissed
Hide dismissed
| * You can use the refresh token to get an access token for the My Account API. Refer to [com.auth0.android.authentication.storage.CredentialsManager.getApiCredentials] | ||
| * , or alternatively [com.auth0.android.authentication.AuthenticationAPIClient.renewAuth] if you are not using CredentialsManager. | ||
| * You can use a refresh token to get an access token for the My Account API. | ||
| * Refer to `CredentialsManager#getApiCredentials` or `AuthenticationAPIClient#renewAuth`. |
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.
Why was the comment changed ? Ensure the comments convey as much detail as possible including samples wherever applicable
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.
done
| * ```kotlin | ||
| * val auth0 = Auth0.getInstance("YOUR_CLIENT_ID", "YOUR_DOMAIN") | ||
| * val client = MyAccountAPIClient(auth0,accessToken) | ||
| * val auth0 = Auth0("YOUR_CLIENT_ID", "YOUR_DOMAIN") |
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.
val auth0 = Auth0.getInstance("YOUR_CLIENT_ID", "YOUR_DOMAIN") is the correct way to access an Auth0 instance . Please correct in the 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.
done
| public fun updateAuthenticationMethodById( | ||
| authenticationMethodId: String, | ||
| preferredAuthenticationMethod: String, | ||
| authenticationMethodName: String |
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.
These are optional parameters
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.
Please make them as optional .Check with @NandanPrabhu
…nd modifying Email, Phone, and TOTP Verification verifyOtp(...)
| public val recoveryCode: String | ||
| ) : EnrollmentChallenge(id, authSession) | ||
|
|
||
| public data class PublicKeyCredentialCreationOptions( |
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.
This seems to be a duplicate of AuthnParamsPublicKey data class in the PasskeyRegistrationChallenge.kt file
| public val identityUserId: String? | ||
| ) : EnrollmentPayload("passkey") | ||
|
|
||
| public data class WebAuthnPlatformEnrollmentPayload( |
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.
This can be a normal class ?
public class WebAuthnPlatformEnrollmentPayload() : EnrollmentPayload("webauthn-roaming")
This way you don't have to add an empty placeholder for the data class
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.
maintain consistency within the EnrollmentPayload sealed class hierarchy.
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.
normal class can also be part of sealed class ?
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") |
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.
Same as above
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.
aintain consistency within the EnrollmentPayload sealed class hierarchy.
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") |
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.
Same
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.
aintain consistency within the EnrollmentPayload sealed class hierarchy.
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null |
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.
Same
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.
aintain consistency within the EnrollmentPayload sealed class hierarchy.
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null |
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.
Same
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.
aintain consistency within the EnrollmentPayload sealed class hierarchy.
| @SerializedName("user_handle") | ||
| val userHandle: String | ||
| ) | ||
| public typealias PasskeyAuthenticationMethod = AuthenticationMethod |
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.
The AuthenticationMethod data class has some mandatory property like usage which the passkeys response might not send . Please cross check them and test this to avoid run time crashes
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.
current code has been updated to use a dedicated data class PasskeyAuthenticationMethod(...) which inherits from the AuthenticationMethod sealed class
auth0/src/main/java/com/auth0/android/result/PasskeyEnrollmentChallenge.kt
Show resolved
Hide resolved
| import com.auth0.android.request.JsonAdapter | ||
| import com.auth0.android.request.PublicKeyCredentials | ||
| import com.auth0.android.request.Request | ||
| import com.auth0.android.request.* |
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.
Please configure your IDE to have specific imports instead of generic one
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.
Done
| */ | ||
| public fun updateAuthenticationMethodById( | ||
| authenticationMethodId: String, | ||
| preferredAuthenticationMethod: String, |
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.
lets limit the preferredAuthenticationMethod type to the accepted values of sms and voice . Lets use an enum type here with string default value
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.
done added enum for the same
| * val apiClient = MyAccountAPIClient(auth0, accessToken) | ||
| * | ||
| * apiClient.enrollPhone("+11234567890", "sms") | ||
| * .start(object : Callback<EnrollmentChallenge, MyAccountException> { |
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.
Format the code samples
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.
done
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.
Fix the indentation
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.
done for all KDOC in [MyAccountAPIClient.kt]
| * @param preferredMethod The preferred method for this factor ("sms" or "voice"). | ||
| * @return a request that will yield an enrollment challenge. | ||
| */ | ||
| public fun enrollPhone(phoneNumber: String, preferredMethod: String): Request<EnrollmentChallenge, MyAccountException> { |
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.
As discussed above ,lets have the preferred method as an enum type
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.
added new enum for the same
| @SerializedName("barcode_uri") | ||
| public val barcodeUri: String, | ||
| @SerializedName("manual_input_code") | ||
| public val manualInputCode: String |
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.
manualInputCode must be optional.we dont get it incase of push notification factor
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 introduces comprehensive My Account API support for managing user authentication methods, including enrollment, verification, and CRUD operations for various multi-factor authentication (MFA) methods.
Key changes include:
- New API methods for authentication method management (CRUD operations)
- Support for enrolling multiple authentication types (email, phone, TOTP, recovery codes)
- Comprehensive verification methods for OTP and non-OTP authentication factors
- Enhanced data models with proper JSON serialization for various authentication method types
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
MyAccountAPIClient.kt |
Core client implementation with new API methods for authentication method management and enrollment flows |
MyAccountAPIClientTest.kt |
Comprehensive test coverage for all new API endpoints and functionality |
AuthenticationMethod.kt |
Sealed class hierarchy for different authentication method types with JSON deserialization |
EnrollmentChallenge.kt |
Sealed class for enrollment challenges with custom deserializer logic |
EnrollmentPayload.kt |
Sealed class hierarchy for enrollment request payloads |
Factor.kt |
Simple data class for factor information |
Factors.kt |
Wrapper class for factor list responses |
ErrorResponse.kt |
Standardized error response structure |
VerificationPayload.kt |
Data class for OTP verification requests |
PasskeyEnrollmentChallenge.kt |
Simplified passkey enrollment challenge without JSON annotations |
PasskeyAuthenticationMethod.kt |
Removed standalone passkey authentication method (now part of sealed class hierarchy) |
PhoneAuthenticationMethod.kt |
Enum for phone authentication method types |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
auth0/src/main/java/com/auth0/android/myaccount/MyAccountAPIClient.kt
Outdated
Show resolved
Hide resolved
auth0/src/main/java/com/auth0/android/myaccount/MyAccountAPIClient.kt
Outdated
Show resolved
Hide resolved
| public data class WebAuthnPlatformEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-platform") | ||
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") | ||
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("recovery-code") |
Copilot
AI
Sep 4, 2025
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.
[nitpick] Multiple classes use identical placeholder properties that serve no functional purpose. Consider using object declarations instead of data classes for payload types that don't require additional properties, or remove the placeholder entirely if not needed for serialization.
| public data class WebAuthnPlatformEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-platform") | |
| public data class WebAuthnRoamingEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-roaming") | |
| public data class TotpEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("totp") | |
| public data class PushNotificationEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("push-notification") | |
| public data class RecoveryCodeEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("recovery-code") | |
| public object WebAuthnPlatformEnrollmentPayload : EnrollmentPayload("webauthn-platform") | |
| public object WebAuthnRoamingEnrollmentPayload : EnrollmentPayload("webauthn-roaming") | |
| public object TotpEnrollmentPayload : EnrollmentPayload("totp") | |
| public object PushNotificationEnrollmentPayload : EnrollmentPayload("push-notification") | |
| public object RecoveryCodeEnrollmentPayload : EnrollmentPayload("recovery-code") |
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.
done
| public data class WebAuthnPlatformEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-platform") | ||
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") | ||
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("recovery-code") |
Copilot
AI
Sep 4, 2025
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.
[nitpick] Multiple classes use identical placeholder properties that serve no functional purpose. Consider using object declarations instead of data classes for payload types that don't require additional properties, or remove the placeholder entirely if not needed for serialization.
| public data class WebAuthnPlatformEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-platform") | |
| public data class WebAuthnRoamingEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-roaming") | |
| public data class TotpEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("totp") | |
| public data class PushNotificationEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("push-notification") | |
| public data class RecoveryCodeEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("recovery-code") | |
| public object WebAuthnPlatformEnrollmentPayload : EnrollmentPayload("webauthn-platform") | |
| public object WebAuthnRoamingEnrollmentPayload : EnrollmentPayload("webauthn-roaming") | |
| public object TotpEnrollmentPayload : EnrollmentPayload("totp") | |
| public object PushNotificationEnrollmentPayload : EnrollmentPayload("push-notification") | |
| public object RecoveryCodeEnrollmentPayload : EnrollmentPayload("recovery-code") |
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.
done
| public data class WebAuthnPlatformEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-platform") | ||
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") | ||
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("recovery-code") |
Copilot
AI
Sep 4, 2025
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.
[nitpick] Multiple classes use identical placeholder properties that serve no functional purpose. Consider using object declarations instead of data classes for payload types that don't require additional properties, or remove the placeholder entirely if not needed for serialization.
| public data class WebAuthnPlatformEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-platform") | |
| public data class WebAuthnRoamingEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-roaming") | |
| public data class TotpEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("totp") | |
| public data class PushNotificationEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("push-notification") | |
| public data class RecoveryCodeEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("recovery-code") | |
| public object WebAuthnPlatformEnrollmentPayload : EnrollmentPayload("webauthn-platform") | |
| public object WebAuthnRoamingEnrollmentPayload : EnrollmentPayload("webauthn-roaming") | |
| public object TotpEnrollmentPayload : EnrollmentPayload("totp") | |
| public object PushNotificationEnrollmentPayload : EnrollmentPayload("push-notification") | |
| public object RecoveryCodeEnrollmentPayload : EnrollmentPayload("recovery-code") |
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.
done
| public data class WebAuthnPlatformEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-platform") | ||
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") | ||
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("recovery-code") |
Copilot
AI
Sep 4, 2025
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.
[nitpick] Multiple classes use identical placeholder properties that serve no functional purpose. Consider using object declarations instead of data classes for payload types that don't require additional properties, or remove the placeholder entirely if not needed for serialization.
| public data class WebAuthnPlatformEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-platform") | |
| public data class WebAuthnRoamingEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-roaming") | |
| public data class TotpEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("totp") | |
| public data class PushNotificationEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("push-notification") | |
| public data class RecoveryCodeEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("recovery-code") | |
| public object WebAuthnPlatformEnrollmentPayload : EnrollmentPayload("webauthn-platform") | |
| public object WebAuthnRoamingEnrollmentPayload : EnrollmentPayload("webauthn-roaming") | |
| public object TotpEnrollmentPayload : EnrollmentPayload("totp") | |
| public object PushNotificationEnrollmentPayload : EnrollmentPayload("push-notification") | |
| public object RecoveryCodeEnrollmentPayload : EnrollmentPayload("recovery-code") |
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.
done
| public data class WebAuthnPlatformEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-platform") | ||
|
|
||
| public data class WebAuthnRoamingEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("webauthn-roaming") | ||
|
|
||
| public data class TotpEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("totp") | ||
|
|
||
| public data class PushNotificationEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("push-notification") | ||
|
|
||
| public data class RecoveryCodeEnrollmentPayload( | ||
| private val placeholder: String? = null | ||
| ) : EnrollmentPayload("recovery-code") |
Copilot
AI
Sep 4, 2025
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.
[nitpick] Multiple classes use identical placeholder properties that serve no functional purpose. Consider using object declarations instead of data classes for payload types that don't require additional properties, or remove the placeholder entirely if not needed for serialization.
| public data class WebAuthnPlatformEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-platform") | |
| public data class WebAuthnRoamingEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("webauthn-roaming") | |
| public data class TotpEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("totp") | |
| public data class PushNotificationEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("push-notification") | |
| public data class RecoveryCodeEnrollmentPayload( | |
| private val placeholder: String? = null | |
| ) : EnrollmentPayload("recovery-code") | |
| public object WebAuthnPlatformEnrollmentPayload : EnrollmentPayload("webauthn-platform") | |
| public object WebAuthnRoamingEnrollmentPayload : EnrollmentPayload("webauthn-roaming") | |
| public object TotpEnrollmentPayload : EnrollmentPayload("totp") | |
| public object PushNotificationEnrollmentPayload : EnrollmentPayload("push-notification") | |
| public object RecoveryCodeEnrollmentPayload : EnrollmentPayload("recovery-code") |
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.
done
auth0/src/test/java/com/auth0/android/myaccount/MyAccountAPIClientTest.kt
Outdated
Show resolved
Hide resolved
auth0/src/test/java/com/auth0/android/myaccount/MyAccountAPIClientTest.kt
Outdated
Show resolved
Hide resolved
| ) | ||
| val location = (metadata[LOCATION_KEY] as? List<*>)?.filterIsInstance<String>()?.firstOrNull() | ||
| val authId = location?.split("/")?.lastOrNull()?.let { URLDecoder.decode(it, "UTF-8") } | ||
| ?: throw MyAccountException("Authentication method ID not found in Location header.") |
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.
nit: Continuous chaining may affect the readability of the code especially for such a long set of actions. Ensure the code is readable and easier to understand
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.
made readable avoided chaining.
| val post = factory.post(url.toString(), passkeyEnrollmentAdapter) | ||
| .addParameters(params) | ||
| return factory.post(url.toString(), passkeyEnrollmentAdapter) | ||
| .addParameters(params.mapValues { it.value.toString() }) // FIX: Safely convert map values to String |
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.
What is the need for this change.params is already a dictionary of strings
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.
fixed
| .asDictionary() | ||
|
|
||
| return factory.post(url.toString(), GsonAdapter(PasskeyAuthenticationMethod::class.java, gson)) | ||
| .addParameters(params.mapValues { it.value.toString() }) // FIX: Safely convert map values to String |
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.
Same as above
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.
fixed
| PasskeyAuthenticationMethod::class.java | ||
| ) | ||
| return factory.patch(url.toString(), GsonAdapter(AuthenticationMethod::class.java, gson)) | ||
| .addParameters(params.mapValues { it.value.toString() }) |
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.
Same as above
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.
fixed
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.
Seems to be a confusion. The change to be done is .addParameters(params) . Why do you need to add mapValues to the params. It is already a map of String. Check any of the existing api in AuthenticationAPIClient or passkeys above. We are just passing params as such.
| * | ||
| * apiClient.enrollEmail("[email protected]") | ||
| * .start(object : Callback<EnrollmentChallenge, MyAccountException> { | ||
| * override fun onSuccess(result: EnrollmentChallenge) { |
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.
Fix the indentation
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.
done
| * override fun onSuccess(result: EnrollmentChallenge) { | ||
| * // The result will be a TotpEnrollmentChallenge containing a barcode_uri | ||
| * Log.d("MyApp", "Enrollment started for Push Notification.") | ||
| * } |
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.
Fix the indentation
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.
done
| private fun createEnrollmentRequest(params: Map<String, Any>): Request<EnrollmentChallenge, MyAccountException> { | ||
| val url = getDomainUrlBuilder().addPathSegment(AUTHENTICATION_METHODS).build() | ||
| return factory.post(url.toString(), GsonAdapter(EnrollmentChallenge::class.java, gson)) | ||
| .addParameters(params.mapValues { it.value.toString() }) // FIX: Safely convert map values to String |
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.
Where is the parameter being created in this method ?
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.
Changed the name for more clarity and refactored the code.
auth0/src/main/java/com/auth0/android/myaccount/PhoneAuthenticationMethod.kt
Show resolved
Hide resolved
|
|
||
| /** | ||
| * Represents a standardized error response from the My Account API. | ||
| */ |
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.
these are already defined in the MyAccountException class . What is the need to redefine this ?
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.
- ErrorResponse: This class is a pure Data Transfer Object (DTO). Its structure is a direct 1-to-1 mapping of the JSON error payload defined in the OpenAPI specification. It represents the raw error data from the server.
- MyAccountException: This is the actual exception class that our SDK uses to signal a failure to the developer. It consumes the data from the error response but also adds SDK-specific context, like the ability to wrap an underlying
cause(e.g., a network error).
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.
Yes I understand that. What I meant is ,the error you receive from the API is mapped to MyAccountException in the network layer. The above parameters are already defined in that class which the user can access . I also don't see any usage of this class also in the code. So delete it ,if not being used
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.
removed [ErrorResponse.kt]
|
Add the samples in Example.md file |
| PasskeyAuthenticationMethod::class.java | ||
| ) | ||
| return factory.patch(url.toString(), GsonAdapter(AuthenticationMethod::class.java, gson)) | ||
| .addParameters(params.mapValues { it.value.toString() }) |
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.
Seems to be a confusion. The change to be done is .addParameters(params) . Why do you need to add mapValues to the params. It is already a map of String. Check any of the existing api in AuthenticationAPIClient or passkeys above. We are just passing params as such.
| return buildEnrollmentRequest(params) | ||
| } | ||
|
|
||
| private fun buildEnrollmentRequest(params: Map<String, Any>): Request<EnrollmentChallenge, MyAccountException> { |
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.
This can be Map<String,String> . Or any place you are sending anything other than String ?
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.
done
EXAMPLES.md
Outdated
| **Scopes required:** `read:me:factors` | ||
| ```kotlin | ||
| myAccountClient.getFactors() | ||
| .start(object : Callback<Factors, MyAccountException> { |
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.
this is List in the actual method. Correct it here
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.
done
EXAMPLES.md
Outdated
|
|
||
| ```java | ||
| myAccountClient.getFactors() | ||
| .start(new Callback<Factors, MyAccountException>() { |
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.
List . Need to be corrected here
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.
done
EXAMPLES.md
Outdated
| myAccountClient.enrollTotp() | ||
| .start(object : Callback<EnrollmentChallenge, MyAccountException> { | ||
| override fun onSuccess(result: EnrollmentChallenge) { | ||
| val totpChallenge = result as TotpEnrollmentChallenge |
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.
Why do the users need to explicitly cast them to TotpEnrollmentChallenge ? Why can't we return an instance of TotpEnrollmentChallenge ?
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.
done
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.
Might want to make the necessary changes in line 1112 and 1113
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.
done
EXAMPLES.md
Outdated
| myAccountClient.enrollPushNotification() | ||
| .start(object : Callback<EnrollmentChallenge, MyAccountException> { | ||
| override fun onSuccess(result: EnrollmentChallenge) { | ||
| val pushChallenge = result as TotpEnrollmentChallenge // Uses the same response format as TOTP |
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.
Same as above
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.
done
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.
Necessary changes in line 1142 and 1143
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.
done
EXAMPLES.md
Outdated
| .start(new Callback<EnrollmentChallenge, MyAccountException>() { | ||
| @Override | ||
| public void onSuccess(EnrollmentChallenge result) { | ||
| TotpEnrollmentChallenge pushChallenge = (TotpEnrollmentChallenge) result; |
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.
Same as above
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.
done
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.
Changes in line 1157 and 1158
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.
done
EXAMPLES.md
Outdated
| myAccountClient.enrollRecoveryCode() | ||
| .start(object : Callback<EnrollmentChallenge, MyAccountException> { | ||
| override fun onSuccess(result: EnrollmentChallenge) { | ||
| val recoveryChallenge = result as RecoveryCodeEnrollmentChallenge |
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.
Why can't we return an instance of RecoveryCodeEnrollmentChallenge ?
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.
done
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.
Changes in line 1173 and 1174
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.
done
EXAMPLES.md
Outdated
| @Override | ||
| public void onSuccess(RecoveryCodeEnrollmentChallenge result) { | ||
| RecoveryCodeEnrollmentChallenge recoveryChallenge = (RecoveryCodeEnrollmentChallenge) result; | ||
| // Display and require the user to save recoveryChallenge.getRecoveryCode() |
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.
Changes in line 1190
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.
done
| } | ||
|
|
||
| /** | ||
| * Confirms the enrollment for factors that do not require an OTP, like Push Notification or Recovery Code. |
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.
Adding the like Push Notification or Recovery Code. may convey the wrong message that this method can be used for push notification and recovery code. Either remove this or convey it to avoid any confusion
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.
done
This PR introduces new sets of API for the MyAccount feature .
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