Add SCIM Groups endpoints with diff-based member sync

Phase 3 of the SCIM v2 implementation:

- Groups CRUD at /scim/v2/<org>/Groups: filters on displayName/externalId,
  pagination, excludedAttributes=members honored on list (Entra sends it
  on every sync cycle).
- PATCH implements true add/remove diffs on groups_users, including
  Entra's filter-path removal form members[value eq "id"] used by
  provisioning apps created without the aadOptscim062020 flag. Duplicate
  adds are idempotent; removals of absent members are no-ops (Entra
  retries removals).
- PUT and PATCH-replace do full member-set replacement, the one place
  ldap_import's replace semantics are actually correct.
- Member values resolve to memberships of the token's org only; unknown
  or cross-org ids are rejected before any write, so a failed create
  leaves nothing behind. Users must be provisioned before group
  assignment (GroupUser keys on MembershipId).
- Group DELETE is a real delete: groups carry no E2EE state, unlike
  memberships.
- Gated on ORG_GROUPS_ENABLED with a loud SCIM 501, not ldap_import's
  silent skip. Audit events use the 1400-range group event types.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/api/scim/error.rs

@@ -72,6 +72,14 @@ impl ScimError {
         }
     }
 
+    pub fn not_implemented(detail: &str) -> Self {
+        Self {
+            status: Status::NotImplemented,
+            scim_type: None,
+            detail: String::from(detail),
+        }
+    }
+
     pub fn internal() -> Self {
         Self {
             status: Status::InternalServerError,

src/api/scim/groups.rs

@@ -0,0 +1,329 @@
+//
+// SCIM /Groups endpoints. The SCIM Group id is Vaultwarden's GroupId, and
+// member values are MembershipIds (the same ids the /Users endpoints expose),
+// which is why User provisioning must precede Group assignment.
+//
+// SCIM owns group existence and membership only. Collection access stays an
+// in-app admin decision: the IdP decides who is in a group, the vault admin
+// decides what the group can see.
+//
+// Requires ORG_GROUPS_ENABLED. Unlike ldap_import, which silently skips
+// groups when disabled, these endpoints fail loudly so a misconfigured
+// deployment is visible in the IdP instead of quietly not syncing.
+//
+use rocket::Route;
+use serde::Deserialize;
+use serde_json::Value;
+
+use crate::{
+    CONFIG,
+    api::{
+        core::log_event,
+        scim::{
+            ScimJson, ScimResponse,
+            error::ScimError,
+            filter::parse_eq_filter,
+            guard::ScimToken,
+            patch::{PatchOp, parse_group_patch},
+        },
+    },
+    db::{
+        DbConn,
+        models::{EventType, Group, GroupId, GroupUser, Membership, MembershipId},
+    },
+};
+
+pub fn routes() -> Vec<Route> {
+    routes![list_groups, get_group, post_group, put_group, patch_group, delete_group]
+}
+
+const SCIM_ACTOR: &str = "vaultwarden-scim-00000-000000000000";
+const SCIM_DEVICE_TYPE: i32 = 14;
+
+fn check_groups_enabled() -> Result<(), ScimError> {
+    if !CONFIG.org_groups_enabled() {
+        return Err(ScimError::not_implemented("Group support is disabled on this server (ORG_GROUPS_ENABLED=false)"));
+    }
+    Ok(())
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct ScimGroupMember {
+    value: String,
+}
+
+#[derive(Debug, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct ScimGroupRequest {
+    display_name: Option<String>,
+    external_id: Option<String>,
+    #[serde(default)]
+    members: Vec<ScimGroupMember>,
+}
+
+async fn to_scim_group(group: &Group, token: &ScimToken, include_members: bool, conn: &DbConn) -> Value {
+    let location = format!("{}/scim/v2/{}/Groups/{}", CONFIG.domain(), token.org_uuid, group.uuid);
+    let mut body = json!({
+        "schemas": [crate::api::scim::discovery::GROUP_SCHEMA_URN],
+        "id": group.uuid,
+        "externalId": group.external_id,
+        "displayName": group.name,
+        "meta": {
+            "resourceType": "Group",
+            "location": location,
+        },
+    });
+    if include_members {
+        let members: Vec<Value> = GroupUser::find_by_group(&group.uuid, &token.org_uuid, conn)
+            .await
+            .iter()
+            .map(|gu| json!({"value": gu.users_organizations_uuid}))
+            .collect();
+        body["members"] = json!(members);
+    }
+    body
+}
+
+async fn log_group_event(event_type: EventType, group: &Group, token: &ScimToken, conn: &DbConn) {
+    log_event(
+        event_type as i32,
+        &group.uuid,
+        &token.org_uuid,
+        &SCIM_ACTOR.into(),
+        SCIM_DEVICE_TYPE,
+        &token.ip.ip,
+        conn,
+    )
+    .await;
+}
+
+// Resolves a SCIM member value to a membership of THIS org, or a 400: group
+// assignment requires the user to be provisioned into the org first.
+async fn resolve_member(value: &str, token: &ScimToken, conn: &DbConn) -> Result<MembershipId, ScimError> {
+    let member_id: MembershipId = value.to_owned().into();
+    match Membership::find_by_uuid_and_org(&member_id, &token.org_uuid, conn).await {
+        Some(member) => Ok(member.uuid),
+        None => Err(ScimError::bad_request(
+            "invalidValue",
+            "members must reference existing members of this organization (provision the user first)",
+        )),
+    }
+}
+
+async fn set_members(
+    group: &Group,
+    member_ids: Vec<MembershipId>,
+    token: &ScimToken,
+    conn: &DbConn,
+) -> Result<(), ScimError> {
+    GroupUser::delete_all_by_group(&group.uuid, &token.org_uuid, conn).await.map_err(|_| ScimError::internal())?;
+    for member_id in member_ids {
+        let mut group_user = GroupUser::new(group.uuid.clone(), member_id);
+        group_user.save(conn).await.map_err(|_| ScimError::internal())?;
+    }
+    Ok(())
+}
+
+#[derive(FromForm)]
+pub struct GroupListParams {
+    filter: Option<String>,
+    #[field(name = "startIndex")]
+    start_index: Option<i64>,
+    count: Option<i64>,
+    #[field(name = "excludedAttributes")]
+    excluded_attributes: Option<String>,
+}
+
+#[get("/v2/<_>/Groups?<params..>")]
+async fn list_groups(params: GroupListParams, token: ScimToken, conn: DbConn) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+
+    let groups: Vec<Group> = if let Some(raw_filter) = params.filter.as_deref() {
+        let eq = parse_eq_filter(raw_filter)?;
+        match eq.attribute.as_str() {
+            "displayname" => Group::find_by_organization(&token.org_uuid, &conn)
+                .await
+                .into_iter()
+                .filter(|g| g.name == eq.value)
+                .collect(),
+            "externalid" => {
+                Group::find_by_external_id_and_org(&eq.value, &token.org_uuid, &conn).await.into_iter().collect()
+            }
+            _ => {
+                return Err(ScimError::bad_request(
+                    "invalidFilter",
+                    "Filterable attributes are displayName and externalId",
+                ));
+            }
+        }
+    } else {
+        Group::find_by_organization(&token.org_uuid, &conn).await
+    };
+
+    // Entra requests excludedAttributes=members on list syncs; honoring it
+    // avoids loading every group's member set.
+    let include_members = !params.excluded_attributes.as_deref().is_some_and(|excluded| excluded.contains("members"));
+
+    let total = groups.len();
+    let start_index = usize::try_from(params.start_index.unwrap_or(1)).unwrap_or(1).max(1);
+    let count = usize::try_from(params.count.unwrap_or(100)).unwrap_or(0).min(200);
+
+    let mut resources = Vec::new();
+    for group in groups.into_iter().skip(start_index - 1).take(count) {
+        resources.push(to_scim_group(&group, &token, include_members, &conn).await);
+    }
+
+    Ok(ScimResponse::ok(json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:ListResponse"],
+        "totalResults": total,
+        "itemsPerPage": resources.len(),
+        "startIndex": start_index,
+        "Resources": resources,
+    })))
+}
+
+#[get("/v2/<_>/Groups/<group_id>")]
+async fn get_group(group_id: GroupId, token: ScimToken, conn: DbConn) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+    let Some(group) = Group::find_by_uuid_and_org(&group_id, &token.org_uuid, &conn).await else {
+        return Err(ScimError::not_found());
+    };
+    Ok(ScimResponse::ok(to_scim_group(&group, &token, true, &conn).await))
+}
+
+#[post("/v2/<_>/Groups", data = "<data>")]
+async fn post_group(
+    data: ScimJson<ScimGroupRequest>,
+    token: ScimToken,
+    conn: DbConn,
+) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+    let request = data.0;
+
+    let Some(display_name) = request.display_name.as_deref().filter(|n| !n.trim().is_empty()) else {
+        return Err(ScimError::bad_request("invalidValue", "displayName is required"));
+    };
+
+    if let Some(external_id) = request.external_id.as_deref()
+        && Group::find_by_external_id_and_org(external_id, &token.org_uuid, &conn).await.is_some()
+    {
+        return Err(ScimError::conflict("uniqueness", "A group with this externalId already exists"));
+    }
+
+    // Resolve members before creating anything, so a bad member list cannot
+    // leave a half-created group behind.
+    let mut member_ids = Vec::with_capacity(request.members.len());
+    for member in &request.members {
+        member_ids.push(resolve_member(&member.value, &token, &conn).await?);
+    }
+
+    let mut group = Group::new(token.org_uuid.clone(), display_name.to_owned(), false, request.external_id.clone());
+    group.save(&conn).await.map_err(|_| ScimError::internal())?;
+    set_members(&group, member_ids, &token, &conn).await?;
+
+    log_group_event(EventType::GroupCreated, &group, &token, &conn).await;
+
+    let location = format!("{}/scim/v2/{}/Groups/{}", CONFIG.domain(), token.org_uuid, group.uuid);
+    let body = to_scim_group(&group, &token, true, &conn).await;
+    Ok(ScimResponse::created(location, body))
+}
+
+// PUT is a full replacement: displayName, externalId, and the member set.
+#[put("/v2/<_>/Groups/<group_id>", data = "<data>")]
+async fn put_group(
+    group_id: GroupId,
+    data: ScimJson<ScimGroupRequest>,
+    token: ScimToken,
+    conn: DbConn,
+) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+    let Some(mut group) = Group::find_by_uuid_and_org(&group_id, &token.org_uuid, &conn).await else {
+        return Err(ScimError::not_found());
+    };
+    let request = data.0;
+
+    let mut member_ids = Vec::with_capacity(request.members.len());
+    for member in &request.members {
+        member_ids.push(resolve_member(&member.value, &token, &conn).await?);
+    }
+
+    if let Some(display_name) = request.display_name.as_deref().filter(|n| !n.trim().is_empty()) {
+        group.name = display_name.to_owned();
+    }
+    if request.external_id.is_some() {
+        group.set_external_id(request.external_id.clone());
+    }
+    group.save(&conn).await.map_err(|_| ScimError::internal())?;
+    set_members(&group, member_ids, &token, &conn).await?;
+
+    log_group_event(EventType::GroupUpdated, &group, &token, &conn).await;
+
+    Ok(ScimResponse::ok(to_scim_group(&group, &token, true, &conn).await))
+}
+
+#[patch("/v2/<_>/Groups/<group_id>", data = "<data>")]
+async fn patch_group(
+    group_id: GroupId,
+    data: ScimJson<PatchOp>,
+    token: ScimToken,
+    conn: DbConn,
+) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+    let Some(mut group) = Group::find_by_uuid_and_org(&group_id, &token.org_uuid, &conn).await else {
+        return Err(ScimError::not_found());
+    };
+
+    let patch = parse_group_patch(&data.0)?;
+
+    if let Some(display_name) = patch.display_name.as_deref().filter(|n| !n.trim().is_empty()) {
+        group.name = display_name.to_owned();
+        group.save(&conn).await.map_err(|_| ScimError::internal())?;
+    }
+    if let Some(external_id) = patch.external_id.clone() {
+        group.set_external_id(Some(external_id));
+        group.save(&conn).await.map_err(|_| ScimError::internal())?;
+    }
+
+    if let Some(replacement) = patch.replace_members {
+        let mut member_ids = Vec::with_capacity(replacement.len());
+        for value in &replacement {
+            member_ids.push(resolve_member(value, &token, &conn).await?);
+        }
+        set_members(&group, member_ids, &token, &conn).await?;
+    } else {
+        // True diff semantics: adds insert (idempotently via replace_into in
+        // GroupUser::save), removes delete only the listed member links.
+        for value in &patch.add_members {
+            let member_id = resolve_member(value, &token, &conn).await?;
+            let mut group_user = GroupUser::new(group.uuid.clone(), member_id);
+            group_user.save(&conn).await.map_err(|_| ScimError::internal())?;
+        }
+        for value in &patch.remove_members {
+            // Removing a member who is not in the org (or already absent from
+            // the group) is a no-op, not an error: Entra retries removals.
+            let member_id: MembershipId = value.clone().into();
+            GroupUser::delete_by_group_and_member(&group.uuid, &member_id, &conn)
+                .await
+                .map_err(|_| ScimError::internal())?;
+        }
+    }
+
+    log_group_event(EventType::GroupUpdated, &group, &token, &conn).await;
+
+    Ok(ScimResponse::ok(to_scim_group(&group, &token, true, &conn).await))
+}
+
+// Deleting a group only removes an access mapping: it carries no E2EE state
+// (unlike memberships, where delete would destroy the wrapped org key), so a
+// real delete is safe and matches RFC semantics.
+#[delete("/v2/<_>/Groups/<group_id>")]
+async fn delete_group(group_id: GroupId, token: ScimToken, conn: DbConn) -> Result<ScimResponse, ScimError> {
+    check_groups_enabled()?;
+    let Some(group) = Group::find_by_uuid_and_org(&group_id, &token.org_uuid, &conn).await else {
+        return Err(ScimError::not_found());
+    };
+    log_group_event(EventType::GroupDeleted, &group, &token, &conn).await;
+    group.delete(&token.org_uuid, &conn).await.map_err(|_| ScimError::internal())?;
+    Ok(ScimResponse::no_content())
+}

src/api/scim/mod.rs

@@ -10,6 +10,7 @@ pub mod guard;
 mod discovery;
 mod error;
 mod filter;
+mod groups;
 mod manage;
 mod models;
 mod patch;
@@ -33,6 +34,7 @@ pub use manage::routes as manage_routes;
 pub fn routes() -> Vec<Route> {
     let mut routes = discovery::routes();
     routes.append(&mut users::routes());
+    routes.append(&mut groups::routes());
     routes
 }
 

src/api/scim/patch.rs

@@ -226,3 +226,221 @@ mod tests {
         }
     }
 }
+
+// ---------------------------------------------------------------------------
+// Group PATCH
+// ---------------------------------------------------------------------------
+
+// What a Group PATCH asked for, after normalization. Member values are
+// MembershipIds (the SCIM User id).
+#[derive(Debug, Default, PartialEq)]
+pub struct GroupPatch {
+    pub display_name: Option<String>,
+    pub external_id: Option<String>,
+    pub add_members: Vec<String>,
+    pub remove_members: Vec<String>,
+    // Some(list) means full replacement of the member set.
+    pub replace_members: Option<Vec<String>>,
+}
+
+// Entra removes single members with a filter path instead of a value list:
+//     {"op": "Remove", "path": "members[value eq \"<id>\"]"}
+// (the value-array form is only sent by apps created with the
+// aadOptscim062020 feature flag). Both forms must work.
+fn parse_members_filter_path(path: &str) -> Option<String> {
+    let inner = path.strip_prefix("members[")?.strip_suffix(']')?;
+    let (attribute, rest) = inner.split_once(char::is_whitespace)?;
+    if !attribute.eq_ignore_ascii_case("value") {
+        return None;
+    }
+    let (operator, rest) = rest.trim_start().split_once(char::is_whitespace)?;
+    if !operator.eq_ignore_ascii_case("eq") {
+        return None;
+    }
+    let value = rest.trim().strip_prefix('"')?.strip_suffix('"')?;
+    if value.is_empty() {
+        return None;
+    }
+    Some(value.to_owned())
+}
+
+// Extracts member ids from a PATCH value: either [{"value": "id"}, ...] or a
+// single {"value": "id"} object.
+fn member_values(value: Option<&Value>) -> Result<Vec<String>, ScimError> {
+    let invalid = || ScimError::bad_request("invalidValue", "members value must be an array of {value} objects");
+    let items: Vec<&Value> = match value {
+        Some(Value::Array(items)) => items.iter().collect(),
+        Some(single @ Value::Object(_)) => vec![single],
+        _ => return Err(invalid()),
+    };
+    let mut ids = Vec::with_capacity(items.len());
+    for item in items {
+        let Some(Value::String(id)) = item.get("value") else {
+            return Err(invalid());
+        };
+        ids.push(id.clone());
+    }
+    Ok(ids)
+}
+
+pub fn parse_group_patch(patch: &PatchOp) -> Result<GroupPatch, ScimError> {
+    if !patch.schemas.iter().any(|s| s == PATCH_OP_URN) {
+        return Err(ScimError::bad_request("invalidValue", "Missing PatchOp schema"));
+    }
+    if patch.operations.is_empty() {
+        return Err(ScimError::bad_request("invalidValue", "No operations provided"));
+    }
+
+    let mut result = GroupPatch::default();
+
+    for operation in &patch.operations {
+        let op = operation.op.to_lowercase();
+        match operation.path.as_deref() {
+            Some(op_path) if op_path.eq_ignore_ascii_case("members") => match op.as_str() {
+                "add" => result.add_members.append(&mut member_values(operation.value.as_ref())?),
+                "remove" => result.remove_members.append(&mut member_values(operation.value.as_ref())?),
+                "replace" => {
+                    result
+                        .replace_members
+                        .get_or_insert_with(Vec::new)
+                        .append(&mut member_values(operation.value.as_ref())?);
+                }
+                _ => return Err(ScimError::bad_request("invalidValue", "Unsupported operation on members")),
+            },
+            Some(op_path) if op_path.to_lowercase().starts_with("members[") => {
+                let Some(member_id) = parse_members_filter_path(op_path) else {
+                    return Err(ScimError::bad_request("invalidPath", "Unsupported members filter path"));
+                };
+                match op.as_str() {
+                    // Entra's single-member removal form.
+                    "remove" => result.remove_members.push(member_id),
+                    "add" | "replace" => result.add_members.push(member_id),
+                    _ => return Err(ScimError::bad_request("invalidValue", "Unsupported operation on members")),
+                }
+            }
+            Some(op_path) if op_path.eq_ignore_ascii_case("displayname") => {
+                let Some(Value::String(name)) = operation.value.as_ref() else {
+                    return Err(ScimError::bad_request("invalidValue", "displayName must be a string"));
+                };
+                result.display_name = Some(name.clone());
+            }
+            Some(op_path) if op_path.eq_ignore_ascii_case("externalid") => {
+                let Some(Value::String(external_id)) = operation.value.as_ref() else {
+                    return Err(ScimError::bad_request("invalidValue", "externalId must be a string"));
+                };
+                result.external_id = Some(external_id.clone());
+            }
+            Some(_) => return Err(ScimError::bad_request("invalidPath", "Unsupported patch path for Groups")),
+            None => {
+                let Some(Value::Object(map)) = operation.value.as_ref() else {
+                    return Err(ScimError::bad_request(
+                        "invalidValue",
+                        "Operation without path must carry an object value",
+                    ));
+                };
+                for (attribute, value) in map {
+                    if attribute.eq_ignore_ascii_case("displayname") {
+                        let Value::String(name) = value else {
+                            return Err(ScimError::bad_request("invalidValue", "displayName must be a string"));
+                        };
+                        result.display_name = Some(name.clone());
+                    } else if attribute.eq_ignore_ascii_case("externalid") {
+                        let Value::String(external_id) = value else {
+                            return Err(ScimError::bad_request("invalidValue", "externalId must be a string"));
+                        };
+                        result.external_id = Some(external_id.clone());
+                    } else if attribute.eq_ignore_ascii_case("members") {
+                        match op.as_str() {
+                            "add" => result.add_members.append(&mut member_values(Some(value))?),
+                            "remove" => result.remove_members.append(&mut member_values(Some(value))?),
+                            "replace" => {
+                                result
+                                    .replace_members
+                                    .get_or_insert_with(Vec::new)
+                                    .append(&mut member_values(Some(value))?);
+                            }
+                            _ => {
+                                return Err(ScimError::bad_request("invalidValue", "Unsupported operation on members"));
+                            }
+                        }
+                    } else {
+                        return Err(ScimError::bad_request("invalidPath", "Unsupported patch attribute for Groups"));
+                    }
+                }
+            }
+        }
+    }
+
+    Ok(result)
+}
+
+#[cfg(test)]
+mod group_tests {
+    use super::*;
+
+    fn patch(payload: Value) -> Result<GroupPatch, ScimError> {
+        let parsed: PatchOp = serde_json::from_value(payload).expect("valid PatchOp json");
+        parse_group_patch(&parsed)
+    }
+
+    #[test]
+    fn add_and_remove_member_value_lists() {
+        let parsed = patch(json!({
+            "schemas": [PATCH_OP_URN],
+            "Operations": [
+                {"op": "Add", "path": "members", "value": [{"value": "member-1"}, {"value": "member-2"}]},
+                {"op": "remove", "path": "members", "value": [{"value": "member-3"}]},
+            ],
+        }))
+        .expect("valid");
+        assert_eq!(parsed.add_members, vec!["member-1", "member-2"]);
+        assert_eq!(parsed.remove_members, vec!["member-3"]);
+    }
+
+    #[test]
+    fn entra_filter_path_removal() {
+        let parsed = patch(json!({
+            "schemas": [PATCH_OP_URN],
+            "Operations": [{"op": "Remove", "path": "members[value eq \"member-9\"]"}],
+        }))
+        .expect("valid");
+        assert_eq!(parsed.remove_members, vec!["member-9"]);
+    }
+
+    #[test]
+    fn replace_members_and_rename() {
+        let parsed = patch(json!({
+            "schemas": [PATCH_OP_URN],
+            "Operations": [
+                {"op": "replace", "path": "members", "value": [{"value": "only-member"}]},
+                {"op": "replace", "path": "displayName", "value": "New Group Name"},
+            ],
+        }))
+        .expect("valid");
+        assert_eq!(parsed.replace_members.as_deref(), Some(&["only-member".to_owned()][..]));
+        assert_eq!(parsed.display_name.as_deref(), Some("New Group Name"));
+    }
+
+    #[test]
+    fn group_patch_rejects_garbage() {
+        for (payload, expected_type) in [
+            (
+                json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "remove", "path": "members[display eq \"x\"]"}]}),
+                "invalidPath",
+            ),
+            (
+                json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "add", "path": "members", "value": ["bare-string"]}]}),
+                "invalidValue",
+            ),
+            (
+                json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "add", "path": "wibble", "value": 1}]}),
+                "invalidPath",
+            ),
+        ] {
+            match patch(payload.clone()) {
+                Err(e) => assert_eq!(e.scim_type, Some(expected_type), "wrong scimType for {payload}"),
+                Ok(p) => panic!("group patch {payload} unexpectedly parsed to {p:?}"),
+            }
+        }
+    }
+}

src/api/scim/tests/mod.rs

@@ -688,3 +688,158 @@ async fn pagination_edges() {
     assert_eq!(parsed["totalResults"], json!(3));
     assert_eq!(parsed["itemsPerPage"], json!(0));
 }
+
+// ---------------------------------------------------------------------------
+// Groups (Phase 3)
+// ---------------------------------------------------------------------------
+
+#[rocket::async_test]
+async fn group_crud_and_member_diff_lifecycle() {
+    let _guard = TEST_LOCK.lock().await;
+    let (client, pool) = scim_client().await;
+    let conn = pool.get().await.expect("conn");
+    let org = seed_org(&conn, "scim-group-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let member_a = seed_member(&conn, &org, "group.a@example.com", 1, MembershipType::User).await;
+    let member_b = seed_member(&conn, &org, "group.b@example.com", 2, MembershipType::User).await;
+
+    // POST with one initial member.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
+        "displayName": "Engineering",
+        "externalId": "entra-group-1",
+        "members": [{"value": member_a}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response = client.post(format!("/scim/v2/{org}/Groups")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Created);
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["displayName"], json!("Engineering"));
+    assert_eq!(parsed["members"].as_array().expect("members").len(), 1);
+    let group_id = parsed["id"].as_str().expect("group id").to_owned();
+
+    // PATCH add member_b via value list.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "Add", "path": "members", "value": [{"value": member_b}]}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Groups/{group_id}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Ok);
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["members"].as_array().expect("members").len(), 2);
+
+    // Adding the same member twice is idempotent.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "add", "path": "members", "value": [{"value": member_b}]}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Groups/{group_id}")).header(auth).header(ct).body(body).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["members"].as_array().expect("members").len(), 2, "duplicate add must not duplicate the link");
+
+    // Entra's filter-path removal form removes member_a only.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "Remove", "path": format!("members[value eq \"{member_a}\"]")}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Groups/{group_id}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Ok);
+    let parsed = parse_json(&body_of(response).await);
+    let members = parsed["members"].as_array().expect("members");
+    assert_eq!(members.len(), 1);
+    assert_eq!(members[0]["value"], json!(member_b.to_string()));
+
+    // excludedAttributes=members on list omits the member arrays.
+    let response =
+        client.get(format!("/scim/v2/{org}/Groups?excludedAttributes=members")).header(bearer(&token)).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(1));
+    assert!(parsed["Resources"][0].get("members").is_none(), "members must be excluded");
+
+    // Filter by displayName.
+    let response = client
+        .get(format!("/scim/v2/{org}/Groups?filter={}", url_escape("displayName eq \"Engineering\"")))
+        .header(bearer(&token))
+        .dispatch()
+        .await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(1));
+
+    // DELETE is a real delete for groups (no E2EE state involved).
+    let response = client.delete(format!("/scim/v2/{org}/Groups/{group_id}")).header(bearer(&token)).dispatch().await;
+    assert_eq!(response.status(), Status::NoContent);
+    let response = client.get(format!("/scim/v2/{org}/Groups/{group_id}")).header(bearer(&token)).dispatch().await;
+    assert_eq!(response.status(), Status::NotFound);
+}
+
+#[rocket::async_test]
+async fn group_member_must_be_provisioned_first() {
+    let _guard = TEST_LOCK.lock().await;
+    let (client, pool) = scim_client().await;
+    let conn = pool.get().await.expect("conn");
+    let org = seed_org(&conn, "scim-group-order-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let org_other = seed_org(&conn, "scim-group-other-org").await;
+    let foreign_member = seed_member(&conn, &org_other, "other.org@example.com", 1, MembershipType::User).await;
+
+    // A member id from another org must be rejected, same as an unknown id:
+    // GroupUser keys on MembershipId, so cross-org references would otherwise
+    // link silently.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
+        "displayName": "Bad Members",
+        "members": [{"value": foreign_member}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response = client.post(format!("/scim/v2/{org}/Groups")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::BadRequest);
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["scimType"], json!("invalidValue"));
+
+    // And the failed create must not leave a group behind.
+    let response = client.get(format!("/scim/v2/{org}/Groups")).header(bearer(&token)).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(0));
+}
+
+#[rocket::async_test]
+async fn group_put_replaces_member_set() {
+    let _guard = TEST_LOCK.lock().await;
+    let (client, pool) = scim_client().await;
+    let conn = pool.get().await.expect("conn");
+    let org = seed_org(&conn, "scim-group-put-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let member_a = seed_member(&conn, &org, "put.a@example.com", 1, MembershipType::User).await;
+    let member_b = seed_member(&conn, &org, "put.b@example.com", 1, MembershipType::User).await;
+
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
+        "displayName": "Replace Me",
+        "members": [{"value": member_a}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response = client.post(format!("/scim/v2/{org}/Groups")).header(auth).header(ct).body(body).dispatch().await;
+    let group_id = parse_json(&body_of(response).await)["id"].as_str().expect("id").to_owned();
+
+    // PUT swaps the whole member set and renames.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:Group"],
+        "displayName": "Replaced",
+        "members": [{"value": member_b}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.put(format!("/scim/v2/{org}/Groups/{group_id}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Ok);
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["displayName"], json!("Replaced"));
+    let members = parsed["members"].as_array().expect("members");
+    assert_eq!(members.len(), 1);
+    assert_eq!(members[0]["value"], json!(member_b.to_string()));
+}