-
Notifications
You must be signed in to change notification settings - Fork 562
fix: authorize custom org repository roles via base permission #45641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8928729
8712729
05e9d3c
d2d24f5
9c58a7d
54f1658
152d9e5
6ab493c
b3c0864
6d19a2a
7f36978
85758a1
8148beb
e3cc02e
9224d6c
a5b0da2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -325,6 +325,125 @@ describe("check_permissions_utils", () => { | |
| expect(mockCore.warning).toHaveBeenCalledWith("User permission 'maintain' does not meet requirements: write"); | ||
| }); | ||
|
|
||
| it("should authorize custom org role via base permission when base permission matches", async () => { | ||
|
pelikhan marked this conversation as resolved.
|
||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "Security Champions", inherited_role: "write" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["admin", "maintain", "write"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: true, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission API fields for 'testuser': permission='write', role='Security Champions', inherited='write'"); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission computed roles for 'testuser': effective='Security Champions', custom_role=true, inherited_standard_role='write'"); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission matched required role 'write' via inherited-standard-role"); | ||
| expect(mockCore.info).toHaveBeenCalledWith("✅ User has Security Champions access to repository"); | ||
| }); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] The two new test cases cover 💡 Suggested additional casesAdd:
These cases ensure the @copilot please address this. |
||
|
|
||
| it("should reject maintain-based custom org role when only write is required", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "Security Champions", inherited_role: "maintain" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["write"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: false, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.warning).toHaveBeenCalledWith("User permission 'Security Champions' does not meet requirements: write"); | ||
| }); | ||
|
|
||
| it("should authorize maintain-based custom org role when maintain is required", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "Security Champions", inherited_role: "maintain" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["maintain"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: true, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.info).toHaveBeenCalledWith("✅ User has Security Champions access to repository"); | ||
| }); | ||
|
|
||
| it("should authorize read-based custom org role when read is required", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "read", role_name: "Security Champions", inherited_role: "read" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["read"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: true, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.info).toHaveBeenCalledWith("✅ User has Security Champions access to repository"); | ||
| }); | ||
|
|
||
| it("should reject read-based custom org role when required permission does not match", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "read", role_name: "Security Champions", inherited_role: "read" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["write"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: false, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.warning).toHaveBeenCalledWith("User permission 'Security Champions' does not meet requirements: write"); | ||
| }); | ||
|
|
||
| it("should authorize when required permissions include the exact custom role name", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "Security Champions", inherited_role: "maintain" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["Security Champions"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: true, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.info).toHaveBeenCalledWith("✅ User has Security Champions access to repository"); | ||
| }); | ||
|
|
||
| it("should not treat an empty role_name as a custom org role", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "", inherited_role: "maintain" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["maintain"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: false, | ||
| permission: "write", | ||
| }); | ||
| expect(mockCore.warning).toHaveBeenCalledWith("User permission 'write' does not meet requirements: maintain"); | ||
| }); | ||
|
|
||
| it("should fail closed for custom org role when inherited role metadata is unavailable", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write", role_name: "Security Champions" }, | ||
| }); | ||
|
|
||
| const result = await checkRepositoryPermission("testuser", "testowner", "testrepo", ["write"]); | ||
|
|
||
| expect(result).toEqual({ | ||
| authorized: false, | ||
| permission: "Security Champions", | ||
| }); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission API fields for 'testuser': permission='write', role='Security Champions', inherited='<empty>'"); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission computed roles for 'testuser': effective='Security Champions', custom_role=true, inherited_standard_role='<empty>'"); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission fallback unavailable for custom role 'Security Champions' because GitHub did not provide an inherited standard role"); | ||
| expect(mockCore.debug).toHaveBeenCalledWith("Repository permission did not match required roles: write"); | ||
| expect(mockCore.warning).toHaveBeenCalledWith("User permission 'Security Champions' does not meet requirements: write"); | ||
| }); | ||
|
|
||
| it("should check permissions in order and stop at first match", async () => { | ||
| mockGithub.rest.repos.getCollaboratorPermissionLevel.mockResolvedValue({ | ||
| data: { permission: "write" }, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.