feat: api v1 admin api access for orgs #22544

sean-brydon · 2025-07-16T08:44:03Z

Add PBAC Permission Checking to Admin API Guard

Summary

Integrates Permission-Based Access Control (PBAC) system with the existing admin API guard functionality
Updates isAdminGuard to check organization.adminApi permission when PBAC is enabled for a team
Maintains backward compatibility with traditional role-based admin checks when PBAC is disabled

Changes Made

Permission Flow

System-wide admin check: Unchanged - users with UserPermissionRole.ADMIN get system-wide access
Organization admin check: Now uses PBAC when enabled:
- If PBAC is enabled for the organization → checks organization.adminApi permission via custom roles
- If PBAC is disabled → falls back to traditional OWNER/ADMIN role check
Fallback behavior: Seamless transition between PBAC and traditional role systems

Test Coverage

Added test cases covering:

✅ PBAC enabled with permission granted
✅ PBAC enabled with permission denied
✅ PBAC disabled with fallback to traditional roles
✅ Existing functionality preserved

Testing Setup

SQL Script to Create Developer Role with Admin API Permission

-- Create developer role with organization.adminApi permission
-- This script creates a developer role and enables PBAC for dunder-mifflin team

-- First, get the dunder-mifflin team ID
DO $$
DECLARE
    dunder_team_id INT;
    developer_role_id TEXT;
BEGIN
    -- Get the dunder-mifflin team ID
    SELECT id INTO dunder_team_id 
    FROM "Team" 
    WHERE slug = 'dunder-mifflin' AND "isOrganization" = true;
    
    IF dunder_team_id IS NULL THEN
        RAISE EXCEPTION 'Team with slug "dunder-mifflin" not found';
    END IF;
    
    -- Create the developer role for the dunder-mifflin organization
    INSERT INTO "Role" (id, name, color, description, "teamId", type, "createdAt", "updatedAt")
    VALUES (
        gen_random_uuid()::text,
        'developer',
        '#10B981', -- Green color
        'Developer role with admin API access',
        dunder_team_id,
        'CUSTOM',
        NOW(),
        NOW()
    )
    RETURNING id INTO developer_role_id;
    
    -- Add organization.adminApi permission to the developer role
    INSERT INTO "RolePermission" (id, "roleId", resource, action, "createdAt")
    VALUES (
        gen_random_uuid()::text,
        developer_role_id,
        'organization',
        'adminApi',
        NOW()
    );
    
    -- Enable PBAC feature for the dunder-mifflin team
    INSERT INTO "TeamFeatures" ("teamId", "featureId", "assignedAt", "assignedBy", "updatedAt")
    VALUES (
        dunder_team_id,
        'pbac',
        NOW(),
        'system',
        NOW()
    )
    ON CONFLICT ("teamId", "featureId") DO NOTHING;
    
    -- Ensure the pbac feature exists in the Feature table
    INSERT INTO "Feature" (slug, enabled, description, type, "stale", "lastUsedAt", "createdAt", "updatedAt")
    VALUES (
        'pbac',
        true,
        'Permission-based access control system',
        'PERMISSION',
        false,
        NOW(),
        NOW(),
        NOW()
    )
    ON CONFLICT (slug) DO UPDATE SET
        enabled = true,
        "lastUsedAt" = NOW(),
        "updatedAt" = NOW();
    
    RAISE NOTICE 'Successfully created developer role with ID: % for team: %', developer_role_id, dunder_team_id;
    RAISE NOTICE 'PBAC feature enabled for dunder-mifflin team';
END $$;

Test Plan

Run integration tests to verify PBAC permission checking
Test fallback behavior when PBAC is disabled
Verify system-wide admin access remains unchanged
Execute SQL script to create test role and verify admin API access manually

coderabbitai · 2025-07-16T08:44:11Z

Caution

Review failed

The pull request is closed.

Walkthrough

The admin check now queries accepted memberships for organizations with admin API enabled and uses PermissionCheckService to evaluate the organization.adminApi permission per membership, using OWNER/ADMIN as fallback roles. The AdminApi action and its registry entry were added to the PBAC permission registry, a DB migration assigns that permission to the admin role, and i18n strings were added. Tests for isAdminGuard were updated to mock and assert PermissionCheckService behavior. No exported function signatures changed.

Possibly related PRs

fix: (PBAC) Add organization fallback on resources #22733: Adds organization-level fallback handling in permission queries used by PermissionCheckService, directly affecting isAdminGuard permission evaluation.
feat: team roles ui pbac #22585: Modifies the PBAC permission registry and scopes API, touching the same permission-registry file updated here.
feat: (PBAC) Introduce depends on permission registery #23440: Extends the permission registry with dependency metadata and edits the same registry file that now includes AdminApi.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9bd6cda and 7d05a88.

📒 Files selected for processing (1)

apps/web/public/static/locales/en/common.json (1 hunks)

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/api-v1-pbac

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

graphite-app · 2025-07-16T08:45:23Z

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (07/16/25)

1 reviewer was added to this PR based on Keith Williams's automation.

delve-auditor · 2025-07-16T08:46:05Z

✅ No security or compliance issues detected. Reviewed everything up to 66957f3.

Security Overview

🔎 Scanned files: 5 changed file(s)

Detected Code Changes

Change Type	Relevant files
Enhancement	► isAdmin.ts Integrate PBAC permission checks for admin API access ► isAdmin.integration-test.ts Add PBAC integration tests ► permission-registry.ts Add AdminApi to CustomAction enum and registry ► common.json Add PBAC admin API translations
Configuration changes	► migration.sql Add organization admin API permission migration

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

apps/web/public/static/locales/en/common.json (1)

718-720: Consider aligning new PBAC keys with existing permission sections

The new keys (pbac_action_admin_api, pbac_desc_access_organization_admin_api) are added much earlier in the file than the rest of the PBAC‐related strings (which start around line 3270).
Keeping all PBAC keys together makes future maintenance easier and avoids “hidden” duplicates further down the file.

No functional issue here—just a suggestion to move these two entries next to the other pbac_action_* / pbac_desc_* keys.
apps/api/v1/test/lib/utils/isAdmin.integration-test.ts (1)
160-177: Good test for fallback behavior verification.

This test validates the fallback mechanism when PBAC is disabled:

Fallback scenario: Tests that admin access is granted when permission check returns true (via fallback roles)

Backward compatibility: Ensures existing role-based access continues to work

However, the test name could be more descriptive of what it's actually testing.

Consider updating the test name to better reflect what it's testing:
-  it("Returns admin when PBAC is disabled and user has fallback admin role", async () => {
+  it("Returns admin when permission check succeeds via fallback roles", async () => {

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a84e898 and d5f4f88.

📒 Files selected for processing (5)

apps/api/v1/lib/utils/isAdmin.ts (2 hunks)
apps/api/v1/test/lib/utils/isAdmin.integration-test.ts (2 hunks)
apps/web/public/static/locales/en/common.json (1 hunks)
packages/features/pbac/domain/types/permission-registry.ts (2 hunks)
packages/prisma/migrations/20250716084913_add_organization_admin_api_permission/migration.sql (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

apps/web/public/static/locales/en/common.json (1)

undefined

<retrieved_learning>
Learnt from: bandhan-majumder
PR: #22359
File: packages/lib/server/locales/en/common.json:1336-1339
Timestamp: 2025-07-14T16:31:45.201Z
Learning: When making localization changes for new features, it's often safer to add new strings rather than modify existing ones to avoid breaking existing functionality that depends on the original strings. This approach allows for feature-specific customization while maintaining backward compatibility.
</retrieved_learning>

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

GitHub Check: Detect changes
GitHub Check: Security Check

🔇 Additional comments (12)

packages/prisma/migrations/20250716084913_add_organization_admin_api_permission/migration.sql (1)

1-8: Well-structured migration with proper idempotency handling.

The migration correctly adds the new organization.adminApi permission to the admin role. The use of ON CONFLICT DO NOTHING ensures the migration is idempotent and safe to run multiple times.

The values ('admin_role', 'organization', 'adminApi') align with the corresponding TypeScript enum and registry definitions, maintaining consistency across the database and application layers.

packages/features/pbac/domain/types/permission-registry.ts (3)

28-28: Consistent enum addition follows established patterns.

The AdminApi = "adminApi" addition to the CustomAction enum is well-placed and follows the existing naming convention. The string value matches what's used in the database migration.

232-237: Complete permission registry entry with proper structure.

The permission registry entry for AdminApi is well-structured and includes all required fields:

Description clearly explains the permission's purpose

Category "org" is consistent with other Organization resource permissions

i18n keys follow the established naming convention

The entry aligns with the database migration and maintains consistency across the permission system.

235-236: i18n keys verified in English locale

Both pbac_action_admin_api and pbac_desc_access_organization_admin_api are defined in
apps/web/public/static/locales/en/common.json.

No missing-translation warnings will occur for the default locale.

If you support additional locales, please add these entries to their respective JSON files.

apps/api/v1/lib/utils/isAdmin.ts (2)

3-3: LGTM: New PBAC service import added correctly.

The import of PermissionCheckService is properly added to support the new permission-based admin checks.

36-56: Excellent implementation of PBAC integration with proper fallback handling.

The refactored logic successfully integrates Permission-Based Access Control while maintaining backward compatibility. Key strengths:

Proper permission check: Uses "organization.adminApi" permission with correct fallback roles

Maintains existing query structure: Still filters for accepted memberships in organizations with admin API enabled

Fallback mechanism: Specifies OWNER and ADMIN roles for backward compatibility when PBAC is disabled

Correct scope assignment: Returns OrgOwnerOrAdmin scope when permission is granted

The implementation aligns perfectly with the PR objectives of seamless PBAC integration.

apps/api/v1/test/lib/utils/isAdmin.integration-test.ts (6)

4-4: LGTM: Test imports updated correctly.

The addition of vi and beforeEach imports is appropriate for the new test setup requirements.

6-6: LGTM: Permission service import added for testing.

The import of PermissionCheckService is correctly added to support mocking in the tests.

12-12: LGTM: Proper mock setup for permission service.

The mock setup for PermissionCheckService is correctly implemented using Vitest's mocking capabilities.

18-29: Excellent test setup with proper mock management.

The test setup properly:

Mock service type definition: Correctly types the mock service with checkPermission method

beforeEach cleanup: Clears mocks between tests to prevent interference

Mock implementation: Properly mocks the PermissionCheckService constructor

Mock method setup: Configures the checkPermission method for testing

This ensures isolated and reliable test execution.

110-133: Comprehensive test coverage for PBAC permission granted scenario.

This test case excellently validates:

Permission check flow: Verifies that the permission service is called when PBAC is enabled

Correct parameters: Validates that checkPermission is called with proper userId, teamId, permission, and fallback roles

Expected behavior: Confirms admin access is granted when permission check returns true

Proper scope: Ensures OrgOwnerOrAdmin scope is returned

The test aligns perfectly with the PR objectives of testing PBAC functionality.

135-158: Thorough test coverage for PBAC permission denied scenario.

This test case provides excellent validation for the negative case:

Permission denial: Verifies behavior when checkPermission returns false

Correct response: Confirms admin access is denied and scope is null

Service interaction: Validates that the permission service is still called with correct parameters

This ensures robust testing of both positive and negative PBAC scenarios.

github-actions · 2025-07-16T09:34:36Z

E2E results are ready!

Workflow #74897.1 latest results

sean-brydon · 2025-07-16T11:36:05Z

apps/api/v1/lib/utils/isAdmin.ts

@@ -22,7 +23,6 @@ export const isAdminGuard = async (req: NextApiRequest) => {
          isAdminAPIEnabled: true,
        },
      },
-      OR: [{ role: MembershipRole.OWNER }, { role: MembershipRole.ADMIN }],


Remove this check because we just want to pull back their membership for now

sean-brydon · 2025-07-16T11:37:05Z

apps/api/v1/lib/utils/isAdmin.ts

@@ -33,7 +33,27 @@ export const isAdminGuard = async (req: NextApiRequest) => {
      },
    },
  });
-  if (orgOwnerOrAdminMemberships.length > 0) return { isAdmin: true, scope: ScopeOfAdmin.OrgOwnerOrAdmin };
+
+  if (orgOwnerOrAdminMemberships.length > 0) {


If they have a membership for any orgs with admin enabled

sean-brydon · 2025-07-16T11:37:33Z

apps/api/v1/lib/utils/isAdmin.ts

+
+    // Check PBAC permissions for each organization
+    // This should only ever be one membership as we only support one org currently.
+    for (const membership of orgOwnerOrAdminMemberships) {


Should only be one loop iteration due to only one org per user

apps/api/v1/lib/utils/isAdmin.ts

sean-brydon · 2025-07-16T11:38:39Z

packages/features/pbac/domain/types/permission-registry.ts

@@ -228,6 +229,12 @@ export const PERMISSION_REGISTRY: PermissionRegistry = {
      i18nKey: "pbac_action_update",
      descriptionI18nKey: "pbac_desc_edit_organization_settings",
    },
+    [CustomAction.AdminApi]: {


This auto adds everything to the UI to add this permission to new/existing roles

vercel · 2025-07-16T12:28:47Z

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

2 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
cal	Ignored			Sep 2, 2025 10:44am
cal-eu	Ignored			Sep 2, 2025 10:44am

joeauyeung

LGTM. Nice refactor of an existing check. Fallback is hit inside of the permissionCheckService.

joeauyeung · 2025-07-18T18:26:46Z

apps/api/v1/lib/utils/isAdmin.ts

@@ -12,7 +13,7 @@ export const isAdminGuard = async (req: NextApiRequest) => {
  const { role: userRole } = user;
  if (userRole === UserPermissionRole.ADMIN) return { isAdmin: true, scope: ScopeOfAdmin.SystemWide };

-  const orgOwnerOrAdminMemberships = await prisma.membership.findMany({
+  const usersOrgMemberships = await prisma.membership.findMany({


If we're refactoring this query, can we use a repository here?

apps/api/v1/lib/utils/isAdmin.ts

github-actions · 2025-08-08T00:27:39Z

This PR is being marked as stale due to inactivity.

github-actions · 2025-08-24T00:27:19Z

This PR is being marked as stale due to inactivity.

keithwillcode · 2025-08-28T12:08:29Z

@sean-brydon Now that API v1 is deprecated, I'd rather not ship more code out to it. Do we definitely need this?

eunjae-lee

Regarding @keithwillcode 's comment, I'll let you @sean-brydon decide.

Anyway, the change looks good to me.

sean-brydon · 2025-09-02T10:44:47Z

@sean-brydon Now that API v1 is deprecated, I'd rather not ship more code out to it. Do we definitely need this?
Likely not! Will close!

feat: api v1 admin api access for orgs

d5f4f88

sean-brydon requested a review from a team July 16, 2025 08:44

sean-brydon requested review from a team as code owners July 16, 2025 08:44

graphite-app bot requested a review from a team July 16, 2025 08:44

github-actions bot added the ❗️ migrations contains migration files label Jul 16, 2025

keithwillcode added consumer core area: core, team members only labels Jul 16, 2025

dosubot bot added the ✨ feature New feature or request label Jul 16, 2025

coderabbitai bot reviewed Jul 16, 2025

View reviewed changes

vercel bot deployed to Preview – dev July 16, 2025 09:00 View deployment

sean-brydon added the ready-for-e2e label Jul 16, 2025

sean-brydon commented Jul 16, 2025

View reviewed changes

apps/api/v1/lib/utils/isAdmin.ts Show resolved Hide resolved

sean-brydon commented Jul 16, 2025

View reviewed changes

rename var + fix test

5aa7458

vercel bot deployed to Preview – dev July 16, 2025 12:47 View deployment

Merge branch 'main' into feat/api-v1-pbac

23edd9c

vercel bot deployed to Preview – dev July 18, 2025 09:02 View deployment

joeauyeung approved these changes Jul 18, 2025

View reviewed changes

Merge branch 'main' into feat/api-v1-pbac

66957f3

vercel bot deployed to Preview – dev July 22, 2025 15:04 View deployment

sean-brydon enabled auto-merge (squash) July 24, 2025 11:24

Merge branch 'main' into feat/api-v1-pbac

bb59b74

vercel bot deployed to Preview – dev July 24, 2025 11:46 View deployment

github-actions bot added the Stale label Aug 8, 2025

Merge branch 'main' into feat/api-v1-pbac

9bd6cda

vercel bot deployed to Preview – dev August 8, 2025 08:46 View deployment

github-actions bot removed the Stale label Aug 9, 2025

github-actions bot added the Stale label Aug 24, 2025

eunjae-lee approved these changes Aug 28, 2025

View reviewed changes

github-actions bot removed the Stale label Aug 29, 2025

Merge branch 'main' into feat/api-v1-pbac

7d05a88

sean-brydon closed this Sep 2, 2025

auto-merge was automatically disabled September 2, 2025 10:44
Pull request was closed

vercel bot deployed to Preview – dev September 2, 2025 11:17 View deployment

feat: api v1 admin api access for orgs #22544

feat: api v1 admin api access for orgs #22544

Conversation

sean-brydon commented Jul 16, 2025

Add PBAC Permission Checking to Admin API Guard

Summary

Changes Made

Permission Flow

Test Coverage

Testing Setup

Test Plan

Uh oh!

coderabbitai bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Possibly related PRs

Chat

Support

CodeRabbit Commands (Invoked using PR/Issue comments)

Other keywords and placeholders

Status, Documentation and Community

Uh oh!

graphite-app bot commented Jul 16, 2025

Graphite Automations

Uh oh!

delve-auditor bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E results are ready!

Uh oh!

sean-brydon Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

sean-brydon Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

sean-brydon Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sean-brydon Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

vercel bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joeauyeung left a comment

Choose a reason for hiding this comment

Uh oh!

joeauyeung Jul 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 8, 2025

Uh oh!

github-actions bot commented Aug 24, 2025

Uh oh!

keithwillcode commented Aug 28, 2025

Uh oh!

eunjae-lee left a comment

Choose a reason for hiding this comment

Uh oh!

sean-brydon commented Sep 2, 2025

Uh oh!

Uh oh!

coderabbitai bot commented Jul 16, 2025 •

edited

Loading

delve-auditor bot commented Jul 16, 2025 •

edited

Loading

github-actions bot commented Jul 16, 2025 •

edited

Loading

vercel bot commented Jul 16, 2025 •

edited

Loading