fix: validate a dispatch address against the principal's live binding

backend/.sqlx/query-61ecedf0e73a136073dfbcfbc2f955c38de41c29e8ee182ddfada92ea6c7ff34.json

@@ -0,0 +1,22 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT email FROM password WHERE (username = $1 OR email = $1) AND super_admin = true ORDER BY email LIMIT 1",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "email",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text"
+      ]
+    },
+    "nullable": [
+      false
+    ]
+  },
+  "hash": "61ecedf0e73a136073dfbcfbc2f955c38de41c29e8ee182ddfada92ea6c7ff34"
+}

backend/.sqlx/query-dada076fe622e9902606bf95f9e6df004f8cb7091588309a1f484b91fe7183fc.json

@@ -0,0 +1,35 @@
+{
+  "db_name": "PostgreSQL",
+  "query": "SELECT is_admin, operator, email FROM usr where username = $1 AND workspace_id = $2 AND disabled = false",
+  "describe": {
+    "columns": [
+      {
+        "ordinal": 0,
+        "name": "is_admin",
+        "type_info": "Bool"
+      },
+      {
+        "ordinal": 1,
+        "name": "operator",
+        "type_info": "Bool"
+      },
+      {
+        "ordinal": 2,
+        "name": "email",
+        "type_info": "Varchar"
+      }
+    ],
+    "parameters": {
+      "Left": [
+        "Text",
+        "Text"
+      ]
+    },
+    "nullable": [
+      false,
+      false,
+      false
+    ]
+  },
+  "hash": "dada076fe622e9902606bf95f9e6df004f8cb7091588309a1f484b91fe7183fc"
+}

backend/windmill-common/src/auth.rs

@@ -377,6 +377,44 @@ async fn fetch_authed_from_permissioned_as_inner(
     w_id: &str,
     conn: &mut sqlx::PgConnection,
 ) -> Result<Authed> {
+    // The `usr` row is the live binding between a `u/` principal and an address, and it is read
+    // here anyway for the workspace role. Callers may hand us a cached address, so read it before
+    // anything is granted: `super_admin` and `email_to_igroup` below are keyed on the address
+    // while the role is keyed on the principal, and an address that no longer belongs to this
+    // principal — a username freed and reassigned while its previous holder keeps a privileged
+    // account — would mix one account's role with another's instance privileges.
+    let member = match permissioned_as.split_once('/') {
+        Some(("u", name)) => sqlx::query!(
+            "SELECT is_admin, operator, email FROM usr where username = $1 AND \
+                                         workspace_id = $2 AND disabled = false",
+            name,
+            &w_id
+        )
+        .fetch_optional(&mut *conn)
+        .await?,
+        _ => None,
+    };
+    let resolved_email;
+    let email = match member.as_ref() {
+        Some(m) => m.email.as_str(),
+        // No `usr` row: the principal names an account only `password` knows, so validate against
+        // the same fallback `resolve_username_to_email` uses. Reached only for a superadmin acting
+        // outside their workspaces, which is why it is worth a query the member path does not pay.
+        None => match permissioned_as.split_once('/') {
+            Some(("u", name)) => {
+                resolved_email = sqlx::query_scalar!(
+                    "SELECT email FROM password WHERE (username = $1 OR email = $1) \
+                     AND super_admin = true ORDER BY email LIMIT 1",
+                    name
+                )
+                .fetch_optional(&mut *conn)
+                .await?;
+                resolved_email.as_deref().unwrap_or(email)
+            }
+            _ => email,
+        },
+    };
+
     let is_super_admin = permissioned_as == SUPERADMIN_SYNC_EMAIL
         || email == SUPERADMIN_SECRET_EMAIL
         || email == SUPERADMIN_NOTIFICATION_EMAIL
@@ -390,22 +428,12 @@ async fn fetch_authed_from_permissioned_as_inner(
         if prefix == "u" {
             let (is_admin, is_operator) = if is_super_admin {
                 (true, false)
+            } else if let Some(m) = member.as_ref() {
+                (m.is_admin, m.operator)
             } else {
-                let r = sqlx::query!(
-                    "SELECT is_admin, operator FROM usr where username = $1 AND \
-                                                 workspace_id = $2 AND disabled = false",
-                    name,
-                    &w_id
-                )
-                .fetch_optional(&mut *conn)
-                .await?;
-                if let Some(r) = r {
-                    (r.is_admin, r.operator)
-                } else {
-                    return Err(Error::NotFound(format!(
-                        "user {name} not found in workspace {w_id}"
-                    )));
-                }
+                return Err(Error::NotFound(format!(
+                    "user {name} not found in workspace {w_id}"
+                )));
             };
 
             let groups = get_groups_for_user(w_id, &name, email, &mut *conn).await?;

backend/windmill-common/tests/permissioned_as_authz.rs

@@ -0,0 +1,50 @@
+use sqlx::{Pool, Postgres};
+use windmill_common::auth::fetch_authed_from_permissioned_as;
+
+/// The address handed to `fetch_authed_from_permissioned_as` may come from a cache that a
+/// username reassignment has outrun. It must not be believed: the workspace role is keyed on the
+/// principal while `super_admin` and `email_to_igroup` are keyed on the address, so trusting a
+/// stale one would run the new holder's job with the previous holder's instance privileges.
+#[sqlx::test(migrations = "../migrations", fixtures("base"))]
+async fn test_stale_address_cannot_carry_the_previous_holders_privileges(db: Pool<Postgres>) {
+    // `test-user` in the fixture is a superadmin with the address `test@windmill.dev`. Free the
+    // username and hand it to somebody who is not, exactly as an offboard-then-onboard would.
+    sqlx::query("DELETE FROM usr WHERE workspace_id = 'test-workspace' AND username = 'test-user'")
+        .execute(&db)
+        .await
+        .expect("free the username");
+    sqlx::query(
+        "INSERT INTO password(email, password_hash, login_type, super_admin, verified, name)
+         VALUES ('newcomer@windmill.dev', 'x', 'password', false, true, 'Newcomer')",
+    )
+    .execute(&db)
+    .await
+    .expect("create the new account");
+    sqlx::query(
+        "INSERT INTO usr(workspace_id, email, username, is_admin, role)
+         VALUES ('test-workspace', 'newcomer@windmill.dev', 'test-user', false, 'User')",
+    )
+    .execute(&db)
+    .await
+    .expect("reassign the username");
+
+    // What a replica that has not yet consumed the eviction would pass: the principal is the
+    // reassigned username, the address is the one it cached for the previous holder.
+    let authed = fetch_authed_from_permissioned_as(
+        "u/test-user",
+        "test@windmill.dev",
+        "test-workspace",
+        &db,
+    )
+    .await
+    .expect("should authenticate the current holder");
+
+    assert_eq!(
+        authed.email, "newcomer@windmill.dev",
+        "the principal's live address must win over the one supplied"
+    );
+    assert!(
+        !authed.is_admin,
+        "the new holder must not inherit the previous holder's superadmin"
+    );
+}