From ff8866888032792a0ba1fa04754f7164524965aa Mon Sep 17 00:00:00 2001 From: rafaumeu Date: Tue, 7 Jul 2026 22:14:10 -0300 Subject: [PATCH 1/2] fix: use membership role as source-of-truth for organization roles (#1641) The getEffectiveOrganizationRole function previously used organizations.ownerId as the primary source for determining the 'owner' role, with memberRole as a fallback. This caused two problems: 1. When a user was the organization owner (ownerId match) but their membership record had a different role (e.g. 'member'), the ownerId check would incorrectly override their actual membership role. 2. When a user's membership record had role='owner' but they weren't the ownerId, the function would force their role to 'member', silently stripping their owner privileges. This fix makes memberRole from organization_members the primary source of truth, with ownerId only used as a fallback when no membership record exists. Also updates isMemberOwner in MembersCard and isOrganizationOwnerTarget to use membership role instead of ownerId comparison. Closes #1641 --- .../unit/desktop-organization-branding.test.ts | 3 ++- apps/web/__tests__/unit/roles-permissions.test.ts | 15 ++++++++++++--- .../organization/components/MembersCard.tsx | 3 ++- apps/web/lib/permissions/roles.ts | 15 ++++++++++++--- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/apps/web/__tests__/unit/desktop-organization-branding.test.ts b/apps/web/__tests__/unit/desktop-organization-branding.test.ts index 4a29a8b30b3..6b5ef372bc6 100644 --- a/apps/web/__tests__/unit/desktop-organization-branding.test.ts +++ b/apps/web/__tests__/unit/desktop-organization-branding.test.ts @@ -144,12 +144,13 @@ describe("desktop organization branding", () => { expect( canEditOrganizationBranding(row({ tombstoneAt: new Date() }), "user-1"), ).toBe(false); + // memberRole "owner" takes precedence — user CAN edit expect( canEditOrganizationBranding( row({ ownerId: "user-2", role: "owner" }), "user-1", ), - ).toBe(false); + ).toBe(true); }); it("validates and normalizes branding patch payloads", () => { diff --git a/apps/web/__tests__/unit/roles-permissions.test.ts b/apps/web/__tests__/unit/roles-permissions.test.ts index 3aa19016cce..20dcdbee537 100644 --- a/apps/web/__tests__/unit/roles-permissions.test.ts +++ b/apps/web/__tests__/unit/roles-permissions.test.ts @@ -27,14 +27,15 @@ describe("organization role permissions", () => { expect(normalizeAssignableOrganizationRole("owner")).toBeNull(); }); - it("derives owner from organization ownership even when membership role differs", () => { + it("uses membership role as source-of-truth for organization roles (#1641)", () => { + // memberRole takes precedence over ownerId expect( getEffectiveOrganizationRole({ userId: "user-1", ownerId: "user-1", memberRole: "member", }), - ).toBe("owner"); + ).toBe("member"); expect( getEffectiveOrganizationRole({ userId: "user-2", @@ -48,7 +49,15 @@ describe("organization role permissions", () => { ownerId: "real-owner", memberRole: "owner", }), - ).toBe("member"); + ).toBe("owner"); + // ownerId fallback when membership record is missing + expect( + getEffectiveOrganizationRole({ + userId: "user-1", + ownerId: "user-1", + memberRole: null, + }), + ).toBe("owner"); }); it("allows only owners and admins to view and manage organization members", () => { diff --git a/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx b/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx index e8d59b05127..4e9691db8e0 100644 --- a/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx +++ b/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx @@ -205,7 +205,8 @@ export const MembersCard = ({ setIsInviteDialogOpen }: MembersCardProps) => { }; const isMemberOwner = (id: string) => { - return id === activeOrganization?.organization.ownerId; + const member = activeOrganization?.members.find((m) => m.userId === id); + return member?.role === "owner"; }; return ( diff --git a/apps/web/lib/permissions/roles.ts b/apps/web/lib/permissions/roles.ts index 2c01192780b..2a5d3987655 100644 --- a/apps/web/lib/permissions/roles.ts +++ b/apps/web/lib/permissions/roles.ts @@ -66,9 +66,15 @@ export function getEffectiveOrganizationRole({ ownerId: string | null | undefined; memberRole: string | null | undefined; }): OrganizationRole | null { + // Prefer organization_members.role as source-of-truth over organization.ownerId + // to avoid drift between the two fields (see #1641). + const normalizedRole = normalizeOrganizationRole(memberRole); + if (normalizedRole === "owner") return "owner"; + if (normalizedRole) return normalizedRole; + // Fallback: if membership record is missing (e.g. legacy data), + // derive owner from the organization.ownerId field. if (userId && ownerId && userId === ownerId) return "owner"; - const role = normalizeOrganizationRole(memberRole); - return role === "owner" ? "member" : role; + return null; } export function normalizeSpaceRole( @@ -131,7 +137,10 @@ export function isOrganizationOwnerTarget({ ownerId: string | null | undefined; targetRole: OrganizationRole | null | undefined; }) { - return targetRole === "owner" || (!!targetUserId && targetUserId === ownerId); + // Prefer membership role over ownerId (see #1641). + if (targetRole === "owner") return true; + // Fallback for legacy/missing membership records. + return !!targetUserId && !!ownerId && targetUserId === ownerId; } export function canChangeOrganizationMemberRole({ From f7d0dfd65d12afb1069931d0ca42384119d918d7 Mon Sep 17 00:00:00 2001 From: rafaumeu Date: Tue, 7 Jul 2026 22:35:08 -0300 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20restrict=20ownerId=20fallback=20to=20missing=20memb?= =?UTF-8?q?ership?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - isOrganizationOwnerTarget: only use ownerId fallback when targetRole is null/undefined (not when it's a known non-owner role) - MembersCard.isMemberOwner: fall back to ownerId comparison when no membership record exists (legacy/migration state) Co-authored-by: tembo (suggestion) --- .../dashboard/settings/organization/components/MembersCard.tsx | 2 +- apps/web/lib/permissions/roles.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx b/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx index 4e9691db8e0..2d479d0e055 100644 --- a/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx +++ b/apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx @@ -206,7 +206,7 @@ export const MembersCard = ({ setIsInviteDialogOpen }: MembersCardProps) => { const isMemberOwner = (id: string) => { const member = activeOrganization?.members.find((m) => m.userId === id); - return member?.role === "owner"; + return member ? member.role === "owner" : id === activeOrganization?.organization.ownerId; }; return ( diff --git a/apps/web/lib/permissions/roles.ts b/apps/web/lib/permissions/roles.ts index 2a5d3987655..79566c14cea 100644 --- a/apps/web/lib/permissions/roles.ts +++ b/apps/web/lib/permissions/roles.ts @@ -139,6 +139,7 @@ export function isOrganizationOwnerTarget({ }) { // Prefer membership role over ownerId (see #1641). if (targetRole === "owner") return true; + if (targetRole) return false; // Fallback for legacy/missing membership records. return !!targetUserId && !!ownerId && targetUserId === ownerId; }