Skip to content

Commit 4878c50

Browse files
authored
fix(auth_identity): accept @ in down-level account names (#719)
1 parent 6d17708 commit 4878c50

1 file changed

Lines changed: 65 additions & 4 deletions

File tree

‎src/auth_identity.rs‎

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,14 @@ impl Username {
174174
}
175175

176176
/// Builds a down-level logon name from an account name and a NetBIOS domain name
177+
///
178+
/// The account name may contain `@`: once the down-level format is established, the account
179+
/// name is opaque. Windows itself accepts `MicrosoftAccount\user@example.com`, where the
180+
/// entire e-mail address *is* the account name (a Microsoft account, not an AD UPN), and
181+
/// rejecting the `@` here left such accounts with no representable form at all (#718).
177182
pub fn new_down_level_logon_name(account_name: &str, netbios_domain_name: &str) -> Result<Self, UsernameError> {
178-
if account_name.contains(['\\', '@']) {
183+
// NOTE: account names may contain `@` (Microsoft accounts, AD names with an embedded `@`)
184+
if account_name.contains('\\') || (netbios_domain_name.is_empty() && account_name.contains('@')) {
179185
return Err(UsernameError::MixedFormat);
180186
}
181187

@@ -219,6 +225,11 @@ impl Username {
219225
///
220226
/// If there is no `\` or `@` separator, the value is considered to be a down-level logon name with
221227
/// an empty NetBIOS domain.
228+
///
229+
/// A value containing both separators is a down-level logon name: the `\` takes precedence and
230+
/// everything after it is the account name, so `MicrosoftAccount\user@example.com` parses as
231+
/// NetBIOS domain `MicrosoftAccount` with account name `user@example.com` — matching how
232+
/// Windows itself reads that qualification.
222233
pub fn parse(value: &str) -> Result<Self, UsernameError> {
223234
match (value.split_once('\\'), value.rsplit_once('@')) {
224235
(None, None) => Ok(Self {
@@ -809,8 +820,9 @@ mod tests {
809820
);
810821
}
811822

812-
// A down-level user name can't contain a @ in the account name
813-
if !initial_username.account_name().contains('@') {
823+
// With no NetBIOS domain, `Username::new` falls back to `parse`, which reads an `@` as
824+
// a UPN separator — only that combination can't reconstruct as a down-level logon name.
825+
if domain.is_some() || !initial_username.account_name().contains('@') {
814826
let netbios_name = Username::new(initial_username.account_name(), domain).expect("NetBIOS");
815827
assert_eq!(netbios_name.format(), UserNameFormat::DownLevelLogonName);
816828
assert_eq!(netbios_name.account_name(), initial_username.account_name());
@@ -850,7 +862,9 @@ mod tests {
850862

851863
#[test]
852864
fn down_level_logon_name_round_trip() {
853-
proptest!(|(account_name in "[a-zA-Z0-9.]{1,3}", domain_name in "[A-Z0-9.]{1,3}")| {
865+
// The account-name alphabet includes `@`: `DOMAIN\user@example.com` is a valid down-level
866+
// logon name (the whole e-mail address is the account name — a Microsoft account, #718).
867+
proptest!(|(account_name in "[a-zA-Z0-9@.]{1,3}", domain_name in "[A-Z0-9.]{1,3}")| {
854868
let username = Username::new_down_level_logon_name(&account_name, &domain_name).expect("down-level logon name");
855869

856870
assert_eq!(username.account_name(), account_name);
@@ -888,6 +902,53 @@ mod tests {
888902
})
889903
}
890904

905+
#[test]
906+
fn down_level_logon_name_with_at_requires_a_domain() {
907+
// Pins the fix in 2e05ed1 (Copilot's review finding, confirmed by @TheBestTvarynka):
908+
// with no NetBIOS domain there is no `\` in the serialized string, so an `@`-bearing
909+
// account name came back from an `AuthIdentityBuffers` round trip as a UPN, breaking the
910+
// format-preservation invariant. The qualifier is what makes such a name representable.
911+
assert_eq!(
912+
Username::new_down_level_logon_name("frank@example.com", ""),
913+
Err(UsernameError::MixedFormat),
914+
);
915+
916+
// Qualified, the same account name is representable and survives the round trip.
917+
let qualified =
918+
Username::new_down_level_logon_name("frank@example.com", "MicrosoftAccount").expect("qualified");
919+
assert_eq!(qualified.account_name(), "frank@example.com");
920+
check_round_trip_property(&qualified);
921+
}
922+
923+
/// #718: a Microsoft account's account name is the entire e-mail address. Every qualified way
924+
/// of writing one must parse with the address intact, and the unqualified form keeps its
925+
/// (AD-correct) UPN reading.
926+
#[test]
927+
fn microsoft_account_forms_are_representable() {
928+
// `MicrosoftAccount\me@example.com` — the qualification Microsoft documents for users.
929+
let qualified = Username::parse("MicrosoftAccount\\me@example.com").expect("qualified form");
930+
assert_eq!(qualified.format(), UserNameFormat::DownLevelLogonName);
931+
assert_eq!(qualified.account_name(), "me@example.com");
932+
assert_eq!(
933+
qualified.parts(),
934+
UsernameParts::DownLevelLogonName(DownLevelLogonNameParts {
935+
account_name: "me@example.com",
936+
netbios_domain: Some("MicrosoftAccount"),
937+
})
938+
);
939+
check_round_trip_property(&qualified);
940+
941+
// The same identity supplied as separate username + domain fields.
942+
let split = Username::new("me@example.com", Some("MicrosoftAccount")).expect("split form");
943+
assert_eq!(split, qualified);
944+
945+
// The unqualified e-mail alone still parses as a UPN: it is indistinguishable from an
946+
// AD user principal name, and guessing "Microsoft account" would break AD logons.
947+
let bare = Username::parse("me@example.com").expect("bare form");
948+
assert_eq!(bare.format(), UserNameFormat::UserPrincipalName);
949+
assert_eq!(bare.account_name(), "me");
950+
}
951+
891952
#[test]
892953
fn eq_ignore_ascii_case_matches_within_format() {
893954
// UPNs that differ only by ASCII case are equal.

0 commit comments

Comments
 (0)