fix(auth_identity): accept @ in down-level account names

[GlassOnTin]

Fixes #718.

What

Username::new_down_level_logon_name rejected any account name containing @ as MixedFormat. That left a Microsoft account — where the entire e-mail address is the account name — with no representable form: MicrosoftAccount\me@example.com (the qualification Microsoft documents for users) and Username::new("me@example.com", Some("MicrosoftAccount")) both errored, and the bare e-mail parses as a UPN whose account_name() is truncated to me.

The check now rejects only \. Once the down-level format is established — by the backslash in the string or by an explicit domain argument — the account name is opaque. Windows itself accepts such logon names, and FreeRDP likewise splits only on the backslash.

What deliberately does not change

• Username::parse("me@example.com") still parses as a UPN with account_name() == "me". The bare e-mail is indistinguishable from an AD UPN, and guessing "Microsoft account" on sight would break AD logons. Under NLA a Microsoft account therefore needs the qualified form — but now it has one.
• Every previously-valid input parses exactly as before; the change strictly widens the accepted set.

Semantics note, now documented on parse

A value containing both separators is a down-level logon name: the \ takes precedence and everything after it is the account name, matching how Windows reads the qualification. The inverse asymmetry also exists and is inherent to the string form: a domain-less down-level name built directly via new_down_level_logon_name("me@example.com", "") stringifies to something parse reads back as a UPN. The typed constructors are the unambiguous path; happy to instead reject @ in the empty-domain case if you'd rather preserve universal round-tripping.

Tests

• microsoft_account_forms_are_representable pins the three forms from Microsoft account usernames are unrepresentable: account name truncated at '@', qualified forms rejected #718: qualified string ✓, split username+domain ✓ (equal to the qualified form), bare e-mail unchanged as UPN.
• down_level_logon_name_round_trip proptest alphabet now includes @ in account names.
• The username_format_conversion proptest guard is narrowed to the one genuinely non-reconstructible combination (no domain + @ in the account).
• 249/249 lib tests pass; fmt and clippy clean.

Downstream context

Hit via IronRDP → CredSSP → NTLM: a Haven for Android user RDPing to a consumer Windows 11 box (Microsoft-account login) gets user = "me", domain = "example.com" on the wire and lands on the lock screen; the qualified spelling they were told to use is rejected before it reaches the wire (GlassHaven/Haven#461). With this change the qualified forms reach NTLM as user = "me@example.com", domain = "MicrosoftAccount", which is what mstsc sends.

[Pavlo Myroniuk (TheBestTvarynka)]

LGTM.

I also did some small dev testing using FreeRDP and MsRdpEx. Seems like everything works just fine.

[Copilot]

🟡 Not ready to approve

Domain-less names containing @ can change from down-level format to UPN during buffer round-trips.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Allows qualified Microsoft account email addresses to remain intact as down-level logon names.

Changes:

• Permits @ in down-level account names.
• Documents separator precedence.
• Adds Microsoft-account and property-test coverage.

File summaries

File	Description
src/auth_identity.rs	Updates validation, documentation, and username tests.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Benoît Cortier (CBenoit)]

Thank you! Can you double check the Copilot comment please?

[Benoît Cortier (CBenoit)]

I’ve reviewing myself a little bit more in depth -- thanks for the very thorough write-up, the description made this much easier to review.

Pavlo and I went through this together, and neither of us is particularly familiar with the MicrosoftAccount\me@example.com shape. I didn't know it was a thing at all when I wrote the Username helper, so the code simply assumed down-level and UPN were mutually exclusive: MixedFormat was meant to reject what I believed was nonsense input, not a legitimate Windows form.

What gives us pause is less the patch than the shape it leaves behind:

• Username::parse("MicrosoftAccount\\me@example.com").account_name() == "me@example.com"
• Username::parse("me@example.com").account_name() == "me"

Same identity, and the answer to "what is the account name?" changes depending on whether the qualification is present. You document the precedence rule clearly and the reasoning behind it is sound, so this isn't an objection to the behavior as such, but it's that the type now seems to be modelling the domain slightly wrong. It has no good answer for "can down-level and UPN mix, and if so what do the accessors mean?", and the patch answers that locally rather than at the level of the model.

Which brings me to the question we can't settle on our own: is MicrosoftAccount\ (and AzureAD\, presumably) the only place where the two formats legitimately mix, or is this a general property of the down-level form that we'll keep hitting? If it's the former, a targeted widening like this one is probably the right size of fix. If it's the latter, we would rather revisit Username as a whole: what an account name is, what a domain name is, which combinations are representable and which aren't, instead of relaxing the validation one case at a time. You've clearly dug into this more than we have; do you have a sense of which it is?

To be clear about where we land: the change is strictly widening and every previously-valid input parses as before, so we're happy to merge as-is if unblocking the downstream case matters now, and treat the API rework as a follow-up. I mainly wanted to raise the modelling concern before it gets baked in rather than after.

On your offer at the end (rejecting @ in the empty-domain case to preserve universal round-tripping): I think that folds into the same decision. If we keep the narrow fix, round-tripping is probably worth preserving; if we rework the API, the typed constructors become the answer anyway.

[GlassOnTin]

Double-checked the Copilot comment: it's right, and I reproduced it independently before looking at the suggested fix — adding new_down_level_logon_name("frank@example.com", "") to auth_identity_buffers_round_trip_preserves_format fails with format: UserPrincipalName against the expected DownLevelLogonName. Pavlo Myroniuk (@TheBestTvarynka) has already pushed the fix (2e05ed1), which is tidier than the version I'd written, so I've dropped mine and pushed only a regression test on top (fd7d203). I checked it earns its place: it fails if the netbios_domain_name.is_empty() && account_name.contains('@') clause is removed. 250/250 lib tests pass.

On the modelling question

You asked whether MicrosoftAccount\ (and AzureAD\) is the only place the two formats legitimately mix, or whether this is a general property of the down-level form. It's the general case, and I think that settles which way to go.

The SAM-Account-Name schema reference gives the disallowed set verbatim:

This attribute must be 20 characters or fewer to support earlier clients, and cannot contain any of these characters:
" / \ [ ] : ; | = , + * ? < >

@ is not in that list. So CONTOSO\bob@corp is a perfectly ordinary down-level logon name with nothing to do with Microsoft accounts — MicrosoftAccount\me@example.com is just its most visible instance, not a special case to carve out. Conversely \ is in that list, and @ is disallowed in NetBIOS domain names.

That gives the asymmetry a principled basis rather than a stylistic one:

• \ is a true structural separator. It cannot appear inside a NetBIOS domain name or inside a SAM account name, so "split on the first \" is exactly right and never ambiguous.
• @ is not. It can appear inside a down-level account name, so it only indicates a UPN in the absence of a \ — and even then not reliably, which is precisely the residual me@example.com ambiguity.

So the precedence rule in this PR isn't a local patch to keep an odd input working; it's what the underlying alphabets require. Under that reading the MixedFormat error was never modelling a real constraint — \ in an account name and @ in a NetBIOS domain are genuinely invalid, and both are still rejected, but "an @ in a down-level account name" never was.

Where that leaves your two accessors:

• Username::parse("MicrosoftAccount\\me@example.com").account_name() == "me@example.com" — correct and unambiguous.
• Username::parse("me@example.com").account_name() == "me" — the only defensible answer for a string, because that string genuinely is ambiguous. No parser can resolve it; it's indistinguishable from an AD UPN.

Which is to say the awkwardness you're pointing at is real but isn't in Username — it's in the string form, and it can only be removed by not going through a string. Your instinct that the typed constructors become the answer is where I'd land too: keep parse as the best-effort reading of an ambiguous string, and treat new_upn / new_down_level_logon_name as the way a caller states which format it means. This PR arguably nudges the model that way already, since after it the down-level constructor is the only way to express an MSA name.

If you'd rather rework Username as a whole before taking this, I'm happy to wait — but I don't think this change bakes in anything you'd have to undo, because it removes a restriction that the AD alphabet says was never correct. Merging it now and revisiting the model separately seems right to me, though it's your call and I'm content either way.

For the downstream context, since you asked about urgency: this unblocks Microsoft-account logons over NLA in Haven, an Android RDP client — currently those users have to turn NLA off at both ends. Not an emergency, and I'd rather it land in a shape you're happy to maintain.