Skip to content

fix: getTeamIdsWithPermission return sub teams#25387

Merged
volnei merged 16 commits intomainfrom
fix/getTeamIdsWithPermission-org
Dec 2, 2025
Merged

fix: getTeamIdsWithPermission return sub teams#25387
volnei merged 16 commits intomainfrom
fix/getTeamIdsWithPermission-org

Conversation

@sean-brydon
Copy link
Member

What does this PR do?

Pr fixes the issue where getTeamIdsWithPermissions would only return the direct teamId queried permission if it exist and did not count the org Id if they had the permission granted in the parent TeamId

@graphite-app graphite-app bot requested a review from a team November 25, 2025 09:50
@keithwillcode keithwillcode added consumer core area: core, team members only labels Nov 25, 2025
SELECT child."id" as "teamId"
FROM "Membership" org_m
INNER JOIN "Role" org_r ON org_m."customRoleId" = org_r.id
INNER JOIN "Team" child ON child."parentId" = org_m."teamId"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ParentID

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/features/pbac/infrastructure/repositories/PermissionRepository.ts">

<violation number="1" location="packages/features/pbac/infrastructure/repositories/PermissionRepository.ts:326">
PBAC-disabled fallback filtering is applied to the parent org instead of the child team, so PBAC-enabled child teams can be unlocked via fallback roles. Ensure the NOT EXISTS check inspects the child team’s feature flag.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

volnei
volnei previously approved these changes Nov 25, 2025
@volnei volnei enabled auto-merge (squash) November 25, 2025 11:03
@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2025

E2E results are ready!

@vercel
Copy link

vercel bot commented Nov 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
cal-companion Error Error Dec 1, 2025 4:01pm
2 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
cal Ignored Ignored Dec 1, 2025 4:01pm
cal-eu Ignored Ignored Dec 1, 2025 4:01pm

Udit-takkar
Udit-takkar previously approved these changes Nov 25, 2025
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 3 files

Prompt for AI agents (all 2 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/features/pbac/services/__tests__/permission-check.service.test.ts">

<violation number="1" location="packages/features/pbac/services/__tests__/permission-check.service.test.ts:558">
The new “PBAC permissions” test is a duplicate of the fallback-role test, so it does not verify PBAC behavior and will never fail if PBAC support breaks. Please rewrite it to exercise PBAC-specific logic (or drop it) instead of duplicating the previous test.</violation>
</file>

<file name="packages/features/pbac/infrastructure/repositories/PermissionRepository.ts">

<violation number="1" location="packages/features/pbac/infrastructure/repositories/PermissionRepository.ts:326">
Child fallback role query only checks the parent org’s PBAC flag, so PBAC-enabled child teams become accessible via fallback roles, bypassing PBAC safeguards.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

});
});

it("should include child teams where user has org-level PBAC permissions", async () => {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new “PBAC permissions” test is a duplicate of the fallback-role test, so it does not verify PBAC behavior and will never fail if PBAC support breaks. Please rewrite it to exercise PBAC-specific logic (or drop it) instead of duplicating the previous test.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/features/pbac/services/__tests__/permission-check.service.test.ts, line 558:

<comment>The new “PBAC permissions” test is a duplicate of the fallback-role test, so it does not verify PBAC behavior and will never fail if PBAC support breaks. Please rewrite it to exercise PBAC-specific logic (or drop it) instead of duplicating the previous test.</comment>

<file context>
@@ -534,6 +534,46 @@ describe(&quot;PermissionCheckService&quot;, () =&gt; {
+      });
+    });
+
+    it(&quot;should include child teams where user has org-level PBAC permissions&quot;, async () =&gt; {
+      // User has PBAC permission in org (teamId: 100) but not in child team (teamId: 1)
+      // Should get access to child team via org-level PBAC permission
</file context>

✅ Addressed in 381c1b8

devin-ai-integration bot and others added 4 commits December 1, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

consumer core area: core, team members only ready-for-e2e size/XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants