Stop anyone from acting as an admin through the MCP token or sign-in

api/role_requests.py

@@ -20,6 +20,9 @@
 from security import PlatformUser
 
 PLATFORM_ROLES = ("researcher", "pi", "equipment_owner", "admin")
+# The administrator role is never requested: an admin assigns it directly on
+# the Users page, so a request (or approving one) can't grant it.
+REQUESTABLE_ROLES = ("researcher", "pi", "equipment_owner")
 ROLE_LABELS = {
     "researcher": "Researcher",
     "pi": "Principal Investigator",
@@ -207,6 +210,8 @@ def create_role_request_pg(
     justification = (justification or "").strip()
     if requested_role not in PLATFORM_ROLES:
         raise RoleChangeError(400, "Invalid platform role")
+    if requested_role not in REQUESTABLE_ROLES:
+        raise RoleChangeError(400, "The administrator role can't be requested")
     if not justification:
         raise RoleChangeError(400, "Please explain why you need this role")
     if len(justification) > MAX_JUSTIFICATION_LENGTH:
@@ -400,6 +405,9 @@ def decide_role_request_pg(
                 raise RoleChangeError(409, f"This request is already {request['status']}")
 
             new_status = "approved" if decision == "approve" else "rejected"
+            if decision == "approve" and request["requested_role"] not in REQUESTABLE_ROLES:
+                # Requests made before admin stopped being requestable.
+                raise RoleChangeError(400, "The administrator role can't be granted through a request")
             if decision == "approve":
                 apply_user_role_change(
                     cur,

api/routers/admin.py

@@ -32,7 +32,7 @@
     list_my_role_requests_pg,
     list_role_requests_pg,
 )
-from user_identity import sync_canonical_user
+from user_identity import IdentityConflictError, sync_canonical_user
 
 logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/admin", tags=["admin"])
@@ -45,6 +45,7 @@ def get_or_create_user_role(
     name: str | None = None,
     organization: str | None = None,
     user_id: str | None = None,
+    id_source: str | None = None,
     x_user_id: str | None = Header(None, alias="X-User-Id"),
     _: None = Depends(require_system_token),
 ):
@@ -69,6 +70,7 @@ def get_or_create_user_role(
                 name=name,
                 organization=organization,
                 requested_status="active",
+                id_from_email=id_source == "email",
             )
             notified = False
             if user.get("first_sign_in"):
@@ -104,6 +106,9 @@ def get_or_create_user_role(
         if user.get("is_new"):
             response["is_new"] = True
         return response
+    except IdentityConflictError as e:
+        logger.error(f"Refused sign-in for {email}: {e}")
+        raise HTTPException(status_code=409, detail="This sign-in conflicts with an existing account")
     except Exception as e:
         logger.error(f"Error syncing user role for {email}: {e}")
         raise HTTPException(status_code=500, detail="Database lookup failed")

api/tests/test_platform_notifications.py

@@ -4,7 +4,7 @@
 import sys
 import unittest
 from pathlib import Path
-from unittest.mock import patch
+from unittest.mock import MagicMock, patch
 
 API_ROOT = Path(__file__).resolve().parents[1]
 if str(API_ROOT) not in sys.path:
@@ -128,6 +128,7 @@ def test_invalid_role_and_missing_or_long_justification_are_rejected_before_data
         with patch.object(role_requests, "get_pg_connection") as connect:
             for kwargs, message in (
                 ({"requested_role": "superuser", "justification": "x"}, "Invalid platform role"),
+                ({"requested_role": "admin", "justification": "x"}, "can't be requested"),
                 ({"requested_role": "pi", "justification": "  "}, "explain"),
                 ({"requested_role": "pi", "justification": "x" * 1001}, "at most"),
             ):
@@ -137,6 +138,26 @@ def test_invalid_role_and_missing_or_long_justification_are_rejected_before_data
                 self.assertEqual(caught.exception.status_code, 400)
         connect.assert_not_called()
 
+    def test_pending_admin_request_cannot_be_approved(self):
+        cursor = MagicMock()
+        cursor.__enter__.return_value = cursor
+        cursor.fetchone.return_value = {
+            "id": "r1",
+            "user_id": "someone",
+            "requested_role": "admin",
+            "status": "pending",
+        }
+        conn = MagicMock()
+        conn.cursor.return_value = cursor
+        with patch.object(role_requests, "get_pg_connection", return_value=conn), patch.object(
+            role_requests, "apply_user_role_change"
+        ) as change:
+            with self.assertRaises(role_requests.RoleChangeError) as caught:
+                role_requests.decide_role_request_pg(admin_user=self.USER, request_id="r1", decision="approve")
+        self.assertIn("can't be granted", caught.exception.detail)
+        change.assert_not_called()
+        conn.commit.assert_not_called()
+
     def test_decision_must_be_approve_or_reject(self):
         with patch.object(role_requests, "get_pg_connection") as connect:
             with self.assertRaises(role_requests.RoleChangeError):

api/tests/test_user_identity.py

@@ -7,7 +7,12 @@
 if str(API_DIR) not in sys.path:
     sys.path.insert(0, str(API_DIR))
 
-from user_identity import canonical_user_id, highest_role, sync_canonical_user
+from user_identity import (
+    IdentityConflictError,
+    canonical_user_id,
+    highest_role,
+    sync_canonical_user,
+)
 
 
 class FakeCursor:
@@ -118,6 +123,55 @@ def test_duplicate_identity_references_move_to_canonical_user(self):
             )
         )
 
+    def _admin_cursor(self):
+        return FakeCursor(
+            users=[
+                {
+                    "id": "ngholiza",
+                    "name": "Navid",
+                    "email": "ngholiza@purdue.edu",
+                    "role": "admin",
+                    "organization": "Purdue University",
+                    "status": "active",
+                }
+            ],
+            available_columns=set(),
+        )
+
+    def test_email_derived_id_cannot_take_over_another_account(self):
+        cursor = self._admin_cursor()
+        with self.assertRaises(IdentityConflictError):
+            sync_canonical_user(
+                cursor,
+                email="ngholiza@other.example",
+                requested_user_id="ngholiza",
+                name="Someone else",
+                organization="Elsewhere",
+                id_from_email=True,
+            )
+        self.assertFalse(any("UPDATE users" in query for query, _ in cursor.executions))
+
+    def test_email_derived_id_with_the_same_email_signs_in(self):
+        result = sync_canonical_user(
+            self._admin_cursor(),
+            email="NGHOLIZA@purdue.edu",
+            requested_user_id="ngholiza",
+            name="Navid",
+            organization="Purdue University",
+            id_from_email=True,
+        )
+        self.assertEqual(result["role"], "admin")
+
+    def test_asserted_username_keeps_its_account_after_an_email_change(self):
+        result = sync_canonical_user(
+            self._admin_cursor(),
+            email="navid@new.example",
+            requested_user_id="ngholiza",
+            name="Navid",
+            organization="Purdue University",
+        )
+        self.assertEqual(result["id"], "ngholiza")
+
 
 if __name__ == "__main__":
     unittest.main()

api/user_identity.py

@@ -36,6 +36,10 @@
 )
 
 
+class IdentityConflictError(Exception):
+    """A sign-in whose id was derived from its email matches another person's account."""
+
+
 def canonical_user_id(email: str, requested_user_id: str | None = None) -> str:
     requested = (requested_user_id or "").strip()
     if requested:
@@ -184,7 +188,18 @@ def sync_canonical_user(
     organization: str | None,
     requested_role: str = "researcher",
     requested_status: str = "active",
+    id_from_email: bool = False,
 ) -> dict[str, Any]:
+    """
+    Create or update the user for a sign-in or invitation, merging duplicate
+    rows that share the email or id.
+
+    id_from_email: the id was derived from the email's local part because the
+    identity provider sent no username. Such an id is not proof of identity
+    (ngholiza@other.example also yields "ngholiza"), so if it matches an
+    account with a different email, the sign-in is refused instead of merged,
+    which would hand over that account and its role.
+    """
     normalized_email = email.strip().lower()
     resolved_user_id = canonical_user_id(normalized_email, requested_user_id)
 
@@ -211,6 +226,14 @@ def sync_canonical_user(
         (normalized_email, resolved_user_id),
     )
     rows = [dict(row) for row in cur.fetchall()]
+    if id_from_email and any(
+        row["id"] == resolved_user_id
+        and (row.get("email") or "").strip().lower() not in ("", normalized_email)
+        for row in rows
+    ):
+        raise IdentityConflictError(
+            f"User id {resolved_user_id!r} belongs to an account with a different email"
+        )
     is_new_user = not rows
     canonical = next(
         (row for row in rows if row["id"] == resolved_user_id),

web/auth.ts

@@ -24,6 +24,8 @@ export interface PlatformIdentity {
   samlNameId?: string;
   samlNameIdFormat?: string;
   samlSessionIndex?: string;
+  /** "email" when the id was derived from the email (no username from the IdP). */
+  idSource?: "asserted" | "email";
 }
 
 interface RoleSyncResponse {
@@ -61,6 +63,7 @@ function buildRoleSyncUrl(params: {
   name?: string;
   organization?: string;
   userId?: string;
+  idSource?: "asserted" | "email";
 }): string {
   const baseUrl = getDtApiBaseUrl();
   const url = new URL(
@@ -73,6 +76,7 @@ function buildRoleSyncUrl(params: {
     url.searchParams.set("organization", params.organization);
   }
   if (params.userId) url.searchParams.set("user_id", params.userId);
+  if (params.idSource) url.searchParams.set("id_source", params.idSource);
   return url.toString();
 }
 
@@ -86,6 +90,7 @@ export async function syncPlatformIdentity(
       name: identity.name,
       organization: identity.organization,
       userId: identity.id,
+      idSource: identity.idSource,
     }),
     {
       headers: { "X-System-Token": requireServerEnv("DT_SYSTEM_TOKEN") },

web/components/role-request-card.tsx

@@ -11,7 +11,8 @@ import {
 } from "@/lib/api-client";
 import { ArrowRightLeft, Clock, CheckCircle2, XCircle, Ban, Loader2 } from "lucide-react";
 
-const REQUESTABLE_ROLES: UserRole[] = ["pi", "equipment_owner", "researcher", "admin"];
+// Admin is assigned only by an administrator on the Users page, never requested.
+const REQUESTABLE_ROLES: UserRole[] = ["pi", "equipment_owner", "researcher"];
 const MAX_JUSTIFICATION = 1000;
 
 const STATUS_STYLES: Record<RoleChangeRequest["status"], { label: string; className: string; icon: typeof Clock }> = {

web/lib/mcp/bearer-identity.test.mjs

@@ -0,0 +1,26 @@
+// Run from web/: node --test lib/mcp/bearer-identity.test.mjs
+import assert from "node:assert/strict";
+import { test } from "node:test";
+
+import { bearerIdentityFromEnv } from "./bearer-identity.ts";
+
+const BASE = { MCP_TEST_USER_ID: "tester", MCP_TEST_USER_EMAIL: "tester@purdue.edu" };
+
+test("the bearer token never acts as an admin", () => {
+  for (const role of ["admin", "Administrator", " ADMIN "]) {
+    assert.ok("refused" in bearerIdentityFromEnv({ ...BASE, MCP_TEST_USER_ROLE: role }));
+  }
+});
+
+test("the bearer token needs an explicit identity", () => {
+  assert.ok("refused" in bearerIdentityFromEnv({}));
+  assert.ok("refused" in bearerIdentityFromEnv({ MCP_TEST_USER_ID: "tester" }));
+  assert.ok("refused" in bearerIdentityFromEnv({ MCP_TEST_USER_EMAIL: "tester@purdue.edu" }));
+});
+
+test("the default role is researcher", () => {
+  const identity = bearerIdentityFromEnv(BASE);
+  assert.equal(identity.role, "researcher");
+  assert.equal(identity.id, "tester");
+  assert.equal(bearerIdentityFromEnv({ ...BASE, MCP_TEST_USER_ROLE: "pi" }).role, "pi");
+});

web/lib/mcp/bearer-identity.ts

@@ -0,0 +1,38 @@
+/**
+ * bearer-identity.ts — who the shared MCP bearer token acts as (pure, no
+ * imports, so it can be unit-tested). Whoever holds the token acts as this
+ * user, so it must be set explicitly (no default account) and can never be an
+ * administrator.
+ */
+
+export interface BearerIdentity {
+  id: string;
+  email: string;
+  name: string;
+  role: string;
+  organization: string;
+}
+
+function normalizePlatformRole(role: string): string {
+  const normalized = role.trim().toLowerCase();
+  if (normalized === "administrator") return "admin";
+  return normalized || "researcher";
+}
+
+/** The configured identity, or the reason the token may not be used. */
+export function bearerIdentityFromEnv(
+  env: Record<string, string | undefined>,
+): BearerIdentity | { refused: string } {
+  const id = env.MCP_TEST_USER_ID?.trim() ?? "";
+  const email = env.MCP_TEST_USER_EMAIL?.trim() ?? "";
+  const role = normalizePlatformRole(env.MCP_TEST_USER_ROLE ?? "researcher");
+  if (!id || !email) return { refused: "MCP_TEST_USER_ID and MCP_TEST_USER_EMAIL must be set" };
+  if (role === "admin") return { refused: "the MCP bearer token may not act as an admin" };
+  return {
+    id,
+    email,
+    name: env.MCP_TEST_USER_NAME?.trim() || id,
+    role,
+    organization: env.MCP_TEST_USER_ORG?.trim() || "Birck Nanotechnology Center",
+  };
+}

web/lib/mcp/dt-call.ts

@@ -1,6 +1,7 @@
 import "server-only";
 
 import { auth } from "@/auth";
+import { bearerIdentityFromEnv } from "@/lib/mcp/bearer-identity";
 import { requireServerEnv } from "@/lib/server-env";
 import { timingSafeEqual } from "crypto";
 
@@ -75,26 +76,23 @@ function constantTimeEquals(left: string, right: string): boolean {
   return timingSafeEqual(leftBuffer, rightBuffer);
 }
 
-function normalizePlatformRole(role: string): string {
-  const normalized = role.trim().toLowerCase();
-  if (normalized === "administrator") return "admin";
-  return normalized || "researcher";
-}
-
+/**
+ * The identity behind the shared MCP bearer token (external MCP / OpenAPI
+ * clients); see bearer-identity.ts. An admin identity, or a missing id/email,
+ * makes the token unusable.
+ */
 export function resolveMcpBearerSession(authorization: string | null): PlatformSession | null {
   const expectedToken = process.env.MCP_BEARER_TOKEN?.trim() ?? "";
   const receivedToken = bearerTokenFromHeader(authorization);
   if (!expectedToken || !receivedToken) return null;
   if (!constantTimeEquals(receivedToken, expectedToken)) return null;
 
-  return {
-    id: process.env.MCP_TEST_USER_ID?.trim() || "ngholiza",
-    email: process.env.MCP_TEST_USER_EMAIL?.trim() || "ngholiza@purdue.edu",
-    name: process.env.MCP_TEST_USER_NAME?.trim() || "N. Gholiza",
-    role: normalizePlatformRole(process.env.MCP_TEST_USER_ROLE ?? "pi"),
-    organization:
-      process.env.MCP_TEST_USER_ORG?.trim() || "Birck Nanotechnology Center",
-  };
+  const identity = bearerIdentityFromEnv(process.env);
+  if ("refused" in identity) {
+    console.error(`[MCP_AUTH] token refused: ${identity.refused}`);
+    return null;
+  }
+  return identity;
 }
 
 export interface DtCallResult<T = unknown> {

web/lib/saml/profile.ts

@@ -165,6 +165,9 @@ export function mapSamlProfile(profile: Profile): Omit<PlatformIdentity, "role">
 
   return {
     id: username,
+    // An id derived from the email is not proof of identity; the role sync
+    // refuses it when it matches an account with a different email.
+    idSource: explicitUsername || nonEmailNameId ? "asserted" : "email",
     nanohubUsername: username,
     email,
     name: displayName || username,