From 2030f4b69250ea1f740f3341ff33bbc4371a1187 Mon Sep 17 00:00:00 2001 From: kipavy Date: Wed, 14 Oct 2026 14:48:30 +0000 Subject: [PATCH] feat(auth): compare-and-swap on wrapped user secrets uploads A device repairing its stored team identity must not overwrite a password change made on another device. The upload now names the value it replaces and the server answers 409 when the row holds something else. --- src/routes/auth.rs | 68 ++++++++++++++++++++++++++++++++++++++++++---- src/routes/meta.rs | 9 ++++++ 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/routes/auth.rs b/src/routes/auth.rs index 1c25ed0..40f38c4 100644 --- a/src/routes/auth.rs +++ b/src/routes/auth.rs @@ -974,11 +974,13 @@ pub async fn logout_all( Ok(Json(session)) } -// ─── Upload wrapped user secrets (migration) ────────────────────────────────── +// ─── Upload wrapped user secrets ────────────────────────────────── #[derive(Deserialize)] pub struct UploadWrappedSecretsRequest { pub wrapped_user_secrets: String, + #[serde(default)] + pub expected_wrapped_user_secrets: Option, } pub async fn upload_wrapped_user_secrets( @@ -986,19 +988,28 @@ pub async fn upload_wrapped_user_secrets( axum::Extension(auth): axum::Extension, Json(body): Json, ) -> Result { - sqlx::query( - "UPDATE users SET wrapped_user_secrets = $1, updated_at = now() WHERE id = $2", + // No expectation means NULL: the legacy migration, the only caller that sends none, runs only then. + let updated = sqlx::query( + "UPDATE users SET wrapped_user_secrets = $1, updated_at = now() + WHERE id = $2 AND wrapped_user_secrets IS NOT DISTINCT FROM $3", ) .bind(&body.wrapped_user_secrets) .bind(auth.0) + .bind(&body.expected_wrapped_user_secrets) .execute(&pool) .await .map_err(|e| { error!(error = %e, user_id = %auth.0, "Failed to upload wrapped_user_secrets"); StatusCode::INTERNAL_SERVER_ERROR - })?; + })? + .rows_affected(); - info!(user_id = %auth.0, "Uploaded wrapped_user_secrets (migration)"); + if updated == 0 { + warn!(user_id = %auth.0, "Refused a wrapped_user_secrets upload that expected another value"); + return Err(StatusCode::CONFLICT); + } + + info!(user_id = %auth.0, "Uploaded wrapped_user_secrets"); Ok(StatusCode::NO_CONTENT) } @@ -1842,4 +1853,51 @@ mod handler_tests { verify(&pool, false, off_token).await; assert_eq!(handle_row(&pool, off_id).await, (generated, false)); } + + async fn stored_wrapped(pool: &PgPool, id: Uuid) -> Option { + sqlx::query_scalar("SELECT wrapped_user_secrets FROM users WHERE id = $1") + .bind(id) + .fetch_one(pool) + .await + .unwrap() + } + + async fn upload_wrapped(pool: &PgPool, id: Uuid, next: &str, expected: Option<&str>) -> Result { + upload_wrapped_user_secrets( + State(pool.clone()), + Extension(AuthUser(id)), + Json(UploadWrappedSecretsRequest { + wrapped_user_secrets: next.to_string(), + expected_wrapped_user_secrets: expected.map(str::to_string), + }), + ) + .await + } + + #[tokio::test] + async fn wrapped_secrets_first_upload_needs_no_expectation() { + let pool = test_pool_or_skip!(); + let id = crate::test_support::seed_user(&pool).await; + assert_eq!(upload_wrapped(&pool, id, "W1", None).await, Ok(StatusCode::NO_CONTENT)); + assert_eq!(stored_wrapped(&pool, id).await.as_deref(), Some("W1")); + } + + #[tokio::test] + async fn wrapped_secrets_replace_only_the_value_they_expect() { + let pool = test_pool_or_skip!(); + let id = crate::test_support::seed_user(&pool).await; + upload_wrapped(&pool, id, "W1", None).await.unwrap(); + assert_eq!(upload_wrapped(&pool, id, "W2", Some("W1")).await, Ok(StatusCode::NO_CONTENT)); + assert_eq!(upload_wrapped(&pool, id, "W3", Some("W1")).await, Err(StatusCode::CONFLICT)); + assert_eq!(stored_wrapped(&pool, id).await.as_deref(), Some("W2")); + } + + #[tokio::test] + async fn wrapped_secrets_are_never_overwritten_without_an_expectation() { + let pool = test_pool_or_skip!(); + let id = crate::test_support::seed_user(&pool).await; + upload_wrapped(&pool, id, "W1", None).await.unwrap(); + assert_eq!(upload_wrapped(&pool, id, "W2", None).await, Err(StatusCode::CONFLICT)); + assert_eq!(stored_wrapped(&pool, id).await.as_deref(), Some("W1")); + } } diff --git a/src/routes/meta.rs b/src/routes/meta.rs index 1e6f71b..d560e02 100644 --- a/src/routes/meta.rs +++ b/src/routes/meta.rs @@ -23,6 +23,8 @@ pub struct MetaResponse { pub two_factor: bool, /// Whether a passkey can be a second factor here: needs a relying party and the portal. pub passkeys: bool, + /// `PUT /v1/auth/wrapped-user-secrets` honours `expected_wrapped_user_secrets`; absent on servers that predate it. + pub wrapped_secrets_cas: bool, } pub async fn get_meta( @@ -41,6 +43,7 @@ pub async fn get_meta( team_secret_types: team_objects::secret_types(), two_factor: cipher.available(), passkeys: passkeys.available(), + wrapped_secrets_cas: true, }) } @@ -86,4 +89,10 @@ mod tests { get_meta(Extension(Features::open()), Extension(SeedCipher::absent()), Extension(Passkeys::for_tests())).await; assert_eq!(serde_json::to_value(&meta).unwrap()["passkeys"], serde_json::json!(true)); } + + #[tokio::test] + async fn meta_advertises_wrapped_secrets_cas() { + let Json(meta) = get_meta(Extension(Features::open()), Extension(SeedCipher::for_tests()), Extension(Passkeys::for_tests())).await; + assert_eq!(serde_json::to_value(&meta).unwrap()["wrapped_secrets_cas"], serde_json::json!(true)); + } }