feat(api): can grant external user (via email)
- add possibility to grant an external user.
- route /api/users/{id} added (rate limited for security)
- security: start route limitation for external users
ex: they must not browse /api/users/{id} nor addressbook
This commit is contained in:
@@ -24,6 +24,16 @@ pub struct UserDto {
|
||||
/// can't own storage; their quota is always 0. Internal users
|
||||
/// default to `false`.
|
||||
pub is_external: bool,
|
||||
/// Optional first/given name. Populated from the OIDC `given_name`
|
||||
/// claim at JIT provisioning, or via a profile-edit endpoint.
|
||||
/// `None` until explicitly set — `skip_serializing_if = "Option::is_none"`
|
||||
/// keeps the wire format compact for the common case.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub given_name: Option<String>,
|
||||
/// Optional last/family name. Same provenance + serde rules as
|
||||
/// `given_name`.
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub family_name: Option<String>,
|
||||
}
|
||||
|
||||
impl From<User> for UserDto {
|
||||
@@ -43,6 +53,8 @@ impl From<User> for UserDto {
|
||||
image: user.image().map(|s| s.to_string()),
|
||||
can_edit_image: !user.is_oidc_user(),
|
||||
is_external: user.is_external(),
|
||||
given_name: user.given_name().map(str::to_string),
|
||||
family_name: user.family_name().map(str::to_string),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -897,6 +897,115 @@ impl AuthApplicationService {
|
||||
self.get_user(user_id).await
|
||||
}
|
||||
|
||||
/// Visibility-checked profile lookup for `GET /api/users/{id}`.
|
||||
///
|
||||
/// Returns `NotFound` (not `AccessDenied`) when the caller has no
|
||||
/// legitimate relationship with the target — anti-enumeration: an
|
||||
/// attacker probing random UUIDs cannot distinguish "user doesn't
|
||||
/// exist" from "exists but you can't see them".
|
||||
///
|
||||
/// External callers (`is_external = TRUE`) are locked out of the
|
||||
/// endpoint entirely. They have no legitimate need to enumerate
|
||||
/// users — their session exists only to interact with resources
|
||||
/// they were explicitly granted. Returns `AccessDenied`, which the
|
||||
/// handler surfaces as 403; the external caller's own role is not
|
||||
/// a secret to themselves, so the honest status is appropriate.
|
||||
///
|
||||
/// Visibility rule for internal callers:
|
||||
/// 1. caller_id == target_id → always visible (self).
|
||||
/// 2. caller is admin → always visible (admin needs every user).
|
||||
/// 3. target is internal AND `expose_system_users` is on → already
|
||||
/// broadly visible via the system address book; no extra check.
|
||||
/// 4. caller and target share at least one grant — either
|
||||
/// direction, either as subject or granter. Subject-group
|
||||
/// co-membership is intentionally NOT included in v1; can be
|
||||
/// added later if a concrete need surfaces.
|
||||
/// 5. Anything else → `NotFound`.
|
||||
pub async fn get_user_profile(
|
||||
&self,
|
||||
caller_id: Uuid,
|
||||
target_id: Uuid,
|
||||
expose_system_users: bool,
|
||||
pool: &sqlx::PgPool,
|
||||
) -> Result<UserDto, DomainError> {
|
||||
// External-caller lockout. Load the caller eagerly so the
|
||||
// is_external check covers every branch (including self-lookup
|
||||
// — an external user reading their own profile via this route
|
||||
// is still off-limits; the frontend should rely on the existing
|
||||
// /api/auth/me endpoint for that).
|
||||
let caller = self.user_storage.get_user_by_id(caller_id).await?;
|
||||
if caller.is_external() {
|
||||
return Err(DomainError::new(
|
||||
ErrorKind::AccessDenied,
|
||||
"User",
|
||||
"External users cannot query /api/users/{id}",
|
||||
));
|
||||
}
|
||||
|
||||
// Self: always (now that the external lockout already filtered
|
||||
// external self-lookups above).
|
||||
if caller_id == target_id {
|
||||
return Ok(UserDto::from(caller));
|
||||
}
|
||||
|
||||
// Anti-enumeration: NotFound for everything that doesn't pass.
|
||||
// Convert a real NotFound on `target` to the same anonymous 404,
|
||||
// so existence isn't leaked through differential responses.
|
||||
let target = match self.user_storage.get_user_by_id(target_id).await {
|
||||
Ok(u) => u,
|
||||
Err(e) if e.kind == ErrorKind::NotFound => {
|
||||
return Err(DomainError::new(
|
||||
ErrorKind::NotFound,
|
||||
"User",
|
||||
"User not found",
|
||||
));
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
};
|
||||
|
||||
// Internal target + system-address-book exposed: already public.
|
||||
if !target.is_external() && expose_system_users {
|
||||
return Ok(UserDto::from(target));
|
||||
}
|
||||
|
||||
// Admin caller: always visible.
|
||||
if caller.role() == UserRole::Admin {
|
||||
return Ok(UserDto::from(target));
|
||||
}
|
||||
|
||||
// Shared grant: caller and target appear together in at least one
|
||||
// access_grants row (either as the granted-by + user-subject pair,
|
||||
// or symmetrically). LIMIT 1 + the (granted_by) + (subject_type,
|
||||
// subject_id) indexes keep this cheap.
|
||||
let related: Option<i32> = sqlx::query_scalar(
|
||||
r#"
|
||||
SELECT 1
|
||||
FROM storage.access_grants
|
||||
WHERE (granted_by = $1 AND subject_type = 'user' AND subject_id = $2)
|
||||
OR (granted_by = $2 AND subject_type = 'user' AND subject_id = $1)
|
||||
LIMIT 1
|
||||
"#,
|
||||
)
|
||||
.bind(caller_id)
|
||||
.bind(target_id)
|
||||
.fetch_optional(pool)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("UserProfile", format!("visibility query: {}", e))
|
||||
})?;
|
||||
|
||||
if related.is_some() {
|
||||
return Ok(UserDto::from(target));
|
||||
}
|
||||
|
||||
// No relationship — anti-enumeration NotFound.
|
||||
Err(DomainError::new(
|
||||
ErrorKind::NotFound,
|
||||
"User",
|
||||
"User not found",
|
||||
))
|
||||
}
|
||||
|
||||
// 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?;
|
||||
|
||||
@@ -925,6 +925,13 @@ impl AppServiceFactory {
|
||||
email_sender: None, // populated below
|
||||
mock_email_sender: None, // populated below
|
||||
magic_link_invite_service: None, // populated below
|
||||
// 60 lookups / minute / caller; cap at 50 000 tracked
|
||||
// callers to bound memory. The same limiter instance is
|
||||
// shared by every clone of AppState since it lives in an
|
||||
// Arc.
|
||||
user_profile_rate_limiter: Arc::new(
|
||||
crate::interfaces::middleware::rate_limit::RateLimiter::new(60, 60, 50_000),
|
||||
),
|
||||
};
|
||||
let email_bundle = build_email_sender(&self.config.smtp);
|
||||
app_state.email_sender = email_bundle.sender;
|
||||
@@ -1302,6 +1309,13 @@ pub struct AppState {
|
||||
pub magic_link_invite_service: Option<
|
||||
Arc<crate::application::services::magic_link_invite_service::MagicLinkInviteService>,
|
||||
>,
|
||||
/// Per-caller sliding-window limiter for `GET /api/users/{id}`. The
|
||||
/// endpoint's primary defense is the visibility check, but a stale
|
||||
/// JWT could in theory iterate UUIDs against the related-by-grant
|
||||
/// branch of that check. 60 lookups per minute keyed on the
|
||||
/// authenticated caller covers any legitimate UI rendering while
|
||||
/// throttling enumeration.
|
||||
pub user_profile_rate_limiter: Arc<crate::interfaces::middleware::rate_limit::RateLimiter>,
|
||||
}
|
||||
|
||||
// All AppState construction is done via struct literal in build_app_state().
|
||||
|
||||
@@ -254,7 +254,22 @@ pub async fn list_address_books(
|
||||
})
|
||||
.collect();
|
||||
|
||||
if state.expose_system_users && state.auth_service.is_some() {
|
||||
// Skip the system address book for external callers so they
|
||||
// don't see an internal-user directory entry (let alone its
|
||||
// contents). The system book is only useful to internal
|
||||
// users picking sharees out of the directory.
|
||||
let hide_system_for_external = match state.auth_service.as_ref() {
|
||||
Some(svc) => {
|
||||
crate::interfaces::middleware::user::require_internal_user(svc, auth_user.id)
|
||||
.await
|
||||
.is_err()
|
||||
}
|
||||
None => false,
|
||||
};
|
||||
if state.expose_system_users
|
||||
&& state.auth_service.is_some()
|
||||
&& !hide_system_for_external
|
||||
{
|
||||
let now = Utc::now();
|
||||
response.push(AddressBookResponse {
|
||||
id: SYSTEM_BOOK_ID.to_string(),
|
||||
@@ -451,6 +466,14 @@ pub async fn list_contacts(
|
||||
let Some(auth_service) = &state.auth_service else {
|
||||
return system_book_unavailable();
|
||||
};
|
||||
// External callers must not enumerate the internal-user
|
||||
// directory through the system address book.
|
||||
if let Err(e) =
|
||||
crate::interfaces::middleware::user::require_internal_user(auth_service, auth_user.id)
|
||||
.await
|
||||
{
|
||||
return e.into_response();
|
||||
}
|
||||
let caller_id = auth_user.id.to_string();
|
||||
match auth_service.list_users(params.limit, params.offset).await {
|
||||
Ok(users) => {
|
||||
@@ -562,6 +585,12 @@ pub async fn get_contact(
|
||||
let Some(auth_service) = &state.auth_service else {
|
||||
return system_book_unavailable();
|
||||
};
|
||||
if let Err(e) =
|
||||
crate::interfaces::middleware::user::require_internal_user(auth_service, auth_user.id)
|
||||
.await
|
||||
{
|
||||
return e.into_response();
|
||||
}
|
||||
let Ok(uuid) = Uuid::parse_str(&contact_id) else {
|
||||
return (
|
||||
StatusCode::BAD_REQUEST,
|
||||
|
||||
@@ -21,6 +21,7 @@ pub mod search_handler;
|
||||
pub mod share_handler;
|
||||
pub mod subject_group_handler;
|
||||
pub mod trash_handler;
|
||||
pub mod users_handler;
|
||||
pub mod webdav_handler;
|
||||
pub mod wopi_handler;
|
||||
|
||||
|
||||
@@ -0,0 +1,91 @@
|
||||
//! User-profile lookup for the frontend.
|
||||
//!
|
||||
//! `GET /api/users/{id}` returns a [`UserDto`] for the target user iff
|
||||
//! the authenticated caller has a legitimate relationship with them.
|
||||
//! The visibility rule lives in
|
||||
//! [`AuthApplicationService::get_user_profile`] — handlers never embed
|
||||
//! their own authz check (CLAUDE.md § Authorization).
|
||||
//!
|
||||
//! In addition to the per-request visibility check, every call is
|
||||
//! throttled by a per-caller sliding-window limiter (60/min) so that a
|
||||
//! stale JWT can't iterate UUIDs against the related-by-grant branch
|
||||
//! of the visibility rule. The limiter shares the same `RateLimiter`
|
||||
//! type as the login / register / refresh middlewares; this handler
|
||||
//! invokes it inline rather than through a layer because the key is
|
||||
//! the authenticated caller_id (not the client IP).
|
||||
|
||||
use axum::{
|
||||
Json, Router,
|
||||
extract::{Path, State},
|
||||
http::StatusCode,
|
||||
response::IntoResponse,
|
||||
routing::get,
|
||||
};
|
||||
use std::sync::Arc;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::common::di::AppState;
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::AuthUser;
|
||||
|
||||
/// Build the `/users` router — mounted at the `/api/users` prefix by
|
||||
/// `main.rs`. Auth + CSRF middlewares are applied by the caller.
|
||||
pub fn user_routes() -> Router<Arc<AppState>> {
|
||||
Router::new().route("/{id}", get(get_user_profile))
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
get,
|
||||
path = "/api/users/{id}",
|
||||
params(("id" = String, Path, description = "User UUID")),
|
||||
responses(
|
||||
(status = 200, description = "Profile of a user the caller can see"),
|
||||
(status = 404, description = "User does not exist OR caller has no visibility (anti-enumeration: indistinguishable)"),
|
||||
(status = 429, description = "Per-caller rate limit exceeded"),
|
||||
),
|
||||
security(("bearerAuth" = [])),
|
||||
tag = "users",
|
||||
)]
|
||||
async fn get_user_profile(
|
||||
State(state): State<Arc<AppState>>,
|
||||
auth_user: AuthUser,
|
||||
Path(target_id): Path<Uuid>,
|
||||
) -> Result<impl IntoResponse, AppError> {
|
||||
let caller_id = auth_user.id;
|
||||
|
||||
// Rate limit FIRST so an attacker can't exhaust the visibility
|
||||
// query (which touches `access_grants`) by hammering with random
|
||||
// UUIDs.
|
||||
if let Err(()) = state
|
||||
.user_profile_rate_limiter
|
||||
.check_and_increment(&caller_id.to_string())
|
||||
{
|
||||
return Err(AppError::new(
|
||||
StatusCode::TOO_MANY_REQUESTS,
|
||||
"Too many user lookups; please retry shortly",
|
||||
"RateLimited",
|
||||
));
|
||||
}
|
||||
|
||||
let auth_svc = state
|
||||
.auth_service
|
||||
.as_ref()
|
||||
.ok_or_else(|| AppError::internal_error("Auth service not configured"))?;
|
||||
let pool = state
|
||||
.db_pool
|
||||
.as_ref()
|
||||
.ok_or_else(|| AppError::internal_error("Database pool not available"))?;
|
||||
|
||||
let dto = auth_svc
|
||||
.auth_application_service
|
||||
.get_user_profile(
|
||||
caller_id,
|
||||
target_id,
|
||||
state.core.config.features.expose_system_users,
|
||||
pool,
|
||||
)
|
||||
.await
|
||||
.map_err(AppError::from)?;
|
||||
|
||||
Ok(Json(dto))
|
||||
}
|
||||
@@ -571,6 +571,13 @@ pub fn create_api_routes(app_state: &Arc<AppState>) -> Router<Arc<AppState>> {
|
||||
.with_state(app_state.clone());
|
||||
router = router.nest("/groups", group_router);
|
||||
|
||||
// Per-user profile lookup `/api/users/{id}` — authenticated only,
|
||||
// throttled by a per-caller limiter inside the handler. External
|
||||
// callers are 403'd in the service layer.
|
||||
let users_router = crate::interfaces::api::handlers::users_handler::user_routes()
|
||||
.with_state(app_state.clone());
|
||||
router = router.nest("/users", users_router);
|
||||
|
||||
// Transparent compression (gzip + brotli) for all API responses.
|
||||
// tower-http negotiates via Accept-Encoding and skips already-compressed
|
||||
// content types automatically. No manual compression in handlers.
|
||||
|
||||
@@ -4,3 +4,4 @@ pub mod csrf;
|
||||
pub mod rate_limit;
|
||||
pub mod trace_span;
|
||||
pub mod trusted_proxy;
|
||||
pub mod user;
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
//! Caller-id-based user guards.
|
||||
//!
|
||||
//! All guards in this module take `(auth, caller_id) → Result<(), AppError>`
|
||||
//! so handlers compose them uniformly as one-liners. They assume the
|
||||
//! caller has already been authenticated by the
|
||||
//! [`AuthUser`](super::auth::AuthUser) extractor, and pull the current
|
||||
//! user state from the database via `AuthApplicationService` so role /
|
||||
//! external-flag changes take effect on the next request without
|
||||
//! waiting for token rotation.
|
||||
//!
|
||||
//! ```ignore
|
||||
//! let caller_id = auth_user.id;
|
||||
//! require_internal_user(&auth, caller_id).await?;
|
||||
//! require_admin_user(&auth, caller_id).await?;
|
||||
//! ```
|
||||
//!
|
||||
//! Future role-based guards (e.g. `require_active_user`) should follow
|
||||
//! the same shape so they slot in next to these without ceremony.
|
||||
//!
|
||||
//! For the legacy header-based admin guard (`require_admin`), see
|
||||
//! [`super::admin`] — that variant exists because some handlers take
|
||||
//! `headers: HeaderMap` directly instead of `AuthUser`.
|
||||
|
||||
use axum::http::StatusCode;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::application::services::auth_application_service::AuthApplicationService;
|
||||
use crate::interfaces::errors::AppError;
|
||||
|
||||
/// Require the caller to be an internal user. Returns `Ok(())` for
|
||||
/// internal callers, `Err(403)` for externals.
|
||||
///
|
||||
/// External users authenticate via magic-link / OIDC-only / OCM and
|
||||
/// exist solely to interact with resources they were explicitly
|
||||
/// granted. They have no business enumerating the user directory, the
|
||||
/// address book, subject groups, or any other instance-wide listing —
|
||||
/// this guard locks them out of those surfaces.
|
||||
///
|
||||
/// DB lookup errors fall back to `Ok(())` so a transient outage doesn't
|
||||
/// lock everyone out — this guard is defense in depth. The canonical
|
||||
/// filter is at the service / repository layer (`include_external =
|
||||
/// false` on `list_users`, the visibility rule in `get_user_profile`,
|
||||
/// etc.); this helper just opts a surface in to "internal only" with
|
||||
/// one extra line.
|
||||
///
|
||||
/// The 403 status is honest (not 404 stealth) because the caller's own
|
||||
/// `is_external` flag is not a secret to themselves — the UI already
|
||||
/// surfaces "you came in through a magic link".
|
||||
pub async fn require_internal_user(
|
||||
auth: &AuthApplicationService,
|
||||
caller_id: Uuid,
|
||||
) -> Result<(), AppError> {
|
||||
match auth.get_user_by_id(caller_id).await {
|
||||
Ok(dto) if dto.is_external => Err(AppError::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
"External users cannot access this endpoint",
|
||||
"Forbidden",
|
||||
)),
|
||||
_ => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Require the caller to hold the admin role. Returns `Ok(())` for
|
||||
/// admins, `Err(403)` otherwise.
|
||||
///
|
||||
/// The check pulls the role from the user record (not from JWT
|
||||
/// claims) so a role change takes effect on the next request without
|
||||
/// waiting for token rotation. Mirrors [`require_internal_user`]'s
|
||||
/// shape so handlers compose either of them as a one-liner via `?`.
|
||||
///
|
||||
/// Use this in handlers that already have an
|
||||
/// [`AuthUser`](super::auth::AuthUser) extractor (and thus a validated
|
||||
/// `caller_id`); use the legacy [`super::admin::require_admin`] variant
|
||||
/// when the handler signature is `headers: HeaderMap` instead.
|
||||
pub async fn require_admin_user(
|
||||
auth: &AuthApplicationService,
|
||||
caller_id: Uuid,
|
||||
) -> Result<(), AppError> {
|
||||
let user = auth
|
||||
.get_user_by_id(caller_id)
|
||||
.await
|
||||
.map_err(AppError::from)?;
|
||||
|
||||
if user.role != "admin" {
|
||||
return Err(AppError::new(
|
||||
StatusCode::FORBIDDEN,
|
||||
"Admin access required",
|
||||
"Forbidden",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -187,8 +187,63 @@ body contains "{{ext_folder_id}}"
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 11 — Second redemption of the same token is rejected.
|
||||
# single-use is enforced by the SQL UPDATE in
|
||||
# Step 11 — External-user lockouts (PR 11.1 + ContactsHandler).
|
||||
# Bob (external) must NOT reach the system address book
|
||||
# or the per-user profile endpoint. Defense-in-depth on
|
||||
# top of the PR 6 service-level filter.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
|
||||
# 11a — system address book: visible at the catalog level
|
||||
# (`GET /api/address-books`) for bob? It must NOT list the system entry.
|
||||
GET {{base_url}}/api/address-books
|
||||
Authorization: Bearer {{bob_access_token}}
|
||||
|
||||
HTTP 200
|
||||
[Asserts]
|
||||
body not contains "OxiCloud Users"
|
||||
body not contains "\"id\":\"system\""
|
||||
|
||||
# 11b — system contacts listing: 403 for bob.
|
||||
GET {{base_url}}/api/address-books/system/contacts
|
||||
Authorization: Bearer {{bob_access_token}}
|
||||
|
||||
HTTP 403
|
||||
|
||||
# 11c — /api/users/{id}: bob cannot query anyone's profile, not even
|
||||
# Alice's. Service-level external lockout in get_user_profile.
|
||||
GET {{base_url}}/api/users/{{bob_user_id}}
|
||||
Authorization: Bearer {{bob_access_token}}
|
||||
|
||||
HTTP 403
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 12 — /api/users/{id} happy path (Alice → Bob).
|
||||
# Visibility rule: they share a grant, so Alice sees
|
||||
# Bob's profile (with is_external=true).
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
GET {{base_url}}/api/users/{{bob_user_id}}
|
||||
Authorization: Bearer {{alice_token}}
|
||||
|
||||
HTTP 200
|
||||
[Asserts]
|
||||
jsonpath "$.id" == "{{bob_user_id}}"
|
||||
jsonpath "$.is_external" == true
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 13 — /api/users/{id} 404 anti-enumeration for an
|
||||
# unrelated UUID (random Uuid that doesn't exist).
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
GET {{base_url}}/api/users/00000000-0000-0000-0000-deadbeefcafe
|
||||
Authorization: Bearer {{alice_token}}
|
||||
|
||||
HTTP 404
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
# Step 14 — Second redemption of the same magic-link token is
|
||||
# rejected — single-use is enforced by the SQL UPDATE in
|
||||
# magic_link_token_pg_repository::mark_used.
|
||||
# ─────────────────────────────────────────────────────────────
|
||||
GET {{magic_url}}
|
||||
|
||||
Reference in New Issue
Block a user