From b0423fb27666a474c5b2435eb1c511b5d099b0ae Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 08:41:43 +0000 Subject: [PATCH 01/21] feat(perms): apply per-member allow/deny masks when resolving permissions --- src/services/permissions.test.ts | 43 ++++++++++++++++++++++++++++++-- src/services/permissions.ts | 8 ++++-- src/services/teamService.ts | 4 +++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/services/permissions.test.ts b/src/services/permissions.test.ts index fd0578844..f8a7a9206 100644 --- a/src/services/permissions.test.ts +++ b/src/services/permissions.test.ts @@ -1,5 +1,5 @@ -import { test, expect } from "vitest"; -import { resolveCan, PERM_BITS, type PermissionSnapshot } from "./permissions.ts"; +import { test, expect, describe, it } from "vitest"; +import { resolveCan, PERM_BITS, effectivePermissions, type PermissionSnapshot } from "./permissions.ts"; import type { Team, TeamMember, TeamRole } from "@/services/teamService"; import type { Vault } from "@/stores/vaultStore"; @@ -76,3 +76,42 @@ test("fallback denies when team unknown or roles empty", () => { resolveCan(snap({ teams: [team("t1", ["r1"])], rolesByTeam: { t1: [] } }), "VIEW_SECRETS", "t1"), ).toBe(false); }); + +describe("effectivePermissions with member overrides", () => { + const roles: TeamRole[] = [ + { id: "r1", team_id: "t1", name: "editor", permissions: PERM_BITS.CONNECT | PERM_BITS.VIEW_SECRETS, is_builtin: true, position: 2, created_at: "" }, + ]; + + it("returns the role union when no override is present", () => { + expect(effectivePermissions({ role_ids: ["r1"] }, roles)).toBe( + PERM_BITS.CONNECT | PERM_BITS.VIEW_SECRETS, + ); + }); + + it("adds allowed bits the roles do not grant", () => { + expect( + effectivePermissions({ role_ids: ["r1"], permission_allow: PERM_BITS.EDIT_KEYS }, roles), + ).toBe(PERM_BITS.CONNECT | PERM_BITS.VIEW_SECRETS | PERM_BITS.EDIT_KEYS); + }); + + it("removes denied bits the roles do grant", () => { + expect( + effectivePermissions({ role_ids: ["r1"], permission_deny: PERM_BITS.VIEW_SECRETS }, roles), + ).toBe(PERM_BITS.CONNECT); + }); + + it("lets deny win when a bit is in both masks", () => { + expect( + effectivePermissions( + { role_ids: [], permission_allow: PERM_BITS.CONNECT, permission_deny: PERM_BITS.CONNECT }, + roles, + ), + ).toBe(0); + }); + + it("grants an allowed bit to a member holding no roles", () => { + expect( + effectivePermissions({ role_ids: [], permission_allow: PERM_BITS.CONNECT }, roles), + ).toBe(PERM_BITS.CONNECT); + }); +}); diff --git a/src/services/permissions.ts b/src/services/permissions.ts index 98ecffaa1..4e39320d9 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -42,11 +42,15 @@ export const PERM_BITS: Record = { }; /** OR together all permission bits for a member's assigned roles. */ -export function effectivePermissions(member: { role_ids: string[] }, roles: TeamRole[]): number { - return member.role_ids.reduce((acc, rid) => { +export function effectivePermissions( + member: { role_ids: string[]; permission_allow?: number; permission_deny?: number }, + roles: TeamRole[], +): number { + const union = member.role_ids.reduce((acc, rid) => { const role = roles.find((r) => r.id === rid); return acc | (role?.permissions ?? 0); }, 0); + return (union | (member.permission_allow ?? 0)) & ~(member.permission_deny ?? 0); } /** True if member holds the builtin role with the given name in this team. */ diff --git a/src/services/teamService.ts b/src/services/teamService.ts index b3e074b76..6aa3bfa7b 100644 --- a/src/services/teamService.ts +++ b/src/services/teamService.ts @@ -11,6 +11,8 @@ export interface Team { owner_tier: string; created_at: string; role_ids: string[]; + permission_allow?: number; + permission_deny?: number; } export interface TeamMember { @@ -21,6 +23,8 @@ export interface TeamMember { joined_at: string; public_key: string; role_ids: string[]; + permission_allow?: number; + permission_deny?: number; is_online?: boolean; /** An older server (no migration 035) omits this. Never render a bare "@" when absent. */ handle?: string; From ca37258c20ab0e44f8cfd811fa615a472949e7ef Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 08:55:55 +0000 Subject: [PATCH 02/21] fix(perms): strengthen deny-precedence test, fix team-fallback masks, remove stale comment --- src/services/permissions.test.ts | 15 ++++++++++++--- src/services/permissions.ts | 3 +-- src/services/teamDataManager.ts | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/services/permissions.test.ts b/src/services/permissions.test.ts index f8a7a9206..86f6b87be 100644 --- a/src/services/permissions.test.ts +++ b/src/services/permissions.test.ts @@ -100,13 +100,13 @@ describe("effectivePermissions with member overrides", () => { ).toBe(PERM_BITS.CONNECT); }); - it("lets deny win when a bit is in both masks", () => { + it("lets deny win when a role-granted bit is in both allow and deny masks", () => { expect( effectivePermissions( - { role_ids: [], permission_allow: PERM_BITS.CONNECT, permission_deny: PERM_BITS.CONNECT }, + { role_ids: ["r1"], permission_allow: PERM_BITS.VIEW_SECRETS, permission_deny: PERM_BITS.VIEW_SECRETS }, roles, ), - ).toBe(0); + ).toBe(PERM_BITS.CONNECT); }); it("grants an allowed bit to a member holding no roles", () => { @@ -115,3 +115,12 @@ describe("effectivePermissions with member overrides", () => { ).toBe(PERM_BITS.CONNECT); }); }); + +test("resolveCan uses team-level deny in the team fallback (before membersByTeam loads)", () => { + const s = snap({ + teams: [{ id: "t1", name: "t1", owner_id: "o", owner_tier: "team", created_at: "", role_ids: ["r1"], permission_deny: PERM_BITS.VIEW_SECRETS }], + rolesByTeam: { t1: [role("r1", PERM_BITS.VIEW_SECRETS | PERM_BITS.CONNECT)] }, + }); + expect(resolveCan(s, "VIEW_SECRETS", "t1")).toBe(false); + expect(resolveCan(s, "CONNECT", "t1")).toBe(true); +}); diff --git a/src/services/permissions.ts b/src/services/permissions.ts index 4e39320d9..4de809732 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -41,7 +41,6 @@ export const PERM_BITS: Record = { EDIT_SNIPPETS: 1 << 16, // 65536 }; -/** OR together all permission bits for a member's assigned roles. */ export function effectivePermissions( member: { role_ids: string[]; permission_allow?: number; permission_deny?: number }, roles: TeamRole[], @@ -99,7 +98,7 @@ export function resolveCan( const myTeam = snapshot.teams.find((t) => t.id === teamId); if (!myTeam || roles.length === 0) return false; - return (effectivePermissions({ role_ids: myTeam.role_ids }, roles) & PERM_BITS[permission]) !== 0; + return (effectivePermissions(myTeam, roles) & PERM_BITS[permission]) !== 0; } /** diff --git a/src/services/teamDataManager.ts b/src/services/teamDataManager.ts index 403be9b3f..71bad8773 100644 --- a/src/services/teamDataManager.ts +++ b/src/services/teamDataManager.ts @@ -110,7 +110,7 @@ function applyFirstViewNav(teamId: string): void { const team = teams.find((t) => t.id === teamId); const roles = rolesByTeam[teamId]; if (!team || !roles || roles.length === 0) return; - const nav = firstViewNav(effectivePermissions({ role_ids: team.role_ids }, roles)); + const nav = firstViewNav(effectivePermissions(team, roles)); if (isMobileShell()) { const { tab, screen } = mobileFirstViewTarget(nav); useMobileNavStore.getState().setTab(tab); From 8fc692e947515891df3d662928afe97fdc213744 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 09:19:39 +0000 Subject: [PATCH 03/21] fix(vault): apply permission overrides to the keychain role mirror --- src/stores/teamStore.cacheVaultRoles.test.ts | 40 ++++++++++++++++++++ src/stores/teamStore.ts | 15 ++++---- 2 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 src/stores/teamStore.cacheVaultRoles.test.ts diff --git a/src/stores/teamStore.cacheVaultRoles.test.ts b/src/stores/teamStore.cacheVaultRoles.test.ts new file mode 100644 index 000000000..8f7267800 --- /dev/null +++ b/src/stores/teamStore.cacheVaultRoles.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { PERM_BITS } from "@/services/permissions"; + +const invoke = vi.fn().mockResolvedValue(undefined); +vi.mock("@tauri-apps/api/core", () => ({ invoke: (...a: unknown[]) => invoke(...a) })); +vi.mock("@/services/teamService", () => ({ listRoles: vi.fn() })); + +describe("cacheVaultRoles", () => { + beforeEach(() => invoke.mockClear()); + + it("writes role bits minus the caller's denied bits", async () => { + const { cacheVaultRoles } = await import("@/stores/teamStore"); + await cacheVaultRoles( + [{ + id: "t1", name: "T", owner_id: "u1", owner_tier: "pro", created_at: "", + role_ids: ["r1"], permission_allow: 0, permission_deny: PERM_BITS.EDIT_CONNECTIONS, + }] as never, + { t1: [{ id: "r1", team_id: "t1", name: "editor", permissions: PERM_BITS.EDIT_CONNECTIONS | PERM_BITS.CONNECT, is_builtin: true, position: 2, created_at: "" }] }, + () => {}, + ); + + const written = JSON.parse(invoke.mock.calls[0][1].value as string); + expect(written.t1).toBe(PERM_BITS.CONNECT); + }); + + it("writes role bits plus the caller's allowed bits", async () => { + const { cacheVaultRoles } = await import("@/stores/teamStore"); + await cacheVaultRoles( + [{ + id: "t1", name: "T", owner_id: "u1", owner_tier: "pro", created_at: "", + role_ids: ["r1"], permission_allow: PERM_BITS.EDIT_KEYS, permission_deny: 0, + }] as never, + { t1: [{ id: "r1", team_id: "t1", name: "connect", permissions: PERM_BITS.CONNECT, is_builtin: true, position: 4, created_at: "" }] }, + () => {}, + ); + + const written = JSON.parse(invoke.mock.calls[0][1].value as string); + expect(written.t1).toBe(PERM_BITS.CONNECT | PERM_BITS.EDIT_KEYS); + }); +}); diff --git a/src/stores/teamStore.ts b/src/stores/teamStore.ts index b0714ec34..d611a5db4 100644 --- a/src/stores/teamStore.ts +++ b/src/stores/teamStore.ts @@ -41,17 +41,17 @@ interface TeamStore { } /** - * Mirrors {teamId -> the union of the user's role permission bits} into the - * keychain for the Rust vault-write check. Bits, not role names: a team using a - * custom-named role with write permissions would be denied locally by a name - * match even though the server allows it. The union mirrors the server's - * `bit_or` over every assigned role. + * Mirrors {teamId -> the user's effective permission bits (role union with + * allow/deny overrides applied)} into the keychain for the Rust vault-write + * check. Bits, not role names: a team using a custom-named role with write + * permissions would be denied locally by a name match even though the server + * allows it. * * A team whose roles can't be resolved is left out of the map rather than * written as "no permissions" — the server stays authoritative, and guessing * would lock the user out of a vault they can write to. */ -async function cacheVaultRoles( +export async function cacheVaultRoles( teams: Team[], rolesByTeam: Record, onRolesLoaded: (teamId: string, roles: TeamRole[]) => void, @@ -72,7 +72,8 @@ async function cacheVaultRoles( .map((rid) => roles!.find((r) => r.id === rid)?.permissions) .filter((p): p is number => typeof p === "number"); if (resolved.length < t.role_ids.length) return; - bits[t.id] = resolved.reduce((acc, p) => acc | p, 0); + bits[t.id] = (resolved.reduce((acc, p) => acc | p, 0) | (t.permission_allow ?? 0)) + & ~(t.permission_deny ?? 0); }), ); await invoke("keychain_set", { From 77d64873f09b5ed2886e52ab74093a49cd7418dd Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 09:46:28 +0000 Subject: [PATCH 04/21] refactor(vault): reuse effectivePermissions for the keychain role mirror --- src/stores/teamStore.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stores/teamStore.ts b/src/stores/teamStore.ts index d611a5db4..3b5679e36 100644 --- a/src/stores/teamStore.ts +++ b/src/stores/teamStore.ts @@ -3,6 +3,7 @@ import { persist } from "zustand/middleware"; import { invoke } from "@tauri-apps/api/core"; import * as api from "@/services/teamService"; import { logFailure } from "@/lib/logger"; +import { effectivePermissions } from "@/services/permissions"; import type { Team, TeamMember, TeamRole, PendingInvitation, MyPendingInvitation } from "@/services/teamService"; export type { Team, TeamMember, TeamRole, PendingInvitation, MyPendingInvitation }; @@ -72,8 +73,7 @@ export async function cacheVaultRoles( .map((rid) => roles!.find((r) => r.id === rid)?.permissions) .filter((p): p is number => typeof p === "number"); if (resolved.length < t.role_ids.length) return; - bits[t.id] = (resolved.reduce((acc, p) => acc | p, 0) | (t.permission_allow ?? 0)) - & ~(t.permission_deny ?? 0); + bits[t.id] = effectivePermissions(t, roles!); }), ); await invoke("keychain_set", { From e364627cc9e15673277b11ee8991ae9c13fe6d4d Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 09:52:00 +0000 Subject: [PATCH 05/21] feat(members): add the set-member-permissions service call and store action --- src/i18n/locales/en/common.json | 2 ++ src/i18n/locales/fr/common.json | 2 ++ src/i18n/locales/ru/common.json | 2 ++ src/i18n/locales/zh/common.json | 2 ++ src/services/teamService.ts | 18 +++++++++++ .../teamStore.setMemberPermissions.test.ts | 32 +++++++++++++++++++ src/stores/teamStore.ts | 13 ++++++++ 7 files changed, 71 insertions(+) create mode 100644 src/stores/teamStore.setMemberPermissions.test.ts diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 53c4f421c..580af76bf 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -120,6 +120,8 @@ "failedToRemoveMember": "Failed to remove member: {{status}}", "failedToAssignRole": "Failed to assign role: {{status}}", "failedToRemoveRole": "Failed to remove role: {{status}}", + "insufficientPermissionSetMemberPermissions": "You do not have permission to change this member's permissions", + "failedToSetMemberPermissions": "Failed to set member permissions ({{status}})", "failedToCreateRole": "Failed to create role: {{status}}", "failedToUpdateRole": "Failed to update role: {{status}}", "failedToDeleteRole": "Failed to delete role: {{status}}", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index eae8096d3..512addd10 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -120,6 +120,8 @@ "failedToRemoveMember": "Échec de la suppression du membre : {{status}}", "failedToAssignRole": "Échec de l'attribution du rôle : {{status}}", "failedToRemoveRole": "Échec du retrait du rôle : {{status}}", + "insufficientPermissionSetMemberPermissions": "Vous n'avez pas la permission de modifier les permissions de ce membre", + "failedToSetMemberPermissions": "Échec de la définition des permissions du membre ({{status}})", "failedToCreateRole": "Échec de la création du rôle : {{status}}", "failedToUpdateRole": "Échec de la mise à jour du rôle : {{status}}", "failedToDeleteRole": "Échec de la suppression du rôle : {{status}}", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index bd4cc39c6..3393a580e 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -120,6 +120,8 @@ "failedToRemoveMember": "Не удалось удалить участника: {{status}}", "failedToAssignRole": "Не удалось назначить роль: {{status}}", "failedToRemoveRole": "Не удалось удалить роль: {{status}}", + "insufficientPermissionSetMemberPermissions": "У вас нет прав на изменение разрешений этого участника", + "failedToSetMemberPermissions": "Не удалось задать разрешения участника ({{status}})", "failedToCreateRole": "Не удалось создать роль: {{status}}", "failedToUpdateRole": "Не удалось обновить роль: {{status}}", "failedToDeleteRole": "Не удалось удалить роль: {{status}}", diff --git a/src/i18n/locales/zh/common.json b/src/i18n/locales/zh/common.json index aa0a201fd..b19cf5926 100644 --- a/src/i18n/locales/zh/common.json +++ b/src/i18n/locales/zh/common.json @@ -120,6 +120,8 @@ "failedToRemoveMember": "移除成员失败：{{status}}", "failedToAssignRole": "分配角色失败：{{status}}", "failedToRemoveRole": "移除角色失败：{{status}}", + "insufficientPermissionSetMemberPermissions": "您没有权限修改该成员的权限", + "failedToSetMemberPermissions": "设置成员权限失败（{{status}}）", "failedToCreateRole": "创建角色失败：{{status}}", "failedToUpdateRole": "更新角色失败：{{status}}", "failedToDeleteRole": "删除角色失败：{{status}}", diff --git a/src/services/teamService.ts b/src/services/teamService.ts index 6aa3bfa7b..a60cc7428 100644 --- a/src/services/teamService.ts +++ b/src/services/teamService.ts @@ -205,6 +205,24 @@ export async function removeMemberRole( } } +export async function setMemberPermissions( + teamId: string, + userId: string, + allow: number, + deny: number, +): Promise { + const serverUrl = await getServerUrl(); + if (!serverUrl) throw new Error(i18n.t("common.error.notConnectedToServer")); + const res = await fetchAuth(`${serverUrl}/v1/teams/${teamId}/members/${userId}/permissions`, { + method: "PUT", + body: JSON.stringify({ allow, deny }), + }); + if (!res.ok) { + if (res.status === 403) throw new Error(i18n.t("common.error.insufficientPermissionSetMemberPermissions")); + throw new Error(i18n.t("common.error.failedToSetMemberPermissions", { status: res.status })); + } +} + // ─── Roles CRUD ─────────────────────────────────────────────────────────────── export async function listRoles(teamId: string): Promise { diff --git a/src/stores/teamStore.setMemberPermissions.test.ts b/src/stores/teamStore.setMemberPermissions.test.ts new file mode 100644 index 000000000..81f795cba --- /dev/null +++ b/src/stores/teamStore.setMemberPermissions.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { PERM_BITS } from "@/services/permissions"; + +const setMemberPermissions = vi.fn().mockResolvedValue(undefined); +vi.mock("@/services/teamService", async (orig) => ({ + ...(await orig>()), + setMemberPermissions: (...a: unknown[]) => setMemberPermissions(...a), +})); + +describe("teamStore.setMemberPermissions", () => { + beforeEach(() => setMemberPermissions.mockClear()); + + it("updates the cached member optimistically", async () => { + const { useTeamStore } = await import("@/stores/teamStore"); + useTeamStore.setState({ + membersByTeam: { + t1: [{ + team_id: "t1", user_id: "u1", handle: "alice", public_key: "k", + invited_by_display_name: null, joined_at: "", role_ids: [], + permission_allow: 0, permission_deny: 0, + }], + }, + } as never); + + await useTeamStore.getState().setMemberPermissions("t1", "u1", PERM_BITS.CONNECT, PERM_BITS.VIEW_SECRETS); + + expect(setMemberPermissions).toHaveBeenCalledWith("t1", "u1", PERM_BITS.CONNECT, PERM_BITS.VIEW_SECRETS); + const member = useTeamStore.getState().membersByTeam.t1[0]; + expect(member.permission_allow).toBe(PERM_BITS.CONNECT); + expect(member.permission_deny).toBe(PERM_BITS.VIEW_SECRETS); + }); +}); diff --git a/src/stores/teamStore.ts b/src/stores/teamStore.ts index 3b5679e36..92a3d5aa6 100644 --- a/src/stores/teamStore.ts +++ b/src/stores/teamStore.ts @@ -39,6 +39,7 @@ interface TeamStore { deleteRole: (teamId: string, roleId: string) => Promise; assignMemberRole: (teamId: string, userId: string, roleId: string) => Promise; removeMemberRole: (teamId: string, userId: string, roleId: string) => Promise; + setMemberPermissions: (teamId: string, userId: string, allow: number, deny: number) => Promise; } /** @@ -268,6 +269,18 @@ export const useTeamStore = create()( })); }, + setMemberPermissions: async (teamId, userId, allow, deny) => { + await api.setMemberPermissions(teamId, userId, allow, deny); + set((s) => ({ + membersByTeam: { + ...s.membersByTeam, + [teamId]: (s.membersByTeam[teamId] ?? []).map((m) => + m.user_id === userId ? { ...m, permission_allow: allow, permission_deny: deny } : m, + ), + }, + })); + }, + getActiveMembers: () => { const { activeTeamId, membersByTeam } = get(); if (!activeTeamId) return []; From f884fce4af20d392428fb7e236c6cc5f0c7de34f Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 10:00:31 +0000 Subject: [PATCH 06/21] test(members): pin the set-member-permissions failure path --- .../teamStore.setMemberPermissions.test.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/stores/teamStore.setMemberPermissions.test.ts b/src/stores/teamStore.setMemberPermissions.test.ts index 81f795cba..102f5033a 100644 --- a/src/stores/teamStore.setMemberPermissions.test.ts +++ b/src/stores/teamStore.setMemberPermissions.test.ts @@ -29,4 +29,26 @@ describe("teamStore.setMemberPermissions", () => { expect(member.permission_allow).toBe(PERM_BITS.CONNECT); expect(member.permission_deny).toBe(PERM_BITS.VIEW_SECRETS); }); + + it("leaves the cache unchanged when the service call rejects", async () => { + const { useTeamStore } = await import("@/stores/teamStore"); + useTeamStore.setState({ + membersByTeam: { + t1: [{ + team_id: "t1", user_id: "u1", handle: "alice", public_key: "k", + invited_by_display_name: null, joined_at: "", role_ids: [], + permission_allow: 0, permission_deny: 0, + }], + }, + } as never); + setMemberPermissions.mockRejectedValueOnce(new Error("boom")); + + await expect( + useTeamStore.getState().setMemberPermissions("t1", "u1", PERM_BITS.CONNECT, PERM_BITS.VIEW_SECRETS), + ).rejects.toThrow("boom"); + + const member = useTeamStore.getState().membersByTeam.t1[0]; + expect(member.permission_allow).toBe(0); + expect(member.permission_deny).toBe(0); + }); }); From 7a812cc2de7475bcd611d484ff8acc1a62530993 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 10:07:12 +0000 Subject: [PATCH 07/21] refactor(members): share one reversible-action wrapper between both role toggles --- .../members/panels/MemberDetailPanel.tsx | 73 +++++++++---------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index 7312304af..71fb66c79 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -27,8 +27,6 @@ export function MemberDetailPanel({ member, isMe, teamId, teamRoles, canManageMembers, isTargetOwner, onClose, onUpdated, }: MemberDetailPanelProps) { const { t } = useTranslation(); - const assignMemberRole = useTeamStore((s) => s.assignMemberRole); - const removeMemberRole = useTeamStore((s) => s.removeMemberRole); const push = useHistoryStore((s) => s.push); const [error, setError] = useState(""); @@ -43,51 +41,50 @@ export function MemberDetailPanel({ // offering Leave to an owner would promise something that 403s. const canLeave = isMe && !isTargetOwner; + const runReversible = async (opts: { + pending: string; + success: string; + label: string; + run: () => Promise; + undo: () => Promise; + redo: () => Promise; + }) => { + await runTeamAction({ pending: opts.pending, success: opts.success, run: opts.run }); + push({ + label: opts.label, + undo: async () => { await opts.undo(); onUpdated(); }, + redo: async () => { await opts.redo(); onUpdated(); }, + }); + }; + const handleToggleRole = async (role: TeamRole) => { const hasRole = member.role_ids.includes(role.id); - // Block removing the owner role from an owner if (hasRole && isTargetOwner && role.is_builtin && role.name === "owner") { setError(t("members.error.cannotRemoveOwnerRole")); return; } + const store = useTeamStore.getState(); + const assign = () => store.assignMemberRole(teamId, member.user_id, role.id); + const remove = () => store.removeMemberRole(teamId, member.user_id, role.id); + setToggling(role.id); setError(""); try { - if (hasRole) { - await runTeamAction({ - pending: t("members.toast.removingRoleFrom", { role: role.name, name: member.handle }), - success: t("members.toast.roleRemovedFrom", { role: role.name, name: member.handle }), - run: () => removeMemberRole(teamId, member.user_id, role.id), - }); - push({ - label: t("members.history.removeRole", { name: member.handle }), - undo: async () => { - await useTeamStore.getState().assignMemberRole(teamId, member.user_id, role.id); - onUpdated(); - }, - redo: async () => { - await useTeamStore.getState().removeMemberRole(teamId, member.user_id, role.id); - onUpdated(); - }, - }); - } else { - await runTeamAction({ - pending: t("members.toast.assigningRoleTo", { role: role.name, name: member.handle }), - success: t("members.toast.roleAssignedTo", { role: role.name, name: member.handle }), - run: () => assignMemberRole(teamId, member.user_id, role.id), - }); - push({ - label: t("members.history.assignRole", { name: member.handle }), - undo: async () => { - await useTeamStore.getState().removeMemberRole(teamId, member.user_id, role.id); - onUpdated(); - }, - redo: async () => { - await useTeamStore.getState().assignMemberRole(teamId, member.user_id, role.id); - onUpdated(); - }, - }); - } + await runReversible( + hasRole + ? { + pending: t("members.toast.removingRoleFrom", { role: role.name, name: member.handle }), + success: t("members.toast.roleRemovedFrom", { role: role.name, name: member.handle }), + label: t("members.history.removeRole", { name: member.handle }), + run: remove, undo: assign, redo: remove, + } + : { + pending: t("members.toast.assigningRoleTo", { role: role.name, name: member.handle }), + success: t("members.toast.roleAssignedTo", { role: role.name, name: member.handle }), + label: t("members.history.assignRole", { name: member.handle }), + run: assign, undo: remove, redo: assign, + }, + ); onUpdated(); setJustToggled(role.id); setTimeout(() => setJustToggled(null), 700); From f54662c8d64811160f7534c430b2720200571151 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 14:22:19 +0000 Subject: [PATCH 08/21] feat(members): add a tri-state permission override row --- .../panels/PermissionOverrideRow.test.tsx | 78 ++++++++++++++++ .../members/panels/PermissionOverrideRow.tsx | 91 +++++++++++++++++++ src/components/members/roleChips.tsx | 12 ++- src/i18n/locales/en/members.json | 14 +++ src/i18n/locales/fr/members.json | 14 +++ src/i18n/locales/ru/members.json | 14 +++ src/i18n/locales/zh/members.json | 14 +++ 7 files changed, 232 insertions(+), 5 deletions(-) create mode 100644 src/components/members/panels/PermissionOverrideRow.test.tsx create mode 100644 src/components/members/panels/PermissionOverrideRow.tsx diff --git a/src/components/members/panels/PermissionOverrideRow.test.tsx b/src/components/members/panels/PermissionOverrideRow.test.tsx new file mode 100644 index 000000000..535afc887 --- /dev/null +++ b/src/components/members/panels/PermissionOverrideRow.test.tsx @@ -0,0 +1,78 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup, fireEvent } from "@testing-library/react"; + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (k: string, o?: { roles?: string }) => (o?.roles ? `${k} ${o.roles}` : k), + }), + initReactI18next: { type: "3rdParty", init: () => {} }, +})); + +import { PERM_BITS } from "@/services/permissions"; +import { + PermissionOverrideRow, overrideStateOf, applyOverrideState, +} from "./PermissionOverrideRow"; + +afterEach(cleanup); + +describe("overrideStateOf", () => { + it("reads inherit when the bit is in neither mask", () => { + expect(overrideStateOf("CONNECT", 0, 0)).toBe("inherit"); + }); + it("reads allow when the bit is in the allow mask", () => { + expect(overrideStateOf("CONNECT", PERM_BITS.CONNECT, 0)).toBe("allow"); + }); + it("reads deny when the bit is in the deny mask", () => { + expect(overrideStateOf("CONNECT", 0, PERM_BITS.CONNECT)).toBe("deny"); + }); + it("prefers deny when the bit is in both", () => { + expect(overrideStateOf("CONNECT", PERM_BITS.CONNECT, PERM_BITS.CONNECT)).toBe("deny"); + }); +}); + +describe("applyOverrideState", () => { + it("moves a bit from allow to deny without leaving it in both", () => { + expect(applyOverrideState("CONNECT", PERM_BITS.CONNECT, 0, "deny")).toEqual({ + allow: 0, deny: PERM_BITS.CONNECT, + }); + }); + it("clears the bit from both masks on inherit", () => { + expect(applyOverrideState("CONNECT", 0, PERM_BITS.CONNECT, "inherit")).toEqual({ + allow: 0, deny: 0, + }); + }); + it("leaves other bits untouched", () => { + const { allow, deny } = applyOverrideState("CONNECT", PERM_BITS.EDIT_KEYS, PERM_BITS.VIEW_SECRETS, "allow"); + expect(allow).toBe(PERM_BITS.EDIT_KEYS | PERM_BITS.CONNECT); + expect(deny).toBe(PERM_BITS.VIEW_SECRETS); + }); +}); + +describe("PermissionOverrideRow", () => { + const base = { + permission: "CONNECT" as const, + state: "inherit" as const, + inheritedFrom: ["editor"], + inheritedGrants: true, + disabled: false, + }; + + it("names the roles the bit is inherited from", () => { + render(); + expect(screen.getByText(/editor/)).toBeTruthy(); + }); + + it("emits the chosen state", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("radio", { name: /deny/i })); + expect(onChange).toHaveBeenCalledWith("deny"); + }); + + it("does not emit while disabled", () => { + const onChange = vi.fn(); + render(); + fireEvent.click(screen.getByRole("radio", { name: /deny/i })); + expect(onChange).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/members/panels/PermissionOverrideRow.tsx b/src/components/members/panels/PermissionOverrideRow.tsx new file mode 100644 index 000000000..daa38f617 --- /dev/null +++ b/src/components/members/panels/PermissionOverrideRow.tsx @@ -0,0 +1,91 @@ +import { useTranslation } from "react-i18next"; +import { Icon } from "@iconify/react"; +import { PERM_BITS, type Permission } from "@/services/permissions"; +import { permissionLabel } from "@/components/members/roleChips"; + +export type OverrideState = "deny" | "inherit" | "allow"; + +const STATES: { value: OverrideState; icon: string; color: string }[] = [ + { value: "deny", icon: "lucide:x", color: "var(--t-status-error)" }, + { value: "inherit", icon: "lucide:minus", color: "var(--t-text-dim)" }, + { value: "allow", icon: "lucide:check", color: "#34d399" }, +]; + +export function overrideStateOf(permission: Permission, allow: number, deny: number): OverrideState { + const bit = PERM_BITS[permission]; + if ((deny & bit) !== 0) return "deny"; + if ((allow & bit) !== 0) return "allow"; + return "inherit"; +} + +export function applyOverrideState( + permission: Permission, + allow: number, + deny: number, + next: OverrideState, +): { allow: number; deny: number } { + const bit = PERM_BITS[permission]; + const clearedAllow = allow & ~bit; + const clearedDeny = deny & ~bit; + if (next === "allow") return { allow: clearedAllow | bit, deny: clearedDeny }; + if (next === "deny") return { allow: clearedAllow, deny: clearedDeny | bit }; + return { allow: clearedAllow, deny: clearedDeny }; +} + +export interface PermissionOverrideRowProps { + permission: Permission; + state: OverrideState; + inheritedFrom: string[]; + inheritedGrants: boolean; + disabled: boolean; + onChange: (next: OverrideState) => void; +} + +export function PermissionOverrideRow({ + permission, state, inheritedFrom, inheritedGrants, disabled, onChange, +}: PermissionOverrideRowProps) { + const { t } = useTranslation(); + const label = permissionLabel(t, permission); + + const source = inheritedFrom.length > 0 + ? t("members.permissions.inheritedFrom", { roles: inheritedFrom.join(", ") }) + : t("members.permissions.notGranted"); + + return ( +
+
+

{label}

+

+ {source} + {state === "inherit" && ( + <> · {inheritedGrants ? t("members.permissions.effectiveAllowed") : t("members.permissions.effectiveDenied")} + )} +

+
+
+ {STATES.map((s) => { + const active = s.value === state; + return ( + + ); + })} +
+
+ ); +} diff --git a/src/components/members/roleChips.tsx b/src/components/members/roleChips.tsx index 9ab7a49cb..b251fadb5 100644 --- a/src/components/members/roleChips.tsx +++ b/src/components/members/roleChips.tsx @@ -1,10 +1,11 @@ import { Icon } from "@iconify/react"; import { useTranslation } from "react-i18next"; +import type { TFunction } from "i18next"; import type { TeamRole } from "@/services/teamService"; // Both from the service, not the hook: @/hooks/usePermission pulls teamService // and i18n in at runtime, which every consumer of this leaf module would then // have to mock. -import { PERM_BITS, PERM_META } from "@/services/permissions"; +import { PERM_BITS, PERM_META, type Permission } from "@/services/permissions"; export const ROLE_META: Record = { owner: { label: "Owner", color: "#a78bfa", bg: "rgba(167,139,250,0.12)" }, @@ -24,6 +25,10 @@ export function roleChipColors(name: string, override?: string | null, fallback return { meta, color, bg: meta?.bg ?? `${color}1a` }; } +export function permissionLabel(t: TFunction, permission: Permission): string { + return t(`members.permission.${permission}`, { defaultValue: PERM_META[permission]?.label ?? permission }); +} + /** * The precise half of the role explanation: exactly which permissions a role * grants, derived from its bits. The plain-language counterpart is @@ -33,10 +38,7 @@ export function RolePermissionTooltip({ role, color }: { role: TeamRole; color: const { t } = useTranslation(); const permLabels = Object.entries(PERM_BITS) .filter(([, bit]) => (role.permissions & bit) !== 0) - .map(([p]) => { - const key = p as keyof typeof PERM_META; - return t(`members.permission.${key}`, { defaultValue: PERM_META[key]?.label ?? p }); - }); + .map(([p]) => permissionLabel(t, p as Permission)); if (permLabels.length === 0) return null; diff --git a/src/i18n/locales/en/members.json b/src/i18n/locales/en/members.json index c8082f252..a4562a95b 100644 --- a/src/i18n/locales/en/members.json +++ b/src/i18n/locales/en/members.json @@ -51,6 +51,20 @@ "JOIN_TERMINAL_SESSION": "Join sessions", "VIEW_TERMINAL_SESSIONS": "View sessions" }, + "permissions": { + "title": "Permissions", + "inheritedFrom": "From {{roles}}", + "notGranted": "No role grants this", + "effectiveAllowed": "allowed", + "effectiveDenied": "denied", + "state": { "deny": "Deny", "inherit": "Inherit", "allow": "Allow" }, + "readOnlyNoManage": "You need the Manage members permission", + "readOnlyOwner": "An owner's permissions cannot be overridden", + "readOnlySelf": "You cannot change your own permissions", + "readOnlyHigherRole": "This member holds a role at or above yours", + "readOnlyNotHeld": "You cannot grant a permission you do not hold", + "overrideMarker": "Has permission overrides" + }, "convert": { "title": "Share \"{{vault}}\" with other people?", "body": "This turns your private vault into a team vault. Everything already in it moves across, so the people you add see it too.", diff --git a/src/i18n/locales/fr/members.json b/src/i18n/locales/fr/members.json index cf1f3048f..241762a2a 100644 --- a/src/i18n/locales/fr/members.json +++ b/src/i18n/locales/fr/members.json @@ -51,6 +51,20 @@ "JOIN_TERMINAL_SESSION": "Rejoindre des sessions", "VIEW_TERMINAL_SESSIONS": "Voir les sessions" }, + "permissions": { + "title": "Permissions", + "inheritedFrom": "Via {{roles}}", + "notGranted": "Aucun rôle ne l'accorde", + "effectiveAllowed": "autorisé", + "effectiveDenied": "refusé", + "state": { "deny": "Refuser", "inherit": "Hériter", "allow": "Autoriser" }, + "readOnlyNoManage": "Vous avez besoin de la permission Gérer les membres", + "readOnlyOwner": "Les permissions d'un propriétaire ne peuvent pas être modifiées", + "readOnlySelf": "Vous ne pouvez pas modifier vos propres permissions", + "readOnlyHigherRole": "Ce membre a un rôle égal ou supérieur au vôtre", + "readOnlyNotHeld": "Vous ne pouvez pas accorder une permission que vous n'avez pas", + "overrideMarker": "Permissions personnalisées" + }, "convert": { "title": "Partager « {{vault}} » avec d'autres personnes ?", "body": "Ceci transforme votre coffre-fort privé en coffre-fort d'équipe. Tout ce qu'il contient déjà y est transféré, et devient donc visible par les personnes que vous ajoutez.", diff --git a/src/i18n/locales/ru/members.json b/src/i18n/locales/ru/members.json index e4632c4e3..e02f1cf1c 100644 --- a/src/i18n/locales/ru/members.json +++ b/src/i18n/locales/ru/members.json @@ -53,6 +53,20 @@ "JOIN_TERMINAL_SESSION": "Присоединение к сессиям", "VIEW_TERMINAL_SESSIONS": "Просмотр сессий" }, + "permissions": { + "title": "Разрешения", + "inheritedFrom": "От {{roles}}", + "notGranted": "Ни одна роль не даёт этого", + "effectiveAllowed": "разрешено", + "effectiveDenied": "запрещено", + "state": { "deny": "Запретить", "inherit": "Наследовать", "allow": "Разрешить" }, + "readOnlyNoManage": "Требуется разрешение «Управление участниками»", + "readOnlyOwner": "Разрешения владельца изменить нельзя", + "readOnlySelf": "Нельзя изменить собственные разрешения", + "readOnlyHigherRole": "У этого участника роль не ниже вашей", + "readOnlyNotHeld": "Нельзя выдать разрешение, которого у вас нет", + "overrideMarker": "Есть переопределения разрешений" + }, "convert": { "title": "Предоставить общий доступ к \"{{vault}}\" другим людям?", "body": "Это превратит ваше личное хранилище в командное. Всё, что уже в нём есть, переносится туда, поэтому добавленные вами люди тоже это увидят.", diff --git a/src/i18n/locales/zh/members.json b/src/i18n/locales/zh/members.json index f5d694521..07b24fa24 100644 --- a/src/i18n/locales/zh/members.json +++ b/src/i18n/locales/zh/members.json @@ -51,6 +51,20 @@ "JOIN_TERMINAL_SESSION": "加入会话", "VIEW_TERMINAL_SESSIONS": "查看会话" }, + "permissions": { + "title": "权限", + "inheritedFrom": "来自 {{roles}}", + "notGranted": "没有角色授予此权限", + "effectiveAllowed": "已允许", + "effectiveDenied": "已拒绝", + "state": { "deny": "拒绝", "inherit": "继承", "allow": "允许" }, + "readOnlyNoManage": "需要「管理成员」权限", + "readOnlyOwner": "无法覆盖所有者的权限", + "readOnlySelf": "无法修改自己的权限", + "readOnlyHigherRole": "该成员的角色不低于你", + "readOnlyNotHeld": "无法授予你自己没有的权限", + "overrideMarker": "存在权限覆盖" + }, "convert": { "title": "要与他人共享 \"{{vault}}\" 吗？", "body": "这会将您的私有保险库转变为团队保险库。其中已有的所有内容都会一并转移，因此您添加的人也能看到。", From 088b9d014670d29b59b919a31ea5b726acc8b129 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 14:38:09 +0000 Subject: [PATCH 09/21] feat(members): edit per-member permissions from the member detail panel --- .../MembersPage.MemberDetailPanel.test.tsx | 137 +++++++++++++++++- src/components/members/MembersPage.tsx | 1 + .../members/panels/MemberDetailPanel.tsx | 93 +++++++++++- src/i18n/locales/en/members.json | 6 +- src/i18n/locales/fr/members.json | 6 +- src/i18n/locales/ru/members.json | 6 +- src/i18n/locales/zh/members.json | 6 +- 7 files changed, 249 insertions(+), 6 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index dc37beb70..d3402c5ee 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -1,6 +1,7 @@ import { test, expect, vi, beforeEach, afterEach } from "vitest"; -import { render, screen, cleanup, fireEvent, waitFor } from "@testing-library/react"; +import { render, screen, cleanup, fireEvent, waitFor, within } from "@testing-library/react"; import type { TeamMember, TeamRole } from "@/stores/teamStore"; +import { PERM_BITS } from "@/services/permissions"; const h = vi.hoisted(() => ({ assign: vi.fn(), @@ -9,6 +10,7 @@ const h = vi.hoisted(() => ({ addMemberById: vi.fn(), loadMembers: vi.fn(), push: vi.fn(), + setPerms: vi.fn(), })); vi.mock("react-i18next", () => ({ @@ -34,6 +36,7 @@ vi.mock("@/stores/teamStore", () => { removeMember: h.removeMember, addMemberById: h.addMemberById, loadMembers: h.loadMembers, + setMemberPermissions: h.setPerms, }; const useTeamStore = Object.assign( (sel: (s: typeof state) => unknown) => sel(state), @@ -90,6 +93,7 @@ beforeEach(() => { h.removeMember.mockResolvedValue(undefined); h.addMemberById.mockResolvedValue(undefined); h.loadMembers.mockResolvedValue(undefined); + h.setPerms.mockResolvedValue(undefined); baseProps.onClose = vi.fn(); baseProps.onUpdated = vi.fn(); }); @@ -196,3 +200,134 @@ test("remove undo closure: re-adds member, reassigns each snapshot role, reloads expect(h.addMemberById.mock.invocationCallOrder[0]).toBeLessThan(h.assign.mock.invocationCallOrder[0]); expect(h.assign.mock.invocationCallOrder[0]).toBeLessThan(h.loadMembers.mock.invocationCallOrder[0]); }); + +// ── Permission overrides ────────────────────────────────────────────────── + +const viewerRole: TeamRole = { + id: "r-viewer", team_id: "t1", name: "viewer-role", is_builtin: false, + permissions: PERM_BITS.MANAGE_MEMBERS, position: 0, created_at: "", +}; +const targetRole: TeamRole = { + id: "r-target", team_id: "t1", name: "target-role", is_builtin: false, + permissions: 0, position: 2, created_at: "", +}; +const highRole: TeamRole = { + id: "r-high", team_id: "t1", name: "high-role", is_builtin: false, + permissions: 0, position: 0, created_at: "", +}; + +const viewerMember: TeamMember = { + team_id: "t1", user_id: "uviewer", handle: "me", public_key: "k", + invited_by_display_name: null, joined_at: "2024-01-01T00:00:00Z", + role_ids: ["r-viewer"], permission_allow: 0, permission_deny: 0, +}; + +const targetMember: TeamMember = { + team_id: "t1", user_id: "u2", handle: "alice", public_key: "k", + invited_by_display_name: null, joined_at: "2024-01-01T00:00:00Z", + role_ids: ["r-target"], permission_allow: 0, permission_deny: 0, +}; + +function permProps(overrides: Partial<{ + member: TeamMember; isMe: boolean; teamRoles: TeamRole[]; + canManageMembers: boolean; isTargetOwner: boolean; viewer: TeamMember; + onUpdated: () => void; +}> = {}) { + return { + member: targetMember, + isMe: false, + teamId: "t1", + teamRoles: [viewerRole, targetRole], + canManageMembers: true, + isTargetOwner: false, + viewer: viewerMember, + onClose: vi.fn(), + onUpdated: vi.fn(), + ...overrides, + }; +} + +test("permission overrides: renders one row per permission and sends the new masks", async () => { + render(); + + expect(screen.getAllByRole("radiogroup")).toHaveLength(16); + + const row = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), + ); +}); + +test.each([ + { + label: "no manage-members permission", + key: "members.permissions.readOnlyNoManage", + overrides: { canManageMembers: false }, + }, + { + label: "target is an owner", + key: "members.permissions.readOnlyOwner", + overrides: { isTargetOwner: true }, + }, + { + label: "target is the viewer themselves", + key: "members.permissions.readOnlySelf", + overrides: { isMe: true }, + }, + { + label: "target holds a role at or above the viewer's", + key: "members.permissions.readOnlyHigherRole", + overrides: { member: { ...targetMember, role_ids: ["r-high"] }, teamRoles: [viewerRole, highRole] }, + }, + { + label: "target already carries an allow bit the viewer lacks", + key: "members.permissions.readOnlyNotHeld", + overrides: { member: { ...targetMember, permission_allow: PERM_BITS.CONNECT } }, + }, +])("read-only reason: $label", ({ overrides, key }) => { + render(); + + expect(screen.getByText(key)).toBeTruthy(); + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + expect((within(row).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(true); +}); + +test("hierarchy: a roleless target is not read-only", () => { + render(); + + expect(screen.queryByText("members.permissions.readOnlyHigherRole")).toBeNull(); + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + expect((within(row).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(false); +}); + +test("choosing allow on a bit the viewer lacks sends no request", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.CONNECT" }); + fireEvent.click(within(row).getByRole("radio", { name: /allow/i })); + + expect(await screen.findByText("members.permissions.readOnlyNotHeld")).toBeTruthy(); + expect(h.setPerms).not.toHaveBeenCalled(); +}); + +test("a write in flight disables the other rows too", async () => { + let resolveSet: () => void = () => {}; + h.setPerms.mockImplementation(() => new Promise((resolve) => { resolveSet = resolve; })); + + render(); + const row1 = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + const row2 = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + + fireEvent.click(within(row1).getByRole("radio", { name: /deny/i })); + + await waitFor(() => + expect((within(row2).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(true), + ); + + resolveSet(); + await waitFor(() => + expect((within(row2).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(false), + ); +}); diff --git a/src/components/members/MembersPage.tsx b/src/components/members/MembersPage.tsx index ef0d8de43..a509c7911 100644 --- a/src/components/members/MembersPage.tsx +++ b/src/components/members/MembersPage.tsx @@ -521,6 +521,7 @@ const vaultTabs = selectedVaultIds.length > 1 teamRoles={teamRoles} canManageMembers={canManageMembers} isTargetOwner={isOwnerMember(detailMember)} + viewer={myMember} onClose={() => setShowDetailPanel(false)} onUpdated={reload} /> diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index 71fb66c79..31b2f7eed 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -11,6 +11,10 @@ import { ROLE_META, RoleBlurb } from "@/components/members/roleChips"; import { RoleBadges } from "@/components/members/roleBadges"; import { OffboardingDialog } from "@/components/members/OffboardingDialog"; import type { DepartMode } from "@/services/teamOffboarding"; +import { PERM_BITS, PERM_META, effectivePermissions, type Permission } from "@/services/permissions"; +import { + PermissionOverrideRow, overrideStateOf, applyOverrideState, type OverrideState, +} from "./PermissionOverrideRow"; export interface MemberDetailPanelProps { member: TeamMember; @@ -19,12 +23,22 @@ export interface MemberDetailPanelProps { teamRoles: TeamRole[]; canManageMembers: boolean; isTargetOwner: boolean; + viewer?: TeamMember; onClose: () => void; onUpdated: () => void; } +/** Lower position = more authority; a role absent from `roles` is skipped. */ +function minRolePosition(roleIds: string[], roles: TeamRole[]): number | null { + return roleIds.reduce((min, rid) => { + const role = roles.find((r) => r.id === rid); + if (!role) return min; + return min === null || role.position < min ? role.position : min; + }, null); +} + export function MemberDetailPanel({ - member, isMe, teamId, teamRoles, canManageMembers, isTargetOwner, onClose, onUpdated, + member, isMe, teamId, teamRoles, canManageMembers, isTargetOwner, viewer, onClose, onUpdated, }: MemberDetailPanelProps) { const { t } = useTranslation(); const push = useHistoryStore((s) => s.push); @@ -34,6 +48,7 @@ export function MemberDetailPanel({ const [justToggled, setJustToggled] = useState(null); const [offboarding, setOffboarding] = useState(null); const [creatingRole, setCreatingRole] = useState(false); + const [overriding, setOverriding] = useState(false); const canChangeRoles = canManageMembers && !isMe; const canRemove = canManageMembers && !isTargetOwner && !isMe; @@ -95,6 +110,59 @@ export function MemberDetailPanel({ } }; + const allow = member.permission_allow ?? 0; + const deny = member.permission_deny ?? 0; + const viewerEffective = viewer ? effectivePermissions(viewer, teamRoles) : 0; + + const editablePermissions = (Object.keys(PERM_META) as Permission[]) + .filter((p) => p !== "CREATE_CUSTOM_ROLES"); + + const rolesGranting = (permission: Permission) => + teamRoles + .filter((r) => member.role_ids.includes(r.id) && (r.permissions & PERM_BITS[permission]) !== 0) + .map((r) => r.name); + + const readOnlyReason: string | null = (() => { + if (!canManageMembers) return t("members.permissions.readOnlyNoManage"); + if (isTargetOwner) return t("members.permissions.readOnlyOwner"); + if (isMe) return t("members.permissions.readOnlySelf"); + const viewerMin = viewer ? minRolePosition(viewer.role_ids, teamRoles) : null; + const targetMin = minRolePosition(member.role_ids, teamRoles); + const hierarchyFails = !viewer || viewerMin === null || (targetMin !== null && viewerMin >= targetMin); + if (hierarchyFails) return t("members.permissions.readOnlyHigherRole"); + if ((allow & ~viewerEffective) !== 0) return t("members.permissions.readOnlyNotHeld"); + return null; + })(); + + const write = (masks: { allow: number; deny: number }) => () => + useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); + + const handleOverride = async (permission: Permission, next: OverrideState) => { + if (next === "allow" && (viewerEffective & PERM_BITS[permission]) === 0) { + setError(t("members.permissions.readOnlyNotHeld")); + return; + } + const previous = { allow, deny }; + const updated = applyOverrideState(permission, allow, deny, next); + setError(""); + setOverriding(true); + try { + await runReversible({ + pending: t("members.toast.updatingPermissions", { name: member.handle }), + success: t("members.toast.permissionsUpdated", { name: member.handle }), + label: t("members.history.changePermissions", { name: member.handle }), + run: write(updated), + undo: write(previous), + redo: write(updated), + }); + onUpdated(); + } catch (e) { + setError(e instanceof Error ? e.message : t("members.error.failedToUpdatePermissions")); + } finally { + setOverriding(false); + } + }; + const joinedDate = new Date(member.joined_at).toLocaleDateString(undefined, { year: "numeric", month: "long", day: "numeric", }); @@ -177,6 +245,29 @@ export function MemberDetailPanel({ )} + {/* Permissions */} + + {readOnlyReason && ( +

{readOnlyReason}

+ )} +
+ {editablePermissions.map((permission) => { + const granting = rolesGranting(permission); + return ( + 0} + disabled={readOnlyReason !== null || overriding} + onChange={(next) => void handleOverride(permission, next)} + /> + ); + })} +
+ + {/* Info */}
diff --git a/src/i18n/locales/en/members.json b/src/i18n/locales/en/members.json index a4562a95b..024330736 100644 --- a/src/i18n/locales/en/members.json +++ b/src/i18n/locales/en/members.json @@ -140,11 +140,14 @@ "grantingKey": "Granting vault access to {{name}}…", "keyGranted": "{{name}} can now open the vault", "leavingTeam": "Leaving team...", - "leftTeam": "You left the team" + "leftTeam": "You left the team", + "updatingPermissions": "Updating {{name}}'s permissions…", + "permissionsUpdated": "Updated {{name}}'s permissions" }, "error": { "cannotRemoveOwnerRole": "Cannot remove the owner role from the team owner", "failedToUpdateRole": "Failed to update role", + "failedToUpdatePermissions": "Failed to update permissions", "failedToRemoveMember": "Failed to remove member", "failedToAddMember": "Failed to add member", "inviteFailed": "Could not invite {{name}} — {{reason}}", @@ -159,6 +162,7 @@ "history": { "removeRole": "Remove role: {{name}}", "assignRole": "Assign role: {{name}}", + "changePermissions": "Change {{name}}'s permissions", "remove": "Remove: {{name}}", "assignRoleBulk": "Assign role ×{{count}}", "removeRoleBulk": "Remove role ×{{count}}", diff --git a/src/i18n/locales/fr/members.json b/src/i18n/locales/fr/members.json index 241762a2a..5ef1d9586 100644 --- a/src/i18n/locales/fr/members.json +++ b/src/i18n/locales/fr/members.json @@ -140,11 +140,14 @@ "grantingKey": "Octroi de l'accès au coffre-fort à {{name}}…", "keyGranted": "{{name}} peut maintenant ouvrir le coffre-fort", "leavingTeam": "Départ de l'équipe…", - "leftTeam": "Vous avez quitté l'équipe" + "leftTeam": "Vous avez quitté l'équipe", + "updatingPermissions": "Mise à jour des permissions de {{name}}…", + "permissionsUpdated": "Permissions de {{name}} mises à jour" }, "error": { "cannotRemoveOwnerRole": "Impossible de retirer le rôle de propriétaire au propriétaire de l'équipe", "failedToUpdateRole": "Échec de la mise à jour du rôle", + "failedToUpdatePermissions": "Échec de la mise à jour des permissions", "failedToRemoveMember": "Échec de la suppression du membre", "failedToAddMember": "Échec de l'ajout du membre", "inviteFailed": "Impossible d'inviter {{name}} — {{reason}}", @@ -159,6 +162,7 @@ "history": { "removeRole": "Retirer le rôle : {{name}}", "assignRole": "Attribuer un rôle : {{name}}", + "changePermissions": "Modifier les permissions de {{name}}", "remove": "Retirer : {{name}}", "assignRoleBulk": "Attribuer un rôle ×{{count}}", "removeRoleBulk": "Retirer un rôle ×{{count}}", diff --git a/src/i18n/locales/ru/members.json b/src/i18n/locales/ru/members.json index e02f1cf1c..5602dca71 100644 --- a/src/i18n/locales/ru/members.json +++ b/src/i18n/locales/ru/members.json @@ -144,11 +144,14 @@ "grantingKey": "Предоставление доступа к хранилищу для {{name}}…", "keyGranted": "{{name}} теперь может открыть хранилище", "leavingTeam": "Выход из команды…", - "leftTeam": "Вы вышли из команды" + "leftTeam": "Вы вышли из команды", + "updatingPermissions": "Обновление разрешений {{name}}…", + "permissionsUpdated": "Разрешения {{name}} обновлены" }, "error": { "cannotRemoveOwnerRole": "Нельзя удалить роль владельца у владельца команды", "failedToUpdateRole": "Не удалось обновить роль", + "failedToUpdatePermissions": "Не удалось обновить разрешения", "failedToRemoveMember": "Не удалось удалить участника", "failedToAddMember": "Не удалось добавить участника", "inviteFailed": "Не удалось пригласить {{name}} — {{reason}}", @@ -163,6 +166,7 @@ "history": { "removeRole": "Удалить роль: {{name}}", "assignRole": "Назначить роль: {{name}}", + "changePermissions": "Изменить разрешения {{name}}", "remove": "Удалить: {{name}}", "assignRoleBulk": "Назначить роль ×{{count}}", "removeRoleBulk": "Удалить роль ×{{count}}", diff --git a/src/i18n/locales/zh/members.json b/src/i18n/locales/zh/members.json index 07b24fa24..46c839746 100644 --- a/src/i18n/locales/zh/members.json +++ b/src/i18n/locales/zh/members.json @@ -140,11 +140,14 @@ "grantingKey": "正在为 {{name}} 授予保险库访问权…", "keyGranted": "{{name}} 现在可以打开保险库", "leavingTeam": "正在退出团队…", - "leftTeam": "你已退出团队" + "leftTeam": "你已退出团队", + "updatingPermissions": "正在更新 {{name}} 的权限…", + "permissionsUpdated": "已更新 {{name}} 的权限" }, "error": { "cannotRemoveOwnerRole": "无法从团队所有者移除所有者角色", "failedToUpdateRole": "更新角色失败", + "failedToUpdatePermissions": "更新权限失败", "failedToRemoveMember": "移除成员失败", "failedToAddMember": "添加成员失败", "inviteFailed": "无法邀请 {{name}} — {{reason}}", @@ -159,6 +162,7 @@ "history": { "removeRole": "移除角色：{{name}}", "assignRole": "分配角色：{{name}}", + "changePermissions": "修改 {{name}} 的权限", "remove": "移除：{{name}}", "assignRoleBulk": "分配角色 ×{{count}}", "removeRoleBulk": "移除角色 ×{{count}}", From 094c8f8b51e3fa567afc199665c01b488bf30815 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 14:45:37 +0000 Subject: [PATCH 10/21] test(members): pin the fail-closed hierarchy guardrail --- .../MembersPage.MemberDetailPanel.test.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index d3402c5ee..89e1ef58a 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -302,6 +302,25 @@ test("hierarchy: a roleless target is not read-only", () => { expect((within(row).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(false); }); +// The hierarchy check must fail CLOSED (read-only) when the viewer side of the +// comparison cannot be resolved at all, not just when it loses the comparison. +test.each([ + { + label: "viewer is undefined", + overrides: { viewer: undefined }, + }, + { + label: "viewer holds no resolvable role", + overrides: { viewer: { ...viewerMember, role_ids: [] } }, + }, +])("hierarchy fails closed when $label", ({ overrides }) => { + render(); + + expect(screen.getByText("members.permissions.readOnlyHigherRole")).toBeTruthy(); + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + expect((within(row).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(true); +}); + test("choosing allow on a bit the viewer lacks sends no request", async () => { render(); From 51ff0a1df96a87ffa731a73bdfe4e07929c704ea Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 15:01:13 +0000 Subject: [PATCH 11/21] fix(members): let an admin clear an override bit they do not hold --- .../MembersPage.MemberDetailPanel.test.tsx | 22 +++++++++++++ .../members/panels/MemberDetailPanel.tsx | 33 +++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index 89e1ef58a..bcb96e484 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -258,6 +258,28 @@ test("permission overrides: renders one row per permission and sends the new mas await waitFor(() => expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), ); + + const entry = h.push.mock.calls[0][0] as { undo: () => Promise }; + await entry.undo(); + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0); +}); + +test("a notHeld lock enables exactly the offending row, not the others", async () => { + const member = { ...targetMember, permission_allow: PERM_BITS.CONNECT }; + render(); + + const connectRow = screen.getByRole("radiogroup", { name: "members.permission.CONNECT" }); + expect((within(connectRow).getByRole("radio", { name: /allow/i }) as HTMLButtonElement).disabled).toBe(false); + expect((within(connectRow).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(false); + + const otherRow = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + expect((within(otherRow).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(true); + + fireEvent.click(within(connectRow).getByRole("radio", { name: /inherit/i })); + + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0), + ); }); test.each([ diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index 31b2f7eed..36ec95eb6 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -122,18 +122,37 @@ export function MemberDetailPanel({ .filter((r) => member.role_ids.includes(r.id) && (r.permissions & PERM_BITS[permission]) !== 0) .map((r) => r.name); - const readOnlyReason: string | null = (() => { - if (!canManageMembers) return t("members.permissions.readOnlyNoManage"); - if (isTargetOwner) return t("members.permissions.readOnlyOwner"); - if (isMe) return t("members.permissions.readOnlySelf"); + const offendingBits = allow & ~viewerEffective; + + type ReadOnlyReasonKind = "noManage" | "owner" | "self" | "higherRole" | "notHeld"; + const READONLY_REASON_KEYS: Record = { + noManage: "members.permissions.readOnlyNoManage", + owner: "members.permissions.readOnlyOwner", + self: "members.permissions.readOnlySelf", + higherRole: "members.permissions.readOnlyHigherRole", + notHeld: "members.permissions.readOnlyNotHeld", + }; + + const readOnlyReasonKind: ReadOnlyReasonKind | null = (() => { + if (!canManageMembers) return "noManage"; + if (isTargetOwner) return "owner"; + if (isMe) return "self"; const viewerMin = viewer ? minRolePosition(viewer.role_ids, teamRoles) : null; const targetMin = minRolePosition(member.role_ids, teamRoles); const hierarchyFails = !viewer || viewerMin === null || (targetMin !== null && viewerMin >= targetMin); - if (hierarchyFails) return t("members.permissions.readOnlyHigherRole"); - if ((allow & ~viewerEffective) !== 0) return t("members.permissions.readOnlyNotHeld"); + if (hierarchyFails) return "higherRole"; + if (offendingBits !== 0) return "notHeld"; return null; })(); + const readOnlyReason: string | null = readOnlyReasonKind ? t(READONLY_REASON_KEYS[readOnlyReasonKind]) : null; + + // A whole-mask notHeld lock still lets the admin clear the very bit that + // caused it — clearing it produces a mask the server accepts. + const rowDisabled = (permission: Permission) => + overriding || (readOnlyReasonKind !== null && + (readOnlyReasonKind !== "notHeld" || (PERM_BITS[permission] & offendingBits) === 0)); + const write = (masks: { allow: number; deny: number }) => () => useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); @@ -260,7 +279,7 @@ export function MemberDetailPanel({ state={overrideStateOf(permission, allow, deny)} inheritedFrom={granting} inheritedGrants={granting.length > 0} - disabled={readOnlyReason !== null || overriding} + disabled={rowDisabled(permission)} onChange={(next) => void handleOverride(permission, next)} /> ); From 2ff622301fe03acd409ed83d7bbe25fd9ac648d3 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 15:13:10 +0000 Subject: [PATCH 12/21] feat(members): mark members carrying permission overrides in the roster --- src/components/members/roleBadges.test.tsx | 62 ++++++++++++++++++++++ src/components/members/roleBadges.tsx | 40 ++++++++++---- 2 files changed, 92 insertions(+), 10 deletions(-) create mode 100644 src/components/members/roleBadges.test.tsx diff --git a/src/components/members/roleBadges.test.tsx b/src/components/members/roleBadges.test.tsx new file mode 100644 index 000000000..2fe7f980a --- /dev/null +++ b/src/components/members/roleBadges.test.tsx @@ -0,0 +1,62 @@ +import { test, expect, vi, afterEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; + +vi.mock("@iconify/react", () => ({ Icon: ({ icon }: { icon: string }) => })); +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (k: string) => k }), + initReactI18next: { type: "3rdParty", init: () => {} }, +})); + +import { PERM_BITS } from "@/services/permissions"; +import type { TeamMember, TeamRole } from "@/services/teamService"; +import { RoleBadges } from "./roleBadges"; + +afterEach(cleanup); + +const member = (allow: number, deny: number, role_ids: string[] = []): TeamMember => ({ + team_id: "t1", user_id: "u1", handle: "alice", public_key: "k", + invited_by_display_name: null, joined_at: "", role_ids, + permission_allow: allow, permission_deny: deny, +}); + +const role: TeamRole = { + id: "r1", team_id: "t1", name: "editor", permissions: 0, + is_builtin: true, position: 0, created_at: "", +}; + +test("no-role member with an allow override shows the marker", () => { + render(); + expect(screen.getByTestId("override-marker")).toBeTruthy(); +}); + +test("no-role member with a deny override shows the marker", () => { + render(); + expect(screen.getByTestId("override-marker")).toBeTruthy(); +}); + +test("no-role member with both masks empty shows no marker", () => { + render(); + expect(screen.queryByTestId("override-marker")).toBeNull(); +}); + +test("no-role member with canManage and onAddRole still shows the marker", () => { + render( + {}} />, + ); + expect(screen.getByTestId("override-marker")).toBeTruthy(); +}); + +test("no-role member with canManage and onAddRole and no overrides shows no marker", () => { + render( {}} />); + expect(screen.queryByTestId("override-marker")).toBeNull(); +}); + +test("member with a resolvable role and an override shows the marker", () => { + render(); + expect(screen.getByTestId("override-marker")).toBeTruthy(); +}); + +test("member with a resolvable role and no overrides shows no marker", () => { + render(); + expect(screen.queryByTestId("override-marker")).toBeNull(); +}); diff --git a/src/components/members/roleBadges.tsx b/src/components/members/roleBadges.tsx index 9835c6dac..fdc24e720 100644 --- a/src/components/members/roleBadges.tsx +++ b/src/components/members/roleBadges.tsx @@ -43,25 +43,45 @@ export function RoleBadges({ .map((rid) => roles.find((r) => r.id === rid)) .filter(Boolean) as TeamRole[]; memberRoles.sort((a, b) => a.position - b.position); + const hasOverrides = ((member.permission_allow ?? 0) | (member.permission_deny ?? 0)) !== 0; + const marker = hasOverrides ? ( + + + + ) : null; if (memberRoles.length === 0) { if (canManage && onAddRole) { return ( - +
+ + {marker} +
); } - return {t("members.noRole")}; + return ( +
+ {t("members.noRole")} + {marker} +
+ ); } return (
{memberRoles.map((r) => )} + {marker}
); } From e33a53d2311ef7c6ef0dfdbfe461cb0d8a3235cd Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 15:24:25 +0000 Subject: [PATCH 13/21] fix(members): keep the role-badge wrapper inline so the roster line does not break --- src/components/members/roleBadges.test.tsx | 12 ++++++++++++ src/components/members/roleBadges.tsx | 10 ++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/components/members/roleBadges.test.tsx b/src/components/members/roleBadges.test.tsx index 2fe7f980a..bb181aa18 100644 --- a/src/components/members/roleBadges.test.tsx +++ b/src/components/members/roleBadges.test.tsx @@ -51,6 +51,18 @@ test("no-role member with canManage and onAddRole and no overrides shows no mark expect(screen.queryByTestId("override-marker")).toBeNull(); }); +test("the noRole wrapper is inline, not a block, so surrounding text does not break", () => { + render(); + expect(screen.getByTestId("override-marker").parentElement?.tagName).toBe("SPAN"); +}); + +test("the add-role wrapper is inline, not a block, so surrounding text does not break", () => { + render( + {}} />, + ); + expect(screen.getByTestId("override-marker").parentElement?.tagName).toBe("SPAN"); +}); + test("member with a resolvable role and an override shows the marker", () => { render(); expect(screen.getByTestId("override-marker")).toBeTruthy(); diff --git a/src/components/members/roleBadges.tsx b/src/components/members/roleBadges.tsx index fdc24e720..fd7a9a32f 100644 --- a/src/components/members/roleBadges.tsx +++ b/src/components/members/roleBadges.tsx @@ -5,6 +5,8 @@ import type { TeamMember, TeamRole } from "@/stores/teamStore"; import { avatarColor } from "@/components/shared/AvatarStack"; import { ROLE_META, RolePermissionTooltip } from "@/components/members/roleChips"; +const INLINE_WRAPPER_CLASS = "inline-flex items-center gap-1"; + export function RoleChip({ role }: { role: TeamRole }) { const [showTip, setShowTip] = useState(false); const meta = ROLE_META[role.name]; @@ -57,7 +59,7 @@ export function RoleBadges({ if (memberRoles.length === 0) { if (canManage && onAddRole) { return ( -
+ {marker} -
+ ); } return ( -
+ {t("members.noRole")} {marker} -
+ ); } return ( From 978f090be2d808e2a99d69a4a9d25408090fe0fe Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 19:27:23 +0000 Subject: [PATCH 14/21] feat(members): confirm before an override revokes vault key access --- .../MembersPage.MemberDetailPanel.test.tsx | 91 +++++++++++++++++++ .../members/panels/MemberDetailPanel.tsx | 44 +++++++-- src/i18n/locales/en/members.json | 5 + src/i18n/locales/fr/members.json | 5 + src/i18n/locales/ru/members.json | 5 + src/i18n/locales/zh/members.json | 5 + src/services/permissions.test.ts | 52 ++++++++++- src/services/permissions.ts | 15 +++ 8 files changed, 215 insertions(+), 7 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index bcb96e484..7dde19541 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -11,6 +11,7 @@ const h = vi.hoisted(() => ({ loadMembers: vi.fn(), push: vi.fn(), setPerms: vi.fn(), + rotate: vi.fn(), })); vi.mock("react-i18next", () => ({ @@ -56,6 +57,9 @@ vi.mock("@/stores/historyStore", () => ({ vi.mock("@/services/teamActionFeedback", () => ({ runTeamAction: async (o: { run: () => Promise }) => o.run(), })); +vi.mock("@/services/teamKeyRotation", () => ({ + checkAndRotateTeamKey: (...a: unknown[]) => h.rotate(...a), +})); import { MemberDetailPanel } from "./panels/MemberDetailPanel"; @@ -94,6 +98,7 @@ beforeEach(() => { h.addMemberById.mockResolvedValue(undefined); h.loadMembers.mockResolvedValue(undefined); h.setPerms.mockResolvedValue(undefined); + h.rotate.mockResolvedValue(undefined); baseProps.onClose = vi.fn(); baseProps.onUpdated = vi.fn(); }); @@ -277,6 +282,10 @@ test("a notHeld lock enables exactly the offending row, not the others", async ( fireEvent.click(within(connectRow).getByRole("radio", { name: /inherit/i })); + // Clearing the member's only source of CONNECT crosses the vault key gate, + // so this now routes through the confirmation dialog before writing. + fireEvent.click(await screen.findByRole("button", { name: "members.revokeKeyAccess.confirm" })); + await waitFor(() => expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0), ); @@ -353,6 +362,88 @@ test("choosing allow on a bit the viewer lacks sends no request", async () => { expect(h.setPerms).not.toHaveBeenCalled(); }); +// ── Vault key gate confirmation ──────────────────────────────────────────── + +const keyRole: TeamRole = { + id: "r-key", team_id: "t1", name: "key-role", is_builtin: false, + permissions: PERM_BITS.VIEW_SECRETS, position: 2, created_at: "", +}; +const keyMember: TeamMember = { ...targetMember, role_ids: ["r-key"] }; + +test("a gate-crossing change opens the dialog and writes nothing yet", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + + expect(await screen.findByText("members.revokeKeyAccess.title")).toBeTruthy(); + expect(h.setPerms).not.toHaveBeenCalled(); +}); + +test("confirming the dialog writes, then kicks rotation after the write resolves", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + fireEvent.click(await screen.findByRole("button", { name: "members.revokeKeyAccess.confirm" })); + + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.VIEW_SECRETS), + ); + await waitFor(() => expect(h.rotate).toHaveBeenCalledWith("t1")); + expect(h.setPerms.mock.invocationCallOrder[0]).toBeLessThan(h.rotate.mock.invocationCallOrder[0]); +}); + +test("cancelling the dialog writes nothing and rotates nothing", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + fireEvent.click(await screen.findByRole("button", { name: "common.action.cancel" })); + + expect(screen.queryByText("members.revokeKeyAccess.title")).toBeNull(); + expect(h.setPerms).not.toHaveBeenCalled(); + expect(h.rotate).not.toHaveBeenCalled(); +}); + +test("a non-crossing change writes immediately with no dialog and no rotation", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), + ); + expect(screen.queryByText("members.revokeKeyAccess.title")).toBeNull(); + expect(h.rotate).not.toHaveBeenCalled(); +}); + +test("a rejected write after confirming does not rotate", async () => { + h.setPerms.mockRejectedValueOnce(new Error("boom")); + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + fireEvent.click(await screen.findByRole("button", { name: "members.revokeKeyAccess.confirm" })); + + expect(await screen.findByText("boom")).toBeTruthy(); + expect(h.rotate).not.toHaveBeenCalled(); +}); + +test("clearing an allow grant crosses the gate too", async () => { + const rolelessMember = { + ...targetMember, role_ids: [], permission_allow: PERM_BITS.VIEW_SECRETS, permission_deny: 0, + }; + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /inherit/i })); + + expect(await screen.findByText("members.revokeKeyAccess.title")).toBeTruthy(); + expect(h.setPerms).not.toHaveBeenCalled(); +}); + test("a write in flight disables the other rows too", async () => { let resolveSet: () => void = () => {}; h.setPerms.mockImplementation(() => new Promise((resolve) => { resolveSet = resolve; })); diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index 36ec95eb6..5d0034a25 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -10,8 +10,12 @@ import { RoleModal } from "@/components/settings/sections/RolesSection"; import { ROLE_META, RoleBlurb } from "@/components/members/roleChips"; import { RoleBadges } from "@/components/members/roleBadges"; import { OffboardingDialog } from "@/components/members/OffboardingDialog"; +import { ConfirmModal } from "@/components/shared/ConfirmModal"; import type { DepartMode } from "@/services/teamOffboarding"; -import { PERM_BITS, PERM_META, effectivePermissions, type Permission } from "@/services/permissions"; +import { + PERM_BITS, PERM_META, effectivePermissions, crossesVaultKeyGate, type Permission, +} from "@/services/permissions"; +import { checkAndRotateTeamKey } from "@/services/teamKeyRotation"; import { PermissionOverrideRow, overrideStateOf, applyOverrideState, type OverrideState, } from "./PermissionOverrideRow"; @@ -49,6 +53,9 @@ export function MemberDetailPanel({ const [offboarding, setOffboarding] = useState(null); const [creatingRole, setCreatingRole] = useState(false); const [overriding, setOverriding] = useState(false); + // Stores the intent, not the computed masks — commitOverride recomputes them + // from the render current at confirm time, in case member state changed meanwhile. + const [pendingRevoke, setPendingRevoke] = useState<{ permission: Permission; next: OverrideState } | null>(null); const canChangeRoles = canManageMembers && !isMe; const canRemove = canManageMembers && !isTargetOwner && !isMe; @@ -156,11 +163,7 @@ export function MemberDetailPanel({ const write = (masks: { allow: number; deny: number }) => () => useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); - const handleOverride = async (permission: Permission, next: OverrideState) => { - if (next === "allow" && (viewerEffective & PERM_BITS[permission]) === 0) { - setError(t("members.permissions.readOnlyNotHeld")); - return; - } + const commitOverride = async (permission: Permission, next: OverrideState, rotate: boolean) => { const previous = { allow, deny }; const updated = applyOverrideState(permission, allow, deny, next); setError(""); @@ -175,6 +178,7 @@ export function MemberDetailPanel({ redo: write(updated), }); onUpdated(); + if (rotate) void checkAndRotateTeamKey(teamId); } catch (e) { setError(e instanceof Error ? e.message : t("members.error.failedToUpdatePermissions")); } finally { @@ -182,6 +186,19 @@ export function MemberDetailPanel({ } }; + const handleOverride = async (permission: Permission, next: OverrideState) => { + if (next === "allow" && (viewerEffective & PERM_BITS[permission]) === 0) { + setError(t("members.permissions.readOnlyNotHeld")); + return; + } + const updated = applyOverrideState(permission, allow, deny, next); + if (crossesVaultKeyGate(member, teamRoles, updated)) { + setPendingRevoke({ permission, next }); + return; + } + await commitOverride(permission, next, false); + }; + const joinedDate = new Date(member.joined_at).toLocaleDateString(undefined, { year: "numeric", month: "long", day: "numeric", }); @@ -336,6 +353,21 @@ export function MemberDetailPanel({ onDone={() => { onClose(); onUpdated(); }} /> )} + + {pendingRevoke && ( + setPendingRevoke(null)} + onConfirm={() => { + const { permission, next } = pendingRevoke; + setPendingRevoke(null); + void commitOverride(permission, next, true); + }} + /> + )} ); } diff --git a/src/i18n/locales/en/members.json b/src/i18n/locales/en/members.json index 024330736..24ddb51bf 100644 --- a/src/i18n/locales/en/members.json +++ b/src/i18n/locales/en/members.json @@ -65,6 +65,11 @@ "readOnlyNotHeld": "You cannot grant a permission you do not hold", "overrideMarker": "Has permission overrides" }, + "revokeKeyAccess": { + "title": "Revoke {{name}}'s access to the vault key?", + "body": "{{name}} will no longer be able to connect or view secrets in this vault. They keep a copy of the current key and can still read secrets already synced to their device, so the vault key will be rotated for the remaining members.", + "confirm": "Revoke and rotate key" + }, "convert": { "title": "Share \"{{vault}}\" with other people?", "body": "This turns your private vault into a team vault. Everything already in it moves across, so the people you add see it too.", diff --git a/src/i18n/locales/fr/members.json b/src/i18n/locales/fr/members.json index 5ef1d9586..4180e9f2a 100644 --- a/src/i18n/locales/fr/members.json +++ b/src/i18n/locales/fr/members.json @@ -65,6 +65,11 @@ "readOnlyNotHeld": "Vous ne pouvez pas accorder une permission que vous n'avez pas", "overrideMarker": "Permissions personnalisées" }, + "revokeKeyAccess": { + "title": "Retirer à {{name}} l'accès à la clé du coffre ?", + "body": "{{name}} ne pourra plus se connecter ni voir les secrets de ce coffre. Cette personne conserve une copie de la clé actuelle et peut encore lire les secrets déjà synchronisés sur son appareil ; la clé du coffre sera donc changée pour les membres restants.", + "confirm": "Retirer et changer la clé" + }, "convert": { "title": "Partager « {{vault}} » avec d'autres personnes ?", "body": "Ceci transforme votre coffre-fort privé en coffre-fort d'équipe. Tout ce qu'il contient déjà y est transféré, et devient donc visible par les personnes que vous ajoutez.", diff --git a/src/i18n/locales/ru/members.json b/src/i18n/locales/ru/members.json index 5602dca71..8d6e01e3f 100644 --- a/src/i18n/locales/ru/members.json +++ b/src/i18n/locales/ru/members.json @@ -67,6 +67,11 @@ "readOnlyNotHeld": "Нельзя выдать разрешение, которого у вас нет", "overrideMarker": "Есть переопределения разрешений" }, + "revokeKeyAccess": { + "title": "Отозвать у {{name}} доступ к ключу хранилища?", + "body": "{{name}} больше не сможет подключаться и просматривать секреты этого хранилища. У участника остаётся копия текущего ключа, и уже синхронизированные на его устройство секреты по-прежнему доступны для чтения, поэтому для остальных участников будет выполнена ротация ключа хранилища.", + "confirm": "Отозвать и выполнить ротацию ключа" + }, "convert": { "title": "Предоставить общий доступ к \"{{vault}}\" другим людям?", "body": "Это превратит ваше личное хранилище в командное. Всё, что уже в нём есть, переносится туда, поэтому добавленные вами люди тоже это увидят.", diff --git a/src/i18n/locales/zh/members.json b/src/i18n/locales/zh/members.json index 46c839746..16d212220 100644 --- a/src/i18n/locales/zh/members.json +++ b/src/i18n/locales/zh/members.json @@ -65,6 +65,11 @@ "readOnlyNotHeld": "无法授予你自己没有的权限", "overrideMarker": "存在权限覆盖" }, + "revokeKeyAccess": { + "title": "撤销 {{name}} 对保管库密钥的访问权限？", + "body": "{{name}} 将无法再连接或查看此保管库中的机密。该成员仍持有当前密钥的副本，已同步到其设备上的机密依然可读，因此将为其余成员轮换保管库密钥。", + "confirm": "撤销并轮换密钥" + }, "convert": { "title": "要与他人共享 \"{{vault}}\" 吗？", "body": "这会将您的私有保险库转变为团队保险库。其中已有的所有内容都会一并转移，因此您添加的人也能看到。", diff --git a/src/services/permissions.test.ts b/src/services/permissions.test.ts index 86f6b87be..b8a6a7233 100644 --- a/src/services/permissions.test.ts +++ b/src/services/permissions.test.ts @@ -1,5 +1,5 @@ import { test, expect, describe, it } from "vitest"; -import { resolveCan, PERM_BITS, effectivePermissions, type PermissionSnapshot } from "./permissions.ts"; +import { resolveCan, PERM_BITS, effectivePermissions, crossesVaultKeyGate, type PermissionSnapshot } from "./permissions.ts"; import type { Team, TeamMember, TeamRole } from "@/services/teamService"; import type { Vault } from "@/stores/vaultStore"; @@ -116,6 +116,56 @@ describe("effectivePermissions with member overrides", () => { }); }); +describe("crossesVaultKeyGate", () => { + it("role grants VIEW_SECRETS only; deny VIEW_SECRETS crosses the gate", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.VIEW_SECRETS }), + ).toBe(true); + }); + + it("already denying VIEW_SECRETS; submitting the identical deny again does not cross", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: PERM_BITS.VIEW_SECRETS }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.VIEW_SECRETS }), + ).toBe(false); + }); + + it("role grants VIEW_SECRETS and CONNECT; deny VIEW_SECRETS only does not cross (CONNECT still gates)", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS | PERM_BITS.CONNECT)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.VIEW_SECRETS }), + ).toBe(false); + }); + + it("role grants VIEW_SECRETS and COPY_SECRETS; deny COPY_SECRETS only does not cross", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS | PERM_BITS.COPY_SECRETS)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.COPY_SECRETS }), + ).toBe(false); + }); + + it("roleless member with allow VIEW_SECRETS; clearing to inherit crosses", () => { + const roles: TeamRole[] = []; + const m = { ...member("u1", []), permission_allow: PERM_BITS.VIEW_SECRETS, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: 0 }), + ).toBe(true); + }); + + it("role grants VIEW_SECRETS; deny EDIT_KEYS does not cross", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.EDIT_KEYS }), + ).toBe(false); + }); +}); + test("resolveCan uses team-level deny in the team fallback (before membersByTeam loads)", () => { const s = snap({ teams: [{ id: "t1", name: "t1", owner_id: "o", owner_tier: "team", created_at: "", role_ids: ["r1"], permission_deny: PERM_BITS.VIEW_SECRETS }], diff --git a/src/services/permissions.ts b/src/services/permissions.ts index 4de809732..0d4befc31 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -52,6 +52,21 @@ export function effectivePermissions( return (union | (member.permission_allow ?? 0)) & ~(member.permission_deny ?? 0); } +const VAULT_KEY_GATE = PERM_BITS.CONNECT | PERM_BITS.VIEW_SECRETS; + +export function crossesVaultKeyGate( + member: { role_ids: string[]; permission_allow?: number; permission_deny?: number }, + roles: TeamRole[], + next: { allow: number; deny: number }, +): boolean { + const before = effectivePermissions(member, roles) & VAULT_KEY_GATE; + const after = effectivePermissions( + { role_ids: member.role_ids, permission_allow: next.allow, permission_deny: next.deny }, + roles, + ) & VAULT_KEY_GATE; + return before !== 0 && after === 0; +} + /** True if member holds the builtin role with the given name in this team. */ export function hasBuiltinRole(member: TeamMember, roleName: string, roles: TeamRole[]): boolean { const target = roles.find((r) => r.is_builtin && r.name === roleName); From ed2bdb187d347b5c002e9ffe919d05b0112a2019 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 19:45:46 +0000 Subject: [PATCH 15/21] fix(members): keep rows inert while the revoke dialog is open --- .../MembersPage.MemberDetailPanel.test.tsx | 20 +++++++++++++++++-- src/components/members/MembersPage.tsx | 1 + .../members/panels/MemberDetailPanel.tsx | 2 +- src/services/permissions.test.ts | 8 ++++++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index 7dde19541..9e51da1ce 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -283,7 +283,7 @@ test("a notHeld lock enables exactly the offending row, not the others", async ( fireEvent.click(within(connectRow).getByRole("radio", { name: /inherit/i })); // Clearing the member's only source of CONNECT crosses the vault key gate, - // so this now routes through the confirmation dialog before writing. + // so this routes through the confirmation dialog before writing. fireEvent.click(await screen.findByRole("button", { name: "members.revokeKeyAccess.confirm" })); await waitFor(() => @@ -380,7 +380,21 @@ test("a gate-crossing change opens the dialog and writes nothing yet", async () expect(h.setPerms).not.toHaveBeenCalled(); }); +test("every row is inert while the revoke dialog is open", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + await screen.findByText("members.revokeKeyAccess.title"); + + const otherRow = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + expect((within(otherRow).getByRole("radio", { name: /deny/i }) as HTMLButtonElement).disabled).toBe(true); +}); + test("confirming the dialog writes, then kicks rotation after the write resolves", async () => { + let resolveSet: () => void = () => {}; + h.setPerms.mockImplementation(() => new Promise((resolve) => { resolveSet = resolve; })); + render(); const row = screen.getByRole("radiogroup", { name: "members.permission.VIEW_SECRETS" }); @@ -390,8 +404,10 @@ test("confirming the dialog writes, then kicks rotation after the write resolves await waitFor(() => expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.VIEW_SECRETS), ); + expect(h.rotate).not.toHaveBeenCalled(); + + resolveSet(); await waitFor(() => expect(h.rotate).toHaveBeenCalledWith("t1")); - expect(h.setPerms.mock.invocationCallOrder[0]).toBeLessThan(h.rotate.mock.invocationCallOrder[0]); }); test("cancelling the dialog writes nothing and rotates nothing", async () => { diff --git a/src/components/members/MembersPage.tsx b/src/components/members/MembersPage.tsx index a509c7911..c0d9e9ef2 100644 --- a/src/components/members/MembersPage.tsx +++ b/src/components/members/MembersPage.tsx @@ -515,6 +515,7 @@ const vaultTabs = selectedVaultIds.length > 1 showDetailPanel && detailMember ? ( - overriding || (readOnlyReasonKind !== null && + overriding || pendingRevoke !== null || (readOnlyReasonKind !== null && (readOnlyReasonKind !== "notHeld" || (PERM_BITS[permission] & offendingBits) === 0)); const write = (masks: { allow: number; deny: number }) => () => diff --git a/src/services/permissions.test.ts b/src/services/permissions.test.ts index b8a6a7233..7e5398953 100644 --- a/src/services/permissions.test.ts +++ b/src/services/permissions.test.ts @@ -164,6 +164,14 @@ describe("crossesVaultKeyGate", () => { crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.EDIT_KEYS }), ).toBe(false); }); + + it("role grants VIEW_SECRETS and an unrelated bit; deny VIEW_SECRETS still crosses", () => { + const roles: TeamRole[] = [role("r1", PERM_BITS.VIEW_SECRETS | PERM_BITS.EDIT_KEYS)]; + const m = { ...member("u1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + expect( + crossesVaultKeyGate(m, roles, { allow: 0, deny: PERM_BITS.VIEW_SECRETS }), + ).toBe(true); + }); }); test("resolveCan uses team-level deny in the team fallback (before membersByTeam loads)", () => { From 65f3c220d13ffcc53f3dfb202c62361246e2c42a Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 19:53:13 +0000 Subject: [PATCH 16/21] feat(logs): render the member permissions-changed audit event --- src/components/logs/AuditEventRow.tsx | 9 +++++---- src/components/logs/AuditFilters.tsx | 11 ++++++----- src/i18n/locales/en/logs.json | 2 ++ src/i18n/locales/fr/logs.json | 2 ++ src/i18n/locales/ru/logs.json | 2 ++ src/i18n/locales/zh/logs.json | 2 ++ 6 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/components/logs/AuditEventRow.tsx b/src/components/logs/AuditEventRow.tsx index c8f2b913d..d02af5b09 100644 --- a/src/components/logs/AuditEventRow.tsx +++ b/src/components/logs/AuditEventRow.tsx @@ -19,10 +19,11 @@ const fallbackResource = () => i18n.t("logs.eventLabels.fallbackResource"); const fallbackHost = () => i18n.t("logs.eventLabels.fallbackHost"); export const ACTION_META: Record = { - "member.invited": { icon: "lucide:user-plus", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberInvited", { name: l.target_name ?? l.target_id ?? fallbackUser() }) }, - "member.joined": { icon: "lucide:user-check", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberJoined", { role: l.metadata?.role ?? fallbackRole() }) }, - "member.removed": { icon: "lucide:user-minus", color: "#ef4444", label: (l) => i18n.t("logs.eventLabels.memberRemoved", { name: l.target_name ?? l.target_id ?? fallbackMember() }) }, - "member.role_changed": { icon: "lucide:user-cog", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberRoleChanged", { name: l.target_name ?? l.target_id ?? fallbackMember() }) }, + "member.invited": { icon: "lucide:user-plus", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberInvited", { name: l.target_name ?? l.target_id ?? fallbackUser() }) }, + "member.joined": { icon: "lucide:user-check", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberJoined", { role: l.metadata?.role ?? fallbackRole() }) }, + "member.removed": { icon: "lucide:user-minus", color: "#ef4444", label: (l) => i18n.t("logs.eventLabels.memberRemoved", { name: l.target_name ?? l.target_id ?? fallbackMember() }) }, + "member.role_changed": { icon: "lucide:user-cog", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberRoleChanged", { name: l.target_name ?? l.target_id ?? fallbackMember() }) }, + "member.permissions_changed": { icon: "lucide:sliders-horizontal", color: "#3b82f6", label: (l) => i18n.t("logs.eventLabels.memberPermissionsChanged", { name: l.target_name ?? l.target_id ?? fallbackMember() }) }, "vault.created": { icon: "lucide:database", color: "#8b5cf6", label: (l) => i18n.t("logs.eventLabels.vaultCreated", { name: l.target_name ?? l.target_id ?? "" }) }, "vault.deleted": { icon: "lucide:database", color: "#ef4444", label: (l) => i18n.t("logs.eventLabels.vaultDeleted", { name: l.target_name ?? l.target_id ?? "" }) }, "vault.renamed": { icon: "lucide:database", color: "#8b5cf6", label: (l) => i18n.t("logs.eventLabels.vaultRenamed", { name: l.target_name ?? l.target_id ?? "" }) }, diff --git a/src/components/logs/AuditFilters.tsx b/src/components/logs/AuditFilters.tsx index 589a66c5a..426319fde 100644 --- a/src/components/logs/AuditFilters.tsx +++ b/src/components/logs/AuditFilters.tsx @@ -12,11 +12,12 @@ import { getAuditTimeRange, type AuditTimeRange } from "./auditLogToolbarUtils"; function getActionOptions(t: TFunction) { return [ { value: "", label: t("logs.filters.actionOptions.all") }, - { value: "member.invited", label: t("logs.filters.actionOptions.memberInvited") }, - { value: "member.joined", label: t("logs.filters.actionOptions.memberJoined") }, - { value: "member.removed", label: t("logs.filters.actionOptions.memberRemoved") }, - { value: "member.role_changed", label: t("logs.filters.actionOptions.memberRoleChanged") }, - { value: "connection.created", label: t("logs.filters.actionOptions.connectionCreated") }, + { value: "member.invited", label: t("logs.filters.actionOptions.memberInvited") }, + { value: "member.joined", label: t("logs.filters.actionOptions.memberJoined") }, + { value: "member.removed", label: t("logs.filters.actionOptions.memberRemoved") }, + { value: "member.role_changed", label: t("logs.filters.actionOptions.memberRoleChanged") }, + { value: "member.permissions_changed", label: t("logs.filters.actionOptions.memberPermissionsChanged") }, + { value: "connection.created", label: t("logs.filters.actionOptions.connectionCreated") }, { value: "connection.updated", label: t("logs.filters.actionOptions.connectionUpdated") }, { value: "connection.deleted", label: t("logs.filters.actionOptions.connectionDeleted") }, { value: "identity.created", label: t("logs.filters.actionOptions.identityCreated") }, diff --git a/src/i18n/locales/en/logs.json b/src/i18n/locales/en/logs.json index 17f7ce71b..b6c1510de 100644 --- a/src/i18n/locales/en/logs.json +++ b/src/i18n/locales/en/logs.json @@ -23,6 +23,7 @@ "memberJoined": "Member joined", "memberRemoved": "Member removed", "memberRoleChanged": "Role changed", + "memberPermissionsChanged": "Permissions changed", "connectionCreated": "Host created", "connectionUpdated": "Host updated", "connectionDeleted": "Host deleted", @@ -79,6 +80,7 @@ "memberJoined": "joined the team as {{role}}", "memberRemoved": "removed {{name}}", "memberRoleChanged": "changed role for {{name}}", + "memberPermissionsChanged": "changed permissions for {{name}}", "vaultCreated": "created vault \"{{name}}\"", "vaultDeleted": "deleted vault \"{{name}}\"", "vaultRenamed": "renamed vault to \"{{name}}\"", diff --git a/src/i18n/locales/fr/logs.json b/src/i18n/locales/fr/logs.json index dec13eb31..a4c16f1dc 100644 --- a/src/i18n/locales/fr/logs.json +++ b/src/i18n/locales/fr/logs.json @@ -23,6 +23,7 @@ "memberJoined": "Membre rejoint", "memberRemoved": "Membre retiré", "memberRoleChanged": "Rôle modifié", + "memberPermissionsChanged": "Permissions modifiées", "connectionCreated": "Hôte créé", "connectionUpdated": "Hôte modifié", "connectionDeleted": "Hôte supprimé", @@ -79,6 +80,7 @@ "memberJoined": "a rejoint l'équipe en tant que {{role}}", "memberRemoved": "a retiré {{name}}", "memberRoleChanged": "a changé le rôle de {{name}}", + "memberPermissionsChanged": "a changé les permissions de {{name}}", "vaultCreated": "a créé le coffre « {{name}} »", "vaultDeleted": "a supprimé le coffre « {{name}} »", "vaultRenamed": "a renommé le coffre en « {{name}} »", diff --git a/src/i18n/locales/ru/logs.json b/src/i18n/locales/ru/logs.json index 5ab1a34c4..ba899a787 100644 --- a/src/i18n/locales/ru/logs.json +++ b/src/i18n/locales/ru/logs.json @@ -23,6 +23,7 @@ "memberJoined": "Участник присоединился", "memberRemoved": "Участник удалён", "memberRoleChanged": "Роль изменена", + "memberPermissionsChanged": "Разрешения изменены", "connectionCreated": "Хост создан", "connectionUpdated": "Хост обновлён", "connectionDeleted": "Хост удалён", @@ -79,6 +80,7 @@ "memberJoined": "присоединился(лась) к команде как {{role}}", "memberRemoved": "удалил(а) {{name}}", "memberRoleChanged": "изменил(а) роль для {{name}}", + "memberPermissionsChanged": "изменил(а) разрешения для {{name}}", "vaultCreated": "создал(а) хранилище \"{{name}}\"", "vaultDeleted": "удалил(а) хранилище \"{{name}}\"", "vaultRenamed": "переименовал(а) хранилище в \"{{name}}\"", diff --git a/src/i18n/locales/zh/logs.json b/src/i18n/locales/zh/logs.json index 572d636fd..9ea8b138d 100644 --- a/src/i18n/locales/zh/logs.json +++ b/src/i18n/locales/zh/logs.json @@ -23,6 +23,7 @@ "memberJoined": "成员已加入", "memberRemoved": "已移除成员", "memberRoleChanged": "角色已更改", + "memberPermissionsChanged": "权限已更改", "connectionCreated": "已创建主机", "connectionUpdated": "已更新主机", "connectionDeleted": "已删除主机", @@ -79,6 +80,7 @@ "memberJoined": "以 {{role}} 身份加入了团队", "memberRemoved": "移除了 {{name}}", "memberRoleChanged": "更改了 {{name}} 的角色", + "memberPermissionsChanged": "更改了 {{name}} 的权限", "vaultCreated": "创建了保险库 \"{{name}}\"", "vaultDeleted": "删除了保险库 \"{{name}}\"", "vaultRenamed": "将保险库重命名为 \"{{name}}\"", From 55256f5fb28513ae81b419d3fe93100684cd9517 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 20:35:37 +0000 Subject: [PATCH 17/21] fix(teams): keep override masks in the loadTeams change predicate The unchanged-list comparison ignored permission_allow/permission_deny, so an override change with no other team change was discarded and cacheVaultRoles mirrored stale bits into the keychain the Rust vault-write gate reads. --- src/stores/teamStore.test.ts | 10 ++++++++++ src/stores/teamStore.ts | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/stores/teamStore.test.ts b/src/stores/teamStore.test.ts index f0434fcb0..a7dfc511a 100644 --- a/src/stores/teamStore.test.ts +++ b/src/stores/teamStore.test.ts @@ -148,6 +148,16 @@ test("loadTeams unions the bits of every role a member holds", async () => { expect(cachedRoles()).toEqual({ t1: 12 }); }); +test("loadTeams caches freshly served override masks even when nothing else changed", async () => { + const before = { ...team("t1", ["r1"]), permission_allow: 0, permission_deny: 0 }; + useTeamStore.setState({ teams: [before] }); + api.listTeams.mockResolvedValue([{ ...before, permission_deny: 8 }]); + api.listRoles.mockResolvedValue([role("r1", 12)]); + await get().loadTeams(); + expect(get().teams[0].permission_deny).toBe(8); + expect(cachedRoles()).toEqual({ t1: 4 }); +}); + test("loadTeams omits a team whose roles cannot be resolved", async () => { api.listTeams.mockResolvedValue([team("t1", ["r1"])]); api.listRoles.mockRejectedValue(new Error("offline")); diff --git a/src/stores/teamStore.ts b/src/stores/teamStore.ts index 92a3d5aa6..efcec50c2 100644 --- a/src/stores/teamStore.ts +++ b/src/stores/teamStore.ts @@ -103,6 +103,8 @@ export const useTeamStore = create()( const same = prev.length === fresh.length && fresh.every((t, i) => t.id === prev[i].id && t.name === prev[i].name && + t.permission_allow === prev[i].permission_allow && + t.permission_deny === prev[i].permission_deny && JSON.stringify(t.role_ids) === JSON.stringify(prev[i].role_ids)); const teams = same ? prev : fresh; set({ teams, loading: false }); From d1350812b4d1f37eec3e9a50e28dc317446dd136 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 20:35:44 +0000 Subject: [PATCH 18/21] fix(members): gate the permissions section, scope undo, keep the retired bit clearable Hide the section entirely when the server serves neither mask, so an older server does not render 16 live rows that 404 on click. Recompute undo and redo from current store state instead of replaying a whole-mask snapshot, so the full-replace PUT no longer erases a concurrent admin's change. Render CREATE_CUSTOM_ROLES when either mask carries it, so an offending bit always has a row that can clear it. --- .../MembersPage.MemberDetailPanel.test.tsx | 57 +++++++++++++++++++ .../members/panels/MemberDetailPanel.tsx | 22 +++++-- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index 9e51da1ce..99ff34ae2 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -32,6 +32,7 @@ vi.mock("@/components/settings/sections/RolesSection", () => ({ })); vi.mock("@/stores/teamStore", () => { const state = { + membersByTeam: {} as Record, assignMemberRole: h.assign, removeMemberRole: h.remove, removeMember: h.removeMember, @@ -62,6 +63,9 @@ vi.mock("@/services/teamKeyRotation", () => ({ })); import { MemberDetailPanel } from "./panels/MemberDetailPanel"; +import { useTeamStore } from "@/stores/teamStore"; + +const mockStore = useTeamStore.getState() as unknown as { membersByTeam: Record }; const baseMember: TeamMember = { team_id: "t1", @@ -99,6 +103,7 @@ beforeEach(() => { h.loadMembers.mockResolvedValue(undefined); h.setPerms.mockResolvedValue(undefined); h.rotate.mockResolvedValue(undefined); + mockStore.membersByTeam = {}; baseProps.onClose = vi.fn(); baseProps.onUpdated = vi.fn(); }); @@ -269,6 +274,58 @@ test("permission overrides: renders one row per permission and sends the new mas expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0); }); +// An older server omits both mask fields entirely; every row would otherwise +// render live and 404 on click. +test("no permissions section at all when the server serves neither mask", () => { + const legacy = { ...targetMember }; + delete legacy.permission_allow; + delete legacy.permission_deny; + render(); + + expect(screen.queryAllByRole("radiogroup")).toHaveLength(0); + expect(screen.queryByText("members.permissions.title")).toBeNull(); +}); + +test("undo re-reads the masks so a concurrent change survives the full replace", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), + ); + + mockStore.membersByTeam = { + t1: [{ ...targetMember, permission_deny: PERM_BITS.EDIT_KEYS | PERM_BITS.CONNECT }], + }; + + const entry = h.push.mock.calls[0][0] as { undo: () => Promise }; + await entry.undo(); + + expect(h.setPerms).toHaveBeenLastCalledWith("t1", "u2", 0, PERM_BITS.CONNECT); +}); + +// CREATE_CUSTOM_ROLES is retired and normally hidden, but it is inside the +// server's ALL_PERMISSIONS, so a mask carrying it must stay clearable. +test("a retired bit set in a mask renders an enabled row that can clear it", async () => { + const member = { ...targetMember, permission_allow: PERM_BITS.CREATE_CUSTOM_ROLES }; + render(); + + expect(screen.getByText("members.permissions.readOnlyNotHeld")).toBeTruthy(); + const row = screen.getByRole("radiogroup", { name: "members.permission.CREATE_CUSTOM_ROLES" }); + expect((within(row).getByRole("radio", { name: /inherit/i }) as HTMLButtonElement).disabled).toBe(false); + + fireEvent.click(within(row).getByRole("radio", { name: /inherit/i })); + + await waitFor(() => expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0)); +}); + +test("the retired bit renders no row when neither mask carries it", () => { + render(); + + expect(screen.queryByRole("radiogroup", { name: "members.permission.CREATE_CUSTOM_ROLES" })).toBeNull(); +}); + test("a notHeld lock enables exactly the offending row, not the others", async () => { const member = { ...targetMember, permission_allow: PERM_BITS.CONNECT }; render(); diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index f1c964c3e..136870086 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -121,8 +121,14 @@ export function MemberDetailPanel({ const deny = member.permission_deny ?? 0; const viewerEffective = viewer ? effectivePermissions(viewer, teamRoles) : 0; + // A server predating overrides omits both masks; a zero mask serializes as 0. + const serverSupportsOverrides = + member.permission_allow !== undefined || member.permission_deny !== undefined; + + // Retired, but shown when set: otherwise no row can clear it as an offending bit. const editablePermissions = (Object.keys(PERM_META) as Permission[]) - .filter((p) => p !== "CREATE_CUSTOM_ROLES"); + .filter((p) => p !== "CREATE_CUSTOM_ROLES" + || ((allow | deny) & PERM_BITS.CREATE_CUSTOM_ROLES) !== 0); const rolesGranting = (permission: Permission) => teamRoles @@ -164,8 +170,14 @@ export function MemberDetailPanel({ useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); const commitOverride = async (permission: Permission, next: OverrideState, rotate: boolean) => { - const previous = { allow, deny }; const updated = applyOverrideState(permission, allow, deny, next); + // Undo/redo re-read the masks so a concurrent admin's unrelated bits survive + // the full-replace PUT; only the bit this entry owns moves. + const at = (state: OverrideState) => () => { + const m = useTeamStore.getState().membersByTeam[teamId]?.find((x) => x.user_id === member.user_id); + const masks = applyOverrideState(permission, m?.permission_allow ?? 0, m?.permission_deny ?? 0, state); + return useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); + }; setError(""); setOverriding(true); try { @@ -174,8 +186,8 @@ export function MemberDetailPanel({ success: t("members.toast.permissionsUpdated", { name: member.handle }), label: t("members.history.changePermissions", { name: member.handle }), run: write(updated), - undo: write(previous), - redo: write(updated), + undo: at(overrideStateOf(permission, allow, deny)), + redo: at(next), }); onUpdated(); if (rotate) void checkAndRotateTeamKey(teamId); @@ -282,6 +294,7 @@ export function MemberDetailPanel({ {/* Permissions */} + {serverSupportsOverrides && ( {readOnlyReason && (

{readOnlyReason}

@@ -303,6 +316,7 @@ export function MemberDetailPanel({ })}
+ )} {/* Info */} From 25237b584ec2751fae8e94f603a553e63fe83c54 Mon Sep 17 00:00:00 2001 From: kipavy Date: Fri, 11 Sep 2026 20:35:44 +0000 Subject: [PATCH 19/21] =?UTF-8?q?fix(i18n):=20match=20the=20failedTo*=20fo?= =?UTF-8?q?rmat,=20use=20=E4=BF=9D=E9=99=A9=E5=BA=93=20for=20vault=20in=20?= =?UTF-8?q?zh?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also note the 32-bit ceiling on PERM_BITS. --- src/i18n/locales/en/common.json | 2 +- src/i18n/locales/fr/common.json | 2 +- src/i18n/locales/ru/common.json | 2 +- src/i18n/locales/zh/common.json | 2 +- src/i18n/locales/zh/members.json | 4 ++-- src/services/permissions.ts | 1 + 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index 580af76bf..3a93b056b 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -121,7 +121,7 @@ "failedToAssignRole": "Failed to assign role: {{status}}", "failedToRemoveRole": "Failed to remove role: {{status}}", "insufficientPermissionSetMemberPermissions": "You do not have permission to change this member's permissions", - "failedToSetMemberPermissions": "Failed to set member permissions ({{status}})", + "failedToSetMemberPermissions": "Failed to set member permissions: {{status}}", "failedToCreateRole": "Failed to create role: {{status}}", "failedToUpdateRole": "Failed to update role: {{status}}", "failedToDeleteRole": "Failed to delete role: {{status}}", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index 512addd10..f1cfba2c5 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -121,7 +121,7 @@ "failedToAssignRole": "Échec de l'attribution du rôle : {{status}}", "failedToRemoveRole": "Échec du retrait du rôle : {{status}}", "insufficientPermissionSetMemberPermissions": "Vous n'avez pas la permission de modifier les permissions de ce membre", - "failedToSetMemberPermissions": "Échec de la définition des permissions du membre ({{status}})", + "failedToSetMemberPermissions": "Échec de la définition des permissions du membre : {{status}}", "failedToCreateRole": "Échec de la création du rôle : {{status}}", "failedToUpdateRole": "Échec de la mise à jour du rôle : {{status}}", "failedToDeleteRole": "Échec de la suppression du rôle : {{status}}", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 3393a580e..079e2c0e3 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -121,7 +121,7 @@ "failedToAssignRole": "Не удалось назначить роль: {{status}}", "failedToRemoveRole": "Не удалось удалить роль: {{status}}", "insufficientPermissionSetMemberPermissions": "У вас нет прав на изменение разрешений этого участника", - "failedToSetMemberPermissions": "Не удалось задать разрешения участника ({{status}})", + "failedToSetMemberPermissions": "Не удалось задать разрешения участника: {{status}}", "failedToCreateRole": "Не удалось создать роль: {{status}}", "failedToUpdateRole": "Не удалось обновить роль: {{status}}", "failedToDeleteRole": "Не удалось удалить роль: {{status}}", diff --git a/src/i18n/locales/zh/common.json b/src/i18n/locales/zh/common.json index b19cf5926..275b5a92e 100644 --- a/src/i18n/locales/zh/common.json +++ b/src/i18n/locales/zh/common.json @@ -121,7 +121,7 @@ "failedToAssignRole": "分配角色失败：{{status}}", "failedToRemoveRole": "移除角色失败：{{status}}", "insufficientPermissionSetMemberPermissions": "您没有权限修改该成员的权限", - "failedToSetMemberPermissions": "设置成员权限失败（{{status}}）", + "failedToSetMemberPermissions": "设置成员权限失败：{{status}}", "failedToCreateRole": "创建角色失败：{{status}}", "failedToUpdateRole": "更新角色失败：{{status}}", "failedToDeleteRole": "删除角色失败：{{status}}", diff --git a/src/i18n/locales/zh/members.json b/src/i18n/locales/zh/members.json index 16d212220..ce6585685 100644 --- a/src/i18n/locales/zh/members.json +++ b/src/i18n/locales/zh/members.json @@ -66,8 +66,8 @@ "overrideMarker": "存在权限覆盖" }, "revokeKeyAccess": { - "title": "撤销 {{name}} 对保管库密钥的访问权限？", - "body": "{{name}} 将无法再连接或查看此保管库中的机密。该成员仍持有当前密钥的副本，已同步到其设备上的机密依然可读，因此将为其余成员轮换保管库密钥。", + "title": "撤销 {{name}} 对保险库密钥的访问权限？", + "body": "{{name}} 将无法再连接或查看此保险库中的机密。该成员仍持有当前密钥的副本，已同步到其设备上的机密依然可读，因此将为其余成员轮换保险库密钥。", "confirm": "撤销并轮换密钥" }, "convert": { diff --git a/src/services/permissions.ts b/src/services/permissions.ts index 0d4befc31..fbcfafce2 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -21,6 +21,7 @@ export type Permission = | "EDIT_SNIPPETS"; // Bitmask values for each permission — must stay in sync with server/src/permissions.rs +// JS bitwise ops coerce to 32-bit signed, so bit 31 and above cannot be used here. export const PERM_BITS: Record = { VIEW_SECRETS: 1 << 0, // 1 COPY_SECRETS: 1 << 1, // 2 From 8453065457bc87b75103e7a25170f54da9e5c8d5 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sat, 12 Sep 2026 09:47:40 +0000 Subject: [PATCH 20/21] fix(members): undo bails instead of writing empty masks for an absent member at() defaulted to 0/0 when the member was missing from membersByTeam, silently discarding real permission bits on undo/redo. --- .../MembersPage.MemberDetailPanel.test.tsx | 15 +++++++++++++++ .../members/panels/MemberDetailPanel.tsx | 6 ++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/components/members/MembersPage.MemberDetailPanel.test.tsx b/src/components/members/MembersPage.MemberDetailPanel.test.tsx index 99ff34ae2..387c7305c 100644 --- a/src/components/members/MembersPage.MemberDetailPanel.test.tsx +++ b/src/components/members/MembersPage.MemberDetailPanel.test.tsx @@ -269,11 +269,26 @@ test("permission overrides: renders one row per permission and sends the new mas expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), ); + mockStore.membersByTeam = { t1: [targetMember] }; + const entry = h.push.mock.calls[0][0] as { undo: () => Promise }; await entry.undo(); expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, 0); }); +test("permission overrides: undo throws instead of writing empty masks when the member is gone from the store", async () => { + render(); + + const row = screen.getByRole("radiogroup", { name: "members.permission.EDIT_KEYS" }); + fireEvent.click(within(row).getByRole("radio", { name: /deny/i })); + await waitFor(() => + expect(h.setPerms).toHaveBeenCalledWith("t1", "u2", 0, PERM_BITS.EDIT_KEYS), + ); + + const entry = h.push.mock.calls[0][0] as { undo: () => Promise }; + await expect(entry.undo()).rejects.toThrow(); +}); + // An older server omits both mask fields entirely; every row would otherwise // render live and 404 on click. test("no permissions section at all when the server serves neither mask", () => { diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index 136870086..cabf4b0be 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -172,10 +172,12 @@ export function MemberDetailPanel({ const commitOverride = async (permission: Permission, next: OverrideState, rotate: boolean) => { const updated = applyOverrideState(permission, allow, deny, next); // Undo/redo re-read the masks so a concurrent admin's unrelated bits survive - // the full-replace PUT; only the bit this entry owns moves. + // the full-replace PUT; only the bit this entry owns moves. A member missing + // from the store can't be safely masked to 0/0 — bail instead of writing empty masks. const at = (state: OverrideState) => () => { const m = useTeamStore.getState().membersByTeam[teamId]?.find((x) => x.user_id === member.user_id); - const masks = applyOverrideState(permission, m?.permission_allow ?? 0, m?.permission_deny ?? 0, state); + if (!m) throw new Error(t("members.error.failedToUpdatePermissions")); + const masks = applyOverrideState(permission, m.permission_allow ?? 0, m.permission_deny ?? 0, state); return useTeamStore.getState().setMemberPermissions(teamId, member.user_id, masks.allow, masks.deny); }; setError(""); From acfa0e37e69ecf92855b48e242522815e9a72dbb Mon Sep 17 00:00:00 2001 From: kipavy Date: Sat, 12 Sep 2026 09:59:21 +0000 Subject: [PATCH 21/21] refactor(members): extract the read-only reason chain into services/permissions.ts minRolePosition + the guardrail chain move beside crossesVaultKeyGate, the file that already carries the "stay in sync with server/src/permissions.rs" contract. Returns the reason enum, not a translated string; READONLY_REASON_KEYS moves to module scope so it isn't rebuilt every render. Behaviour-preserving. --- .../members/panels/MemberDetailPanel.tsx | 47 ++++++--------- src/services/permissions.test.ts | 59 ++++++++++++++++++- src/services/permissions.ts | 37 ++++++++++++ 3 files changed, 113 insertions(+), 30 deletions(-) diff --git a/src/components/members/panels/MemberDetailPanel.tsx b/src/components/members/panels/MemberDetailPanel.tsx index cabf4b0be..89529744e 100644 --- a/src/components/members/panels/MemberDetailPanel.tsx +++ b/src/components/members/panels/MemberDetailPanel.tsx @@ -13,7 +13,8 @@ import { OffboardingDialog } from "@/components/members/OffboardingDialog"; import { ConfirmModal } from "@/components/shared/ConfirmModal"; import type { DepartMode } from "@/services/teamOffboarding"; import { - PERM_BITS, PERM_META, effectivePermissions, crossesVaultKeyGate, type Permission, + PERM_BITS, PERM_META, effectivePermissions, crossesVaultKeyGate, resolveMemberReadOnlyReason, + type Permission, type MemberReadOnlyReason, } from "@/services/permissions"; import { checkAndRotateTeamKey } from "@/services/teamKeyRotation"; import { @@ -32,14 +33,13 @@ export interface MemberDetailPanelProps { onUpdated: () => void; } -/** Lower position = more authority; a role absent from `roles` is skipped. */ -function minRolePosition(roleIds: string[], roles: TeamRole[]): number | null { - return roleIds.reduce((min, rid) => { - const role = roles.find((r) => r.id === rid); - if (!role) return min; - return min === null || role.position < min ? role.position : min; - }, null); -} +const READONLY_REASON_KEYS: Record = { + noManage: "members.permissions.readOnlyNoManage", + owner: "members.permissions.readOnlyOwner", + self: "members.permissions.readOnlySelf", + higherRole: "members.permissions.readOnlyHigherRole", + notHeld: "members.permissions.readOnlyNotHeld", +}; export function MemberDetailPanel({ member, isMe, teamId, teamRoles, canManageMembers, isTargetOwner, viewer, onClose, onUpdated, @@ -137,26 +137,15 @@ export function MemberDetailPanel({ const offendingBits = allow & ~viewerEffective; - type ReadOnlyReasonKind = "noManage" | "owner" | "self" | "higherRole" | "notHeld"; - const READONLY_REASON_KEYS: Record = { - noManage: "members.permissions.readOnlyNoManage", - owner: "members.permissions.readOnlyOwner", - self: "members.permissions.readOnlySelf", - higherRole: "members.permissions.readOnlyHigherRole", - notHeld: "members.permissions.readOnlyNotHeld", - }; - - const readOnlyReasonKind: ReadOnlyReasonKind | null = (() => { - if (!canManageMembers) return "noManage"; - if (isTargetOwner) return "owner"; - if (isMe) return "self"; - const viewerMin = viewer ? minRolePosition(viewer.role_ids, teamRoles) : null; - const targetMin = minRolePosition(member.role_ids, teamRoles); - const hierarchyFails = !viewer || viewerMin === null || (targetMin !== null && viewerMin >= targetMin); - if (hierarchyFails) return "higherRole"; - if (offendingBits !== 0) return "notHeld"; - return null; - })(); + const readOnlyReasonKind = resolveMemberReadOnlyReason({ + canManageMembers, + isTargetOwner, + isMe, + viewerRoleIds: viewer ? viewer.role_ids : null, + targetRoleIds: member.role_ids, + teamRoles, + offendingBits, + }); const readOnlyReason: string | null = readOnlyReasonKind ? t(READONLY_REASON_KEYS[readOnlyReasonKind]) : null; diff --git a/src/services/permissions.test.ts b/src/services/permissions.test.ts index 7e5398953..3c3932d28 100644 --- a/src/services/permissions.test.ts +++ b/src/services/permissions.test.ts @@ -1,5 +1,8 @@ import { test, expect, describe, it } from "vitest"; -import { resolveCan, PERM_BITS, effectivePermissions, crossesVaultKeyGate, type PermissionSnapshot } from "./permissions.ts"; +import { + resolveCan, PERM_BITS, effectivePermissions, crossesVaultKeyGate, resolveMemberReadOnlyReason, + type PermissionSnapshot, +} from "./permissions.ts"; import type { Team, TeamMember, TeamRole } from "@/services/teamService"; import type { Vault } from "@/stores/vaultStore"; @@ -174,6 +177,60 @@ describe("crossesVaultKeyGate", () => { }); }); +describe("resolveMemberReadOnlyReason", () => { + const admin = role("r-admin", 0, { position: 0 }); + const target = role("r-target", 0, { position: 1 }); + + function reason(over: Partial[0]> = {}) { + return resolveMemberReadOnlyReason({ + canManageMembers: true, + isTargetOwner: false, + isMe: false, + viewerRoleIds: ["r-admin"], + targetRoleIds: ["r-target"], + teamRoles: [admin, target], + offendingBits: 0, + ...over, + }); + } + + it("no manage permission wins first", () => { + expect(reason({ canManageMembers: false })).toBe("noManage"); + }); + + it("target is owner", () => { + expect(reason({ isTargetOwner: true })).toBe("owner"); + }); + + it("editing yourself", () => { + expect(reason({ isMe: true })).toBe("self"); + }); + + it("viewer strictly above target: no reason", () => { + expect(reason()).toBeNull(); + }); + + it("viewer at or below target's position: higherRole", () => { + expect(reason({ viewerRoleIds: ["r-target"], targetRoleIds: ["r-admin"] })).toBe("higherRole"); + }); + + it("absent viewer fails closed: higherRole", () => { + expect(reason({ viewerRoleIds: null })).toBe("higherRole"); + }); + + it("roleless viewer fails closed: higherRole", () => { + expect(reason({ viewerRoleIds: [] })).toBe("higherRole"); + }); + + it("roleless target passes hierarchy", () => { + expect(reason({ targetRoleIds: [] })).toBeNull(); + }); + + it("hierarchy passes but offending bits remain: notHeld", () => { + expect(reason({ offendingBits: PERM_BITS.VIEW_SECRETS })).toBe("notHeld"); + }); +}); + test("resolveCan uses team-level deny in the team fallback (before membersByTeam loads)", () => { const s = snap({ teams: [{ id: "t1", name: "t1", owner_id: "o", owner_tier: "team", created_at: "", role_ids: ["r1"], permission_deny: PERM_BITS.VIEW_SECRETS }], diff --git a/src/services/permissions.ts b/src/services/permissions.ts index fbcfafce2..7ccfb7877 100644 --- a/src/services/permissions.ts +++ b/src/services/permissions.ts @@ -75,6 +75,43 @@ export function hasBuiltinRole(member: TeamMember, roleName: string, roles: Team return member.role_ids.includes(target.id); } +export type MemberReadOnlyReason = "noManage" | "owner" | "self" | "higherRole" | "notHeld"; + +/** Lower position = more authority; a role absent from `roles` is skipped. */ +function minRolePosition(roleIds: string[], roles: TeamRole[]): number | null { + return roleIds.reduce((min, rid) => { + const role = roles.find((r) => r.id === rid); + if (!role) return min; + return min === null || role.position < min ? role.position : min; + }, null); +} + +/** + * One section-level reason a member's permission overrides are read-only, first + * failure wins, in the server's own guardrail order. Mirrors `assign_member_role`'s + * `(Some(_), None) => Ok(())`: an absent or roleless viewer fails closed, a roleless + * target passes. + */ +export function resolveMemberReadOnlyReason(params: { + canManageMembers: boolean; + isTargetOwner: boolean; + isMe: boolean; + viewerRoleIds: string[] | null; + targetRoleIds: string[]; + teamRoles: TeamRole[]; + offendingBits: number; +}): MemberReadOnlyReason | null { + if (!params.canManageMembers) return "noManage"; + if (params.isTargetOwner) return "owner"; + if (params.isMe) return "self"; + const viewerMin = params.viewerRoleIds ? minRolePosition(params.viewerRoleIds, params.teamRoles) : null; + const targetMin = minRolePosition(params.targetRoleIds, params.teamRoles); + const hierarchyFails = !params.viewerRoleIds || viewerMin === null || (targetMin !== null && viewerMin >= targetMin); + if (hierarchyFails) return "higherRole"; + if (params.offendingBits !== 0) return "notHeld"; + return null; +} + export interface PermissionSnapshot { myUserId: string; teams: Team[];