dir-sim: #321 review follow-up (exact bag-member names, SID transform, DNS-label tenant)

crates/ogar-ad/src/lib.rs

@@ -19,6 +19,7 @@ pub mod ldif;
 use ogar_dir_core::dn::Dn;
 use ogar_dir_core::label::{self, CloudLabel};
 use ogar_dir_core::record::{FLAG_DN_UNENCODED, FLAG_NON_OU_CONTAINER};
+use ogar_dir_core::schema::bag_member_of;
 use ogar_dir_core::{
     AttrDef, AttrKind, BAG_LEN, DirRecord, Guid128, OuDictionary, SchemaFamily, SchemaId, ValuePool,
 };
@@ -355,13 +356,7 @@ fn is_bag_member(base: &str) -> bool {
     SCHEMA_V1
         .iter()
         .filter(|d| d.kind == AttrKind::Bag)
-        .any(|d| {
-            base.len() > d.name.len()
-                && base[..d.name.len()].eq_ignore_ascii_case(d.name)
-                && base[d.name.len()..]
-                    .parse::<usize>()
-                    .is_ok_and(|n| (1..=BAG_LEN).contains(&n))
-        })
+        .any(|d| bag_member_of(d.name, base).is_some())
 }
 
 fn identity_or_dn(name: &str) -> bool {

crates/ogar-ad/tests/main.rs

@@ -376,4 +376,9 @@ fn extension_attributes_are_one_bag() {
     );
     let (none, _) = enc("").unwrap();
     assert_eq!(none.record.str_ref(slot("extensionAttribute")), None);
+    // Codex P2: a non-canonical spelling is not member 1. It is neither
+    // stored nor silently dropped: it is reported as ignored.
+    let (odd, _) = enc("extensionAttribute01: x\n").unwrap();
+    assert_eq!(odd.record.str_ref(slot("extensionAttribute")), None);
+    assert_eq!(odd.ignored, vec!["extensionAttribute01"]);
 }

crates/ogar-az/src/lib.rs

@@ -19,6 +19,7 @@
 use ogar_dir_core::dn::Dn;
 use ogar_dir_core::edge::{DirEdge, EdgeEvidence, EdgeKind};
 use ogar_dir_core::record::{FLAG_DN_UNENCODED, FLAG_NON_OU_CONTAINER};
+use ogar_dir_core::schema::bag_member_of;
 use ogar_dir_core::{
     AttrDef, AttrKind, BAG_LEN, DirRecord, Guid128, OuDictionary, SchemaFamily, SchemaId,
     ValuePool, base64,
@@ -184,9 +185,14 @@ pub enum SyncTransform {
     /// `onPremisesImmutableId` is the base64 of the 16
     /// `mS-DS-ConsistencyGuid` bytes.
     ImmutableId,
-    /// The cloud value is the AD value prefixed `onPremises*` evidence
-    /// (DN, sAMAccountName, SID): copied, not mapped to a cloud property.
+    /// The cloud value is the AD value, copied as `onPremises*` evidence
+    /// (DN, sAMAccountName), not mapped to a cloud property.
     OnPremisesEvidence,
+    /// `onPremisesSecurityIdentifier` is the string form (`S-1-5-21-...`)
+    /// of the binary `objectSid`: `ogar_dir_core::sid::sid_to_string`.
+    /// Comparing the raw bytes with the string would report drift on every
+    /// synced user.
+    SecurityIdentifier,
 }
 
 /// One attribute AD and the cloud both hold, under their own names.
@@ -249,7 +255,7 @@ pub const SYNCED: &[SyncedAttribute] = &[
     synced(
         "objectSid",
         "onPremisesSecurityIdentifier",
-        SyncTransform::OnPremisesEvidence,
+        SyncTransform::SecurityIdentifier,
     ),
 ];
 
@@ -265,9 +271,21 @@ const fn synced(
     }
 }
 
-/// The cloud counterpart of an AD attribute, if it is synced.
+/// The cloud counterpart of an AD attribute, if it is synced. A numbered
+/// member of a bag (`extensionAttribute7`) resolves to its bag's entry; only
+/// the canonical spelling does (`ogar_dir_core::schema::bag_member_of`).
 pub fn cloud_of(ad: &str) -> Option<&'static SyncedAttribute> {
-    SYNCED.iter().find(|s| s.ad.eq_ignore_ascii_case(ad))
+    SYNCED
+        .iter()
+        .find(|s| s.ad.eq_ignore_ascii_case(ad))
+        .or_else(|| {
+            SYNCED.iter().find(|s| {
+                let bag = SCHEMA_V1
+                    .iter()
+                    .any(|d| d.name == s.cloud && d.kind == AttrKind::Bag);
+                bag && bag_member_of(s.ad, ad).is_some()
+            })
+        })
 }
 
 /// AZ object kinds (family-scoped codes). Only users are ingested in v1.

crates/ogar-az/tests/main.rs

@@ -195,6 +195,16 @@ fn every_synced_attribute_exists_on_both_sides_once() {
         None,
         "the routing address is not synced"
     );
+    // A numbered extension attribute is synced as a member of the bag.
+    let ext = cloud_of("extensionAttribute7").unwrap();
+    assert_eq!(ext.cloud, "onPremisesExtensionAttributes");
+    assert_eq!(cloud_of("extensionAttribute16"), None);
+    assert_eq!(cloud_of("extensionAttribute07"), None);
+    // objectSid is binary in AD and a string in Graph: not copied as is.
+    assert_eq!(
+        cloud_of("objectSid").unwrap().transform,
+        SyncTransform::SecurityIdentifier
+    );
 }
 
 // onPremisesExtensionAttributes is a bag: one positional slot, entry n is

crates/ogar-dir-core/src/lib.rs

@@ -36,6 +36,7 @@ pub mod label;
 pub mod pool;
 pub mod record;
 pub mod schema;
+pub mod sid;
 
 pub use dn::{Dn, DnError, Rdn};
 pub use dn128::{DN_CHILDREN, DN_LEVELS, DirectoryScope, Dn128, Dn128Error};

crates/ogar-dir-core/src/schema.rs

@@ -74,6 +74,27 @@ pub enum AttrKind {
 /// Members of an [`AttrKind::Bag`]: the 15 extension attributes.
 pub const BAG_LEN: usize = 15;
 
+/// The member number (1..=[`BAG_LEN`]) `name` names in the bag `bag`
+/// (`extensionAttribute7` in `extensionAttribute` is 7), or `None`.
+///
+/// Only the canonical spelling counts: the bag name in any ASCII case,
+/// then the number without sign, space or leading zero. Any other spelling
+/// (`extensionAttribute01`) is not a member, so an encoder reports it as
+/// ignored instead of dropping it. A name whose bag-length prefix ends
+/// inside a multi-byte character is not a member either.
+pub fn bag_member_of(bag: &str, name: &str) -> Option<usize> {
+    let (head, n) = name.split_at_checked(bag.len())?;
+    if !head.eq_ignore_ascii_case(bag)
+        || n.starts_with('0')
+        || n.is_empty()
+        || !n.bytes().all(|b| b.is_ascii_digit())
+    {
+        return None;
+    }
+    let n: usize = n.parse().ok()?;
+    (1..=BAG_LEN).contains(&n).then_some(n)
+}
+
 /// Which slot space an attribute occupies.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum SlotSpace {

crates/ogar-dir-core/src/sid.rs

@@ -0,0 +1,35 @@
+//! Windows security identifiers. AD stores `objectSid` as the binary form
+//! ([MS-DTYP] 2.4.2.2); Graph's `onPremisesSecurityIdentifier` is the
+//! string form ([MS-DTYP] 2.4.2.1), `S-{revision}-{authority}-{sub}...`.
+
+use std::fmt::Write;
+
+/// The string form of a binary SID, or `None` if it is malformed (shorter
+/// than the 8-byte header, or a length that disagrees with its
+/// sub-authority count).
+///
+/// The 48-bit identifier authority is big-endian and printed in decimal
+/// when it fits 32 bits, otherwise as `0x` and 12 hex digits; each
+/// sub-authority is a little-endian `u32` in decimal.
+pub fn sid_to_string(b: &[u8]) -> Option<String> {
+    let [revision, count, ..] = *b else {
+        return None;
+    };
+    let authority = b
+        .get(2..8)?
+        .iter()
+        .fold(0u64, |a, &x| a << 8 | u64::from(x));
+    let subs = &b[8..];
+    if subs.len() != usize::from(count) * 4 {
+        return None;
+    }
+    let mut s = if authority < 1 << 32 {
+        format!("S-{revision}-{authority}")
+    } else {
+        format!("S-{revision}-0x{authority:012X}")
+    };
+    for c in subs.as_chunks::<4>().0 {
+        let _ = write!(s, "-{}", u32::from_le_bytes(*c));
+    }
+    Some(s)
+}

crates/ogar-dir-core/tests/main.rs

@@ -272,3 +272,55 @@ fn schema_validation_knows_the_guid_space() {
     assert!(schema::validate(&[def("a", 1, AttrKind::Guid), def("b", 1, AttrKind::Guid)]).is_err());
     assert!(schema::validate(&[def("a", GUID_SLOTS as u8, AttrKind::Guid)]).is_err());
 }
+
+// A bag member is named by its exact canonical spelling only: `01` is not
+// member 1, and a name with a multi-byte character at the prefix boundary
+// is simply not a member (it must not panic).
+#[test]
+fn bag_members_are_named_exactly() {
+    use ogar_dir_core::schema::bag_member_of;
+    let bag = "extensionAttribute";
+    assert_eq!(bag_member_of(bag, "extensionAttribute7"), Some(7));
+    assert_eq!(bag_member_of(bag, "ExtensionAttribute15"), Some(15));
+    assert_eq!(bag_member_of(bag, "extensionattribute1"), Some(1));
+    for not in [
+        "extensionAttribute",
+        "extensionAttribute0",
+        "extensionAttribute01",
+        "extensionAttribute16",
+        "extensionAttribute+1",
+        "extensionAttribute 1",
+        "abcdefghijklmnopqé",
+        "extensionAttributé1",
+        "mail",
+    ] {
+        assert_eq!(bag_member_of(bag, not), None, "{not}");
+    }
+}
+
+// objectSid is binary; Graph's onPremisesSecurityIdentifier is its SDDL
+// string form, S-{revision}-{authority}-{subauthority}...
+#[test]
+fn a_binary_sid_renders_as_its_string_form() {
+    use ogar_dir_core::sid::sid_to_string;
+    let sid = [
+        0x01, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x15, 0x00, 0x00, 0x00, 0xdc, 0xf4, 0xdc,
+        0x3b, 0x83, 0x3d, 0x2b, 0x46, 0x82, 0x8b, 0xa6, 0x28, 0x00, 0x02, 0x00, 0x00,
+    ];
+    assert_eq!(
+        sid_to_string(&sid).as_deref(),
+        Some("S-1-5-21-1004336348-1177238915-682003330-512")
+    );
+    // Well-known SID with no sub-authorities beyond one: Everyone, S-1-1-0.
+    assert_eq!(
+        sid_to_string(&[1, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0]).as_deref(),
+        Some("S-1-1-0")
+    );
+    // The length must match the sub-authority count exactly.
+    assert_eq!(sid_to_string(&sid[..27]), None);
+    let mut long = sid.to_vec();
+    long.push(0);
+    assert_eq!(sid_to_string(&long), None);
+    assert_eq!(sid_to_string(&[]), None);
+    assert_eq!(sid_to_string(&[1]), None);
+}

crates/ogar-dir-sim/src/exchange.rs

@@ -543,12 +543,21 @@ pub const ROUTING: AddressTemplate = AddressTemplate("{0}@{1}.mail.onmicrosoft.c
 /// `(alias, tenant)` of a routing address, or `None` if it is not one.
 pub fn routing_parts(address: &str) -> Option<(&str, &str)> {
     match ROUTING.matches(address)?.as_slice() {
-        // One `@` in the whole address, and the tenant is a single label.
-        [alias, tenant] if !tenant.contains(['@', '.']) => Some((alias, tenant)),
+        // The tenant is a single DNS label, so the address has one `@`.
+        [alias, tenant] if is_dns_label(tenant) => Some((alias, tenant)),
         _ => None,
     }
 }
 
+/// One DNS label: 1..=63 ASCII letters, digits or hyphens, not starting or
+/// ending with a hyphen.
+fn is_dns_label(s: &str) -> bool {
+    (1..=63).contains(&s.len())
+        && !s.starts_with('-')
+        && !s.ends_with('-')
+        && s.bytes().all(|b| b.is_ascii_alphanumeric() || b == b'-')
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -577,13 +586,20 @@ mod tests {
             routing_parts("alice@contoso.mail.onmicrosoft.com"),
             Some(("alice", "contoso"))
         );
+        assert_eq!(
+            routing_parts("alice@contoso-eu.mail.onmicrosoft.com"),
+            Some(("alice", "contoso-eu"))
+        );
         for bad in [
             "alice@contoso.onmicrosoft.com",          // not the routing domain
             "alice@contoso.mail.onmicrosoft.co",      // suffix
             "@contoso.mail.onmicrosoft.com",          // empty alias
             "alice@.mail.onmicrosoft.com",            // empty tenant
             "alice@sub.contoso.mail.onmicrosoft.com", // tenant with a dot
             "a@b@contoso.mail.onmicrosoft.com",       // alias with an @
+            "alice@bad tenant.mail.onmicrosoft.com",  // tenant not a DNS label
+            "alice@-contoso.mail.onmicrosoft.com",    // leading hyphen
+            "alice@contoso_x.mail.onmicrosoft.com",   // underscore
             "alice@contoso.mail.onmicrosoft.com.x",
         ] {
             assert_eq!(routing_parts(bad), None, "{bad}");

docs/DIRECTORY-SIMULATION-POC.md

@@ -279,10 +279,15 @@ no-population-intermediate rule.
 - **V9 — `onPremisesExtensionAttributes` is a bag.** Graph returns it as
   one object of fifteen `extensionAttributeN` keys; AD stores fifteen
   separate attributes. Both land in one pooled slot of `AttrKind::Bag`:
-  member *n* is `extensionAttribute(n+1)`, an empty member is absent, and a
-  bag with no values sets no slot. Member names are not stored, so the AD
-  and Graph bags are byte-identical and the pair syncs `Same`. Read a
-  member with `ValuePool::bag_member(r, n)` (1-based, `BAG_LEN` = 15).
+  the entry at zero-based offset *i* is `extensionAttribute(i+1)`, an empty
+  entry is an absent member, and a bag with no values sets no slot. Member
+  names are not stored, so the AD and Graph bags are byte-identical and the
+  pair syncs `Same`. Read a member with `ValuePool::bag_member(r, n)`,
+  which is 1-based: `n = 1` is `extensionAttribute1`, `n = BAG_LEN` (15)
+  is `extensionAttribute15`. Only the canonical member spelling counts
+  (`schema::bag_member_of`): `extensionAttribute01` is reported as ignored.
+  `objectSid` syncs through `SyncTransform::SecurityIdentifier`, the
+  binary-to-`S-1-...` string form (`ogar_dir_core::sid::sid_to_string`).
   `ogar-ad` v5 gathers the bag (and stops reporting its members as
   ignored); `ogar-az` v2 requests it. `onPremisesDistinguishedName` and
   `onPremisesSamAccountName` were already ingested: the DN's OU path