diff --git a/migrations/036_drop_user_display_name.sql b/migrations/036_drop_user_display_name.sql new file mode 100644 index 0000000..fefeffe --- /dev/null +++ b/migrations/036_drop_user_display_name.sql @@ -0,0 +1,8 @@ +-- users.handle (035) is the sole human-facing identifier. display_name was a +-- second name for the same person and a weaker one: it defaulted to +-- split_part(email, '@', 1), so it leaked the email local part by construction. +-- +-- DEPLOY WARNING: irreversible, no down-migration. The old server still +-- reads/writes display_name, so it cannot keep serving once this lands — +-- no rolling restart across this migration. +ALTER TABLE users DROP COLUMN IF EXISTS display_name; diff --git a/src/main.rs b/src/main.rs index 4a859d9..4924a81 100644 --- a/src/main.rs +++ b/src/main.rs @@ -275,7 +275,6 @@ async fn main() { let protected = Router::new() .route("/v1/auth/account", delete(routes::auth::delete_account)) .route("/v1/auth/me", get(routes::auth::get_me)) - .route("/v1/auth/display-name", put(routes::auth::update_display_name)) .route("/v1/auth/email", put(routes::auth::update_email)) .route("/v1/auth/password", put(routes::auth::update_password)) .route( diff --git a/src/models/team.rs b/src/models/team.rs index f6304b4..75c2ac4 100644 --- a/src/models/team.rs +++ b/src/models/team.rs @@ -27,9 +27,12 @@ pub struct TeamRole { pub struct TeamMember { pub team_id: Uuid, pub user_id: Uuid, + /// ALIAS for pre-0.26 clients. Value is the handle; there is no stored + /// `display_name`. Delete this field in 0.27, and never repopulate it. pub display_name: String, pub handle: String, pub public_key: String, + /// The inviter's handle. The field name is the alias, the value is not. pub invited_by_display_name: Option, pub joined_at: DateTime, pub role_ids: Vec, diff --git a/src/routes/admin.rs b/src/routes/admin.rs index 6c7d9a7..bfd3b4b 100644 --- a/src/routes/admin.rs +++ b/src/routes/admin.rs @@ -1803,8 +1803,8 @@ mod admin_handler_tests { .await .expect("generate handle"); sqlx::query( - "INSERT INTO users (id, email, account_id, auth_hash, public_key, display_name, last_seen_on, handle) - VALUES ($1, $2, $3, 'test-hash', 'test-pubkey', 'Test User', + "INSERT INTO users (id, email, account_id, auth_hash, public_key, last_seen_on, handle) + VALUES ($1, $2, $3, 'test-hash', 'test-pubkey', CASE WHEN $4::int IS NULL THEN NULL ELSE current_date - $4::int END, $5)", ) .bind(id) diff --git a/src/routes/audit.rs b/src/routes/audit.rs index 1f5b78b..d3a477f 100644 --- a/src/routes/audit.rs +++ b/src/routes/audit.rs @@ -159,7 +159,7 @@ pub async fn list_audit_logs( let logs = sqlx::query_as::<_, AuditLogRow>( r#"SELECT al.id, al.team_id, al.vault_id, al.actor_id, - u.display_name AS actor_name, + u.handle AS actor_name, al.action, al.source, al.target_type, al.target_id, al.target_name, al.metadata, al.ip_address::text AS ip_address, al.created_at FROM audit_logs al @@ -219,7 +219,7 @@ pub async fn export_audit_logs( let logs = sqlx::query_as::<_, AuditLogRow>( r#"SELECT al.id, al.team_id, al.vault_id, al.actor_id, - u.display_name AS actor_name, + u.handle AS actor_name, al.action, al.source, al.target_type, al.target_id, al.target_name, al.metadata, al.ip_address::text AS ip_address, al.created_at FROM audit_logs al diff --git a/src/routes/auth.rs b/src/routes/auth.rs index f60b9de..e71699d 100644 --- a/src/routes/auth.rs +++ b/src/routes/auth.rs @@ -184,8 +184,8 @@ pub async fn register( })?; let row = sqlx::query_as::<_, (Uuid,)>( - "INSERT INTO users (email, display_name, account_id, auth_hash, public_key, wrapped_user_secrets, subscription_tier, trial_ends_at, handle) - VALUES ($1, split_part($1, '@', 1), $2, $3, $4, $5, $6, $7, $8) RETURNING id", + "INSERT INTO users (email, account_id, auth_hash, public_key, wrapped_user_secrets, subscription_tier, trial_ends_at, handle) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id", ) .bind(&email) .bind(body.account_id) @@ -574,6 +574,7 @@ pub async fn resend_verification_email( #[derive(Serialize)] pub struct MeResponse { pub email: String, + /// ALIAS for pre-0.26 clients. Value is the handle. Delete in 0.27. pub display_name: String, pub account_id: Uuid, pub tier: String, @@ -585,24 +586,21 @@ pub struct MeResponse { pub allow_stranger_invites: bool, } -pub async fn get_me( - State(pool): State, - axum::Extension(auth): axum::Extension, -) -> Result, StatusCode> { +pub(crate) async fn fetch_me_inner(pool: &PgPool, user_id: Uuid) -> Result { let row = sqlx::query_as::<_, (String, String, Uuid, Option, String, bool, bool)>( - "SELECT email, display_name, account_id, wrapped_user_secrets, handle, handle_is_custom, allow_stranger_invites FROM users WHERE id = $1", + "SELECT email, handle AS display_name, account_id, wrapped_user_secrets, handle, handle_is_custom, allow_stranger_invites FROM users WHERE id = $1", ) - .bind(auth.0) - .fetch_one(&pool) + .bind(user_id) + .fetch_one(pool) .await .map_err(|e| { - error!(error = %e, user_id = %auth.0, "Failed to fetch user in get_me"); + error!(error = %e, user_id = %user_id, "Failed to fetch user in get_me"); StatusCode::INTERNAL_SERVER_ERROR })?; - let tier = fetch_tier(&pool, auth.0).await?; + let tier = fetch_tier(pool, user_id).await?; - Ok(Json(MeResponse { + Ok(MeResponse { email: row.0, display_name: row.1, account_id: row.2, @@ -613,38 +611,14 @@ pub async fn get_me( handle: row.4, handle_is_custom: row.5, allow_stranger_invites: row.6, - })) -} - -// ─── Update display name ────────────────────────────────────────────────────── - -#[derive(Deserialize)] -pub struct UpdateDisplayNameRequest { - pub display_name: String, + }) } -pub async fn update_display_name( +pub async fn get_me( State(pool): State, axum::Extension(auth): axum::Extension, - Json(body): Json, -) -> Result { - let display_name = body.display_name.trim().to_string(); - if display_name.is_empty() || display_name.len() > 50 { - return Err(StatusCode::UNPROCESSABLE_ENTITY); - } - - sqlx::query("UPDATE users SET display_name = $1 WHERE id = $2") - .bind(&display_name) - .bind(auth.0) - .execute(&pool) - .await - .map_err(|e| { - error!(error = %e, user_id = %auth.0, "Failed to update display name"); - StatusCode::INTERNAL_SERVER_ERROR - })?; - - info!(user_id = %auth.0, display_name = %display_name, "Display name updated"); - Ok(StatusCode::NO_CONTENT) +) -> Result, StatusCode> { + Ok(Json(fetch_me_inner(&pool, auth.0).await?)) } // ─── Update email ───────────────────────────────────────────────────────────── @@ -1357,4 +1331,22 @@ mod handler_tests { Ok(_) => panic!("expected NOT_FOUND for soft-deleted account"), } } + + #[tokio::test] + async fn me_reports_the_handle_under_both_names() { + let pool = test_pool_or_skip!(); + let user = crate::test_support::seed_user(&pool).await; + let handle = crate::test_support::unique_handle("merry-quartz"); + sqlx::query("UPDATE users SET handle = $1 WHERE id = $2") + .bind(&handle) + .bind(user) + .execute(&pool) + .await + .unwrap(); + + let me = fetch_me_inner(&pool, user).await.unwrap(); + assert_eq!(me.handle, handle); + // ALIAS for pre-0.26 clients. + assert_eq!(me.display_name, handle); + } } diff --git a/src/routes/invitations.rs b/src/routes/invitations.rs index 14b492b..2bfc18a 100644 --- a/src/routes/invitations.rs +++ b/src/routes/invitations.rs @@ -25,7 +25,7 @@ pub async fn get_invitation( axum::extract::Path(token): axum::extract::Path, ) -> Result, StatusCode> { let row = sqlx::query_as::<_, (String, Option, String, chrono::DateTime)>( - r#"SELECT t.name, u.display_name, pi.role, pi.expires_at + r#"SELECT t.name, u.handle, pi.role, pi.expires_at FROM pending_invitations pi JOIN teams t ON t.id = pi.team_id LEFT JOIN users u ON u.id = pi.invited_by @@ -182,7 +182,7 @@ pub async fn list_my_pending_invitations( axum::Extension(auth): axum::Extension, ) -> Result>, StatusCode> { let rows = sqlx::query_as::<_, (Uuid, Uuid, String, Option, String, chrono::DateTime, chrono::DateTime)>( - r#"SELECT pi.id, pi.team_id, t.name, u.display_name, pi.role, pi.created_at, pi.expires_at + r#"SELECT pi.id, pi.team_id, t.name, u.handle, pi.role, pi.created_at, pi.expires_at FROM pending_invitations pi JOIN teams t ON t.id = pi.team_id LEFT JOIN users u ON u.id = pi.invited_by diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 26d69fe..81cac4b 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -262,8 +262,8 @@ pub async fn list_members( ), >( r#" - SELECT tm.team_id, tm.user_id, inv.display_name AS invited_by_display_name, tm.joined_at, - u.display_name, u.handle, u.public_key, tmr.role_id + SELECT tm.team_id, tm.user_id, inv.handle AS invited_by_display_name, tm.joined_at, + u.handle AS display_name, u.handle, u.public_key, tmr.role_id FROM team_members tm JOIN users u ON u.id = tm.user_id LEFT JOIN users inv ON inv.id = tm.invited_by @@ -412,7 +412,7 @@ pub async fn add_member( } let (invitee_email, invitee_display_name) = sqlx::query_as::<_, (String, String)>( - "SELECT email, display_name FROM users WHERE id = $1", + "SELECT email, handle FROM users WHERE id = $1", ) .bind(invitee_id) .fetch_one(&pool) @@ -526,7 +526,7 @@ pub async fn remove_member( error!(error = %e, team_id = %team_id, user_id = %user_id, "Failed to revoke session invitee grants"); } - let removed_display_name = sqlx::query_scalar::<_, String>("SELECT display_name FROM users WHERE id = $1") + let removed_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(user_id) .fetch_optional(&pool) .await @@ -620,12 +620,13 @@ pub(crate) const TEAMMATE_PAIR_SQL: &str = "EXISTS (SELECT 1 FROM team_members a #[derive(Serialize, sqlx::FromRow)] pub struct UserSearchResult { pub user_id: Uuid, + /// ALIAS for pre-0.26 clients. Value is the handle. Delete in 0.27. pub display_name: String, pub handle: String, pub is_teammate: bool, } -/// Resolution rules (D2): teammates fuzzy on name and email; anyone with a +/// Resolution rules (D2): teammates fuzzy on email; anyone with a /// *custom* handle fuzzy on that handle; everyone else on a full email address /// or an exact handle. Email substring matching is gone — it was an enumeration /// oracle, and rate-limiting it would only have slowed the harvest down. @@ -645,19 +646,19 @@ pub(crate) async fn search_users_inner( let sql = format!( r#" - SELECT u.id AS user_id, u.display_name, u.handle, {pair} AS is_teammate + SELECT u.id AS user_id, u.handle AS display_name, u.handle, {pair} AS is_teammate FROM users u WHERE u.id <> $2 AND u.deleted_at IS NULL AND ( - ({pair} AND (LOWER(u.display_name) LIKE $1 OR LOWER(u.email) LIKE $1)) + ({pair} AND LOWER(u.email) LIKE $1) OR (u.handle_is_custom AND LOWER(u.handle) LIKE $1) OR LOWER(u.email) = $3 OR LOWER(u.handle) = $4 ) ORDER BY is_teammate DESC, - CASE WHEN LOWER(u.display_name) LIKE $5 OR LOWER(u.handle) LIKE $5 THEN 0 ELSE 1 END, - u.display_name + CASE WHEN LOWER(u.handle) LIKE $5 THEN 0 ELSE 1 END, + u.handle LIMIT 8 "#, pair = TEAMMATE_PAIR_SQL, @@ -1045,7 +1046,7 @@ pub async fn assign_member_role( return Err(StatusCode::NOT_FOUND); } - let target_display_name = sqlx::query_scalar::<_, String>("SELECT display_name FROM users WHERE id = $1") + let target_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(target_user_id) .fetch_optional(&pool) .await @@ -1134,7 +1135,7 @@ pub async fn remove_member_role( return Err(StatusCode::NOT_FOUND); } - let target_display_name = sqlx::query_scalar::<_, String>("SELECT display_name FROM users WHERE id = $1") + let target_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(target_user_id) .fetch_optional(&pool) .await @@ -1268,7 +1269,7 @@ pub async fn invite_member( .map_err(|e| { error!(error = %e, "Failed to create pending invitation for existing user"); StatusCode::INTERNAL_SERVER_ERROR })?; info!(team_id = %team_id, user_id = %user_id, role = %role, "Pending invitation created for existing user via invite endpoint"); - let invite_display_name = sqlx::query_scalar::<_, String>("SELECT display_name FROM users WHERE id = $1") + let invite_display_name = sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") .bind(user_id) .fetch_optional(&pool) .await @@ -1347,6 +1348,9 @@ pub async fn invite_member( #[derive(Serialize)] pub struct PendingInvitation { pub id: Uuid, + /// NOT an alias — the deliberate exception. Populated by + /// `COALESCE(invitee.handle, pi.email)`: an invitee with no account yet has + /// no handle, so the server has nothing else to send here. pub display_name: String, pub role: String, pub invited_by_display_name: Option, @@ -1373,7 +1377,7 @@ pub async fn list_pending_invitations( } let rows = sqlx::query_as::<_, (Uuid, String, String, Option, chrono::DateTime, chrono::DateTime)>( - r#"SELECT pi.id, COALESCE(invitee.display_name, pi.email), pi.role, inv.display_name, pi.created_at, pi.expires_at + r#"SELECT pi.id, COALESCE(invitee.handle, pi.email), pi.role, inv.handle, pi.created_at, pi.expires_at FROM pending_invitations pi LEFT JOIN users inv ON inv.id = pi.invited_by LEFT JOIN users invitee ON invitee.id = pi.user_id @@ -1445,7 +1449,7 @@ mod authz_tests { use crate::test_pool_or_skip; use crate::test_support::{ add_member as add_team_member, env_lock, member_with_role, seed_role, seed_team, - seed_user, set_user_seats, set_user_tier, set_user_trial, + seed_user, set_user_seats, set_user_tier, set_user_trial, unique_handle, }; use axum::extract::{Path, State}; use axum::{Extension, Json}; @@ -1646,6 +1650,66 @@ mod authz_tests { assert_eq!(handle_of(other), other_handle); } + #[tokio::test] + async fn a_roster_row_carries_the_handle_in_both_fields() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_team_member(&pool, team, owner).await; + + let handle = unique_handle("merry-quartz"); + sqlx::query("UPDATE users SET handle = $1 WHERE id = $2") + .bind(&handle) + .bind(owner) + .execute(&pool) + .await + .unwrap(); + + let presence: PresenceMap = std::sync::Arc::new(dashmap::DashMap::new()); + let members = list_members( + State(pool.clone()), + Extension(AuthUser(owner)), + Extension(presence), + Path(team), + ) + .await + .expect("list members") + .0; + + let me = members.iter().find(|m| m.member.user_id == owner).unwrap(); + assert_eq!(me.member.handle, handle); + // ALIAS for pre-0.26 clients. + assert_eq!(me.member.display_name, handle); + } + + #[tokio::test] + async fn a_pending_invitation_to_an_unregistered_email_still_shows_the_email() { + let pool = test_pool_or_skip!(); + let owner = seed_user(&pool).await; + let team = seed_team(&pool, owner).await; + add_team_member(&pool, team, owner).await; + + sqlx::query( + "INSERT INTO pending_invitations (team_id, email, role, invited_by, expires_at) + VALUES ($1, $2, 'member', $3, now() + interval '7 days')", + ) + .bind(team) + .bind("nobody@example.com") + .bind(owner) + .execute(&pool) + .await + .unwrap(); + + let pending = list_pending_invitations(State(pool.clone()), Extension(AuthUser(owner)), Path(team)) + .await + .expect("list pending invitations") + .0; + + // No account means no handle, so the admin sees the address they typed. + // This is what keeps a handle-only roster mappable back to a person. + assert_eq!(pending[0].display_name, "nobody@example.com"); + } + #[tokio::test] async fn remove_member_forbidden_without_manage_permission() { let pool = test_pool_or_skip!(); @@ -2221,21 +2285,21 @@ mod search_tests { use crate::test_support::unique_handle; use uuid::Uuid; - async fn mk_user(pool: &PgPool, email: &str, name: &str, handle: &str, custom: bool) -> Uuid { + async fn mk_user(pool: &PgPool, email: &str, handle: &str, custom: bool) -> Uuid { sqlx::query_scalar( - "INSERT INTO users (email, display_name, account_id, auth_hash, handle, handle_is_custom, public_key) - VALUES ($1, $2, gen_random_uuid(), 'h', $3, $4, 'pk') RETURNING id", + "INSERT INTO users (email, account_id, auth_hash, handle, handle_is_custom, public_key) + VALUES ($1, gen_random_uuid(), 'h', $2, $3, 'pk') RETURNING id", ) - .bind(email).bind(name).bind(handle).bind(custom) + .bind(email).bind(handle).bind(custom) .fetch_one(pool).await.unwrap() } #[tokio::test] async fn a_stranger_is_not_found_by_an_email_substring() { let pool = crate::test_pool_or_skip!(); - let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Me", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; let email = format!("kevin.parker.{}@corp.test", &Uuid::new_v4().simple().to_string()[..6]); - let them = mk_user(&pool, &email, "Kevin Parker", &unique_handle("quiet-otter"), false).await; + let them = mk_user(&pool, &email, &unique_handle("quiet-otter"), false).await; let hits = search_users_inner(&pool, me, "kevin").await.unwrap(); assert!(!hits.iter().any(|r| r.user_id == them), "email substring must not resolve a stranger"); @@ -2247,9 +2311,9 @@ mod search_tests { #[tokio::test] async fn a_generated_handle_matches_only_exactly() { let pool = crate::test_pool_or_skip!(); - let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Me", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; let handle = unique_handle("swift-otter"); - let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Gen", &handle, false).await; + let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &handle, false).await; assert!(!search_users_inner(&pool, me, "swift-otter").await.unwrap().iter().any(|r| r.user_id == them)); assert!(search_users_inner(&pool, me, &format!("@{handle}")).await.unwrap().iter().any(|r| r.user_id == them)); @@ -2258,9 +2322,9 @@ mod search_tests { #[tokio::test] async fn a_custom_handle_matches_fuzzily() { let pool = crate::test_pool_or_skip!(); - let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Me", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; let handle = unique_handle("kevin-p"); - let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Kev", &handle, true).await; + let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &handle, true).await; // Search on the unique suffix rather than the common "kev" prefix: the // test DB is persistent, and LIMIT 8 means a common substring can be @@ -2271,10 +2335,11 @@ mod search_tests { } #[tokio::test] - async fn a_teammate_still_matches_a_name_substring_and_is_flagged() { + async fn a_teammate_still_matches_an_email_substring_and_is_flagged() { let pool = crate::test_pool_or_skip!(); - let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Me", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; - let mate = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Zoe Teammate", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let email = format!("zoe.teammate.{}@a.test", &Uuid::new_v4().simple().to_string()[..6]); + let mate = mk_user(&pool, &email, &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; let team: Uuid = sqlx::query_scalar("INSERT INTO teams (name, owner_id) VALUES ('t', $1) RETURNING id") .bind(me).fetch_one(&pool).await.unwrap(); for u in [me, mate] { @@ -2282,17 +2347,53 @@ mod search_tests { .bind(team).bind(u).execute(&pool).await.unwrap(); } - let hits = search_users_inner(&pool, me, "zo").await.unwrap(); - let hit = hits.iter().find(|r| r.user_id == mate).expect("teammate must match a name substring"); + let hits = search_users_inner(&pool, me, "zoe.teammate").await.unwrap(); + let hit = hits.iter().find(|r| r.user_id == mate).expect("teammate must match an email substring"); assert!(hit.is_teammate); } + #[tokio::test] + async fn a_teammate_is_no_longer_found_by_a_display_name_substring() { + let pool = crate::test_pool_or_skip!(); + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let mate = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let team: Uuid = sqlx::query_scalar("INSERT INTO teams (name, owner_id) VALUES ('t', $1) RETURNING id") + .bind(me).fetch_one(&pool).await.unwrap(); + for u in [me, mate] { + sqlx::query("INSERT INTO team_members (team_id, user_id) VALUES ($1, $2)") + .bind(team).bind(u).execute(&pool).await.unwrap(); + } + + let handle = unique_handle("merry-quartz"); + let email = format!("ada.lovelace.{}@example.com", &Uuid::new_v4().simple().to_string()[..6]); + sqlx::query("UPDATE users SET handle = $1, email = $2 WHERE id = $3") + .bind(&handle) + .bind(&email) + .bind(mate) + .execute(&pool) + .await + .unwrap(); + + // "lovelace" is in the email, so the teammate email fuzzy still finds them. + let by_email = search_users_inner(&pool, me, "lovelace").await.unwrap(); + assert!(by_email.iter().any(|r| r.user_id == mate)); + + // A prefix of the handle is in the handle, but a generated handle is exact-match only. + let by_handle_substring = search_users_inner(&pool, me, &handle[..handle.len() - 1]).await.unwrap(); + assert!(!by_handle_substring.iter().any(|r| r.user_id == mate)); + + // The result still carries the handle under both keys. + let hit = by_email.iter().find(|r| r.user_id == mate).unwrap(); + assert_eq!(hit.handle, handle); + assert_eq!(hit.display_name, handle); + } + #[tokio::test] async fn the_response_carries_no_public_key() { let pool = crate::test_pool_or_skip!(); - let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Me", &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; + let me = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &crate::handles::generate_unique_handle(&pool).await.unwrap(), false).await; let handle = unique_handle("kevin-pk"); - let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), "Kev", &handle, true).await; + let them = mk_user(&pool, &format!("{}@a.test", Uuid::new_v4()), &handle, true).await; let hits = search_users_inner(&pool, me, &handle).await.unwrap(); let json = serde_json::to_string(&hits).unwrap(); assert!(!json.contains("public_key"), "search must never carry key material: {json}"); diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index 511a9c7..e19df95 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -75,8 +75,8 @@ pub struct ActiveSession { /// (#66) rather than a vault share — names who invited them. pub invited_by: Option, /// `invited_by`'s handle, resolved server-side from `users`. The knock UI - /// renders this and nothing else: every other inviter identity reaching the - /// client (participant `display_name`) is supplied by the sender. + /// renders this and nothing else: `list_active_sessions` deliberately + /// redacts `participants` to empty for an unaccepted stranger. pub invited_by_handle: Option, /// Everyone the host has individually invited (#66). Populated only when /// the caller is the host — a guest must not learn the guest list. @@ -770,10 +770,9 @@ async fn visible_sessions( (SELECT tsi.invited_by FROM terminal_session_invitees tsi WHERE tsi.session_id = ts.id AND tsi.user_id = $1) AS invited_by, -- The inviter's handle, for the caller's own grant only. A knock is - -- the one surface a stranger reads before consenting, so its - -- identity must come from `users` — a participant-supplied - -- display_name there is an impersonation vector the reserved-handle - -- list would otherwise be powerless against. + -- the one surface a stranger reads before consenting, so its identity + -- must come from `users` — same resolution the participant list uses, + -- closing the impersonation vector a client-supplied name would open. (SELECT u.handle FROM terminal_session_invitees tsi JOIN users u ON u.id = tsi.invited_by WHERE tsi.session_id = ts.id AND tsi.user_id = $1) AS invited_by_handle, @@ -1175,31 +1174,27 @@ pub async fn uninvite( #[derive(Deserialize)] pub struct WsQuery { pub token: String, + /// Accepted and discarded. Pre-0.26 clients still append + /// `&display_name=`; a struct without the field would make serde + /// reject their upgrade. Never read this. Delete it in 0.27. + #[allow(dead_code)] pub display_name: Option, /// Required when joining invite_link sessions pub invite_token: Option, } -/// Longest display name the relay will carry. Long enough for any real name or -/// email, short enough that it cannot be used as a payload. -const MAX_DISPLAY_NAME_CHARS: usize = 64; - -/// Sanitizes the caller-supplied `display_name` before it reaches participant -/// lists. `None` (empty or absent) means "fall back to the user id"; `Err` means -/// the value is malformed and the upgrade is refused. -/// -/// Control characters are rejected rather than stripped: no legitimate client -/// sends them, and a name is rendered in enough places that silently reshaping -/// one is worse than telling the caller it was wrong. Length is truncated -/// instead, since a merely long name is plausible input. -fn sanitize_display_name(raw: Option) -> Result, ()> { - let Some(name) = raw.filter(|s| !s.is_empty()) else { - return Ok(None); - }; - if name.chars().any(|c| c.is_control()) { - return Err(()); - } - Ok(Some(name.chars().take(MAX_DISPLAY_NAME_CHARS).collect())) +/// Resolves the name shown on participant lists. Reads `users.handle` by the +/// authenticated user id, so the value cannot be influenced by the caller. +/// Falls back to the user id — matching the previous behaviour for a caller +/// that sent nothing — rather than refusing an upgrade over a missing row. +pub(crate) async fn resolve_participant_handle(pool: &PgPool, user_id: Uuid) -> String { + sqlx::query_scalar::<_, String>("SELECT handle FROM users WHERE id = $1") + .bind(user_id) + .fetch_optional(pool) + .await + .ok() + .flatten() + .unwrap_or_else(|| user_id.to_string()) } pub async fn ws_handler( @@ -1218,20 +1213,14 @@ pub async fn ws_handler( } }; - let display_name = match sanitize_display_name(query.display_name) { - Ok(name) => name.unwrap_or_else(|| user_id.to_string()), - Err(()) => { - warn!(session_id = %session_id, "WS upgrade rejected: malformed display_name"); - return StatusCode::BAD_REQUEST.into_response(); - } - }; + let handle = resolve_participant_handle(&pool, user_id).await; ws.on_upgrade(move |socket| { handle_socket( socket, session_id, user_id, - display_name, + handle, query.invite_token, pool, manager, @@ -1325,7 +1314,7 @@ async fn handle_socket( socket: WebSocket, session_id: Uuid, user_id: Uuid, - display_name: String, + handle: String, invite_token: Option, pool: PgPool, manager: TerminalManager, @@ -1417,13 +1406,7 @@ async fn handle_socket( None => return, }; - state.participants.insert( - user_id, - Participant { - user_id, - display_name: display_name.clone(), - }, - ); + state.participants.insert(user_id, Participant::new(user_id, handle.clone())); let participant_list: Vec<&Participant> = state.participants.values().collect(); let list_json = serde_json::json!({ @@ -1468,7 +1451,9 @@ async fn handle_socket( let joined_msg = serde_json::json!({ "type": "participant_joined", "user_id": user_id, - "display_name": display_name, + "handle": handle, + // ALIAS for pre-0.26 clients. Delete in 0.27. + "display_name": handle, }) .to_string(); let _ = tx.send(joined_msg); @@ -2877,29 +2862,6 @@ mod tests { assert!(for_host.iter().find(|r| r.id == session_id).unwrap().invited_by_handle.is_none()); } - #[test] - fn an_over_long_display_name_is_truncated() { - let name = "k".repeat(500); - let out = sanitize_display_name(Some(name)).unwrap().unwrap(); - assert_eq!(out.chars().count(), MAX_DISPLAY_NAME_CHARS); - } - - #[test] - fn a_display_name_with_control_characters_is_refused() { - assert!(sanitize_display_name(Some("Voltius\u{0}Support".to_string())).is_err()); - assert!(sanitize_display_name(Some("line\nbreak".to_string())).is_err()); - } - - #[test] - fn an_ordinary_display_name_passes_through_unchanged() { - assert_eq!( - sanitize_display_name(Some("Kévin P.".to_string())).unwrap().as_deref(), - Some("Kévin P."), - ); - assert_eq!(sanitize_display_name(Some(String::new())).unwrap(), None); - assert_eq!(sanitize_display_name(None).unwrap(), None); - } - #[tokio::test] async fn ending_a_session_clears_the_suppressed_rows_too() { let pool = test_pool_or_skip!(); @@ -2942,7 +2904,7 @@ mod tests { manager.insert_test_session(session_id, host).await; manager.sessions.lock().await.get_mut(&session_id).unwrap().participants.insert( host, - Participant { user_id: host, display_name: "Real Hostname Owner".to_string() }, + Participant::new(host, "real-hostname-owner".to_string()), ); let Json(sessions) = list_active_sessions( @@ -2967,4 +2929,41 @@ mod tests { let row = sessions.iter().find(|s| s.id == session_id).unwrap(); assert_eq!(row.participant_count, 1); } + + #[tokio::test] + async fn the_participant_handle_comes_from_the_database_not_the_caller() { + let pool = test_pool_or_skip!(); + let user = seed_user(&pool).await; + // Handles are unique and never recycled, so a fixed literal collides + // on a second run against a persistent test database. + let handle = crate::test_support::unique_handle("merry-quartz"); + + sqlx::query("UPDATE users SET handle = $1 WHERE id = $2") + .bind(&handle) + .bind(user) + .execute(&pool) + .await + .unwrap(); + + // The caller cannot influence this value: there is no argument for them to set. + let resolved = resolve_participant_handle(&pool, user).await; + assert_eq!(resolved, handle); + } + + #[tokio::test] + async fn an_unknown_user_resolves_to_its_id_rather_than_failing_the_upgrade() { + let pool = test_pool_or_skip!(); + let ghost = Uuid::new_v4(); + assert_eq!(resolve_participant_handle(&pool, ghost).await, ghost.to_string()); + } + + #[test] + fn a_participant_carries_its_handle_in_both_json_keys() { + let id = Uuid::new_v4(); + let p = Participant::new(id, "merry-quartz-2597".to_string()); + let json = serde_json::to_value(&p).unwrap(); + assert_eq!(json["handle"], "merry-quartz-2597"); + // The alias pre-0.26 clients read. Deleted in 0.27. + assert_eq!(json["display_name"], "merry-quartz-2597"); + } } diff --git a/src/routes/users.rs b/src/routes/users.rs index 3b213c2..0737958 100644 --- a/src/routes/users.rs +++ b/src/routes/users.rs @@ -150,6 +150,7 @@ pub async fn update_preferences( #[derive(Debug, Serialize, sqlx::FromRow)] pub struct UserKeyResponse { pub user_id: Uuid, + /// ALIAS for pre-0.26 clients. Value is the handle. Delete in 0.27. pub display_name: String, pub handle: String, pub public_key: String, @@ -164,7 +165,7 @@ pub(crate) async fn user_public_key_inner( user_id: Uuid, ) -> Result { sqlx::query_as::<_, UserKeyResponse>( - "SELECT id AS user_id, display_name, handle, public_key + "SELECT id AS user_id, handle AS display_name, handle, public_key FROM users WHERE id = $1 AND deleted_at IS NULL AND public_key IS NOT NULL", ) .bind(user_id) @@ -199,8 +200,8 @@ mod tests { .await .expect("generate handle"); let id: Uuid = sqlx::query_scalar( - "INSERT INTO users (email, display_name, account_id, auth_hash, subscription_tier, handle) - VALUES ($1, 'x', gen_random_uuid(), 'h', $2, $3) RETURNING id", + "INSERT INTO users (email, account_id, auth_hash, subscription_tier, handle) + VALUES ($1, gen_random_uuid(), 'h', $2, $3) RETURNING id", ) .bind(format!("{}@example.test", Uuid::new_v4())) .bind(tier) diff --git a/src/terminal_manager.rs b/src/terminal_manager.rs index afca224..eaeb26d 100644 --- a/src/terminal_manager.rs +++ b/src/terminal_manager.rs @@ -1,18 +1,32 @@ use std::{collections::{HashMap, VecDeque}, sync::Arc}; use tokio::sync::{broadcast, Mutex}; use uuid::Uuid; -use serde::{Deserialize, Serialize}; +use serde::Serialize; pub const BROADCAST_CAPACITY: usize = 512; /// Maximum number of encrypted output messages kept per session for late-join replay. pub const OUTPUT_HISTORY_MAX: usize = 500; -#[derive(Debug, Clone, Serialize, Deserialize)] +/// A live participant in a shared session. The name is resolved server-side +/// from `users.handle` — it is never supplied by the client, which is what +/// stops a participant naming themselves "Voltius Support" and what stops the +/// list carrying anyone's email address. +#[derive(Debug, Clone, Serialize)] pub struct Participant { pub user_id: Uuid, + pub handle: String, + /// ALIAS for pre-0.26 clients. Value is the handle; there is no stored + /// `display_name`. Delete this field in 0.27, and never repopulate it + /// from anything. pub display_name: String, } +impl Participant { + pub fn new(user_id: Uuid, handle: String) -> Self { + Self { user_id, display_name: handle.clone(), handle } + } +} + pub struct SessionState { /// Vaults whose members are allowed to join (empty only for invite_link sessions) pub vault_ids: Vec, diff --git a/src/test_support.rs b/src/test_support.rs index 60b13bb..684fc1d 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -83,8 +83,8 @@ pub async fn seed_user(pool: &PgPool) -> Uuid { .await .expect("generate handle"); sqlx::query( - "INSERT INTO users (id, email, account_id, auth_hash, public_key, display_name, handle) - VALUES ($1, $2, $3, 'test-hash', 'test-pubkey', 'Test User', $4)", + "INSERT INTO users (id, email, account_id, auth_hash, public_key, handle) + VALUES ($1, $2, $3, 'test-hash', 'test-pubkey', $4)", ) .bind(id) .bind(format!("{id}@test.local")) @@ -106,8 +106,8 @@ pub async fn seed_user_with_credentials(pool: &PgPool, account_id: Uuid, auth_ke .await .expect("generate handle"); sqlx::query( - "INSERT INTO users (id, email, account_id, auth_hash, public_key, display_name, handle) - VALUES ($1, $2, $3, $4, 'test-pubkey', 'Test User', $5)", + "INSERT INTO users (id, email, account_id, auth_hash, public_key, handle) + VALUES ($1, $2, $3, $4, 'test-pubkey', $5)", ) .bind(id) .bind(format!("{id}@test.local"))