Native Auth V2: Sign-up - #3240
Conversation
|
❌ Work item link check failed. Description does not contain AB#{ID}. Click here to Learn more. |
…-auth-v2-signup # Conflicts: # common4j/src/main/com/microsoft/identity/common/java/eststelemetry/PublicApiId.java
Cross-platform review: CIAM Native Auth V2 sign-up (Android common ↔ MSAL iOS
|
| Location matched | |
|---|---|
Android (mapInteractionError) |
error.innerError.details[].code == "userAlreadyExists" |
iOS (MSALNativeAuthV2ResponseParser.flowError(from:)) |
top-level error.code == "userAlreadyExists" |
iOS's MSALNativeAuthV2HALResponseSerializer.parseError doesn't parse innerError.details at all, so it structurally cannot see the shape your test fixture asserts:
"error": { "code": "invalidRequest", "innerError": { "details": [
{ "attributeIds": ["email"], "code": "userAlreadyExists", "message": "..." } ] } }Impact. At most one of these is correct against the real CIAM v2 HAL contract. Whichever is wrong silently degrades userAlreadyExists to a generic error — on iOS today, .generalError; on Android, APIError. Apps then can't offer "you already have an account, sign in instead."
Recommendation. Confirm the wire shape with the ESTS/CIAM v2 contract owners and align both SDKs. If the server can emit either shape, match both (top-level code OR any innerError.details[].code) and file the corresponding iOS work item. Please link the contract reference in the PR description so reviewers on both platforms can verify.
3. Severity: Medium — attributeValidationError → AttributesInvalid has no iOS counterpart
Issue. INNER_ERROR_ATTRIBUTE_VALIDATION_FAILED = "attributeValidationError" (matched on error.innerError.code) produces NativeAuthV2InteractionApiResult.InvalidAttributes → the public NativeAuthResultV2.AttributesInvalid with a retryable AttributesInvalidStateV2.
The iOS V2 parser has no mapping for attributeValidationError. MSALNativeAuthAttributesInvalidState exists as a class, but nothing in the V2 code path constructs it — the attributeValidationFailed handling on iOS is V1-only (network/responses/validator/sign_up/).
Impact. The same server response (e.g. a password-policy violation on submit) is an actionable, retryable state on Android and an opaque generalError on iOS. This is the largest public-contract divergence in the PR, and it's in the good direction — but it means "mirrors the iOS V2 sign-up scenarios" isn't accurate for this branch.
Recommendation. Keep the Android behaviour and file the iOS gap, or hold this mapping back until iOS lands it. Either way, call it out explicitly in the PR description so the platforms don't quietly diverge.
4. Severity: Medium — parseCollectAttributes has no self fallback for the submitAttributes link
Issue. iOS resolves the submit target defensively:
collectAttributesResponse.href(for: .submitAttributes) ?? collectAttributesResponse.href(for: .self)(the same ?? .self fallback it uses for .update). Android returns missingLinkError(...) the moment links["submitAttributes"] is absent. Compounding it, self isn't in NativeAuthV2ContinuationState.SUPPORTED_RELATIONS, so a self href would be dropped from the continuation state even if the response carried one.
Impact. A collectAttributes body that only self-links — the exact shape iOS defends against — works on iOS and hard-fails mid-flow on Android.
Recommendation. Mirror the iOS fallback and add self to SUPPORTED_RELATIONS. If the fallback is deliberately omitted because the contract guarantees submitAttributes, please note that in the KDoc and file the iOS side as dead code.
5. Severity: Medium — the password wipe doesn't do what the docs claim
Issue. NativeAuthV2FlowController.upfrontAttributeValues:
values[ATTRIBUTE_NAME_PASSWORD] = String(password)That materializes an immutable heap String, which then flows into NativeAuthV2SubmitAttributesRequestParameters.attributes (Map<String, String>), through the request body, and is never cleared. The finally { StringUtil.overwriteWithNull(parameters.password) } in signUpStart only zeroes the char[].
SignUpV2StartCommandParameters's Javadoc states "The password is never retained in continuation or public state; the controller clears the buffer once the request has been issued," and the inline comment says it's cleared "so it never outlives the request." Neither holds for the String copy — it lives until GC and can be captured in a heap dump.
Impact. Not a new leak relative to the rest of the SDK, but the documented security property is stronger than the implementation. That mismatch is the risk: a future reader will trust the comment.
Recommendation. Either (a) thread the password to the JSON body as char[]/CharSequence so no immutable copy is created, or (b) correct the Javadoc and inline comments to describe what actually happens. The same pattern appears in msal#2565 PasswordRequiredStateV2.submitSignUpPassword (mapOf("password" to String(passwordCopy))) with an equally strong comment.
6. Severity: Medium — attribute values are Map<String, String>; iOS uses [String: Any]
Issue. NativeAuthV2SubmitAttributesRequest, createSubmitAttributesRequest, performSubmitAttributes, SignUpV2StartCommandParameters.attributes, and NativeAuthV2SubmitAttributesCommandParameters.attributes are all Map<String, String>. iOS's MSALNativeAuthV2SubmitAttributesRequestBody.attributes is [String: Any], guarded by JSONSerialization.isValidJSONObject, so numeric/boolean CIAM extension attributes serialize with their native JSON type.
Impact. An extension_age: 30 attribute goes out as "30" from Android and 30 from iOS. If the directory schema types the attribute as a number, one of those is rejected.
Assumption: this may be a deliberate Android-wide constraint — V1 UserAttributes is also string-only — in which case disregard. But please confirm the v2 submitAttributes contract coerces stringified values for non-string attribute types; if it doesn't, this is a functional gap rather than a style difference.
7. Severity: Medium — no email-channel guard on the sign-up CodeRequired branch
Issue. iOS's handleSignUpInteractionResult guards the .verificationRequired case:
guard challenge.channelType.isEmailType else { /* general error:
"Sign up currently supports email one-time-code verification only" */ }Android's CodeRequired branch in handleSignUpInteractionResult surfaces whatever challengeChannel the server sent, with no check.
Impact. If a tenant is configured with SMS OTP for sign-up, Android hands the app a CodeRequiredStateV2 for a channel the flow was never validated against, while iOS fails fast with a clear message. Divergent and harder to diagnose.
Recommendation. Add the same guard, or document why Android intentionally accepts all channels.
8. Severity: Medium — AttributesInvalid.invalidAttributes flattens unrelated detail entries
Issue.
serverError.details.flatMap { it.attributeIds }.distinct()details is the full innerError.details array. Entries whose code is unrelated to the validation failure (e.g. a userAlreadyExists detail carried alongside, which your own fixture comment anticipates) contribute their attributeIds too.
Impact. The app can be told to correct an attribute the server didn't actually reject.
Recommendation. Filter to details that belong to the validation failure before flattening, or document that invalidAttributes is a superset.
9. Severity: Medium — upfront password / attributes are silently discarded unless the first response is collectAttributes
Issue. signUpStart passes upfront = parameters into handleSignUpInteractionResult, but every branch except AttributesRequired ignores it. If the server responds to the entry POST with verify or readyToComplete, the caller's password and attributes are dropped with no signal — the app then gets CodeRequired, and the password it supplied to signUpV2() never reaches the server.
iOS has the identical shape, so this is a shared design gap, not an Android regression. Flagging it because the "entry POST omits the username so we always get collectAttributes back" assumption is load-bearing and undefended on both platforms.
Recommendation. At minimum a Logger.warn when upfront is non-null and gets dropped, so the failure is diagnosable from logs.
10. Severity: Medium — retryState for AttributesInvalid is the pre-submit state
Issue. performSignUpSubmitAttributes passes retryState = state — the state before withAdditionalSubmittedAttributes(...) is applied.
Impact. After a rejected upfront submit, the app retries from a state whose submittedAttributes is empty. If the server subsequently re-requests email (already sent), the flow surfaces AttributesRequired to the app instead of the intended "already submitted" hard error, defeating the loop-detection this PR adds.
Recommendation. Use state.withAdditionalSubmittedAttributes(attributes.keys) as the retry state. The continuation token is unchanged either way, so this is a pure bookkeeping fix.
11. Non-blocking — NativeAuthV2RequiredAttribute.required nullability
iOS coerces a missing required to false (attribute["required"] as? Bool ?? false). Android carries Boolean? all the way to the public RequiredUserAttribute.required, so apps see null on Android where iOS sees false. RequiredUserAttribute is a pre-existing V1 type so changing it isn't in scope here, but the V2 NativeAuthV2RequiredAttribute could default it.
Cycle summary
- New issues: 11 (2 High, 8 Medium, 1 non-blocking)
- Resolved issues: —
- Remaining blockers: Update Readme #1 (
SignInAfterSignUpRequiredunreachable fromsignUpStart— account created, generic error returned) and Updated readme. closes #1 #2 (userAlreadyExistswire-location contradiction with iOS).
Verified as not issues, for the record: parcel read/write symmetry in the new V2 states; routing the sign-up password through submitAttributes rather than the sign-in submit-password endpoint; submittedAttributes case-insensitivity (lowercase() ≈ iOS caseInsensitiveCompare); HAL action string values; and the absence of reserved-name filtering in the deferred submitAttributes path (iOS behaves the same).
Summary
Adds the common / common4j layer for Native Auth V2 sign-up, alongside the existing V2 sign-in and SSPR support.
Mirrors the iOS V2 sign-up scenarios (AzureAD/microsoft-authentication-library-for-objc#3093) and follows the established V2 sign-in / SSPR code shape:
submittedAttributesinheritance.NativeAuthV2FlowControllersign-up orchestration; sharedsubmitCodeReadyToComplete → SignInAfterSignUpRequired.Tests
Draft — depends on the paired msal PR.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com