fix(phone): set phone_verified when a phone number is confirmed

[kanakkholwal]

What kind of change does this PR introduce?

Bug fix. Fixes #1906

What is the current behavior?

phone_verified stays false forever after a phone is confirmed, in both
raw_user_meta_data and the phone identity's identity_data. Only
phone_confirmed_at is set.

Not Twilio Verify specific — also happens with the default SMS providers and a
custom send_sms hook.

Cause: Confirm() (email) writes email_verified: true, but ConfirmPhone() only
wrote phone_confirmed_at. All four callers inherited the gap.

What is the new behavior?

Fixed in models.ConfirmPhone instead of at the four call sites, mirroring
Confirm():

• ConfirmPhone sets phone_verified: true on user metadata and on the phone
  identity, plus phone on the identity (signup never populates it).
• ConfirmPhoneChange gets the matching user-metadata update.

Added TestConfirmPhone, extended TestConfirmPhoneChange.
go test ./internal/api/ passes in full.

Additional context

none

[depthfirst-app]

🟡 Severity: MEDIUM

The new raw_user_meta_data.phone_verified value looks like a system verification signal, but an authenticated user can send arbitrary data to PUT /user, which UserUpdate stores through UpdateUserMetaData. They can set it true without OTP, so applications trusting this metadata may grant phone-verified privileges.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: The root cause is that raw_user_meta_data is both a system-managed store (where ConfirmPhone writes phone_verified: true) and a user-writable store (where PUT /user merges arbitrary params.Data). To prevent spoofing, strip protected system-managed keys from user-supplied metadata before persisting it.

In internal/api/user.go, inside the UserUpdate handler (around the if params.Data != nil block at line ~239), delete the reserved keys from the user-supplied map before calling UpdateUserMetaData:

if params.Data != nil {
    // Prevent users from spoofing system-managed verification signals.
    protectedKeys := []string{"phone_verified", "email_verified"}
    for _, k := range protectedKeys {
        delete(params.Data, k)
    }
    if terr = user.UpdateUserMetaData(tx, params.Data); terr != nil {
        return apierrors.NewInternalServerError("Error updating user").WithInternalError(terr)
    }
}

This ensures that even if a client sends {"phone_verified": true} in the PUT /user request body, the field is silently dropped and only the actual OTP flow (via ConfirmPhone) can write it.

[depthfirst-app]

⚪ Severity: LOW

The new identity-level verification signal can be pre-populated from caller-controlled signup metadata: SignupParams.Data is copied into identity_data before confirmation. An attacker can submit data.phone_verified=true, causing consumers that trust this identity field to treat an unconfirmed phone number as verified.
Helpful? Add 👍 / 👎

💡 Fix Suggestion

Suggestion: The vulnerability is not in the ConfirmPhone() function itself (where phone_verified: true is correctly set upon actual confirmation), but in internal/api/signup.go at lines 213–217, where user-controlled params.Data is merged into identityData without stripping privileged verification signals. To fix this, add a blocklist of reserved keys that should never be set from caller-supplied data. In the loop at signup.go lines 213–217, skip keys such as phone_verified, email_verified, phone_confirmed_at, email_confirmed_at, and any other system-controlled fields before copying from params.Data into identityData. For example:

blockedKeys := map[string]struct{}{
    "phone_verified": {}, "email_verified": {},
    "phone_confirmed_at": {}, "email_confirmed_at": {},
}
for k, v := range params.Data {
    if _, blocked := blockedKeys[k]; blocked {
        continue
    }
    if _, ok := identityData[k]; !ok {
        identityData[k] = v
    }
}

This mirrors the approach already used in custom_oauth_admin.go (lines 756–757) where the same fields are explicitly blocked from being set via external input.