Permission Robustness (#591)

<!-- ELLIPSIS_HIDDEN -->


> [!IMPORTANT]
> Enhance permission management by adding unique constraints, handling
duplicate ID errors, and updating frontend and backend logic with
comprehensive tests.
> 
>   - **Database**:
> - Add unique constraint on `Permission` table for `[tenancyId,
queryableId]` in `migration.sql`.
>     - Update `schema.prisma` to reflect new unique constraints.
>   - **Backend**:
> - Update `crud.tsx` files to handle `PERMISSION_ID_ALREADY_EXISTS`
error using `isErrorForNonUniquePermission()`.
> - Add `isPrismaUniqueConstraintViolation()` in `prisma-client.tsx` to
identify unique constraint violations.
>     - Add `PermissionIdAlreadyExists` error in `known-errors.tsx`.
>   - **Frontend**:
> - Update `page-client.tsx` and `permission-table.tsx` to check for
duplicate permission IDs before creation.
>   - **Tests**:
> - Add tests in `project-permission-definitions.test.ts` and
`team-permission-definitions.test.ts` to verify duplicate ID handling.
> - Ensure permissions cannot be created with duplicate IDs across
project and team contexts.
> 
> <sup>This description was created by </sup>[<img alt="Ellipsis"
src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=stack-auth%2Fstack-auth&utm_source=github&utm_medium=referral)<sup>
for b3ccd15. It will automatically
update as commits are pushed.</sup>


<!-- ELLIPSIS_HIDDEN -->

---------

Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
Co-authored-by: Zai Shi <zaishi00@outlook.com>

Author: 3 people

apps/backend/prisma/migrations/20250401220515_permission_unique_constraint/migration.sql

@@ -0,0 +1,8 @@
+/*
+  Warnings:
+
+  - A unique constraint covering the columns `[tenancyId,queryableId]` on the table `Permission` will be added. If there are existing duplicate values, this will fail.
+
+*/
+-- CreateIndex
+CREATE UNIQUE INDEX "Permission_tenancyId_queryableId_key" ON "Permission"("tenancyId", "queryableId");

apps/backend/prisma/schema.prisma

@@ -211,7 +211,7 @@ model Permission {
 
   description String?
 
-  // The scope of the permission. If projectConfigId is set, may be USER or TEAM; if teamId is set, must be TEAM.
+  // The scope of the permission. If projectConfigId is set, may be PROJECT or TEAM; if teamId is set, must be PROJECT.
   scope PermissionScope
 
   projectConfig ProjectConfig? @relation(fields: [projectConfigId], references: [id], onDelete: Cascade)
@@ -224,8 +224,9 @@ model Permission {
 
   isDefaultTeamCreatorPermission Boolean @default(false)
   isDefaultTeamMemberPermission  Boolean @default(false)
-  isDefaultProjectPermission Boolean @default(false)
+  isDefaultProjectPermission     Boolean @default(false)
 
+  @@unique([tenancyId, queryableId])
   @@unique([projectConfigId, queryableId])
   @@unique([tenancyId, teamId, queryableId])
 }

apps/backend/src/app/api/latest/project-permission-definitions/crud.tsx

@@ -1,31 +1,47 @@
-import { createPermissionDefinition, deletePermissionDefinition, listPermissionDefinitions, updatePermissionDefinitions } from "@/lib/permissions";
-import { retryTransaction } from "@/prisma-client";
+import { createPermissionDefinition, deletePermissionDefinition, isErrorForNonUniquePermission, listPermissionDefinitions, updatePermissionDefinitions } from "@/lib/permissions";
+import { isPrismaUniqueConstraintViolation, retryTransaction } from "@/prisma-client";
 import { createCrudHandlers } from "@/route-handlers/crud-handler";
+import { KnownErrors } from "@stackframe/stack-shared";
 import { projectPermissionDefinitionsCrud } from '@stackframe/stack-shared/dist/interface/crud/project-permissions';
 import { permissionDefinitionIdSchema, yupObject } from "@stackframe/stack-shared/dist/schema-fields";
 import { createLazyProxy } from "@stackframe/stack-shared/dist/utils/proxies";
 
+
 export const projectPermissionDefinitionsCrudHandlers = createLazyProxy(() => createCrudHandlers(projectPermissionDefinitionsCrud, {
   paramsSchema: yupObject({
     permission_id: permissionDefinitionIdSchema.defined(),
   }),
   async onCreate({ auth, data }) {
     return await retryTransaction(async (tx) => {
-      return await createPermissionDefinition(tx, {
-        scope: "PROJECT",
-        tenancy: auth.tenancy,
-        data,
-      });
+      try {
+        return await createPermissionDefinition(tx, {
+          scope: "PROJECT",
+          tenancy: auth.tenancy,
+          data,
+        });
+      } catch (error) {
+        if (isErrorForNonUniquePermission(error)) {
+          throw new KnownErrors.PermissionIdAlreadyExists(data.id);
+        }
+        throw error;
+      }
     });
   },
   async onUpdate({ auth, data, params }) {
     return await retryTransaction(async (tx) => {
-      return await updatePermissionDefinitions(tx, {
-        scope: "PROJECT",
-        tenancy: auth.tenancy,
-        permissionId: params.permission_id,
-        data,
-      });
+      try {
+        return await updatePermissionDefinitions(tx, {
+          scope: "PROJECT",
+          tenancy: auth.tenancy,
+          permissionId: params.permission_id,
+          data,
+        });
+      } catch (error) {
+        if (isErrorForNonUniquePermission(error)) {
+          throw new KnownErrors.PermissionIdAlreadyExists(data.id ?? '');
+        }
+        throw error;
+      }
     });
   },
   async onDelete({ auth, params }) {

apps/backend/src/app/api/latest/team-permission-definitions/crud.tsx

@@ -1,6 +1,7 @@
-import { createPermissionDefinition, deletePermissionDefinition, listPermissionDefinitions, updatePermissionDefinitions } from "@/lib/permissions";
+import { createPermissionDefinition, deletePermissionDefinition, isErrorForNonUniquePermission, listPermissionDefinitions, updatePermissionDefinitions } from "@/lib/permissions";
 import { retryTransaction } from "@/prisma-client";
 import { createCrudHandlers } from "@/route-handlers/crud-handler";
+import { KnownErrors } from "@stackframe/stack-shared";
 import { teamPermissionDefinitionsCrud } from '@stackframe/stack-shared/dist/interface/crud/team-permissions';
 import { permissionDefinitionIdSchema, yupObject } from "@stackframe/stack-shared/dist/schema-fields";
 import { createLazyProxy } from "@stackframe/stack-shared/dist/utils/proxies";
@@ -11,21 +12,35 @@ export const teamPermissionDefinitionsCrudHandlers = createLazyProxy(() => creat
   }),
   async onCreate({ auth, data }) {
     return await retryTransaction(async (tx) => {
-      return await createPermissionDefinition(tx, {
-        scope: "TEAM",
-        tenancy: auth.tenancy,
-        data,
-      });
+      try {
+        return await createPermissionDefinition(tx, {
+          scope: "TEAM",
+          tenancy: auth.tenancy,
+          data,
+        });
+      } catch (error) {
+        if (isErrorForNonUniquePermission(error)) {
+          throw new KnownErrors.PermissionIdAlreadyExists(data.id);
+        }
+        throw error;
+      }
     });
   },
   async onUpdate({ auth, data, params }) {
     return await retryTransaction(async (tx) => {
-      return await updatePermissionDefinitions(tx, {
-        scope: "TEAM",
-        tenancy: auth.tenancy,
-        permissionId: params.permission_id,
-        data,
-      });
+      try {
+        return await updatePermissionDefinitions(tx, {
+          scope: "TEAM",
+          tenancy: auth.tenancy,
+          permissionId: params.permission_id,
+          data,
+        });
+      } catch (error) {
+        if (isErrorForNonUniquePermission(error)) {
+          throw new KnownErrors.PermissionIdAlreadyExists(data.id ?? '');
+        }
+        throw error;
+      }
     });
   },
   async onDelete({ auth, params }) {

apps/backend/src/lib/permissions.tsx

@@ -1,7 +1,8 @@
+import { isPrismaUniqueConstraintViolation } from "@/prisma-client";
 import { TeamSystemPermission as DBTeamSystemPermission, Prisma } from "@prisma/client";
 import { KnownErrors } from "@stackframe/stack-shared";
-import { TeamPermissionDefinitionsCrud, TeamPermissionsCrud } from "@stackframe/stack-shared/dist/interface/crud/team-permissions";
 import { ProjectPermissionsCrud } from "@stackframe/stack-shared/dist/interface/crud/project-permissions";
+import { TeamPermissionDefinitionsCrud, TeamPermissionsCrud } from "@stackframe/stack-shared/dist/interface/crud/team-permissions";
 import { groupBy } from "@stackframe/stack-shared/dist/utils/arrays";
 import { StackAssertionError, throwErr } from "@stackframe/stack-shared/dist/utils/errors";
 import { stringCompare, typedToLowercase, typedToUppercase } from "@stackframe/stack-shared/dist/utils/strings";
@@ -649,3 +650,9 @@ export async function grantDefaultProjectPermissions(
 
   return defaultPermissions.length > 0;
 }
+
+export function isErrorForNonUniquePermission(error: unknown): boolean  {
+  return isPrismaUniqueConstraintViolation(error, "Permission", ["tenancyId", "queryableId"]) ||
+    isPrismaUniqueConstraintViolation(error, "Permission", ["projectConfigId", "queryableId"]) ||
+    isPrismaUniqueConstraintViolation(error, "Permission", ["tenancyId", "teamId", "queryableId"]);
+}

apps/backend/src/prisma-client.tsx

@@ -1,7 +1,7 @@
 import { Prisma, PrismaClient } from "@prisma/client";
 import { withAccelerate } from "@prisma/extension-accelerate";
 import { getEnvVariable, getNodeEnvironment } from '@stackframe/stack-shared/dist/utils/env';
-import { filterUndefined, typedFromEntries, typedKeys } from "@stackframe/stack-shared/dist/utils/objects";
+import { deepPlainEquals, filterUndefined, typedFromEntries, typedKeys } from "@stackframe/stack-shared/dist/utils/objects";
 import { Result } from "@stackframe/stack-shared/dist/utils/results";
 import { traceSpan } from "./utils/telemetry";
 
@@ -135,3 +135,21 @@ async function rawQueryArray<Q extends RawQuery<any>[]>(queries: Q): Promise<[]
   });
 }
 
+// not exhaustive
+export const PRISMA_ERROR_CODES = {
+  VALUE_TOO_LONG: "P2000",
+  RECORD_NOT_FOUND: "P2001",
+  UNIQUE_CONSTRAINT_VIOLATION: "P2002",
+  FOREIGN_CONSTRAINT_VIOLATION: "P2003",
+  GENERIC_CONSTRAINT_VIOLATION: "P2004",
+} as const;
+
+export function isPrismaError(error: unknown, code: keyof typeof PRISMA_ERROR_CODES): error is Prisma.PrismaClientKnownRequestError {
+  return error instanceof Prisma.PrismaClientKnownRequestError && error.code === PRISMA_ERROR_CODES[code];
+}
+
+export function isPrismaUniqueConstraintViolation(error: unknown, modelName: string, target: string | string[]): error is Prisma.PrismaClientKnownRequestError {
+  if (!isPrismaError(error, "UNIQUE_CONSTRAINT_VIOLATION")) return false;
+  if (!error.meta?.target) return false;
+  return error.meta.modelName === modelName && deepPlainEquals(error.meta.target, target);
+}

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/project-permissions/page-client.tsx

@@ -40,17 +40,18 @@ function CreateDialog(props: {
   onOpenChange: (open: boolean) => void,
 }) {
   const stackAdminApp = useAdminApp();
-  const permissions = stackAdminApp.useProjectPermissionDefinitions();
+  const projectPermissions = stackAdminApp.useProjectPermissionDefinitions();
+  const combinedPermissions = [...stackAdminApp.useTeamPermissionDefinitions(), ...projectPermissions];
 
   const formSchema = yup.object({
     id: yup.string().defined()
-      .notOneOf(permissions.map((p) => p.id), "ID already exists")
+      .notOneOf(combinedPermissions.map((p) => p.id), "ID already exists")
       .matches(/^[a-z0-9_:]+$/, 'Only lowercase letters, numbers, ":" and "_" are allowed')
       .label("ID"),
     description: yup.string().label("Description"),
     containedPermissionIds: yup.array().of(yup.string().defined()).defined().default([]).meta({
       stackFormFieldRender: (props) => (
-        <PermissionListField {...props} permissions={permissions} type="new" />
+        <PermissionListField {...props} permissions={projectPermissions} type="new" />
       ),
     }),
   });

apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/team-permissions/page-client.tsx

@@ -41,17 +41,18 @@ function CreateDialog(props: {
   onOpenChange: (open: boolean) => void,
 }) {
   const stackAdminApp = useAdminApp();
-  const permissions = stackAdminApp.useTeamPermissionDefinitions();
+  const teamPermissions = stackAdminApp.useTeamPermissionDefinitions();
+  const combinedPermissions = [...teamPermissions, ...stackAdminApp.useProjectPermissionDefinitions()];
 
   const formSchema = yup.object({
     id: yup.string().defined()
-      .notOneOf(permissions.map((p) => p.id), "ID already exists")
+      .notOneOf(combinedPermissions.map((p) => p.id), "ID already exists")
       .matches(/^[a-z0-9_:]+$/, 'Only lowercase letters, numbers, ":" and "_" are allowed')
       .label("ID"),
     description: yup.string().label("Description"),
     containedPermissionIds: yup.array().of(yup.string().defined()).defined().default([]).meta({
       stackFormFieldRender: (props) => (
-        <PermissionListField {...props} permissions={permissions} type="new" />
+        <PermissionListField {...props} permissions={teamPermissions} type="new" />
       ),
     }),
   });

apps/dashboard/src/components/data-table/permission-table.tsx

@@ -31,9 +31,11 @@ function EditDialog(props: {
   permissionType: PermissionType,
 }) {
   const stackAdminApp = useAdminApp();
-  const permissions = props.permissionType === 'project'
-    ? stackAdminApp.useProjectPermissionDefinitions()
-    : stackAdminApp.useTeamPermissionDefinitions();
+  const teamPermissions = stackAdminApp.useTeamPermissionDefinitions();
+  const projectPermissions = stackAdminApp.useProjectPermissionDefinitions();
+  const permissions = props.permissionType === 'project' ? projectPermissions : teamPermissions;
+  const combinedPermissions = [...teamPermissions, ...projectPermissions];
+
   const currentPermission = permissions.find((p) => p.id === props.selectedPermissionId);
   if (!currentPermission) {
     return null;
@@ -42,7 +44,7 @@ function EditDialog(props: {
   const formSchema = yup.object({
     id: yup.string()
       .defined()
-      .notOneOf(permissions.map((p) => p.id).filter(p => p !== props.selectedPermissionId), "ID already exists")
+      .notOneOf(combinedPermissions.map((p) => p.id).filter(p => p !== props.selectedPermissionId), "ID already exists")
       .matches(/^[a-z0-9_:]+$/, 'Only lowercase letters, numbers, ":" and "_" are allowed')
       .label("ID"),
     description: yup.string().label("Description"),

apps/e2e/tests/backend/endpoints/api/v1/project-permission-definitions.test.ts

@@ -173,3 +173,82 @@ it("creates, updates, and deletes a new user permission", async ({ expect }) =>
     }
   `);
 });
+
+it("handles duplicate permission IDs correctly", async ({ expect }) => {
+  backendContext.set({ projectKeys: InternalProjectKeys });
+  const { adminAccessToken } = await Project.createAndGetAdminToken();
+
+  // Create first permission
+  const response1 = await niceBackendFetch(`/api/v1/project-permission-definitions`, {
+    accessType: "admin",
+    method: "POST",
+    body: {
+      id: 'duplicate_test',
+      description: "Test permission"
+    },
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+  expect(response1.status).toBe(201);
+
+  // Try to create another permission with the same ID
+  const response2 = await niceBackendFetch(`/api/v1/project-permission-definitions`, {
+    accessType: "admin",
+    method: "POST",
+    body: {
+      id: 'duplicate_test',
+      description: "Another test permission"
+    },
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+  expect(response2.status).toBe(400);
+  expect(response2.body).toHaveProperty("code", "PERMISSION_ID_ALREADY_EXISTS");
+
+  // Create another permission
+  const response3 = await niceBackendFetch(`/api/v1/project-permission-definitions`, {
+    accessType: "admin",
+    method: "POST",
+    body: {
+      id: 'update_test',
+      description: "Test permission for update"
+    },
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+  expect(response3.status).toBe(201);
+
+  // Update the first permission to have the ID of the second (which should fail)
+  const response4 = await niceBackendFetch(`/api/v1/project-permission-definitions/duplicate_test`, {
+    accessType: "admin",
+    method: "PATCH",
+    body: {
+      id: 'update_test',
+      description: "Updated description"
+    },
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+  expect(response4.status).toBe(400);
+  expect(response4.body).toHaveProperty("code", "PERMISSION_ID_ALREADY_EXISTS");
+
+  // Clean up
+  await niceBackendFetch(`/api/v1/project-permission-definitions/duplicate_test`, {
+    accessType: "admin",
+    method: "DELETE",
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+  await niceBackendFetch(`/api/v1/project-permission-definitions/update_test`, {
+    accessType: "admin",
+    method: "DELETE",
+    headers: {
+      'x-stack-admin-access-token': adminAccessToken
+    },
+  });
+});