From f85f3391cf2ce3009b0a72f3ca5a1a19162ef2de Mon Sep 17 00:00:00 2001 From: kipavy Date: Wed, 11 Nov 2026 20:23:24 +0000 Subject: [PATCH] fix(invitations): re-check the sender's authority when an invite is accepted A pending invitation was validated only when it was sent. If the sender was later demoted, removed from the team, or deleted their account, the invite still admitted its recipient with the stored roles for up to seven days. With Owner invites in the client (VoltiusApp/voltius#778) that let a former Owner mint a new one. Every acceptance path (link token, in-app invite, registration auto-accept) now re-runs the send-time checks against the stored sender: invite permission plus authority over each role still present. A failed check answers 410 Gone. A resend from a current admin stores them as the sender and revives the invite. The duplicated invite-permission guard in add_member and invite_member is now one helper. Test invitations carry the team owner as sender. --- src/routes/invitations.rs | 151 ++++++++++++++++++++++++++++++++++++++ src/routes/teams.rs | 48 ++++++++---- src/test_support.rs | 42 ++++++++++- 3 files changed, 221 insertions(+), 20 deletions(-) diff --git a/src/routes/invitations.rs b/src/routes/invitations.rs index a55c5b9..c897d69 100644 --- a/src/routes/invitations.rs +++ b/src/routes/invitations.rs @@ -133,6 +133,17 @@ pub(crate) async fn accept_pending_invitation( user_email: Option, via: &str, ) -> Result<(), StatusCode> { + let inviter = invitation.invited_by.ok_or(StatusCode::GONE)?; + crate::routes::teams::require_inviter_still_entitled(pool, invitation.team_id, inviter, &invitation.role, &invitation.role_ids) + .await + .map_err(|status| { + if status.is_server_error() { + return status; + } + warn!(invitation_id = %invitation.id, inviter = %inviter, ?status, "Invitation outlived its sender's authority"); + StatusCode::GONE + })?; + let mut tx = pool.begin().await.map_err(|e| { error!(error = %e, "Failed to begin transaction for invitation acceptance"); StatusCode::INTERNAL_SERVER_ERROR @@ -742,3 +753,143 @@ mod admit_tests { assert_eq!(inviter_of(&pool, team, invitee).await, Some(owner)); } } + +#[cfg(test)] +mod inviter_recheck_tests { + //! Acceptance re-checks that the sender may still grant what the invitation carries. + use super::*; + use crate::auth::AuthUser; + use crate::sync_notifier::SyncNotifier; + use crate::test_support::{ + add_member, grant_builtin_role, seed_invitation_from, seed_team_with_roles, seed_user, test_user_email, + }; + use axum::extract::{Path, State}; + use axum::Extension; + + struct Fixture { + pool: PgPool, + team: Uuid, + inviter: Uuid, + recipient: Uuid, + invitation: Uuid, + } + + async fn invited_as(role: &str, inviter_role: &str) -> Option { + let pool = crate::test_support::test_pool().await?; + let primary = seed_user(&pool).await; + let team = seed_team_with_roles(&pool, primary).await; + let inviter = seed_user(&pool).await; + add_member(&pool, team, inviter).await; + grant_builtin_role(&pool, team, inviter, inviter_role).await; + let recipient = seed_user(&pool).await; + let (invitation, _) = + seed_invitation_from(&pool, team, inviter, &test_user_email(recipient), role, Some(recipient)).await; + Some(Fixture { pool, team, inviter, recipient, invitation }) + } + + async fn accept(f: &Fixture) -> Result { + accept_my_pending_invitation( + State(f.pool.clone()), + Extension(AuthUser(f.recipient)), + Extension(SyncNotifier::new()), + Path(f.invitation), + ) + .await + } + + async fn holds(f: &Fixture, user: Uuid, role: &str) -> bool { + sqlx::query_scalar( + "SELECT EXISTS(SELECT 1 FROM team_member_roles tmr JOIN team_roles tr ON tr.id = tmr.role_id + WHERE tmr.team_id = $1 AND tmr.user_id = $2 AND tr.is_builtin AND tr.name = $3)", + ) + .bind(f.team) + .bind(user) + .bind(role) + .fetch_one(&f.pool) + .await + .unwrap() + } + + async fn is_member(f: &Fixture) -> bool { + sqlx::query_scalar("SELECT EXISTS(SELECT 1 FROM team_members WHERE team_id = $1 AND user_id = $2)") + .bind(f.team) + .bind(f.recipient) + .fetch_one(&f.pool) + .await + .unwrap() + } + + async fn strip_roles(f: &Fixture) { + sqlx::query("DELETE FROM team_member_roles WHERE team_id = $1 AND user_id = $2") + .bind(f.team) + .bind(f.inviter) + .execute(&f.pool) + .await + .unwrap(); + } + + #[tokio::test] + async fn an_owner_invite_from_a_current_owner_makes_an_owner() { + let Some(f) = invited_as("owner", "owner").await else { return }; + assert_eq!(accept(&f).await, Ok(StatusCode::NO_CONTENT)); + assert!(holds(&f, f.recipient, "owner").await); + } + + #[tokio::test] + async fn an_owner_invite_dies_when_its_sender_stops_being_owner() { + let Some(f) = invited_as("owner", "owner").await else { return }; + strip_roles(&f).await; + grant_builtin_role(&f.pool, f.team, f.inviter, "manager").await; + + assert_eq!(accept(&f).await, Err(StatusCode::GONE)); + assert!(!is_member(&f).await); + } + + #[tokio::test] + async fn an_invite_dies_when_its_sender_leaves_the_team() { + let Some(f) = invited_as("member", "manager").await else { return }; + strip_roles(&f).await; + sqlx::query("DELETE FROM team_members WHERE team_id = $1 AND user_id = $2") + .bind(f.team) + .bind(f.inviter) + .execute(&f.pool) + .await + .unwrap(); + + assert_eq!(accept(&f).await, Err(StatusCode::GONE)); + assert!(!is_member(&f).await); + } + + #[tokio::test] + async fn an_invite_dies_when_its_sender_deleted_their_account() { + let Some(f) = invited_as("member", "manager").await else { return }; + sqlx::query("UPDATE pending_invitations SET invited_by = NULL WHERE id = $1") + .bind(f.invitation) + .execute(&f.pool) + .await + .unwrap(); + + assert_eq!(accept(&f).await, Err(StatusCode::GONE)); + assert!(!is_member(&f).await); + } + + #[tokio::test] + async fn a_resend_by_a_current_owner_revives_it() { + let Some(f) = invited_as("owner", "owner").await else { return }; + strip_roles(&f).await; + let primary: Uuid = sqlx::query_scalar("SELECT owner_id FROM teams WHERE id = $1") + .bind(f.team) + .fetch_one(&f.pool) + .await + .unwrap(); + sqlx::query("UPDATE pending_invitations SET invited_by = $2 WHERE id = $1") + .bind(f.invitation) + .bind(primary) + .execute(&f.pool) + .await + .unwrap(); + + assert_eq!(accept(&f).await, Ok(StatusCode::NO_CONTENT)); + assert!(holds(&f, f.recipient, "owner").await); + } +} diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 823fa5a..dbd0b48 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -447,14 +447,7 @@ pub async fn add_member( axum::extract::Path(team_id): axum::extract::Path, Json(body): Json, ) -> Result<(StatusCode, Json), StatusCode> { - let can_invite = crate::permissions::has_team_permission( - &pool, team_id, auth.0, crate::permissions::PERM_INVITE_MEMBERS, - ) - .await?; - if !can_invite { - warn!(team_id = %team_id, user_id = %auth.0, "Insufficient permission to invite members"); - return Err(StatusCode::FORBIDDEN); - } + require_can_invite(&pool, team_id, auth.0).await?; let member_name = crate::routes::member_names::invite_member_name(&pool, team_id, auth.0, body.name.as_deref()).await?; @@ -1589,6 +1582,36 @@ impl InvitedRoles { } } +async fn require_can_invite(pool: &PgPool, team_id: Uuid, actor: Uuid) -> Result<(), StatusCode> { + if crate::permissions::has_team_permission(pool, team_id, actor, crate::permissions::PERM_INVITE_MEMBERS).await? { + return Ok(()); + } + warn!(team_id = %team_id, user_id = %actor, "Insufficient permission to invite members"); + Err(StatusCode::FORBIDDEN) +} + +/// The send-time checks, run again at acceptance against whoever sent the invitation. +/// Roles deleted since are left out, as `admit_member` skips them. +pub(crate) async fn require_inviter_still_entitled( + pool: &PgPool, + team_id: Uuid, + inviter: Uuid, + role: &str, + role_ids: &[Uuid], +) -> Result<(), StatusCode> { + require_can_invite(pool, team_id, inviter).await?; + let live = sqlx::query_scalar::<_, Uuid>("SELECT id FROM team_roles WHERE team_id = $1 AND id = ANY($2)") + .bind(team_id) + .bind(role_ids) + .fetch_all(pool) + .await + .map_err(|e| { + error!(error = %e, "Failed to read invited roles"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + invited_roles(pool, team_id, inviter, Some(role), Some(&live)).await.map(drop) +} + async fn invited_roles( pool: &PgPool, team_id: Uuid, @@ -1690,14 +1713,7 @@ pub async fn invite_member( axum::extract::Path(team_id): axum::extract::Path, Json(body): Json, ) -> Result, StatusCode> { - let can_invite = crate::permissions::has_team_permission( - &pool, team_id, auth.0, crate::permissions::PERM_INVITE_MEMBERS, - ) - .await?; - if !can_invite { - warn!(team_id = %team_id, user_id = %auth.0, "Insufficient permission to invite members"); - return Err(StatusCode::FORBIDDEN); - } + require_can_invite(&pool, team_id, auth.0).await?; let member_name = crate::routes::member_names::invite_member_name(&pool, team_id, auth.0, body.name.as_deref()).await?; diff --git a/src/test_support.rs b/src/test_support.rs index fe15f78..3b03833 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -447,20 +447,53 @@ pub fn test_user_email(id: Uuid) -> String { } /// Insert a pending invitation and return `(invitation_id, token)`. `user_id` is the -/// in-app recipient binding (NULL for link-only invites). The token is a fresh UUID -/// hex string so tests can look the invitation up by token. +/// in-app recipient binding (NULL for link-only invites). The team owner sends it and +/// is made a member holding the Owner role, since acceptance re-checks the sender. pub async fn seed_invitation( pool: &PgPool, team: Uuid, email: &str, role: &str, user_id: Option, +) -> (Uuid, String) { + let owner: Uuid = sqlx::query_scalar("SELECT owner_id FROM teams WHERE id = $1") + .bind(team) + .fetch_one(pool) + .await + .expect("team owner"); + sqlx::query("INSERT INTO team_members (team_id, user_id) VALUES ($1, $2) ON CONFLICT DO NOTHING") + .bind(team) + .bind(owner) + .execute(pool) + .await + .expect("owner membership"); + seed_builtin_roles(pool, team).await; + sqlx::query( + "INSERT INTO team_member_roles (team_id, user_id, role_id) + SELECT $1, $2, id FROM team_roles WHERE team_id = $1 AND is_builtin AND name = 'owner' + ON CONFLICT DO NOTHING", + ) + .bind(team) + .bind(owner) + .execute(pool) + .await + .expect("owner role"); + seed_invitation_from(pool, team, owner, email, role, user_id).await +} + +pub async fn seed_invitation_from( + pool: &PgPool, + team: Uuid, + inviter: Uuid, + email: &str, + role: &str, + user_id: Option, ) -> (Uuid, String) { let id = Uuid::new_v4(); let token = Uuid::new_v4().simple().to_string(); sqlx::query( - "INSERT INTO pending_invitations (id, team_id, email, role, token, user_id) - VALUES ($1, $2, $3, $4, $5, $6)", + "INSERT INTO pending_invitations (id, team_id, email, role, token, user_id, invited_by) + VALUES ($1, $2, $3, $4, $5, $6, $7)", ) .bind(id) .bind(team) @@ -468,6 +501,7 @@ pub async fn seed_invitation( .bind(role) .bind(&token) .bind(user_id) + .bind(inviter) .execute(pool) .await .expect("seed invitation");