Skip to content

Conversation

@utkrishtsahu
Copy link
Contributor

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

@utkrishtsahu utkrishtsahu requested a review from a team as a code owner July 21, 2025 03:47
Comment on lines 26 to 49
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

Unused class: Deserializer is not referenced within this codebase. If not used as an external API it should be removed.

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.

* 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`.
Copy link
Contributor

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

Copy link
Contributor Author

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")
Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are optional parameters

Copy link
Contributor

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

public val recoveryCode: String
) : EnrollmentChallenge(id, authSession)

public data class PublicKeyCredentialCreationOptions(
Copy link
Contributor

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(
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

@pmathew92 pmathew92 Sep 4, 2025

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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

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")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

Copy link
Contributor Author

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
Copy link
Contributor

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

Copy link
Contributor Author

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

import com.auth0.android.request.JsonAdapter
import com.auth0.android.request.PublicKeyCredentials
import com.auth0.android.request.Request
import com.auth0.android.request.*
Copy link
Contributor

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

Copy link
Contributor Author

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,
Copy link
Contributor

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

Copy link
Contributor Author

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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Format the code samples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the indentation

Copy link
Contributor Author

@utkrishtsahu utkrishtsahu Sep 4, 2025

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> {
Copy link
Contributor

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

Copy link
Contributor Author

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

@pmathew92 pmathew92 requested a review from Copilot August 19, 2025 05:20

This comment was marked as outdated.

@SerializedName("barcode_uri")
public val barcodeUri: String,
@SerializedName("manual_input_code")
public val manualInputCode: String
Copy link
Contributor

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

@pmathew92 pmathew92 requested a review from Copilot September 4, 2025 04:48
Copy link

Copilot AI left a 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.

Comment on lines 21 to 39
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")
Copy link

Copilot AI Sep 4, 2025

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 21 to 39
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")
Copy link

Copilot AI Sep 4, 2025

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 21 to 39
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")
Copy link

Copilot AI Sep 4, 2025

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 21 to 39
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")
Copy link

Copilot AI Sep 4, 2025

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 21 to 39
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")
Copy link

Copilot AI Sep 4, 2025

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.

Suggested change
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")

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

)
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.")
Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

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() })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Contributor

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the indentation

Copy link
Contributor Author

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.")
* }
Copy link
Contributor

@pmathew92 pmathew92 Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the indentation

Copy link
Contributor Author

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
Copy link
Contributor

@pmathew92 pmathew92 Sep 4, 2025

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 ?

Copy link
Contributor Author

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.


/**
* Represents a standardized error response from the My Account API.
*/
Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 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.
  2. 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).

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed [ErrorResponse.kt]

@pmathew92
Copy link
Contributor

Add the samples in Example.md file
Also there are some indendation issues on code samples in the comments. Fix them as well

PasskeyAuthenticationMethod::class.java
)
return factory.patch(url.toString(), GsonAdapter(AuthenticationMethod::class.java, gson))
.addParameters(params.mapValues { it.value.toString() })
Copy link
Contributor

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> {
Copy link
Contributor

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 ?

Copy link
Contributor Author

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> {
Copy link
Contributor

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

Copy link
Contributor Author

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>() {
Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

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

Copy link
Contributor Author

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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

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

Copy link
Contributor Author

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
Copy link
Contributor

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 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

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

Copy link
Contributor Author

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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in line 1190

Copy link
Contributor Author

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.
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

pmathew92
pmathew92 previously approved these changes Sep 10, 2025
@utkrishtsahu utkrishtsahu merged commit 86a90e7 into main Sep 12, 2025
6 checks passed
@utkrishtsahu utkrishtsahu deleted the SDK-6103-My-Account-Auth-Methods-Android-Support branch September 12, 2025 11:07
@utkrishtsahu utkrishtsahu mentioned this pull request Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants