diff --git a/src/auth/people-search.ts b/src/auth/people-search.ts new file mode 100644 index 0000000..ebcb5db --- /dev/null +++ b/src/auth/people-search.ts @@ -0,0 +1,183 @@ +import { type SQL, and, eq, exists, gt, ilike, or, sql } from "drizzle-orm"; +import type { db } from "../db/index"; +import { account, identityEvidence, passkey, user, userRole } from "../db/schema"; +import { env } from "../env"; +import { matchesSearch } from "../lib/search"; +import { groupMembers } from "./group-listing"; +import type { RbacCatalog } from "./rbac"; +import { type GroupTrait, type MembershipSource, type UserTrait, USER_TRAITS } from "./users"; + +type Transaction = Parameters[0]>[0]; + +// The same trust boundary `readIdentitySubject` applies: evidence counts only from the +// issuer each provider is expected to use. +const TELEGRAM_ISSUER = "https://oauth.telegram.org"; +const POLIMI_EMAIL_ISSUER = "https://mail.polimi.it"; + +function entraIssuer() { + return env.PN_ENTRA_TENANT_ID + ? `https://login.microsoftonline.com/${env.PN_ENTRA_TENANT_ID}/v2.0` + : undefined; +} + +type GroupListing = { source: MembershipSource; members: Set | null }; + +/** + * Who is in each PoliNetwork group right now. Checking people one by one would page + * through the group once per person, so the directory lists each group once instead. When + * Graph cannot answer, it falls back to what each person's last PoliNetwork sign-in + * recorded, and says so. + */ +export async function groupListings(): Promise> { + const listing = async (groupId: string | undefined): Promise => { + if (!groupId || !entraIssuer()) return { source: "unconfigured", members: null }; + const members = await groupMembers(groupId); + return members ? { source: "live", members } : { source: "recorded", members: null }; + }; + const [socio, direttivo] = await Promise.all([ + listing(env.PN_ENTRA_MEMBER_GROUP_ID), + listing(env.PN_ENTRA_DIRETTIVO_GROUP_ID), + ]); + return { socio, direttivo }; +} + +/** Whether the person owns an account matching `where`. */ +function ownsAccount(transaction: Transaction, where: SQL | undefined) { + return exists( + transaction + .select({ found: sql`1` }) + .from(account) + .where(and(eq(account.userId, user.id), where)), + ); +} + +/** Whether the person owns an account whose stored evidence matches `where`. */ +function ownsEvidence(transaction: Transaction, where: SQL | undefined) { + return exists( + transaction + .select({ found: sql`1` }) + .from(account) + .innerJoin( + identityEvidence, + and( + eq(account.issuer, identityEvidence.issuer), + eq(account.accountId, identityEvidence.subject), + eq(account.providerId, identityEvidence.providerId), + ), + ) + .where(and(eq(account.userId, user.id), where)), + ); +} + +function groupCondition(transaction: Transaction, state: GroupTrait, listing: GroupListing): SQL { + const issuer = entraIssuer(); + if (listing.source === "unconfigured" || !issuer) return sql`false`; + const entra = and(eq(account.providerId, "pn-entra"), eq(account.issuer, issuer)); + if (listing.members) { + if (!listing.members.size) return sql`false`; + // One array parameter, however large the group is. + const members = sql.param([...listing.members]); + return ownsEvidence( + transaction, + and(entra, sql`${identityEvidence.externalId} = any(${members}::text[])`), + ); + } + return ownsEvidence(transaction, and(entra, sql`${state} = any(${identityEvidence.states})`)); +} + +/** + * One boolean expression per trait, used both to filter and to report each row, so what + * the list shows can never disagree with what it was filtered by. + */ +export function traitConditions( + transaction: Transaction, + groups: Record, + now: Date, +): Record { + return { + socio: groupCondition(transaction, "socio", groups.socio), + direttivo: groupCondition(transaction, "direttivo", groups.direttivo), + student: ownsEvidence( + transaction, + and( + eq(account.providerId, "polimi-email"), + eq(account.issuer, POLIMI_EMAIL_ISSUER), + sql`'student' = any(${identityEvidence.states})`, + gt(identityEvidence.validUntil, now), + ), + ), + telegram: ownsAccount( + transaction, + and(eq(account.providerId, "telegram"), eq(account.issuer, TELEGRAM_ISSUER)), + ), + google: ownsAccount(transaction, eq(account.providerId, "google")), + polinetwork: ownsAccount(transaction, eq(account.providerId, "pn-entra")), + passkey: exists( + transaction + .select({ found: sql`1` }) + .from(passkey) + .where(eq(passkey.userId, user.id)), + ), + }; +} + +export function searchCondition( + transaction: Transaction, + query: string, + catalog: RbacCatalog, + traits?: Record, + canReadRoles = false, +) { + query = query.trim(); + const escaped = query.replace(/[%_\\]/g, (match) => `\\${match}`); + const term = `%${escaped}%`; + return or( + ...(traits + ? USER_TRAITS.filter((trait) => + matchesSearch(query, [trait.key, trait.label, trait.description]), + ).map((trait) => traits[trait.key]) + : []), + ...(canReadRoles + ? catalog.roles + .filter( + (role) => + !role.managed && matchesSearch(query, [role.key, role.name, role.description]), + ) + .map((role) => + exists( + transaction + .select({ found: sql`1` }) + .from(userRole) + .where(and(eq(userRole.userId, user.id), eq(userRole.roleId, role.id))), + ), + ) + : []), + ilike(user.name, term), + ilike(user.email, term), + eq(user.id, query), + ownsAccount( + transaction, + and( + eq(account.providerId, "polimi-email"), + eq(account.issuer, POLIMI_EMAIL_ISSUER), + ilike(account.accountId, term), + ), + ), + ownsEvidence( + transaction, + and(eq(account.providerId, "pn-entra"), ilike(identityEvidence.email, term)), + ), + ownsEvidence( + transaction, + and( + eq(account.providerId, "telegram"), + eq(account.issuer, TELEGRAM_ISSUER), + or( + ilike(identityEvidence.telegramId, term), + // Usernames are case-insensitive on Telegram; the @ is optional. + ilike(identityEvidence.telegramUsername, `%${escaped.replace(/^@/, "")}%`), + ), + ), + ), + ); +} diff --git a/src/auth/rbac-security.integration.test.mjs b/src/auth/rbac-security.integration.test.mjs index bbff90b..242a82c 100644 --- a/src/auth/rbac-security.integration.test.mjs +++ b/src/auth/rbac-security.integration.test.mjs @@ -26,7 +26,10 @@ vi.mock("../env", () => { }, }; }); -vi.mock("./membership", () => ({ checkEntraGroupMember: mocks.graph })); +vi.mock("./membership", () => ({ + checkEntraGroupMember: mocks.graph, + listEntraGroupMembers: vi.fn().mockResolvedValue(null), +})); // Only session authentication is substituted; routes, authorization, evidence, SQL and // transactions are real. The separate identity HTTP suite exercises signed session cookies. vi.mock("./index", () => ({ diff --git a/src/auth/rbac-store.ts b/src/auth/rbac-store.ts index a899e84..76705b7 100644 --- a/src/auth/rbac-store.ts +++ b/src/auth/rbac-store.ts @@ -1,3 +1,4 @@ +import { groupListings, traitConditions, searchCondition } from "./people-search"; import type { FieldErrors } from "@/lib/action-error"; import { contactEmails } from "./contact-email"; import { mayDelegateMutation } from "./rbac-delegation"; @@ -5,12 +6,10 @@ import { logAuthorizationDenial } from "./denial-log"; import { readIdentitySubject, refreshIdentityMembership } from "./identity-subject"; import type { IdentityClaims } from "./policy"; import { randomUUID } from "node:crypto"; -import { and, count, eq, exists, gt, ilike, or, sql } from "drizzle-orm"; +import { and, count, eq, gt, sql } from "drizzle-orm"; import { db } from "../db/index"; import { authorizationMutationLock } from "../db/security-lock"; import { - account, - identityEvidence, rbacAuditEvent, permission, permissionImplication, @@ -501,7 +500,10 @@ export async function searchUsers( } requireRole(catalog, roleId); } - const term = `%${query.trim().replace(/[%_\\]/g, (match) => `\\${match}`)}%`; + const traits = + query.trim() && access.permissions.includes("idp:users:read") + ? traitConditions(transaction, await groupListings(), new Date()) + : undefined; const people = await transaction .select({ id: user.id, @@ -515,30 +517,12 @@ export async function searchUsers( .from(user) .where( query.trim() - ? or( - ilike(user.name, term), - ilike(user.email, term), - // The PoliNetwork address saved at sign-in, since the stored email is a placeholder. - exists( - transaction - .select({ found: sql`1` }) - .from(account) - .innerJoin( - identityEvidence, - and( - eq(account.issuer, identityEvidence.issuer), - eq(account.accountId, identityEvidence.subject), - eq(account.providerId, identityEvidence.providerId), - ), - ) - .where( - and( - eq(account.userId, user.id), - eq(account.providerId, "pn-entra"), - ilike(identityEvidence.email, term), - ), - ), - ), + ? searchCondition( + transaction, + query, + catalog, + traits, + access.permissions.includes("idp:roles:read"), ) : undefined, ) diff --git a/src/auth/user-directory.integration.test.mjs b/src/auth/user-directory.integration.test.mjs index 75115cc..f516c00 100644 --- a/src/auth/user-directory.integration.test.mjs +++ b/src/auth/user-directory.integration.test.mjs @@ -102,8 +102,8 @@ describe.skipIf(!process.env.RBAC_TEST_DATABASE_URL)("user directory with Postgr async function evidence(issuer, subject, providerId, fields) { await pool.query( - `INSERT INTO identity_evidence (issuer, subject, provider_id, external_id, states, valid_until, telegram_id, email) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + `INSERT INTO identity_evidence (issuer, subject, provider_id, external_id, states, valid_until, telegram_id, email, telegram_username) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, [ issuer, subject, @@ -113,6 +113,7 @@ describe.skipIf(!process.env.RBAC_TEST_DATABASE_URL)("user directory with Postgr fields.validUntil ?? new Date(Date.now() + 86_400_000), fields.telegramId ?? null, fields.email ?? null, + fields.telegramUsername ?? null, ], ); } @@ -145,6 +146,7 @@ describe.skipIf(!process.env.RBAC_TEST_DATABASE_URL)("user directory with Postgr await account(ada, "telegram", "https://oauth.telegram.org", `${ada}-telegram`); await evidence("https://oauth.telegram.org", `${ada}-telegram`, "telegram", { telegramId: "4242424242", + telegramUsername: "Ada_directory_test", }); const adaMail = `${ada}@mail.polimi.it`; await account(ada, "polimi-email", "https://mail.polimi.it", adaMail); @@ -263,6 +265,38 @@ describe.skipIf(!process.env.RBAC_TEST_DATABASE_URL)("user directory with Postgr expect(names(await listUsers(root, { q: `${tag.slice(0, -1)}_` }))).toEqual([]); }); + it("searches Telegram fragments consistently in People and role assignment", async () => { + for (const q of ["@ADA_DIRECTORY", "ada_directory", "424242", `${bruno}@mail.polimi`]) { + const expected = q.includes("polimi") ? bruno : ada; + expect(names(await listUsers(root, { q }))).toEqual([expected]); + expect((await searchUsers(root, q)).map((person) => person.id)).toEqual([expected]); + } + expect(names(await listUsers(root, { q: "@ada_directory%" }))).toEqual([]); + expect(await searchUsers(root, "@ada_directory%")).toEqual([]); + expect(await searchUsers(root, "515151")).toEqual([]); + }); + + it("searches the same traits and roles as the filters, combined with explicit filters", async () => { + for (const [q, expected] of [ + ["Student", ada], + ["Passkey", bruno], + ["Google", bruno], + [moderator.key, bruno], + ]) { + expect(names(await listUsers(root, { q }))).toEqual([expected]); + expect((await searchUsers(root, q)).map((person) => person.id)).toEqual([expected]); + } + expect(names(await listUsers(root, { q: "Google", passkey: "no" }))).toEqual([]); + const reader = await delegate(["idp:users:read", "idp:people:read"]); + expect(names(await listUsers(reader, { q: moderator.key }))).toEqual([]); + expect(await searchUsers(reader, moderator.key)).toEqual([]); + const searcher = await delegate(["idp:people:read"]); + expect(await searchUsers(searcher, "Passkey")).toEqual([]); + expect((await searchUsers(searcher, "@ADA_DIRECTORY")).map((person) => person.id)).toEqual([ + ada, + ]); + }); + it("keeps people without a known email visible, searchable, and assignable", async () => { const search = await call(searchPeople, root, { query: ordinary, roleId: moderator.id }); expect(search.status).toBe(200); diff --git a/src/auth/user-directory.ts b/src/auth/user-directory.ts index 870e791..2f7fd72 100644 --- a/src/auth/user-directory.ts +++ b/src/auth/user-directory.ts @@ -1,187 +1,23 @@ -import { - type SQL, - and, - asc, - count, - desc, - eq, - exists, - gt, - ilike, - inArray, - not, - or, - sql, -} from "drizzle-orm"; +import { type SQL, and, asc, count, desc, eq, exists, inArray, not, or, sql } from "drizzle-orm"; import { alias } from "drizzle-orm/pg-core"; -import { db } from "../db/index"; import { account, identityEvidence, passkey, role, user, userRole } from "../db/schema"; -import { env } from "../env"; import { contactEmails } from "./contact-email"; import { logAuthorizationDenial } from "./denial-log"; -import { groupMembers } from "./group-listing"; +import { groupListings, traitConditions, searchCondition } from "./people-search"; import { readIdentitySubject, refreshIdentityMembership } from "./identity-subject"; import { resolveAccess } from "./rbac"; import { RbacError, assignedRoleKeys, withAuthorizedRbacRead } from "./rbac-store"; import { - type GroupTrait, - type MembershipSource, type UserDetail, type UserListPage, type UserSearch, - type UserTrait, USER_PAGE_SIZE, USER_TRAITS, } from "./users"; -type Transaction = Parameters[0]>[0]; - -// The same trust boundary `readIdentitySubject` applies: evidence counts only from the -// issuer each provider is expected to use. const TELEGRAM_ISSUER = "https://oauth.telegram.org"; const POLIMI_EMAIL_ISSUER = "https://mail.polimi.it"; -function entraIssuer() { - return env.PN_ENTRA_TENANT_ID - ? `https://login.microsoftonline.com/${env.PN_ENTRA_TENANT_ID}/v2.0` - : undefined; -} - -type GroupListing = { source: MembershipSource; members: Set | null }; - -/** - * Who is in each PoliNetwork group right now. Checking people one by one would page - * through the group once per person, so the directory lists each group once instead. When - * Graph cannot answer, it falls back to what each person's last PoliNetwork sign-in - * recorded, and says so. - */ -async function groupListings(): Promise> { - const listing = async (groupId: string | undefined): Promise => { - if (!groupId || !entraIssuer()) return { source: "unconfigured", members: null }; - const members = await groupMembers(groupId); - return members ? { source: "live", members } : { source: "recorded", members: null }; - }; - const [socio, direttivo] = await Promise.all([ - listing(env.PN_ENTRA_MEMBER_GROUP_ID), - listing(env.PN_ENTRA_DIRETTIVO_GROUP_ID), - ]); - return { socio, direttivo }; -} - -/** Whether the person owns an account matching `where`. */ -function ownsAccount(transaction: Transaction, where: SQL | undefined) { - return exists( - transaction - .select({ found: sql`1` }) - .from(account) - .where(and(eq(account.userId, user.id), where)), - ); -} - -/** Whether the person owns an account whose stored evidence matches `where`. */ -function ownsEvidence(transaction: Transaction, where: SQL | undefined) { - return exists( - transaction - .select({ found: sql`1` }) - .from(account) - .innerJoin( - identityEvidence, - and( - eq(account.issuer, identityEvidence.issuer), - eq(account.accountId, identityEvidence.subject), - eq(account.providerId, identityEvidence.providerId), - ), - ) - .where(and(eq(account.userId, user.id), where)), - ); -} - -function groupCondition(transaction: Transaction, state: GroupTrait, listing: GroupListing): SQL { - const issuer = entraIssuer(); - if (listing.source === "unconfigured" || !issuer) return sql`false`; - const entra = and(eq(account.providerId, "pn-entra"), eq(account.issuer, issuer)); - if (listing.members) { - if (!listing.members.size) return sql`false`; - // One array parameter, however large the group is. - const members = sql.param([...listing.members]); - return ownsEvidence( - transaction, - and(entra, sql`${identityEvidence.externalId} = any(${members}::text[])`), - ); - } - return ownsEvidence(transaction, and(entra, sql`${state} = any(${identityEvidence.states})`)); -} - -/** - * One boolean expression per trait, used both to filter and to report each row, so what - * the list shows can never disagree with what it was filtered by. - */ -function traitConditions( - transaction: Transaction, - groups: Record, - now: Date, -): Record { - return { - socio: groupCondition(transaction, "socio", groups.socio), - direttivo: groupCondition(transaction, "direttivo", groups.direttivo), - student: ownsEvidence( - transaction, - and( - eq(account.providerId, "polimi-email"), - eq(account.issuer, POLIMI_EMAIL_ISSUER), - sql`'student' = any(${identityEvidence.states})`, - gt(identityEvidence.validUntil, now), - ), - ), - telegram: ownsAccount( - transaction, - and(eq(account.providerId, "telegram"), eq(account.issuer, TELEGRAM_ISSUER)), - ), - google: ownsAccount(transaction, eq(account.providerId, "google")), - polinetwork: ownsAccount(transaction, eq(account.providerId, "pn-entra")), - passkey: exists( - transaction - .select({ found: sql`1` }) - .from(passkey) - .where(eq(passkey.userId, user.id)), - ), - }; -} - -function searchCondition(transaction: Transaction, query: string) { - const escaped = query.replace(/[%_\\]/g, (match) => `\\${match}`); - const term = `%${escaped}%`; - return or( - ilike(user.name, term), - ilike(user.email, term), - eq(user.id, query), - ownsAccount( - transaction, - and( - eq(account.providerId, "polimi-email"), - eq(account.issuer, POLIMI_EMAIL_ISSUER), - ilike(account.accountId, term), - ), - ), - ownsEvidence( - transaction, - and(eq(account.providerId, "pn-entra"), ilike(identityEvidence.email, term)), - ), - ownsEvidence( - transaction, - and( - eq(account.providerId, "telegram"), - eq(account.issuer, TELEGRAM_ISSUER), - or( - eq(identityEvidence.telegramId, query), - // Usernames are case-insensitive on Telegram; the @ is optional. - ilike(identityEvidence.telegramUsername, escaped.replace(/^@/, "")), - ), - ), - ), - ); -} - function requireRoleRead(actorId: string, permissions: readonly string[]) { if (permissions.includes("idp:roles:read")) return; logAuthorizationDenial(actorId, "user-directory", ["idp:roles:read"]); @@ -208,7 +44,8 @@ export async function listUsers(actorId: string, search: UserSearch): Promise - option.key.toLowerCase().includes(term) || option.label.toLowerCase().includes(term), + ? options.filter((option) => + matchesSearch(term, [ + option.key, + option.label, + option.hint, + option.disabled ? option.disabledReason : undefined, + ]), ) : options; diff --git a/src/components/rbac/role-members.tsx b/src/components/rbac/role-members.tsx index 3498664..4da6a28 100644 --- a/src/components/rbac/role-members.tsx +++ b/src/components/rbac/role-members.tsx @@ -127,7 +127,7 @@ export function RoleMembers({ setQuery(event.target.value)} - placeholder="Search by name or email" + placeholder="Name, email, @Telegram, or ID" aria-label={`Find someone to give ${roleName} to`} className="pl-9" /> diff --git a/src/lib/search.ts b/src/lib/search.ts new file mode 100644 index 0000000..af36a67 --- /dev/null +++ b/src/lib/search.ts @@ -0,0 +1,5 @@ +/** Case-insensitive literal substring search across the fields a section exposes. */ +export function matchesSearch(query: string, values: readonly (string | null | undefined)[]) { + const term = query.trim().toLowerCase(); + return !term || values.some((value) => value?.toLowerCase().includes(term)); +} diff --git a/src/routes/access/permissions/index.tsx b/src/routes/access/permissions/index.tsx index 9cd0672..8e7e958 100644 --- a/src/routes/access/permissions/index.tsx +++ b/src/routes/access/permissions/index.tsx @@ -1,3 +1,4 @@ +import { matchesSearch } from "@/lib/search"; import { useState } from "react"; import { createFileRoute, Link } from "@tanstack/react-router"; import { ChevronRight, KeyRound, Plus, ShieldCheck, Sparkles } from "lucide-react"; @@ -101,12 +102,16 @@ function PermissionsIndex() { const filtered = catalog.permissions.filter( (permission) => (application === null || applicationKey(permission) === application) && - [ + matchesSearch(search, [ permission.name, permission.key, permission.description, labels.get(applicationKey(permission)), - ].some((value) => value?.toLowerCase().includes(search)), + ...expandPermissionKeys(catalog, permission.implies).flatMap((key) => { + const implied = catalog.permissions.find((entry) => entry.key === key); + return [key, implied?.name, implied?.description]; + }), + ]), ); const impliedBy = (permission: PermissionSummary) => expandPermissionKeys(catalog, permission.implies).filter((key) => key !== permission.key); diff --git a/src/routes/access/roles/index.tsx b/src/routes/access/roles/index.tsx index 8156682..de46a8a 100644 --- a/src/routes/access/roles/index.tsx +++ b/src/routes/access/roles/index.tsx @@ -1,3 +1,4 @@ +import { matchesSearch } from "@/lib/search"; import { useState } from "react"; import { createFileRoute, Link } from "@tanstack/react-router"; import { ShieldCheck } from "lucide-react"; @@ -141,7 +142,19 @@ function RolesIndex() { .sort((a, b) => staticRoleOrder(a.key) - staticRoleOrder(b.key)); const created = catalog.roles.filter((role) => !role.managed); const filtered = created.filter((role) => - [role.name, role.key, role.description].some((value) => value?.toLowerCase().includes(search)), + matchesSearch(search, [ + role.name, + role.key, + role.description, + ...role.parents.flatMap((key) => { + const parent = catalog.roles.find((entry) => entry.key === key); + return [key, parent?.name, parent?.description]; + }), + ...effectiveRolePermissions(catalog, role.key).flatMap((key) => { + const permission = catalog.permissions.find((entry) => entry.key === key); + return [key, permission?.name, permission?.description]; + }), + ]), ); const countFor = (role: RoleSummary) => effectiveRolePermissions(catalog, role.key).length; diff --git a/src/routes/users/index.tsx b/src/routes/users/index.tsx index f146ab2..cda2202 100644 --- a/src/routes/users/index.tsx +++ b/src/routes/users/index.tsx @@ -283,7 +283,7 @@ function UsersIndex() { type="search" value={query} onChange={(event) => setQuery(event.target.value)} - placeholder="Name, email, Polimi address, Telegram ID or username, or user ID" + placeholder="Name, email, @Telegram, ID, role, or account status" aria-label="Search users" className="pl-9" />