Skip to content

list_requests enumerates all users' requests with no caller identity or role check #397

Description

@forkwright

Finding

RequestService::list_requests takes optional user_id and status filters. When both are None, the all-records arm calls repo::list_all and returns every request in the database. The trait method accepts no caller identity, so the service layer cannot distinguish who is asking and performs no role check before serving the unfiltered set. Any caller holding a service reference can enumerate every household member's request history.

Evidence

crates/aitesis/src/lib.rs:235-249: the method signature carries no caller parameter, and the (None, None) arm returns all rows unconditionally:

async fn list_requests(
    &self,
    user_id: Option<UserId>,
    status: Option<RequestStatus>,
) -> Result<Vec<MediaRequest>, AitesisError> {
    match (user_id, status) {
        (Some(uid), Some(st)) => {
            let all = repo::list_by_user(&self.read, &uid).await?;
            Ok(all.into_iter().filter(|r| r.status == st).collect())
        }
        (Some(uid), None) => repo::list_by_user(&self.read, &uid).await,
        (None, Some(st)) => repo::list_by_status(&self.read, st).await,
        (None, None) => repo::list_all(&self.read).await,
    }
}

crates/aitesis/src/lib.rs:247: (None, None) => repo::list_all(&self.read).await, — no authorization precedes this arm.

Why this matters

The service is the trust boundary for media-request data; authorization cannot be reliably delegated to every HTTP handler that holds the reference. A Member-role caller whose handler reaches the (None, None) path — through a routing bug, a missing handler-side filter, or a misconfigured paroche endpoint — receives every other household member's request history. Request titles disclose purchasing intent, viewing habits, and denial reasons, so an over-broad read leaks per-person behavioral data to anyone with a member account. Defense-in-depth fails here because the only gate is whatever the caller chose to pass; the data-minimization invariant has no enforcement point inside the crate.

Desired correction

Add a caller_id: UserId parameter to list_requests and resolve the caller's role inside the method. For the all-users query (user_id == None), require caller_role == Admin and return InsufficientPermission otherwise; for a self-scoped query require user_id == Some(caller_id) unless the caller is Admin. This makes the all-records path unreachable for non-admins regardless of handler behavior. Done when: a Member calling list_requests with user_id = None receives an authorization error rather than data, and a Member can only retrieve their own requests.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions