Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions apps/api/src/routes/v1_keys_addPermissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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,
),
),
}),
Expand All @@ -130,30 +130,30 @@ 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({
code: "NOT_FOUND",
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(
Expand All @@ -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(
Expand Down Expand Up @@ -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 })));
});
33 changes: 32 additions & 1 deletion apps/api/src/routes/v1_keys_createKey.happy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<V1KeysCreateKeyRequest, V1KeysCreateKeyResponse>({
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: {
Expand All @@ -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);
Expand Down
20 changes: 10 additions & 10 deletions apps/api/src/routes/v1_keys_createKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,20 +554,20 @@ async function getPermissionIds(
rbac: RBAC,
db: Database,
workspaceId: string,
permissionNames: Array<string>,
permissionsSlugs: Array<string>,
): Promise<Array<string>> {
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);
}

Expand All @@ -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);
Expand Down
69 changes: 9 additions & 60 deletions go/apps/api/routes/v2_permissions_create_permission/409_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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](
Expand All @@ -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](
Expand All @@ -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")
}
})
}
1 change: 0 additions & 1 deletion go/pkg/db/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
);

Expand Down
4 changes: 0 additions & 4 deletions internal/db/src/schema/rbac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading