cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

refactor(user): apply chanoges to hurl tests

Browse Source
This commit is contained in:
Edouard Vanbelle
2026-08-21 23:19:00 +02:00
parent c583b26355
commit a8fa281a02
48 changed files with 310 additions and 244 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/application/services/auth_application_service.rs
+50 -7
View File
@@ -1359,6 +1359,31 @@ impl AuthApplicationService {
&self,
user_id: Uuid,
session: &crate::domain::entities::session::Session,
) -> Result<SelfUserDto, DomainError> {
// Session-context flavour — delegates to the shared builder
// with the DPoP-bound flag derived from the session row's
// thumbprint. See [`build_self_user_dto_for_id`] for the
// handler-context flavour.
self.build_self_user_dto_for_id(user_id, session.dpop_jkt().is_some())
.await
}
/// Handler-context variant of [`build_self_user_dto`]. Called by
/// every endpoint that returns a `SelfUserDto` from a REST handler
/// (`GET /me`, `PATCH /me/profile`, `POST /upgrade-to-internal`)
/// so the wire shape is byte-for-byte identical across them —
/// avoids a "quiet lie" where a client PATCHes one shape and
/// reads another on the very next `/me`.
///
/// `is_dpop_bound` is passed in by the handler because the JWT
/// `cnf.jkt` claim is where handler-scope code learns the caller's
/// binding state (via `auth_user.dpop_jkt.is_some()`). Session-
/// mint paths use [`build_self_user_dto`] and derive the flag from
/// the freshly-created `Session` row instead.
pub async fn build_self_user_dto_for_id(
&self,
user_id: Uuid,
is_dpop_bound: bool,
) -> Result<SelfUserDto, DomainError> {
let (user, flags) =
UserStoragePort::get_user_with_derived_flags(&*self.user_storage, user_id).await?;
@@ -1366,7 +1391,6 @@ impl AuthApplicationService {
let ui_preferences = user.ui_preferences().clone();
let notify_on_share = user.notify_on_share();
let force_password_change = self.read_force_password_change(user_id).await;
let is_dpop_bound = session.dpop_jkt().is_some();
let full = FullUserDto::build(user, flags);
Ok(SelfUserDto::build(
full,
@@ -3414,7 +3438,7 @@ impl AuthApplicationService {
pub async fn admin_create_user(
&self,
dto: crate::application::dtos::settings_dto::AdminCreateUserDto,
) -> Result<PublicUserDto, DomainError> {
) -> Result<FullUserDto, DomainError> {
// Validate username length
if dto.username.len() < 3 || dto.username.len() > 254 {
return Err(DomainError::new(
@@ -3572,7 +3596,16 @@ impl AuthApplicationService {
created.id(),
created.is_external()
);
Ok(PublicUserDto::new(created, false))
// Return `FullUserDto` — same shape as `GET /api/admin/users/{id}`
// and one row of the admin list. Admin surfaces uniformly return
// FullUserDto so the SPA / test asserts don't need to know which
// admin endpoint they came from. Fresh user has no session yet
// (`is_online = false`) and no OPAQUE registration; `has_password`
// reflects whatever the admin passed in the DTO.
let created_id = created.id();
let (user, flags) =
UserStoragePort::get_user_with_derived_flags(&*self.user_storage, created_id).await?;
Ok(FullUserDto::build(user, flags))
}
/// Admin-only: reset a user's password.
@@ -3668,10 +3701,20 @@ impl AuthApplicationService {
Ok(())
}
/// Get a single user by ID (for admin panel)
pub async fn get_user_admin(&self, user_id: Uuid) -> Result<PublicUserDto, DomainError> {
let user = self.user_storage.get_user_by_id(user_id).await?;
Ok(PublicUserDto::new(user, false))
/// Get a single user by ID (for admin panel).
///
/// Returns `FullUserDto` — same shape as one row of
/// `/api/admin/users` — so admin single-user views (detail modal,
/// per-user edit page) render the same fields the list surfaces.
/// The single-row admin view is the canonical observation surface
/// for admin-visible signals like `email_verified_at` /
/// `has_password` / `opaque_registered` / `last_login_at` — none
/// of which live on the peer-view `PublicUserDto`. See
/// `docs/plan/userdto-refactor.md`.
pub async fn get_user_admin(&self, user_id: Uuid) -> Result<FullUserDto, DomainError> {
let (user, flags) =
UserStoragePort::get_user_with_derived_flags(&*self.user_storage, user_id).await?;
Ok(FullUserDto::build(user, flags))
}
/// Delete a user by ID (admin only).
src/interfaces/api/handlers/auth_handler.rs
+53 -62
View File
@@ -11,9 +11,9 @@ use utoipa::ToSchema;
use uuid::Uuid;
use crate::application::dtos::user_dto::{
AuthResponseDto, ChangePasswordDto, FullUserDto, LoginDto, OidcCallbackQueryDto,
OidcExchangeDto, OidcProviderInfoDto, PublicUserDto, RefreshTokenDto, RegisterDto, SelfUserDto,
SetupAdminDto, UpgradeToInternalDto,
AuthResponseDto, ChangePasswordDto, LoginDto, OidcCallbackQueryDto, OidcExchangeDto,
OidcProviderInfoDto, PublicUserDto, RefreshTokenDto, RegisterDto, SelfUserDto, SetupAdminDto,
UpgradeToInternalDto,
};
use crate::application::services::auth_application_service::{OidcCallbackResult, RegisterResult};
use crate::common::di::AppState;
@@ -652,59 +652,20 @@ pub async fn get_current_user(
// Semantics (`docs/plan/drive.md` §7): `storage_used_bytes` is the SUM
// of `used_bytes` across the user's personal drives only. Shared drives
// never count against this envelope — collaborating in a team drive
// costs no personal bytes. The matching cap is
// `storage_quota_bytes` (admin-only mutation).
// costs no personal bytes.
//
// Single-query fetch: `get_user_with_derived_flags` returns the full
// `User` entity + `UserDerivedFlags` (has_password / OPAQUE flags /
// is_online) in one round-trip. That collapses what used to be a
// `get_user_by_id` + separate credential lookups into one wire trip,
// AND populates the OPAQUE flags on `/me` which the fat-PublicUserDto path
// never did (it left them at false — the "quiet lie" that motivated
// this refactor, see `docs/plan/userdto-refactor.md`).
let (user, flags) = auth_service
// Delegate to the shared `build_self_user_dto_for_id` — same code
// path `PATCH /me/profile` and `POST /upgrade-to-internal` use so
// all three self endpoints ship byte-for-byte identical shapes.
// The DPoP-bound signal comes from the JWT `cnf.jkt` claim
// (surfaced by the auth middleware into `AuthUser.dpop_jkt`);
// when present the session that minted this JWT is bound and
// the SPA can skip a redundant `/dpop/bind` call.
let self_dto = auth_service
.auth_application_service
.get_user_with_derived_flags(user_id)
.build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some())
.await?;
// Read the fields we need before moving `user` into FullUserDto below.
// Ordering matters: `can_edit_image` and the self-only bag fields
// must be captured while `user` is still borrowable; the
// `FullUserDto::build` call downstream consumes the entity.
let can_edit_image = !user.is_oidc_user();
let ui_preferences = user.ui_preferences().clone();
let notify_on_share = user.notify_on_share();
// Overlay the cached `force_password_change` flag (see UserFlags).
// Using the cached path (`get_user_flags` → `user_flags_cache`)
// avoids a second DB round-trip on this hot endpoint.
let force_password_change = auth_service
.auth_application_service
.get_user_flags(user_id)
.await
.map(|f| f.force_password_change)
.unwrap_or(false);
// Session-binding state — read from the JWT `cnf.jkt` claim
// (surfaced by the auth middleware into `CurrentUser.dpop_jkt`).
// Present ⇒ the session that minted this JWT was bound; absent ⇒
// the session is unbound and the SPA should call `/dpop/bind` to
// attach the browser's keypair (OIDC / magic-link redirect flow).
// Skips an otherwise-redundant `POST /dpop/bind` on every page load
// which would return 409 `already_bound` and litter the audit
// stream.
let is_dpop_bound = auth_user.dpop_jkt.is_some();
let full = FullUserDto::build(user, flags);
let self_dto = SelfUserDto::build(
full,
ui_preferences,
notify_on_share,
is_dpop_bound,
force_password_change,
can_edit_image,
);
Ok((StatusCode::OK, Json(self_dto)))
}
@@ -860,14 +821,16 @@ pub async fn change_password(
/// self-registration policy. Refused with 403
/// `error_type = "RegistrationDomainNotAllowed"`.
///
/// Response: the updated `PublicUserDto` (post-upgrade view — `is_external`
/// is false, `storage_quota_bytes` is set).
/// Response: the updated `SelfUserDto` (same shape as `GET /me`) so the SPA
/// absorbs the post-upgrade state — new `storage_quota_bytes`,
/// `is_external = false`, updated OPAQUE / auth capability flags — in one
/// round trip without a follow-up `/me` fetch.
#[utoipa::path(
post,
path = "/api/auth/upgrade-to-internal",
request_body = UpgradeToInternalDto,
responses(
(status = 200, description = "Upgrade succeeded", body = PublicUserDto),
(status = 200, description = "Upgrade succeeded — returns SelfUserDto (same shape as GET /me)", body = SelfUserDto),
(status = 400, description = "Password missing / too short"),
(status = 401, description = "Not authenticated"),
(status = 403, description = "OIDC user, or domain not in allowlist"),
@@ -878,9 +841,10 @@ pub async fn change_password(
)]
pub async fn upgrade_to_internal(
State(state): State<Arc<AppState>>,
CurrentUserId(user_id): CurrentUserId,
auth_user: AuthUser,
Json(dto): Json<UpgradeToInternalDto>,
) -> Result<impl IntoResponse, AppError> {
let user_id = auth_user.id;
let auth_service = state
.auth_service
.as_ref()
@@ -928,7 +892,11 @@ pub async fn upgrade_to_internal(
}
}
let updated = auth_service
// Apply the upgrade. Service returns the updated `PublicUserDto`;
// we discard it and rebuild the full self view via the shared
// `build_self_user_dto_for_id` helper so the wire shape matches
// `GET /me` and `PATCH /me/profile` byte-for-byte.
let _ = auth_service
.auth_application_service
.upgrade_to_internal(user_id, dto)
.await
@@ -947,7 +915,11 @@ pub async fn upgrade_to_internal(
_ => AppError::from(err),
})?;
Ok((StatusCode::OK, Json(updated)))
let self_dto = auth_service
.auth_application_service
.build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some())
.await?;
Ok((StatusCode::OK, Json(self_dto)))
}
/// Update the caller's profile (PR 24).
@@ -965,7 +937,7 @@ pub async fn upgrade_to_internal(
path = "/api/auth/me/profile",
request_body = crate::application::dtos::user_dto::UpdateProfileDto,
responses(
(status = 200, description = "Updated profile (PublicUserDto)", body = PublicUserDto),
(status = 200, description = "Updated profile (SelfUserDto) — same shape as GET /me so the SPA sees the just-written state without a follow-up fetch", body = SelfUserDto),
(status = 400, description = "Validation error (e.g. invalid handle format, empty given_name)"),
(status = 401, description = "Not authenticated"),
(status = 403, description = "OIDC-managed profile — edit at the IdP"),
@@ -976,20 +948,39 @@ pub async fn upgrade_to_internal(
)]
pub async fn update_profile(
State(state): State<Arc<AppState>>,
CurrentUserId(user_id): CurrentUserId,
auth_user: AuthUser,
Json(dto): Json<crate::application::dtos::user_dto::UpdateProfileDto>,
) -> Result<impl IntoResponse, AppError> {
let user_id = auth_user.id;
let auth_service = state
.auth_service
.as_ref()
.ok_or_else(|| AppError::internal_error("Authentication service not configured"))?;
let updated = auth_service
// Apply the patch. The service returns the updated `PublicUserDto`
// internally; we discard it and re-fetch the full self view below
// so the response matches `GET /me`'s `SelfUserDto` shape.
//
// Why SelfUserDto instead of PublicUserDto: a self-write endpoint
// whose response mirrors GET /me lets the SPA update its session
// store in one round trip. Returning a slim PublicUserDto would
// force the SPA to follow up with GET /me anyway to observe the
// just-written `ui_preferences` / `notify_on_share` / etc — those
// fields live on SelfUserDto only, not on the public identity
// slice. Same shape for both endpoints avoids "quiet lie" reads
// where a client PATCHes and then reads a stale local value.
let _ = auth_service
.auth_application_service
.update_profile_with_perms(user_id, dto, &state.locale_registry)
.await?;
Ok((StatusCode::OK, Json(updated)))
// Rebuild via the shared helper so the wire shape matches
// `GET /me` and `POST /upgrade-to-internal` byte-for-byte.
let self_dto = auth_service
.auth_application_service
.build_self_user_dto_for_id(user_id, auth_user.dpop_jkt.is_some())
.await?;
Ok((StatusCode::OK, Json(self_dto)))
}
// TODO: add utoipa