From dc009f053e29ae32864330a6ab5a85e8237fa594 Mon Sep 17 00:00:00 2001
From: Edouard Vanbelle <opensource@edouard.vanbelle.fr>
Date: Fri, 17 Jul 2026 19:12:12 +0200
Subject: [PATCH] security(nextcloud): ocs: get only users profile session can
 access to

---
 .../services/auth_application_service.rs      | 53 +++++++++++++++++
 src/interfaces/nextcloud/ocs_handler.rs       | 33 +++++++++--
 tests/api/nc_admin_views_other_user.hurl      | 59 ++++++++++++++-----
 3 files changed, 125 insertions(+), 20 deletions(-)

diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs
index 156f618e..6d86db38 100644
--- a/src/application/services/auth_application_service.rs
+++ b/src/application/services/auth_application_service.rs
@@ -1924,6 +1924,59 @@ impl AuthApplicationService {
         ))
     }
 
+    /// Username-keyed sibling of [`Self::get_user_profile`], routing every
+    /// lookup through the same visibility check as the user-profile REST
+    /// endpoint. Preserves the anti-enum shape end-to-end: whether the
+    /// username doesn't exist OR the caller has no visibility path, the
+    /// response is `NotFound`.
+    ///
+    /// AuthZ audit #11 (2026-07-12): NextCloud OCS user-provisioning
+    /// (`nextcloud/ocs_handler.rs::user_provisioning_response`) used to
+    /// resolve `userid` via bare `get_user_by_username`, gated only by a
+    /// bespoke `caller.role == "admin"` shortcut. Admins bypassed the
+    /// `expose_system_users` gate; non-admins got a `403 Insufficient
+    /// privileges` for any cross-user probe (leaking existence via the
+    /// differential vs a genuine 404); zero audit lines. This wrapper
+    /// closes all three.
+    ///
+    /// The username→id resolution happens here so the target isn't
+    /// leaked through the audit line as a plaintext username on failure:
+    /// the `target_username_not_found` event carries the string
+    /// (unavoidable — we resolved it, we log it), but every other
+    /// downstream event keys off `target_id` after resolution, matching
+    /// the id-based endpoint.
+    pub async fn get_user_profile_by_username_with_perms(
+        &self,
+        caller_id: Uuid,
+        username: &str,
+        expose_system_users: bool,
+        pool: &sqlx::PgPool,
+    ) -> Result<UserDto, DomainError> {
+        let target = match self.user_storage.get_user_by_username(username).await {
+            Ok(u) => u,
+            Err(e) if e.kind == ErrorKind::NotFound => {
+                tracing::info!(
+                    target: "audit",
+                    event = "user_profile.rejected",
+                    reason = "target_username_not_found",
+                    caller_id = %caller_id,
+                    target_username = %username,
+                    "👮🏻‍♂️ user-profile rejected: username '{}' does not exist (caller {})",
+                    username,
+                    caller_id,
+                );
+                return Err(DomainError::new(
+                    ErrorKind::NotFound,
+                    "User",
+                    "User not found",
+                ));
+            }
+            Err(e) => return Err(e),
+        };
+        self.get_user_profile(caller_id, target.id(), expose_system_users, pool)
+            .await
+    }
+
     // New method to get user by username - needed for admin user handling
     pub async fn get_user_by_username(&self, username: &str) -> Result<UserDto, DomainError> {
         let user = self.user_storage.get_user_by_username(username).await?;
diff --git a/src/interfaces/nextcloud/ocs_handler.rs b/src/interfaces/nextcloud/ocs_handler.rs
index 73fc0394..4abb08cb 100644
--- a/src/interfaces/nextcloud/ocs_handler.rs
+++ b/src/interfaces/nextcloud/ocs_handler.rs
@@ -135,19 +135,40 @@ async fn user_provisioning_response(
 ) -> Response {
     let statuscode = if ocs_version == 1 { 100 } else { 200 };
 
-    // Only allow users to view their own profile, unless they are admin.
-    if user.username != userid && user.role != "admin" {
-        return Json(ocs_err(403, "Insufficient privileges")).into_response();
-    }
-
+    // AuthZ audit #11 (2026-07-12): the pre-fix path here rolled its
+    // own gate ("caller is `userid`, else must be admin") and then
+    // called bare `get_user_by_username` — bypassing every visibility
+    // rule the id-keyed `/api/users/{id}` endpoint enforces. Cross-user
+    // probes returned 403 (leaking existence via the differential vs a
+    // genuine 404 for missing users); admins bypassed
+    // `expose_system_users`; no audit line ever fired.
+    //
+    // Now routing through `get_user_profile_by_username_with_perms`,
+    // which delegates to the same visibility engine as the REST
+    // endpoint (self / shared-grant / expose_system_users / admin
+    // paths, all audit-logged on denial). The OCS wire shape stays
+    // `ocs_err(404, ...)` for every denied case — the NC client can't
+    // tell "no such user" from "you can't see this user" from "you're
+    // not admin" apart, which is the anti-enum invariant.
     let auth_service = match state.auth_service.as_ref() {
         Some(svc) => &svc.auth_application_service,
         None => {
             return Json(ocs_err(997, "Authentication not configured")).into_response();
         }
     };
+    let Some(pool) = state.db_pool.as_ref() else {
+        return Json(ocs_err(997, "Database pool not available")).into_response();
+    };
 
-    let user_dto = match auth_service.get_user_by_username(&userid).await {
+    let user_dto = match auth_service
+        .get_user_profile_by_username_with_perms(
+            user.id,
+            &userid,
+            state.core.config.features.expose_system_users,
+            pool,
+        )
+        .await
+    {
         Ok(u) => u,
         Err(_) => {
             return Json(ocs_err(404, "User not found")).into_response();
diff --git a/tests/api/nc_admin_views_other_user.hurl b/tests/api/nc_admin_views_other_user.hurl
index e17dd204..88756e55 100644
--- a/tests/api/nc_admin_views_other_user.hurl
+++ b/tests/api/nc_admin_views_other_user.hurl
@@ -3,18 +3,25 @@
 # =============================================================
 # C4 from BASELINE_TESTS_NC_WEBDAV.md.
 #
-# Deferred from Batch 1 because it needed the bob fixture
-# that `nc_second_user_setup.hurl` now provides. Pins the
-# behaviour of the existing rule in
-# `interfaces/nextcloud/ocs_handler.rs::user_provisioning_response`:
+# Post AuthZ audit #11 (2026-07-17), `user_provisioning_response`
+# no longer rolls its own admin gate — it delegates to
+# `AuthApplicationService::get_user_profile_by_username_with_perms`,
+# which shares the visibility engine with the id-keyed REST
+# endpoint at `/api/users/{id}`. Consequences for this test:
 #
-#     if user.username != userid && user.role != "admin" {
-#         return Json(ocs_err(403, ...)).into_response();
-#     }
-#
-# i.e. you can read your own profile always; you can read
-# anyone's profile if you're admin. Bob is not admin, so bob
-# CANNOT read admin's profile (the symmetric assertion).
+#   - **admin → bob**: still 200 (admin bypass is one of the
+#     five visibility paths; see get_user_profile step 5).
+#   - **bob → admin**: with `OXICLOUD_EXPOSE_SYSTEM_USERS=true`
+#     (tests/common/server.env), both are internal so step 4
+#     of the visibility engine says the target is broadly
+#     visible via the system address book — bob CAN see
+#     admin's basic profile. Pre-fix, the bespoke gate returned
+#     `403 Insufficient privileges` and admin bypassed the
+#     expose gate silently; both anomalies are gone.
+#   - **bob → nonexistent**: `404 User not found`, anti-enum
+#     shape identical to "you can't see this user". Audit line
+#     `user_profile.rejected reason=target_username_not_found`
+#     fires server-side.
 #
 # Uses admin's app password for Basic Auth (same pattern as
 # `nc_ocs_user_info.hurl`).
@@ -82,8 +89,12 @@ jsonpath "$.ocs.data.email"      == "bob@example.com"
 
 
 # ─────────────────────────────────────────────────────────────
-# C4-symmetric — bob (non-admin) CANNOT read admin's profile
-#                (proves the admin-only branch isn't a no-op)
+# C4-symmetric — post-audit-#11: bob CAN read admin's profile
+#                because the visibility engine's
+#                `expose_system_users` branch treats internal
+#                users as broadly visible via the system address
+#                book. The bespoke `403 Insufficient privileges`
+#                the pre-fix handler emitted is gone.
 # ─────────────────────────────────────────────────────────────
 GET {{base_url}}/ocs/v1.php/cloud/users/{{username}}?format=json
 [BasicAuth]
@@ -91,7 +102,27 @@ GET {{base_url}}/ocs/v1.php/cloud/users/{{username}}?format=json
 
 HTTP 200
 [Asserts]
-jsonpath "$.ocs.meta.statuscode" == 403
+jsonpath "$.ocs.meta.statuscode" == 100
+jsonpath "$.ocs.data.id"         == "{{username}}"
+
+
+# ─────────────────────────────────────────────────────────────
+# C4-antienum — bob queries a genuinely nonexistent username.
+#               Response body is the SAME shape as any denial
+#               case: `statuscode=404 status="failure"`. The
+#               NC client cannot distinguish "user doesn't
+#               exist" from "you have no visibility on that
+#               user" (were expose_system_users off) — which
+#               is the anti-enumeration invariant this fix
+#               was meant to preserve.
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/ocs/v1.php/cloud/users/nonexistent-audit-11-canary?format=json
+[BasicAuth]
+{{bob_nc_user}}: {{bob_nc_pw}}
+
+HTTP 200
+[Asserts]
+jsonpath "$.ocs.meta.statuscode" == 404
 jsonpath "$.ocs.meta.status"     == "failure"