Skip to content

Commit f7e2ef5

Browse files
miya0001miya0001
committed
fix: CodeRabbit レビュー対応 — permissions バリデーション追加、CHANGELOG PR番号、update notes 追加
1 parent 4fa52fd commit f7e2ef5

7 files changed

Lines changed: 129 additions & 24 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
- **Breaking**: `admin tenants update` から `--anonymous-access` / `--no-anonymous-access` フラグを削除 — テナントフィーチャーフラグが廃止され、匿名アクセスは XACML ポリシーで制御 (GeonicDB #752)
1313
- **Breaking**: スコープヘルプから `write:X implies read:X` の記述を削除 — `write:X` は `read:X` を暗黙的に含まなくなった (GeonicDB #723)
1414
- **Docs**: `admin policies create` のヘルプに `servicePath` リソース属性、priority の説明（大きい数値=高優先度）、デフォルトロールポリシーの説明を追加 (GeonicDB #747, #751, #752)
15-
- **Docs**: README を更新 — `--permissions` オプション、XACML 認可モデル、スコープ包含関係、API キーヘッダ排他仕様を反映
15+
- **Docs**: README を更新 — `--permissions` オプション、XACML 認可モデル、スコープ包含関係、API キーヘッダ排他仕様を反映 (#79)
1616

1717
### 2026-03-19
1818
- **Feat**: `admin tenants update` に `--anonymous-access` / `--no-anonymous-access` オプションを追加 — テナントの匿名アクセス設定を CLI から管理可能に (#76)

‎src/commands/admin/api-keys.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { loadConfig, saveConfig } from "../../config.js";
44
import { parseJsonInput } from "../../input.js";
55
import { printError, printWarning } from "../../output.js";
66
import { addExamples, addNotes } from "../help.js";
7-
import { API_KEY_SCOPES_HELP_NOTES } from "../../helpers.js";
7+
import { API_KEY_SCOPES_HELP_NOTES, parsePermissions } from "../../helpers.js";
88

99
function validateOrigins(body: unknown, opts: Record<string, unknown>): void {
1010
// Validate origins if provided via flags
@@ -45,7 +45,7 @@ function buildBodyFromFlags(opts: Record<string, unknown>): Record<string, unkno
4545
payload.rateLimit = { perMinute };
4646
}
4747
if (opts.dpopRequired !== undefined) payload.dpopRequired = opts.dpopRequired;
48-
if (opts.permissions) payload.permissions = (opts.permissions as string).split(",").map((s: string) => s.trim()).filter(Boolean);
48+
if (opts.permissions) payload.permissions = parsePermissions(opts.permissions as string);
4949
if (opts.tenantId) payload.tenantId = opts.tenantId;
5050
return payload;
5151
}
@@ -255,13 +255,23 @@ export function registerApiKeysCommand(parent: Command): void {
255255
),
256256
);
257257

258-
addNotes(update, API_KEY_SCOPES_HELP_NOTES);
258+
addNotes(update, [
259+
...API_KEY_SCOPES_HELP_NOTES,
260+
"",
261+
"Valid permissions: read, write, create, update, delete",
262+
" write = create + update + delete",
263+
" Permissions auto-generate XACML policies (allowedEntityTypes respected).",
264+
]);
259265

260266
addExamples(update, [
261267
{
262268
description: "Update an API key name",
263269
command: "geonic admin api-keys update <key-id> --name new-name",
264270
},
271+
{
272+
description: "Update permissions",
273+
command: "geonic admin api-keys update <key-id> --permissions read,write",
274+
},
265275
{
266276
description: "Enable DPoP requirement",
267277
command: "geonic admin api-keys update <key-id> --dpop-required",

‎src/commands/me-api-keys.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { loadConfig, saveConfig } from "../config.js";
44
import { parseJsonInput } from "../input.js";
55
import { printError, printWarning } from "../output.js";
66
import { addExamples, addNotes } from "./help.js";
7-
import { API_KEY_SCOPES_HELP_NOTES } from "../helpers.js";
7+
import { API_KEY_SCOPES_HELP_NOTES, parsePermissions } from "../helpers.js";
88

99
export function addMeApiKeysSubcommand(me: Command): void {
1010
const apiKeys = me
@@ -75,7 +75,7 @@ export function addMeApiKeysSubcommand(me: Command): void {
7575
if (opts.origins) payload.allowedOrigins = opts.origins.split(",").map((s: string) => s.trim()).filter(Boolean);
7676
if (opts.entityTypes) payload.allowedEntityTypes = opts.entityTypes.split(",").map((s: string) => s.trim()).filter(Boolean);
7777
if (opts.dpopRequired !== undefined) payload.dpopRequired = opts.dpopRequired;
78-
if (opts.permissions) payload.permissions = opts.permissions.split(",").map((s: string) => s.trim()).filter(Boolean);
78+
if (opts.permissions) payload.permissions = parsePermissions(opts.permissions);
7979
if (opts.rateLimit) {
8080
const raw = opts.rateLimit.trim();
8181
if (!/^\d+$/.test(raw)) {

‎src/helpers.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,24 @@ export const API_KEY_SCOPES_HELP_NOTES = [
2828
" read:registrations, write:registrations",
2929
];
3030

31+
/**
32+
* Valid permission values for --permissions option.
33+
*/
34+
export const VALID_PERMISSIONS = new Set(["read", "write", "create", "update", "delete"]);
35+
36+
/**
37+
* Parse and validate --permissions flag value.
38+
* Returns the parsed array or calls process.exit(1) on invalid input.
39+
*/
40+
export function parsePermissions(raw: string): string[] {
41+
const permissions = raw.split(",").map((s) => s.trim()).filter(Boolean);
42+
if (permissions.length === 0 || permissions.some((p) => !VALID_PERMISSIONS.has(p))) {
43+
printError("--permissions must be a comma-separated list of: read, write, create, update, delete");
44+
process.exit(1);
45+
}
46+
return permissions;
47+
}
48+
3149
/**
3250
* Resolve merged options from config + CLI flags.
3351
*/

‎tests/admin-api-keys.test.ts‎

Lines changed: 46 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,20 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { createMockClient, mockResponse, createTestProgram, runCommand } from "./test-helpers.js";
33
import type { MockClient } from "./test-helpers.js";
44

5-
vi.mock("../src/helpers.js", () => ({
6-
createClient: vi.fn(),
7-
getFormat: vi.fn(),
8-
outputResponse: vi.fn(),
9-
withErrorHandler: (fn: (...args: unknown[]) => unknown) => fn,
10-
SCOPES_HELP_NOTES: [],
11-
API_KEY_SCOPES_HELP_NOTES: [],
12-
resolveOptions: vi.fn().mockReturnValue({ profile: "default" }),
13-
}));
5+
vi.mock("../src/helpers.js", async (importOriginal) => {
6+
const actual = await importOriginal<typeof import("../src/helpers.js")>();
7+
return {
8+
createClient: vi.fn(),
9+
getFormat: vi.fn(),
10+
outputResponse: vi.fn(),
11+
withErrorHandler: (fn: (...args: unknown[]) => unknown) => fn,
12+
SCOPES_HELP_NOTES: [],
13+
API_KEY_SCOPES_HELP_NOTES: [],
14+
VALID_PERMISSIONS: actual.VALID_PERMISSIONS,
15+
parsePermissions: actual.parsePermissions,
16+
resolveOptions: vi.fn().mockReturnValue({ profile: "default" }),
17+
};
18+
});
1419

1520
vi.mock("../src/input.js", () => ({
1621
parseJsonInput: vi.fn(),
@@ -370,6 +375,38 @@ describe("admin api-keys commands", () => {
370375
}
371376
});
372377

378+
it("rejects invalid --permissions values", async () => {
379+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
380+
throw new Error("process.exit");
381+
});
382+
383+
const program = makeProgram();
384+
await expect(
385+
runCommand(program, ["admin", "api-keys", "create", "--permissions", "read,foo"])
386+
).rejects.toThrow("process.exit");
387+
388+
expect(printError).toHaveBeenCalledWith(
389+
"--permissions must be a comma-separated list of: read, write, create, update, delete",
390+
);
391+
exitSpy.mockRestore();
392+
});
393+
394+
it("rejects --permissions with only invalid values", async () => {
395+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
396+
throw new Error("process.exit");
397+
});
398+
399+
const program = makeProgram();
400+
await expect(
401+
runCommand(program, ["admin", "api-keys", "create", "--permissions", "admin"])
402+
).rejects.toThrow("process.exit");
403+
404+
expect(printError).toHaveBeenCalledWith(
405+
"--permissions must be a comma-separated list of: read, write, create, update, delete",
406+
);
407+
exitSpy.mockRestore();
408+
});
409+
373410
it("accepts valid allowedOrigins in JSON body", async () => {
374411
const body = { name: "valid-key", allowedOrigins: ["https://example.com"] };
375412
vi.mocked(parseJsonInput).mockResolvedValue(body);

‎tests/helpers.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
getFormat,
2121
outputResponse,
2222
withErrorHandler,
23+
parsePermissions,
24+
VALID_PERMISSIONS,
2325
} from "../src/helpers.js";
2426

2527
function fakeCmd(cliOpts: Partial<GlobalOptions> = {}): Command {
@@ -371,4 +373,37 @@ describe("helpers", () => {
371373
expect(printError).toHaveBeenCalledWith("Forbidden");
372374
});
373375
});
376+
377+
describe("parsePermissions", () => {
378+
it("parses valid permissions", () => {
379+
expect(parsePermissions("read,write")).toEqual(["read", "write"]);
380+
expect(parsePermissions("create, update, delete")).toEqual(["create", "update", "delete"]);
381+
expect(parsePermissions("read")).toEqual(["read"]);
382+
});
383+
384+
it("exits on invalid permission value", () => {
385+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
386+
throw new Error("process.exit");
387+
});
388+
expect(() => parsePermissions("read,foo")).toThrow("process.exit");
389+
expect(printError).toHaveBeenCalledWith(
390+
"--permissions must be a comma-separated list of: read, write, create, update, delete",
391+
);
392+
exitSpy.mockRestore();
393+
});
394+
395+
it("exits on empty string", () => {
396+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
397+
throw new Error("process.exit");
398+
});
399+
expect(() => parsePermissions("")).toThrow("process.exit");
400+
exitSpy.mockRestore();
401+
});
402+
});
403+
404+
describe("VALID_PERMISSIONS", () => {
405+
it("contains exactly the expected values", () => {
406+
expect(VALID_PERMISSIONS).toEqual(new Set(["read", "write", "create", "update", "delete"]));
407+
});
408+
});
374409
});

‎tests/me-api-keys.test.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,20 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
22
import { createMockClient, mockResponse, createTestProgram, runCommand } from "./test-helpers.js";
33
import type { MockClient } from "./test-helpers.js";
44

5-
vi.mock("../src/helpers.js", () => ({
6-
createClient: vi.fn(),
7-
getFormat: vi.fn(),
8-
outputResponse: vi.fn(),
9-
withErrorHandler: (fn: (...args: unknown[]) => unknown) => fn,
10-
SCOPES_HELP_NOTES: [],
11-
API_KEY_SCOPES_HELP_NOTES: [],
12-
resolveOptions: vi.fn().mockReturnValue({ profile: "default" }),
13-
}));
5+
vi.mock("../src/helpers.js", async (importOriginal) => {
6+
const actual = await importOriginal<typeof import("../src/helpers.js")>();
7+
return {
8+
createClient: vi.fn(),
9+
getFormat: vi.fn(),
10+
outputResponse: vi.fn(),
11+
withErrorHandler: (fn: (...args: unknown[]) => unknown) => fn,
12+
SCOPES_HELP_NOTES: [],
13+
API_KEY_SCOPES_HELP_NOTES: [],
14+
VALID_PERMISSIONS: actual.VALID_PERMISSIONS,
15+
parsePermissions: actual.parsePermissions,
16+
resolveOptions: vi.fn().mockReturnValue({ profile: "default" }),
17+
};
18+
});
1419

1520
vi.mock("../src/input.js", () => ({
1621
parseJsonInput: vi.fn(),

0 commit comments

Comments
 (0)