security(nextcloud): ocs: get only users profile session can access to

This commit is contained in:
Edouard Vanbelle
2026-07-17 19:12:12 +02:00
parent 9e30018134
commit dc009f053e
3 changed files with 125 additions and 20 deletions
@@ -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?;
+27 -6
View File
@@ -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();
+45 -14
View File
@@ -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"