diff --git a/src/api/scim/patch.rs b/src/api/scim/patch.rs index 45d03a54..17f03d81 100644 --- a/src/api/scim/patch.rs +++ b/src/api/scim/patch.rs @@ -34,6 +34,29 @@ pub struct PatchOperation { #[derive(Debug, Default, PartialEq)] pub struct UserPatch { pub active: Option, + pub external_id: Option, +} + +// 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 { @@ -60,6 +83,13 @@ pub fn parse_user_patch(patch: &PatchOp) -> Result { 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 { 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", ), ( diff --git a/src/api/scim/tests/mod.rs b/src/api/scim/tests/mod.rs index ebb96084..b3dd922d 100644 --- a/src/api/scim/tests/mod.rs +++ b/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)); +} diff --git a/src/api/scim/users.rs b/src/api/scim/users.rs index 664158c2..ffc4f754 100644 --- a/src/api/scim/users.rs +++ b/src/api/scim/users.rs @@ -42,7 +42,7 @@ use crate::{ }; pub fn routes() -> Vec { - 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/", data = "")] +async fn put_user( + member_id: MembershipId, + data: ScimJson, + token: ScimToken, + conn: DbConn, +) -> Result { + 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/")]