diff --git a/apps/api/src/routes/v1_keys_addPermissions.ts b/apps/api/src/routes/v1_keys_addPermissions.ts index 66f3cb41d0..103ee26d2b 100644 --- a/apps/api/src/routes/v1_keys_addPermissions.ts +++ b/apps/api/src/routes/v1_keys_addPermissions.ts @@ -99,7 +99,7 @@ export const registerV1KeysAddPermissions = (app: App) => const { db, rbac, cache } = c.get("services"); const requestedIds = req.permissions.filter((p) => "id" in p).map((p) => p.id!); - const requestedNames = req.permissions.filter((p) => "name" in p).map((p) => p.name!); + const requestedSlugs = req.permissions.filter((p) => "name" in p).map((p) => p.name!); const [key, existingPermissions, connectedPermissions] = await Promise.all([ db.primary.query.keys.findFirst({ @@ -116,7 +116,7 @@ export const registerV1KeysAddPermissions = (app: App) => eq(table.workspaceId, auth.authorizedWorkspaceId), or( requestedIds.length > 0 ? inArray(table.id, requestedIds) : undefined, - requestedNames.length > 0 ? inArray(table.name, requestedNames) : undefined, + requestedSlugs.length > 0 ? inArray(table.slug, requestedSlugs) : undefined, ), ), }), @@ -130,7 +130,7 @@ export const registerV1KeysAddPermissions = (app: App) => throw new UnkeyApiError({ code: "NOT_FOUND", message: `key ${req.keyId} not found` }); } - const missingPermissionNames: string[] = []; + const missingPermissionSlugs: string[] = []; for (const permission of req.permissions) { if ("id" in permission && !existingPermissions.some((ep) => ep.id === permission.id)) { throw new UnkeyApiError({ @@ -138,22 +138,22 @@ export const registerV1KeysAddPermissions = (app: App) => message: `permission ${permission.id} not found`, }); } - if ("name" in permission && !existingPermissions.some((ep) => ep.name === permission.name)) { + if ("name" in permission && !existingPermissions.some((ep) => ep.slug === permission.name)) { if (!permission.create) { throw new UnkeyApiError({ code: "NOT_FOUND", message: `permission ${permission.name} not found`, }); } - missingPermissionNames.push(permission.name!); + missingPermissionSlugs.push(permission.name!); } } - const createPermissions = missingPermissionNames.map((name) => ({ + const createPermissions = missingPermissionSlugs.map((slug) => ({ id: newId("permission"), workspaceId: auth.authorizedWorkspaceId, - name, - slug: name, + name: slug, + slug, })); if (createPermissions.length > 0) { const rbacResp = rbac.evaluatePermissions( @@ -176,8 +176,8 @@ export const registerV1KeysAddPermissions = (app: App) => await db.primary.insert(schema.permissions).values(createPermissions); } const allPermissions = [ - ...existingPermissions.map((p) => ({ id: p.id, name: p.name })), - ...createPermissions.map((p) => ({ id: p.id, name: p.name })), + ...existingPermissions.map((p) => ({ id: p.id, slug: p.name })), + ...createPermissions.map((p) => ({ id: p.id, slug: p.name })), ]; const addPermissions = allPermissions.filter( @@ -263,5 +263,5 @@ export const registerV1KeysAddPermissions = (app: App) => })), ]); - return c.json(allPermissions.map((p) => ({ id: p.id, name: p.name }))); + return c.json(allPermissions.map((p) => ({ id: p.id, name: p.slug }))); }); diff --git a/apps/api/src/routes/v1_keys_createKey.happy.test.ts b/apps/api/src/routes/v1_keys_createKey.happy.test.ts index a091968b89..28bea35fac 100644 --- a/apps/api/src/routes/v1_keys_createKey.happy.test.ts +++ b/apps/api/src/routes/v1_keys_createKey.happy.test.ts @@ -349,7 +349,37 @@ test("upserts the specified permissions", async (t) => { expect(res.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); - const key = await h.db.primary.query.keys.findFirst({ + let key = await h.db.primary.query.keys.findFirst({ + where: (table, { eq }) => eq(table.id, res.body.keyId), + with: { + permissions: { + with: { + permission: true, + }, + }, + }, + }); + expect(key).toBeDefined(); + expect(key!.permissions.length).toBe(2); + for (const p of key!.permissions!) { + expect(permissions).include(p.permission.name); + } + + const res2 = await h.post({ + url: "/v1/keys.createKey", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${root.key}`, + }, + body: { + apiId: h.resources.userApi.id, + permissions, + }, + }); + + expect(res2.status, `expected 200, received: ${JSON.stringify(res, null, 2)}`).toBe(200); + + key = await h.db.primary.query.keys.findFirst({ where: (table, { eq }) => eq(table.id, res.body.keyId), with: { permissions: { @@ -365,6 +395,7 @@ test("upserts the specified permissions", async (t) => { expect(permissions).include(p.permission.name); } }); + describe("with encryption", () => { test("encrypts a key", async (t) => { const h = await IntegrationHarness.init(t); diff --git a/apps/api/src/routes/v1_keys_createKey.ts b/apps/api/src/routes/v1_keys_createKey.ts index 43948e3676..3a8372a674 100644 --- a/apps/api/src/routes/v1_keys_createKey.ts +++ b/apps/api/src/routes/v1_keys_createKey.ts @@ -554,20 +554,20 @@ async function getPermissionIds( rbac: RBAC, db: Database, workspaceId: string, - permissionNames: Array, + permissionsSlugs: Array, ): Promise> { - if (permissionNames.length === 0) { + if (permissionsSlugs.length === 0) { return []; } const permissions = await db.query.permissions.findMany({ where: (table, { inArray, and, eq }) => - and(eq(table.workspaceId, workspaceId), inArray(table.name, permissionNames)), + and(eq(table.workspaceId, workspaceId), inArray(table.slug, permissionsSlugs)), columns: { id: true, - name: true, + slug: true, }, }); - if (permissions.length === permissionNames.length) { + if (permissions.length === permissionsSlugs.length) { return permissions.map((r) => r.id); } @@ -585,15 +585,15 @@ async function getPermissionIds( throw new UnkeyApiError({ code: "INSUFFICIENT_PERMISSIONS", message: val.message }); } - const missingPermissions = permissionNames.filter( - (name) => !permissions.some((permission) => permission.name === name), + const missingPermissionSlugs = permissionsSlugs.filter( + (slug) => !permissions.some((permission) => permission.slug === slug), ); - const newPermissions = missingPermissions.map((name) => ({ + const newPermissions = missingPermissionSlugs.map((slug) => ({ id: newId("permission"), workspaceId, - name, - slug: name, + name: slug, + slug, })); await db.insert(schema.permissions).values(newPermissions); diff --git a/go/apps/api/routes/v2_permissions_create_permission/409_test.go b/go/apps/api/routes/v2_permissions_create_permission/409_test.go index 94937ed7c2..cb4c8fca30 100644 --- a/go/apps/api/routes/v2_permissions_create_permission/409_test.go +++ b/go/apps/api/routes/v2_permissions_create_permission/409_test.go @@ -35,14 +35,14 @@ func TestConflictErrors(t *testing.T) { "Authorization": {fmt.Sprintf("Bearer %s", rootKey)}, } - // Test case for duplicate permission name - t.Run("duplicate permission name", func(t *testing.T) { - permissionName := "test.duplicate.permission" + // Test case for duplicate permission slug + t.Run("duplicate permission slug", func(t *testing.T) { + permissionSlug := "test.duplicate.permission" // First, create a permission req1 := handler.Request{ - Name: permissionName, - Slug: "test-duplicate-permission", + Name: permissionSlug + "-1", + Slug: permissionSlug, } res1 := testutil.CallRoute[handler.Request, handler.Response]( @@ -57,10 +57,10 @@ func TestConflictErrors(t *testing.T) { require.NotNil(t, res1.Body.Data) require.NotEmpty(t, res1.Body.Data.PermissionId) - // Now try to create another permission with the same name + // Now try to create another permission with the same slug req2 := handler.Request{ - Name: permissionName, - Slug: "test-duplicate-permission-2", + Name: permissionSlug + "-2", + Slug: permissionSlug, } res2 := testutil.CallRoute[handler.Request, openapi.ConflictErrorResponse]( @@ -73,59 +73,8 @@ func TestConflictErrors(t *testing.T) { require.Equal(t, http.StatusConflict, res2.Status, "Duplicate permission creation should fail with 409") require.NotNil(t, res2.Body) require.NotNil(t, res2.Body.Error) + // Our implementation returns just "already exists" as the error detail require.Contains(t, res2.Body.Error.Detail, "already exists") }) - - // Test case for duplicate permission name with different case (if case-insensitive) - t.Run("duplicate permission name different case", func(t *testing.T) { - permissionName := "Test.Case.Sensitive.Permission" - - // First, create a permission - req1 := handler.Request{ - Name: permissionName, - Slug: "test-case-sensitive-permission", - } - - res1 := testutil.CallRoute[handler.Request, handler.Response]( - h, - route, - headers, - req1, - ) - - require.Equal(t, http.StatusOK, res1.Status, "First permission creation should succeed") - - // Now try to create another permission with the same name but different case - req2 := handler.Request{ - Name: "test.case.sensitive.permission", // lowercase version - Slug: "test-case-sensitive-permission-lowercase", - } - - // This test might pass or fail depending on if permission names are case-sensitive - // Try both possible assertions based on the expected behavior - res2 := testutil.CallRoute[handler.Request, handler.Response]( - h, - route, - headers, - req2, - ) - - // If permissions are case-sensitive, could be 200 OK - // If permissions are case-insensitive, should be 409 Conflict - // Check either case - if res2.Status == http.StatusConflict { - // Try calling specifically for a conflict response - res2Conflict := testutil.CallRoute[handler.Request, openapi.ConflictErrorResponse]( - h, - route, - headers, - req2, - ) - require.Equal(t, http.StatusConflict, res2Conflict.Status) - require.NotNil(t, res2Conflict.Body.Error) - // Our implementation includes the full context in the error message - require.Contains(t, res2Conflict.Body.Error.Detail, "already exists") - } - }) } diff --git a/go/pkg/db/schema.sql b/go/pkg/db/schema.sql index 275a93152e..8c7e432b4b 100644 --- a/go/pkg/db/schema.sql +++ b/go/pkg/db/schema.sql @@ -47,7 +47,6 @@ CREATE TABLE `permissions` ( `created_at_m` bigint NOT NULL DEFAULT 0, `updated_at_m` bigint, CONSTRAINT `permissions_id` PRIMARY KEY(`id`), - CONSTRAINT `unique_name_per_workspace_idx` UNIQUE(`name`,`workspace_id`), CONSTRAINT `unique_slug_per_workspace_idx` UNIQUE(`slug`,`workspace_id`) ); diff --git a/internal/db/src/schema/rbac.ts b/internal/db/src/schema/rbac.ts index 1bc4855825..6a899c29dd 100644 --- a/internal/db/src/schema/rbac.ts +++ b/internal/db/src/schema/rbac.ts @@ -27,10 +27,6 @@ export const permissions = mysqlTable( }, (table) => { return { - uniqueNamePerWorkspaceIdx: unique("unique_name_per_workspace_idx").on( - table.workspaceId, - table.name, - ), uniqueSlugPerWorkspaceIdx: unique("unique_slug_per_workspace_idx").on( table.workspaceId, table.slug,