Add SCIM Users PUT, externalId updates, and attribute-patch tolerance

Phase 2 of the SCIM v2 implementation:

- PUT /scim/v2/<org>/Users/<id> replaces the attributes SCIM owns
  (externalId, active) and returns the true state for everything else.
  userName/email and role deliberately do not sync: email is the login
  identity, user.name is global to the person across orgs, and roles
  cannot round-trip through Vaultwarden's membership types.
- PATCH now applies externalId changes (path and path-less forms) with
  per-org uniqueness enforced, logged as OrganizationUserUpdated.
- Attributes Entra maps by default but Vaultwarden does not sync
  (displayName, name.*, emails, title, roles, preferredLanguage, the
  enterprise extension) are accepted and ignored instead of rejected: a
  400 would surface as an Entra sync error on every directory rename.
  Genuinely unknown paths still fail with invalidPath.
- Pagination edges covered: middle pages, count=0 (total only), and
  startIndex past the end.

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

src/api/scim/patch.rs

@@ -34,6 +34,29 @@ pub struct PatchOperation {
 #[derive(Debug, Default, PartialEq)]
 pub struct UserPatch {
     pub active: Option<bool>,
+    pub external_id: Option<String>,
+}
+
+// Attributes Entra maps by default but Vaultwarden does not sync after
+// creation. Accepted and ignored (RFC 7644 allows a service provider to
+// treat immutable-for-it attributes this way in practice), because a 400
+// here would surface as a sync error on every rename in the directory.
+// user.name is global to the person across organizations; an org-scoped
+// provisioning channel must not rewrite it.
+fn is_ignored_user_attribute(path: &str) -> bool {
+    let lower = path.to_lowercase();
+    lower == "displayname"
+        || lower == "name"
+        || lower.starts_with("name.")
+        || lower == "emails"
+        || lower.starts_with("emails[")
+        || lower == "username"
+        || lower == "title"
+        || lower == "roles"
+        || lower == "preferredlanguage"
+        || lower.starts_with("addresses")
+        || lower.starts_with("phonenumbers")
+        || lower.starts_with("urn:ietf:params:scim:schemas:extension:enterprise:2.0:user")
 }
 
 pub fn parse_user_patch(patch: &PatchOp) -> Result<UserPatch, ScimError> {
@@ -60,6 +83,13 @@ pub fn parse_user_patch(patch: &PatchOp) -> Result<UserPatch, ScimError> {
                 let value = operation.value.clone().unwrap_or(Value::Null);
                 result.active = Some(coerce_bool(value)?);
             }
+            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(op_path) if is_ignored_user_attribute(op_path) => {}
             Some(_) => {
                 return Err(ScimError::bad_request("invalidPath", "Unsupported patch path"));
             }
@@ -74,6 +104,13 @@ pub fn parse_user_patch(patch: &PatchOp) -> Result<UserPatch, ScimError> {
                 for (attribute, value) in map {
                     if attribute.eq_ignore_ascii_case("active") {
                         result.active = Some(coerce_bool(value.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 is_ignored_user_attribute(attribute) {
+                        // Accepted, not synced; see is_ignored_user_attribute.
                     } else {
                         return Err(ScimError::bad_request("invalidPath", "Unsupported patch attribute"));
                     }
@@ -131,6 +168,38 @@ mod tests {
         assert_eq!(active, Some(true));
     }
 
+    #[test]
+    fn entra_mapped_attributes_are_accepted_and_ignored() {
+        for path in ["displayName", "name.givenName", "emails[type eq \"work\"].value", "title", "roles"] {
+            let parsed = patch(json!({
+                "schemas": [PATCH_OP_URN],
+                "Operations": [
+                    {"op": "replace", "path": path, "value": "whatever"},
+                    {"op": "replace", "path": "active", "value": true},
+                ],
+            }))
+            .unwrap_or_else(|_| panic!("path {path} must be ignored, not rejected"));
+            assert_eq!(parsed.active, Some(true));
+        }
+    }
+
+    #[test]
+    fn externalid_patch_both_forms() {
+        let parsed = patch(json!({
+            "schemas": [PATCH_OP_URN],
+            "Operations": [{"op": "replace", "path": "externalId", "value": "new-ext-1"}],
+        }))
+        .expect("valid");
+        assert_eq!(parsed.external_id.as_deref(), Some("new-ext-1"));
+
+        let parsed = patch(json!({
+            "schemas": [PATCH_OP_URN],
+            "Operations": [{"op": "replace", "value": {"externalId": "new-ext-2", "displayName": "ignored"}}],
+        }))
+        .expect("valid");
+        assert_eq!(parsed.external_id.as_deref(), Some("new-ext-2"));
+    }
+
     #[test]
     fn rejects_bad_patches() {
         for (payload, expected_type) in [
@@ -141,7 +210,7 @@ mod tests {
             (json!({"schemas": [PATCH_OP_URN], "Operations": []}), "invalidValue"),
             (json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "remove", "path": "active"}]}), "invalidValue"),
             (
-                json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "replace", "path": "displayName", "value": "x"}]}),
+                json!({"schemas": [PATCH_OP_URN], "Operations": [{"op": "replace", "path": "wibble", "value": "x"}]}),
                 "invalidPath",
             ),
             (

src/api/scim/tests/mod.rs

@@ -542,7 +542,7 @@ async fn patch_unsupported_path_is_invalid_path() {
 
     let payload = json!({
         "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
-        "Operations": [{"op": "replace", "path": "displayName", "value": "New Name"}],
+        "Operations": [{"op": "replace", "path": "wibble", "value": "New Name"}],
     });
     let (auth, ct, body) = scim_body(&token, &payload);
     let response =
@@ -556,3 +556,135 @@ fn url_escape(raw: &str) -> String {
     // Percent-encode just enough for filter values in test URLs.
     raw.replace('%', "%25").replace(' ', "%20").replace('"', "%22")
 }
+
+// ---------------------------------------------------------------------------
+// Full Users surface (Phase 2: PUT, attribute PATCH, pagination)
+// ---------------------------------------------------------------------------
+
+#[rocket::async_test]
+async fn patch_displayname_rename_is_accepted_noop() {
+    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-rename-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let member = seed_member(&conn, &org, "renamed.user@example.com", 2, MembershipType::User).await;
+
+    // Entra sends this on every directory rename; it must not error and must
+    // not change membership state.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "replace", "path": "displayName", "value": "New Display Name"}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Users/{member}")).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["active"], json!(true));
+    assert_eq!(member_status(&conn, &member, &org).await, 2);
+}
+
+#[rocket::async_test]
+async fn patch_and_put_update_external_id_with_uniqueness() {
+    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-extid-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let member_a = seed_member(&conn, &org, "extid.a@example.com", 1, MembershipType::User).await;
+    let member_b = seed_member(&conn, &org, "extid.b@example.com", 1, MembershipType::User).await;
+
+    // PATCH assigns an externalId.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "replace", "path": "externalId", "value": "ext-a-1"}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Users/{member_a}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Ok);
+    assert_eq!(parse_json(&body_of(response).await)["externalId"], json!("ext-a-1"));
+
+    // PUT replaces it and can toggle active in the same request.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
+        "userName": "extid.a@example.com",
+        "externalId": "ext-a-2",
+        "active": false,
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.put(format!("/scim/v2/{org}/Users/{member_a}")).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["externalId"], json!("ext-a-2"));
+    assert_eq!(parsed["active"], json!(false));
+    assert_eq!(member_status(&conn, &member_a, &org).await, -127, "accepted (1) revoked to -127");
+
+    // Duplicate externalId on another member is a 409.
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+        "Operations": [{"op": "replace", "path": "externalId", "value": "ext-a-2"}],
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.patch(format!("/scim/v2/{org}/Users/{member_b}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Conflict);
+}
+
+#[rocket::async_test]
+async fn put_does_not_change_email_or_role() {
+    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-put-immutable-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    let member = seed_member(&conn, &org, "immutable@example.com", 2, MembershipType::User).await;
+
+    let payload = json!({
+        "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
+        "userName": "changed@example.com",
+        "displayName": "Different Person",
+    });
+    let (auth, ct, body) = scim_body(&token, &payload);
+    let response =
+        client.put(format!("/scim/v2/{org}/Users/{member}")).header(auth).header(ct).body(body).dispatch().await;
+    assert_eq!(response.status(), Status::Ok);
+    let parsed = parse_json(&body_of(response).await);
+    // The response reflects the real state: the email did not change.
+    assert_eq!(parsed["userName"], json!("immutable@example.com"));
+    assert_eq!(member_status(&conn, &member, &org).await, 2);
+}
+
+#[rocket::async_test]
+async fn pagination_edges() {
+    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-page-org").await;
+    let token = seed_scim_key(&conn, &org).await;
+    for i in 1..=3 {
+        seed_member(&conn, &org, &format!("page{i}@example.com"), 1, MembershipType::User).await;
+    }
+
+    // Middle page.
+    let response =
+        client.get(format!("/scim/v2/{org}/Users?startIndex=2&count=1")).header(bearer(&token)).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(3));
+    assert_eq!(parsed["itemsPerPage"], json!(1));
+    assert_eq!(parsed["startIndex"], json!(2));
+
+    // count=0 returns the total with no resources (RFC 7644 s3.4.2.4).
+    let response = client.get(format!("/scim/v2/{org}/Users?count=0")).header(bearer(&token)).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(3));
+    assert_eq!(parsed["itemsPerPage"], json!(0));
+
+    // startIndex beyond the end is an empty page, not an error.
+    let response = client.get(format!("/scim/v2/{org}/Users?startIndex=10")).header(bearer(&token)).dispatch().await;
+    let parsed = parse_json(&body_of(response).await);
+    assert_eq!(parsed["totalResults"], json!(3));
+    assert_eq!(parsed["itemsPerPage"], json!(0));
+}

src/api/scim/users.rs

@@ -42,7 +42,7 @@ use crate::{
 };
 
 pub fn routes() -> Vec<Route> {
-    routes![list_users, get_user, post_user, patch_user, delete_user]
+    routes![list_users, get_user, post_user, put_user, patch_user, delete_user]
 }
 
 // Synthetic acting user recorded in the org event log for SCIM-driven
@@ -267,14 +267,49 @@ async fn patch_user(
     };
 
     let patch = parse_user_patch(&data.0)?;
-    let Some(desired_active) = patch.active else {
-        return Err(ScimError::bad_request("invalidValue", "No supported attributes in patch (supported: active)"));
+
+    if let Some(external_id) = patch.external_id.as_deref() {
+        update_external_id(&mut member, external_id, &token, &conn).await?;
+    }
+
+    match patch.active {
+        Some(true) => restore_member(&mut member, &token, &conn).await?,
+        Some(false) => revoke_member(&mut member, &token, &conn).await?,
+        // Every operation was an accepted-but-ignored attribute (for example
+        // a displayName rename): succeed and return the current state.
+        None => {}
+    }
+
+    let Some(user) = User::find_by_uuid(&member.user_uuid, &conn).await else {
+        return Err(ScimError::internal());
     };
+    Ok(ScimResponse::ok(to_scim_user(&member, &user, &token)))
+}
 
-    if desired_active {
-        restore_member(&mut member, &token, &conn).await?;
-    } else {
-        revoke_member(&mut member, &token, &conn).await?;
+// PUT replaces the attributes SCIM owns: externalId and active. userName,
+// name, and role deliberately do not sync: email is the login identity and
+// user.name is global to the person, while roles cannot round-trip through
+// Vaultwarden's membership types. The response reflects the actual state.
+#[put("/v2/<_>/Users/<member_id>", data = "<data>")]
+async fn put_user(
+    member_id: MembershipId,
+    data: ScimJson<ScimUserRequest>,
+    token: ScimToken,
+    conn: DbConn,
+) -> Result<ScimResponse, ScimError> {
+    let Some(mut member) = Membership::find_by_uuid_and_org(&member_id, &token.org_uuid, &conn).await else {
+        return Err(ScimError::not_found());
+    };
+    let request = data.0;
+
+    if let Some(external_id) = request.external_id.as_deref() {
+        update_external_id(&mut member, external_id, &token, &conn).await?;
+    }
+
+    match request.active.map(|b| b.0) {
+        Some(true) => restore_member(&mut member, &token, &conn).await?,
+        Some(false) => revoke_member(&mut member, &token, &conn).await?,
+        None => {}
     }
 
     let Some(user) = User::find_by_uuid(&member.user_uuid, &conn).await else {
@@ -283,6 +318,25 @@ async fn patch_user(
     Ok(ScimResponse::ok(to_scim_user(&member, &user, &token)))
 }
 
+async fn update_external_id(
+    member: &mut Membership,
+    external_id: &str,
+    token: &ScimToken,
+    conn: &DbConn,
+) -> Result<(), ScimError> {
+    if member.external_id.as_deref() == Some(external_id) {
+        return Ok(());
+    }
+    // The externalId is the correlation key: enforce uniqueness within the org.
+    if Membership::find_by_external_id_and_org(external_id, &token.org_uuid, conn).await.is_some() {
+        return Err(ScimError::conflict("uniqueness", "A member with this externalId already exists"));
+    }
+    member.set_external_id(Some(external_id.to_owned()));
+    member.save(conn).await.map_err(|_| ScimError::internal())?;
+    log_scim_event(EventType::OrganizationUserUpdated, member, token, conn).await;
+    Ok(())
+}
+
 // DELETE deprovisions by revoking, identically to PATCH active:false. The
 // membership row is kept: see the module comment.
 #[delete("/v2/<_>/Users/<member_id>")]