From 1507d530b910f9111dad1ee12fff3d35660d677a Mon Sep 17 00:00:00 2001 From: Erin Millard Date: Mon, 16 Dec 2024 08:57:43 +1000 Subject: [PATCH] Simplify GitHub patterns --- src/github-pattern.ts | 41 +----------- src/name-pattern.ts | 7 -- src/pattern.ts | 1 - src/token-authorizer.ts | 13 ++-- .../any-github-pattern-is-all-repos.spec.ts | 27 -------- .../suite/unit/pattern/github-pattern.spec.ts | 64 ------------------- .../github-patterns-for-account.spec.ts | 19 ------ test/suite/unit/pattern/name-pattern.spec.ts | 14 ---- 8 files changed, 6 insertions(+), 180 deletions(-) delete mode 100644 test/suite/unit/pattern/any-github-pattern-is-all-repos.spec.ts delete mode 100644 test/suite/unit/pattern/github-patterns-for-account.spec.ts diff --git a/src/github-pattern.ts b/src/github-pattern.ts index 2fffe09..06bc207 100644 --- a/src/github-pattern.ts +++ b/src/github-pattern.ts @@ -1,30 +1,12 @@ import { createNamePattern } from "./name-pattern.js"; import type { Pattern } from "./pattern.js"; -export type GitHubPattern = Pattern & { - readonly account: Pattern; - readonly repo: Pattern | undefined; - isAllForAccount: (forAccount: string) => boolean; -}; - -export function createGitHubPattern(pattern: string): GitHubPattern { +export function createGitHubPattern(pattern: string): Pattern { const [accountPart, repoPart] = splitGitHubPattern(pattern); const account = createNamePattern(accountPart); const repo = repoPart ? createNamePattern(repoPart) : undefined; return { - get isAll() { - return repo ? account.isAll && repo.isAll : false; - }, - - get account() { - return account; - }, - - get repo() { - return repo; - }, - test: (string) => { const parts = string.split("/"); @@ -35,10 +17,6 @@ export function createGitHubPattern(pattern: string): GitHubPattern { }, toString: () => pattern, - - isAllForAccount: (forAccount: string) => { - return repo ? account.test(forAccount) && repo.isAll : false; - }, }; } @@ -52,23 +30,6 @@ export function normalizeGitHubPattern( return repoPart == null ? definingAccount : `${definingAccount}/${repoPart}`; } -export function gitHubPatternsForAccount( - account: string, - patterns: GitHubPattern[], -): GitHubPattern[] { - const result: GitHubPattern[] = []; - for (const pattern of patterns) { - if (pattern.account.test(account)) result.push(pattern); - } - - return result; -} - -export function anyGitHubPatternIsAllRepos(patterns: GitHubPattern[]): boolean { - for (const pattern of patterns) if (pattern.repo?.isAll) return true; - return false; -} - function splitGitHubPattern(pattern: string): [string, string | undefined] { const parts = pattern.split("/"); diff --git a/src/name-pattern.ts b/src/name-pattern.ts index 4a7b611..1a4f65c 100644 --- a/src/name-pattern.ts +++ b/src/name-pattern.ts @@ -11,14 +11,7 @@ export function createNamePattern(pattern: string): Pattern { const literals = pattern.split("*"); const expression = patternRegExp(literals); - let isAll = true; - for (const l of literals) if (l) isAll = false; - return { - get isAll() { - return isAll; - }, - test: (string) => expression.test(string), toString: () => pattern, }; diff --git a/src/pattern.ts b/src/pattern.ts index df53216..6e66231 100644 --- a/src/pattern.ts +++ b/src/pattern.ts @@ -1,5 +1,4 @@ export type Pattern = { - readonly isAll: boolean; test: (string: string) => boolean; toString: () => string; }; diff --git a/src/token-authorizer.ts b/src/token-authorizer.ts index 8cf72ce..6e0b757 100644 --- a/src/token-authorizer.ts +++ b/src/token-authorizer.ts @@ -1,4 +1,4 @@ -import { createGitHubPattern, type GitHubPattern } from "./github-pattern.js"; +import { createGitHubPattern } from "./github-pattern.js"; import { createNamePattern } from "./name-pattern.js"; import { anyPatternMatches, type Pattern } from "./pattern.js"; import { isSufficientPermissions } from "./permissions.js"; @@ -225,12 +225,9 @@ export function createTokenAuthorizer( function patternsForRules( rules: PermissionsRule[], - ): [ - Record, - Record, - ] { + ): [Record, Record] { const resourcePatterns: Record = {}; - const consumerPatterns: Record = {}; + const consumerPatterns: Record = {}; for (let i = 0; i < rules.length; ++i) { [resourcePatterns[i], consumerPatterns[i]] = patternsForRule(rules[i]); @@ -241,9 +238,9 @@ export function createTokenAuthorizer( function patternsForRule( rule: PermissionsRule, - ): [ResourceCriteriaPatterns[], GitHubPattern[]] { + ): [ResourceCriteriaPatterns[], Pattern[]] { const resourcePatterns: ResourceCriteriaPatterns[] = []; - const consumerPatterns: GitHubPattern[] = []; + const consumerPatterns: Pattern[] = []; for (const criteria of rule.resources) { resourcePatterns.push(patternsForResourceCriteria(criteria)); diff --git a/test/suite/unit/pattern/any-github-pattern-is-all-repos.spec.ts b/test/suite/unit/pattern/any-github-pattern-is-all-repos.spec.ts deleted file mode 100644 index 07d66b5..0000000 --- a/test/suite/unit/pattern/any-github-pattern-is-all-repos.spec.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { expect, it } from "vitest"; -import { - anyGitHubPatternIsAllRepos, - createGitHubPattern, -} from "../../../../src/github-pattern.js"; - -it("returns true if any GitHub pattern matches all repos in any account", () => { - expect( - anyGitHubPatternIsAllRepos([ - createGitHubPattern("account-a/repo-a"), - createGitHubPattern("account-b/*"), - ]), - ).toBe(true); - expect( - anyGitHubPatternIsAllRepos([ - createGitHubPattern("account-a/*"), - createGitHubPattern("account-b/repo-b"), - ]), - ).toBe(true); - expect( - anyGitHubPatternIsAllRepos([ - createGitHubPattern("account-a"), - createGitHubPattern("account-a/repo-*"), - createGitHubPattern("account-b/repo-b"), - ]), - ).toBe(false); -}); diff --git a/test/suite/unit/pattern/github-pattern.spec.ts b/test/suite/unit/pattern/github-pattern.spec.ts index 8b3530d..4ac00a2 100644 --- a/test/suite/unit/pattern/github-pattern.spec.ts +++ b/test/suite/unit/pattern/github-pattern.spec.ts @@ -29,70 +29,6 @@ it("can be converted to a string", () => { expect(String(createGitHubPattern("a*b/c*d"))).toBe("a*b/c*d"); }); -it.each([["*/*"], ["**/**"], ["**********/**********"]])( - "knows that %s matches all repos", - (pattern) => { - expect(createGitHubPattern(pattern).isAll).toBe(true); - }, -); - -it.each([ - ["a"], - ["a/*"], - ["a*/*"], - ["*a/*"], - ["*a*/*"], - ["*/a"], - ["*/a*"], - ["*/*a"], - ["*/*a*"], - ["a/a"], - ["a*/a*"], - ["*a/*a"], - ["*a*/*a*"], -])("knows that %s doesn't match all repos", (pattern) => { - expect(createGitHubPattern(pattern).isAll).toBe(false); -}); - -it.each` - pattern | account - ${"*/*"} | ${"account-a"} - ${"**/**"} | ${"account-a"} - ${"account-*/*"} | ${"account-a"} - ${"account-**/**"} | ${"account-a"} - ${"*-a/*"} | ${"account-a"} - ${"**-a/**"} | ${"account-a"} - ${"account-a/*"} | ${"account-a"} - ${"account-a/**"} | ${"account-a"} - ${"account-a/**********"} | ${"account-a"} -`( - "knows that $pattern matches all repos with account $account", - ({ pattern, account }) => { - const actual = createGitHubPattern(pattern); - - expect(actual.isAllForAccount(account)).toBe(true); - }, -); - -it.each` - pattern | account - ${"account-b/*"} | ${"account-a"} - ${"account-b/**"} | ${"account-a"} - ${"account-b/**********"} | ${"account-a"} - ${"account-a"} | ${"account-a"} - ${"account-a/a"} | ${"account-a"} - ${"account-a/a*"} | ${"account-a"} - ${"account-a/*a"} | ${"account-a"} - ${"account-a/*a*"} | ${"account-a"} -`( - "knows that $pattern doesn't match all repos with account $account", - ({ pattern, account }) => { - const actual = createGitHubPattern(pattern); - - expect(actual.isAllForAccount(account)).toBe(false); - }, -); - const patterns: [pattern: string, matches: string[], nonMatches: string[]][] = [ ["name-a", ["name-a"], ["name-b"]], ["name.a", ["name.a"], ["name-b"]], diff --git a/test/suite/unit/pattern/github-patterns-for-account.spec.ts b/test/suite/unit/pattern/github-patterns-for-account.spec.ts deleted file mode 100644 index e2006e9..0000000 --- a/test/suite/unit/pattern/github-patterns-for-account.spec.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { expect, it } from "vitest"; -import { - createGitHubPattern, - gitHubPatternsForAccount, -} from "../../../../src/github-pattern.js"; - -it("filters GitHub patterns by account", () => { - const a = createGitHubPattern("account-a"); - const b = createGitHubPattern("account-b"); - const c = createGitHubPattern("account-a/repo-c"); - const d = createGitHubPattern("account-b/repo-d"); - const e = createGitHubPattern("account-a/repo-e"); - - expect(gitHubPatternsForAccount("account-a", [a, b, c, d, e])).toEqual([ - a, - c, - e, - ]); -}); diff --git a/test/suite/unit/pattern/name-pattern.spec.ts b/test/suite/unit/pattern/name-pattern.spec.ts index 43fa245..dc116da 100644 --- a/test/suite/unit/pattern/name-pattern.spec.ts +++ b/test/suite/unit/pattern/name-pattern.spec.ts @@ -18,20 +18,6 @@ it("can be converted to a string", () => { expect(String(createNamePattern("a*b"))).toBe("a*b"); }); -it.each([["*"], ["**"], ["**********"]])( - "knows that %s matches all", - (pattern) => { - expect(createNamePattern(pattern).isAll).toBe(true); - }, -); - -it.each([["a"], ["a*"], ["*a"], ["*a*"]])( - "knows that %s doesn't match all", - (pattern) => { - expect(createNamePattern(pattern).isAll).toBe(false); - }, -); - const patterns: [pattern: string, matches: string[], nonMatches: string[]][] = [ ["name-a", ["name-a"], ["name-b"]], ["name.a", ["name.a"], ["name-b"]],