diff --git a/crates/manager-core/src/admin_user_repo.rs b/crates/manager-core/src/admin_user_repo.rs index dfa8b22..ec62b48 100644 --- a/crates/manager-core/src/admin_user_repo.rs +++ b/crates/manager-core/src/admin_user_repo.rs @@ -69,12 +69,14 @@ pub trait AdminUserRepository: Send + Sync { async fn list(&self) -> Result, AdminUserRepositoryError>; /// Create a new admin. `instance_role` defaults to `Owner` for the /// env-var bootstrap path; admin-creates-admin flows pass an - /// explicit role. + /// explicit role. `email` is optional — pass `None` to leave the + /// column NULL. async fn create( &self, username: &str, password_hash: &str, instance_role: InstanceRole, + email: Option<&str>, ) -> Result; async fn update_username( &self, @@ -86,6 +88,12 @@ pub trait AdminUserRepository: Send + Sync { id: AdminUserId, password_hash: &str, ) -> Result; + /// Set or clear the email address. `None` writes NULL to the column. + async fn update_email( + &self, + id: AdminUserId, + email: Option<&str>, + ) -> Result; /// Update the instance_role. Used by `PATCH /api/v1/admin/admins/{id}`; /// callers enforce the last-owner guard (`count_other_active_owners`) /// before invoking when role transitions away from `Owner`. @@ -192,24 +200,37 @@ impl AdminUserRepository for PostgresAdminUserRepository { username: &str, password_hash: &str, instance_role: InstanceRole, + email: Option<&str>, ) -> Result { let res = sqlx::query_as::<_, AdminUserRecord>( - "INSERT INTO admin_users (username, password_hash, instance_role) \ - VALUES ($1, $2, $3) \ + "INSERT INTO admin_users (username, password_hash, instance_role, email) \ + VALUES ($1, $2, $3, $4) \ RETURNING id, username, is_active, instance_role, email, \ created_at, updated_at, last_login_at", ) .bind(username) .bind(password_hash) .bind(instance_role.as_str()) + .bind(email) .fetch_one(&self.pool) .await; match res { Ok(row) => row.try_into(), - Err(sqlx::Error::Database(e)) if e.is_unique_violation() => Err( - AdminUserRepositoryError::DuplicateUsername(username.to_string()), - ), + Err(sqlx::Error::Database(e)) if e.is_unique_violation() => { + // username and email both have unique constraints; the + // create path can collide on either, so peek at the + // constraint name to surface the right error. + if e.constraint() == Some("admin_users_email_key") { + Err(AdminUserRepositoryError::DuplicateEmail( + email.unwrap_or("").to_string(), + )) + } else { + Err(AdminUserRepositoryError::DuplicateUsername( + username.to_string(), + )) + } + } Err(e) => Err(e.into()), } } @@ -259,6 +280,32 @@ impl AdminUserRepository for PostgresAdminUserRepository { .and_then(TryInto::try_into) } + async fn update_email( + &self, + id: AdminUserId, + email: Option<&str>, + ) -> Result { + let res = sqlx::query_as::<_, AdminUserRecord>( + "UPDATE admin_users SET email = $2, updated_at = NOW() \ + WHERE id = $1 \ + RETURNING id, username, is_active, instance_role, email, \ + created_at, updated_at, last_login_at", + ) + .bind(id.into_inner()) + .bind(email) + .fetch_optional(&self.pool) + .await; + + match res { + Ok(Some(row)) => row.try_into(), + Ok(None) => Err(AdminUserRepositoryError::NotFound(id)), + Err(sqlx::Error::Database(e)) if e.is_unique_violation() => Err( + AdminUserRepositoryError::DuplicateEmail(email.unwrap_or("").to_string()), + ), + Err(e) => Err(e.into()), + } + } + async fn update_instance_role( &self, id: AdminUserId, diff --git a/crates/manager-core/src/admin_users_api.rs b/crates/manager-core/src/admin_users_api.rs index bd9354b..d86bd2e 100644 --- a/crates/manager-core/src/admin_users_api.rs +++ b/crates/manager-core/src/admin_users_api.rs @@ -95,6 +95,9 @@ pub struct CreateAdminRequest { /// channel that defaults to `Owner`. #[serde(default = "default_create_role")] pub instance_role: InstanceRole, + /// Optional contact email. Blank/whitespace is normalized to None. + #[serde(default)] + pub email: Option, } const fn default_create_role() -> InstanceRole { @@ -107,6 +110,26 @@ pub struct PatchAdminRequest { pub password: Option, pub is_active: Option, pub instance_role: Option, + /// JSON Merge Patch (RFC 7396) semantics for email: + /// absent → don't change + /// null → clear (set DB column to NULL) + /// "" → set to that string + /// `Option>` is the idiomatic Rust shape for that + /// tri-state; the custom deserializer below distinguishes the + /// "missing" case from the "present-and-null" case that serde + /// would otherwise collapse together. + #[allow(clippy::option_option)] + #[serde(default, deserialize_with = "deserialize_present_optional")] + pub email: Option>, +} + +#[allow(clippy::option_option)] +fn deserialize_present_optional<'de, T, D>(deserializer: D) -> Result>, D::Error> +where + T: serde::Deserialize<'de>, + D: serde::Deserializer<'de>, +{ + Ok(Some(Option::::deserialize(deserializer)?)) } // ---------------------------------------------------------------------------- @@ -169,10 +192,11 @@ async fn create_admin( let username = input.username.trim(); validate_username(username)?; validate_password(&input.password)?; + let email = normalize_email(input.email.as_deref())?; let hash = hash_password(&input.password).map_err(|e| AdminApiError::Hash(e.to_string()))?; let row = state .users - .create(username, &hash, input.instance_role) + .create(username, &hash, input.instance_role, email.as_deref()) .await?; Ok((StatusCode::CREATED, Json(row.into()))) } @@ -216,6 +240,12 @@ async fn patch_admin( // for the initial cut.) } + if let Some(email_patch) = input.email.as_ref() { + // email_patch is Some(None) → clear, Some(Some(s)) → set. + let normalized = normalize_email(email_patch.as_deref())?; + latest = Some(state.users.update_email(id, normalized.as_deref()).await?); + } + if let Some(new_role) = input.instance_role { // Self-elevation guard: only an owner can promote anyone TO // owner. An admin cannot turn themselves (or anyone else) @@ -358,6 +388,26 @@ fn validate_password(s: &str) -> Result<(), AdminApiError> { Ok(()) } +/// Trim and reject empty / pathological emails, returning the +/// canonical form (or None when the input was blank). The shape +/// check is intentionally loose — we mainly want to reject blanks +/// and obvious junk; real verification is a future concern. +fn normalize_email(raw: Option<&str>) -> Result, AdminApiError> { + let Some(raw) = raw else { + return Ok(None); + }; + let trimmed = raw.trim(); + if trimmed.is_empty() { + return Ok(None); + } + if trimmed.len() > 254 || !trimmed.contains('@') { + return Err(AdminApiError::InvalidEmail( + "email must contain '@' and be at most 254 characters".to_string(), + )); + } + Ok(Some(trimmed.to_string())) +} + // ---------------------------------------------------------------------------- // Errors // ---------------------------------------------------------------------------- @@ -373,6 +423,9 @@ pub enum AdminApiError { #[error("{0}")] InvalidPassword(String), + #[error("{0}")] + InvalidEmail(String), + #[error("cannot leave the system with zero active admins")] LastActiveAdmin, @@ -414,6 +467,7 @@ impl IntoResponse for AdminApiError { ) => (StatusCode::CONFLICT, self.to_string()), Self::InvalidUsername(_) | Self::InvalidPassword(_) + | Self::InvalidEmail(_) | Self::LastActiveAdmin | Self::LastActiveOwner | Self::CannotEscalate diff --git a/crates/manager-core/src/auth_bootstrap.rs b/crates/manager-core/src/auth_bootstrap.rs index fd910c5..0fca45d 100644 --- a/crates/manager-core/src/auth_bootstrap.rs +++ b/crates/manager-core/src/auth_bootstrap.rs @@ -123,6 +123,7 @@ pub async fn bootstrap_first_admin_with( &username, &password_hash, picloud_shared::InstanceRole::Owner, + None, ) .await?; info!(username = %username, "bootstrapped initial admin user"); @@ -176,13 +177,14 @@ mod tests { username: &str, _password_hash: &str, instance_role: InstanceRole, + email: Option<&str>, ) -> Result { let row = AdminUserRow { id: AdminUserId::new(), username: username.to_string(), is_active: true, instance_role, - email: None, + email: email.map(str::to_string), created_at: Utc::now(), updated_at: Utc::now(), last_login_at: None, @@ -204,6 +206,13 @@ mod tests { ) -> Result { unimplemented!() } + async fn update_email( + &self, + _i: AdminUserId, + _e: Option<&str>, + ) -> Result { + unimplemented!() + } async fn update_instance_role( &self, _i: AdminUserId, @@ -272,7 +281,7 @@ mod tests { #[tokio::test] async fn populated_db_is_noop() { let repo = InMemoryRepo::default(); - repo.create("seeded", "x", InstanceRole::Owner) + repo.create("seeded", "x", InstanceRole::Owner, None) .await .unwrap(); let env = BootstrapEnv { diff --git a/crates/picloud/tests/api.rs b/crates/picloud/tests/api.rs index dea99ad..dfcdb2c 100644 --- a/crates/picloud/tests/api.rs +++ b/crates/picloud/tests/api.rs @@ -36,7 +36,7 @@ async fn server_with_app(pool: PgPool) -> (TestServer, String) { let auth = picloud::AuthDeps::from_pool(pool.clone()); let hash = hash_password("test-pw").expect("hash"); auth.users - .create("test-admin", &hash, InstanceRole::Owner) + .create("test-admin", &hash, InstanceRole::Owner, None) .await .expect("seed admin"); @@ -114,6 +114,47 @@ async fn auth_me_returns_principal_with_role_and_email(pool: PgPool) { assert!(body["id"].as_str().is_some()); } +#[ignore = "needs DATABASE_URL pointing at a running Postgres"] +#[sqlx::test(migrations = "../manager-core/migrations")] +async fn create_admin_accepts_email_and_patch_clears_it(pool: PgPool) { + let s = server(pool).await; + // Create with email set. + let created = s + .post("/api/v1/admin/admins") + .json(&json!({ + "username": "alice", + "password": "correct-horse-battery", + "instance_role": "member", + "email": "alice@example.com", + })) + .await; + created.assert_status(axum::http::StatusCode::CREATED); + let body: Value = created.json(); + let alice_id = body["id"].as_str().expect("id").to_string(); + assert_eq!(body["email"], "alice@example.com"); + + // Patch with email present-and-null clears it. + let cleared = s + .patch(&format!("/api/v1/admin/admins/{alice_id}")) + .json(&json!({ "email": null })) + .await; + cleared.assert_status_ok(); + assert!(cleared.json::()["email"].is_null()); + + // Patch with email omitted is a no-op (doesn't clobber a re-set). + let reset = s + .patch(&format!("/api/v1/admin/admins/{alice_id}")) + .json(&json!({ "email": "alice2@example.com" })) + .await; + reset.assert_status_ok(); + let omit = s + .patch(&format!("/api/v1/admin/admins/{alice_id}")) + .json(&json!({ "username": "alice" })) // no email key + .await; + omit.assert_status_ok(); + assert_eq!(omit.json::()["email"], "alice2@example.com"); +} + // ============================================================================ // Script CRUD // ============================================================================ diff --git a/crates/picloud/tests/authz.rs b/crates/picloud/tests/authz.rs index 1e44921..f62f74e 100644 --- a/crates/picloud/tests/authz.rs +++ b/crates/picloud/tests/authz.rs @@ -53,7 +53,7 @@ async fn boot(pool: PgPool) -> Seeded { let hash = hash_password("owner-pw").expect("hash"); let owner = auth .users - .create("owner", &hash, InstanceRole::Owner) + .create("owner", &hash, InstanceRole::Owner, None) .await .expect("seed owner"); @@ -119,7 +119,7 @@ async fn seed_user( ) -> AdminUserId { let repo = PostgresAdminUserRepository::new(pool.clone()); let hash = hash_password(password).expect("hash"); - repo.create(username, &hash, role) + repo.create(username, &hash, role, None) .await .expect("seed user") .id