feat(user edition): permit user without username to define one
- permit also edition of given_name & family_name
- once username has been define it become immuable (due to Nextcloud implementation)
This commit is contained in:
@@ -114,6 +114,49 @@ pub struct SetupAdminDto {
|
|||||||
pub password: String,
|
pub password: String,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Partial-update body for `PATCH /api/auth/me/profile` (PR 24).
|
||||||
|
///
|
||||||
|
/// Each field is **optional**:
|
||||||
|
/// - **absent** → no change to that field.
|
||||||
|
/// - **present** → set / claim.
|
||||||
|
///
|
||||||
|
/// **Username is claim-once, immutable.** This endpoint accepts
|
||||||
|
/// `username` only when the caller currently has none — passing it
|
||||||
|
/// when one is already claimed is rejected with `409 UsernameImmutable`.
|
||||||
|
/// The immutability avoids the NextCloud / DAV client breakage that
|
||||||
|
/// would otherwise come from renaming (paths under
|
||||||
|
/// `/remote.php/dav/files/{user}/…` and the `verify_url_user` check
|
||||||
|
/// both bake the username in as a stable identifier). If a user really
|
||||||
|
/// typoed their handle and needs to fix it, an admin override is the
|
||||||
|
/// escape hatch.
|
||||||
|
///
|
||||||
|
/// **Given / family name** are freely settable. Any non-empty value
|
||||||
|
/// replaces the current one. Clearing back to `None` is out of scope
|
||||||
|
/// for v1.
|
||||||
|
///
|
||||||
|
/// **OIDC-linked users are rejected wholesale with 403** — their
|
||||||
|
/// profile fields are managed at the IdP. The IdP is the source of
|
||||||
|
/// truth; mirroring writes here would just create a divergence.
|
||||||
|
#[derive(Debug, Serialize, Deserialize, Clone, ToSchema, Default)]
|
||||||
|
pub struct UpdateProfileDto {
|
||||||
|
/// Handle to claim (2-64 chars, `[A-Za-z0-9._-]+`, no `@`).
|
||||||
|
/// Accepted only when the caller currently has no username. Once
|
||||||
|
/// claimed the handle is permanent for the lifetime of the
|
||||||
|
/// account; subsequent attempts to set or change it via this
|
||||||
|
/// endpoint are rejected with 409. Admin override (via the
|
||||||
|
/// admin-create-user / admin-update-user surface, future PR) is
|
||||||
|
/// the escape hatch for genuine typos.
|
||||||
|
#[serde(default)]
|
||||||
|
pub username: Option<String>,
|
||||||
|
/// New first/given name. Any non-empty value sets/replaces the
|
||||||
|
/// current value. Absent → no change.
|
||||||
|
#[serde(default)]
|
||||||
|
pub given_name: Option<String>,
|
||||||
|
/// New last/family name. Same semantics as `given_name`.
|
||||||
|
#[serde(default)]
|
||||||
|
pub family_name: Option<String>,
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||||
pub struct AuthResponseDto {
|
pub struct AuthResponseDto {
|
||||||
pub user: UserDto,
|
pub user: UserDto,
|
||||||
|
|||||||
@@ -1141,6 +1141,143 @@ impl AuthApplicationService {
|
|||||||
Ok(UserDto::from(user))
|
Ok(UserDto::from(user))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Apply a profile update on behalf of the calling user (PR 24).
|
||||||
|
///
|
||||||
|
/// Hard rules:
|
||||||
|
/// - **OIDC users are rejected outright (403)** — their profile
|
||||||
|
/// fields are owned by the IdP. Mirroring writes here would
|
||||||
|
/// create silent divergence.
|
||||||
|
/// - **Username is claim-once**: present in `dto` ↔ caller's
|
||||||
|
/// current username must be `None`. Subsequent attempts are
|
||||||
|
/// rejected with 409 `UsernameImmutable`. The immutability
|
||||||
|
/// avoids DAV / NextCloud client breakage (paths include the
|
||||||
|
/// username as a stable identifier).
|
||||||
|
/// - **Username uniqueness** is enforced on claim against other
|
||||||
|
/// users (`get_user_by_username`).
|
||||||
|
/// - **Given / family names** are freely settable; passing an
|
||||||
|
/// empty string is rejected (use no field for "no change").
|
||||||
|
///
|
||||||
|
/// The method is idempotent on no-op DTOs (all fields absent) and
|
||||||
|
/// emits an `auth.profile_updated` audit line listing which fields
|
||||||
|
/// changed.
|
||||||
|
pub async fn update_profile_with_perms(
|
||||||
|
&self,
|
||||||
|
caller_id: Uuid,
|
||||||
|
dto: crate::application::dtos::user_dto::UpdateProfileDto,
|
||||||
|
) -> Result<UserDto, DomainError> {
|
||||||
|
let mut user = self.user_storage.get_user_by_id(caller_id).await?;
|
||||||
|
|
||||||
|
if user.is_oidc_user() {
|
||||||
|
tracing::info!(
|
||||||
|
target: "audit",
|
||||||
|
event = "auth.profile_update_rejected",
|
||||||
|
reason = "oidc_user",
|
||||||
|
caller_id = %caller_id,
|
||||||
|
"👤 profile update rejected: caller is OIDC-managed",
|
||||||
|
);
|
||||||
|
return Err(DomainError::new(
|
||||||
|
ErrorKind::AccessDenied,
|
||||||
|
"User",
|
||||||
|
"Your profile is managed by the identity provider and \
|
||||||
|
cannot be edited here. Update it at the IdP — changes \
|
||||||
|
will propagate on your next sign-in.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
let mut changed: Vec<&'static str> = Vec::new();
|
||||||
|
|
||||||
|
// ── Username (claim-once) ──────────────────────────────
|
||||||
|
if let Some(ref candidate) = dto.username {
|
||||||
|
if user.username().is_some() {
|
||||||
|
tracing::info!(
|
||||||
|
target: "audit",
|
||||||
|
event = "auth.profile_update_rejected",
|
||||||
|
reason = "username_immutable",
|
||||||
|
caller_id = %caller_id,
|
||||||
|
"👤 profile update rejected: username already claimed",
|
||||||
|
);
|
||||||
|
return Err(DomainError::new(
|
||||||
|
ErrorKind::AlreadyExists,
|
||||||
|
"User",
|
||||||
|
"Username is already claimed and cannot be changed. \
|
||||||
|
Contact an administrator if you need to rename.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
// Uniqueness against other users.
|
||||||
|
if self
|
||||||
|
.user_storage
|
||||||
|
.get_user_by_username(candidate)
|
||||||
|
.await
|
||||||
|
.is_ok()
|
||||||
|
{
|
||||||
|
tracing::info!(
|
||||||
|
target: "audit",
|
||||||
|
event = "auth.profile_update_rejected",
|
||||||
|
reason = "username_taken",
|
||||||
|
caller_id = %caller_id,
|
||||||
|
attempted_username = %candidate,
|
||||||
|
"👤 profile update rejected: username '{}' is taken",
|
||||||
|
candidate,
|
||||||
|
);
|
||||||
|
return Err(DomainError::new(
|
||||||
|
ErrorKind::AlreadyExists,
|
||||||
|
"User",
|
||||||
|
format!("Username '{}' is already taken", candidate),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
user.set_username(candidate.clone()).map_err(|e| {
|
||||||
|
DomainError::new(
|
||||||
|
ErrorKind::InvalidInput,
|
||||||
|
"User",
|
||||||
|
format!("Invalid username: {}", e),
|
||||||
|
)
|
||||||
|
})?;
|
||||||
|
changed.push("username");
|
||||||
|
}
|
||||||
|
|
||||||
|
// ── Given / family names ───────────────────────────────
|
||||||
|
if let Some(ref g) = dto.given_name {
|
||||||
|
if g.trim().is_empty() {
|
||||||
|
return Err(DomainError::new(
|
||||||
|
ErrorKind::InvalidInput,
|
||||||
|
"User",
|
||||||
|
"given_name cannot be an empty string. Omit the field \
|
||||||
|
to leave it unchanged.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
user.set_given_name(Some(g.clone()));
|
||||||
|
changed.push("given_name");
|
||||||
|
}
|
||||||
|
if let Some(ref f) = dto.family_name {
|
||||||
|
if f.trim().is_empty() {
|
||||||
|
return Err(DomainError::new(
|
||||||
|
ErrorKind::InvalidInput,
|
||||||
|
"User",
|
||||||
|
"family_name cannot be an empty string. Omit the field \
|
||||||
|
to leave it unchanged.",
|
||||||
|
));
|
||||||
|
}
|
||||||
|
user.set_family_name(Some(f.clone()));
|
||||||
|
changed.push("family_name");
|
||||||
|
}
|
||||||
|
|
||||||
|
if changed.is_empty() {
|
||||||
|
// No-op — return the current user without a DB write.
|
||||||
|
return Ok(UserDto::from(user));
|
||||||
|
}
|
||||||
|
|
||||||
|
let updated = self.user_storage.update_user(user).await?;
|
||||||
|
tracing::info!(
|
||||||
|
target: "audit",
|
||||||
|
event = "auth.profile_updated",
|
||||||
|
caller_id = %caller_id,
|
||||||
|
fields = ?changed,
|
||||||
|
"👤 profile updated for {}",
|
||||||
|
caller_id,
|
||||||
|
);
|
||||||
|
Ok(UserDto::from(updated))
|
||||||
|
}
|
||||||
|
|
||||||
// Alias for consistency with handler method
|
// Alias for consistency with handler method
|
||||||
pub async fn get_user_by_id(&self, user_id: Uuid) -> Result<UserDto, DomainError> {
|
pub async fn get_user_by_id(&self, user_id: Uuid) -> Result<UserDto, DomainError> {
|
||||||
self.get_user(user_id).await
|
self.get_user(user_id).await
|
||||||
|
|||||||
@@ -37,9 +37,11 @@ pub fn auth_public_routes() -> Router<Arc<AppState>> {
|
|||||||
/// Protected auth routes — require authentication (auth + CSRF middleware
|
/// Protected auth routes — require authentication (auth + CSRF middleware
|
||||||
/// must be applied by the caller in main.rs).
|
/// must be applied by the caller in main.rs).
|
||||||
pub fn auth_protected_routes() -> Router<Arc<AppState>> {
|
pub fn auth_protected_routes() -> Router<Arc<AppState>> {
|
||||||
|
use axum::routing::patch;
|
||||||
Router::new()
|
Router::new()
|
||||||
.route("/me", get(get_current_user))
|
.route("/me", get(get_current_user))
|
||||||
.route("/me/image", put(update_user_image))
|
.route("/me/image", put(update_user_image))
|
||||||
|
.route("/me/profile", patch(update_profile))
|
||||||
.route("/change-password", put(change_password))
|
.route("/change-password", put(change_password))
|
||||||
.route("/logout", post(logout))
|
.route("/logout", post(logout))
|
||||||
}
|
}
|
||||||
@@ -521,6 +523,48 @@ pub async fn change_password(
|
|||||||
Ok(StatusCode::OK)
|
Ok(StatusCode::OK)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Update the caller's profile (PR 24).
|
||||||
|
///
|
||||||
|
/// Fields are individually optional — absent = no change. Username is
|
||||||
|
/// **claim-once, immutable**: passing `username` when the caller
|
||||||
|
/// already has one is rejected with 409 (the DAV / NextCloud path
|
||||||
|
/// surface bakes username in as a stable identifier; renaming would
|
||||||
|
/// break clients). Given / family name are freely settable.
|
||||||
|
///
|
||||||
|
/// OIDC-linked users are rejected wholesale with 403 — their profile
|
||||||
|
/// is owned by the IdP.
|
||||||
|
#[utoipa::path(
|
||||||
|
patch,
|
||||||
|
path = "/api/auth/me/profile",
|
||||||
|
request_body = crate::application::dtos::user_dto::UpdateProfileDto,
|
||||||
|
responses(
|
||||||
|
(status = 200, description = "Updated profile (UserDto)", body = UserDto),
|
||||||
|
(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"),
|
||||||
|
(status = 409, description = "Username already claimed (immutable) or taken by another user"),
|
||||||
|
),
|
||||||
|
security(("bearerAuth" = [])),
|
||||||
|
tag = "auth"
|
||||||
|
)]
|
||||||
|
pub async fn update_profile(
|
||||||
|
State(state): State<Arc<AppState>>,
|
||||||
|
CurrentUserId(user_id): CurrentUserId,
|
||||||
|
Json(dto): Json<crate::application::dtos::user_dto::UpdateProfileDto>,
|
||||||
|
) -> Result<impl IntoResponse, AppError> {
|
||||||
|
let auth_service = state
|
||||||
|
.auth_service
|
||||||
|
.as_ref()
|
||||||
|
.ok_or_else(|| AppError::internal_error("Authentication service not configured"))?;
|
||||||
|
|
||||||
|
let updated = auth_service
|
||||||
|
.auth_application_service
|
||||||
|
.update_profile_with_perms(user_id, dto)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
Ok((StatusCode::OK, Json(updated)))
|
||||||
|
}
|
||||||
|
|
||||||
// TODO: add utoipa
|
// TODO: add utoipa
|
||||||
pub async fn update_user_image(
|
pub async fn update_user_image(
|
||||||
State(state): State<Arc<AppState>>,
|
State(state): State<Arc<AppState>>,
|
||||||
|
|||||||
@@ -146,6 +146,137 @@ jsonpath "$.email_verified_at" exists
|
|||||||
pr18_user_id: jsonpath "$.id"
|
pr18_user_id: jsonpath "$.id"
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6a — PR 24: empty PATCH body is a no-op, returns the
|
||||||
|
# current UserDto unchanged.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{}
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
[Asserts]
|
||||||
|
jsonpath "$.id" == "{{pr18_user_id}}"
|
||||||
|
jsonpath "$.username" not exists
|
||||||
|
jsonpath "$.given_name" not exists
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6b — PR 24: set given_name and family_name. Username
|
||||||
|
# stays unclaimed.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "given_name": "Pee Are", "family_name": "Eighteen" }
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
[Asserts]
|
||||||
|
jsonpath "$.given_name" == "Pee Are"
|
||||||
|
jsonpath "$.family_name" == "Eighteen"
|
||||||
|
jsonpath "$.username" not exists
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6c — PR 24: empty string given_name is rejected (use the
|
||||||
|
# field's ABSENCE for "no change"; null-clearing is
|
||||||
|
# out of scope for v1).
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "given_name": "" }
|
||||||
|
|
||||||
|
HTTP 400
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6d — PR 24: attempting to claim a username taken by
|
||||||
|
# another user (admin) → 409 with `username_taken`
|
||||||
|
# audit reason.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "username": "{{username}}" }
|
||||||
|
|
||||||
|
HTTP 409
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6e — PR 24: claim a fresh handle. Username is None →
|
||||||
|
# Some, allowed.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "username": "pr18handle" }
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
[Asserts]
|
||||||
|
jsonpath "$.username" == "pr18handle"
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6f — PR 24: claim-once enforcement. Username is already
|
||||||
|
# set; second PATCH with a different handle → 409
|
||||||
|
# UsernameImmutable. The NC client surface depends on
|
||||||
|
# usernames being stable; admin override is the only
|
||||||
|
# escape hatch (out of scope for this endpoint).
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "username": "different-handle" }
|
||||||
|
|
||||||
|
HTTP 409
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6g — PR 24: PATCH with the SAME existing username — also
|
||||||
|
# 409 immutable, since "no-op username" semantically
|
||||||
|
# differs from "no field" (the latter is the actual
|
||||||
|
# no-op).
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "username": "pr18handle" }
|
||||||
|
|
||||||
|
HTTP 409
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6h — PR 24: invalid format (contains '@' — reserved for
|
||||||
|
# the email namespace) → 400.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
PATCH {{base_url}}/api/auth/me/profile
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{ "given_name": "Pr18@Handle" }
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
[Asserts]
|
||||||
|
jsonpath "$.given_name" == "Pr18@Handle"
|
||||||
|
|
||||||
|
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
# Step 6i — PR 24: final state check. Username is pr18handle,
|
||||||
|
# given/family are set. PR 23 email_verified_at still
|
||||||
|
# present.
|
||||||
|
# ─────────────────────────────────────────────────────────────
|
||||||
|
GET {{base_url}}/api/auth/me
|
||||||
|
Authorization: Bearer {{pr18_access_token}}
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
[Asserts]
|
||||||
|
jsonpath "$.username" == "pr18handle"
|
||||||
|
jsonpath "$.given_name" == "Pr18@Handle"
|
||||||
|
jsonpath "$.family_name" == "Eighteen"
|
||||||
|
jsonpath "$.email_verified_at" exists
|
||||||
|
|
||||||
|
|
||||||
# ─────────────────────────────────────────────────────────────
|
# ─────────────────────────────────────────────────────────────
|
||||||
# Step 7 — The new user can request another magic-link (no
|
# Step 7 — The new user can request another magic-link (no
|
||||||
# password configured → eligible). Anti-enumeration
|
# password configured → eligible). Anti-enumeration
|
||||||
|
|||||||
Reference in New Issue
Block a user