feat: introduce the subgraph-checker role

controlplane/migrations/0131_known_stepford_cuckoos.sql

@@ -0,0 +1 @@
+ALTER TYPE "public"."organization_role" ADD VALUE 'subgraph-checker' BEFORE 'subgraph-viewer';

controlplane/migrations/meta/0129_snapshot.json → controlplane/migrations/meta/0131_snapshot.json

@@ -1,6 +1,6 @@
 {
-  "id": "1bb12880-ed57-4cad-b9aa-dcd0a3d08f67",
-  "prevId": "e8d72d6a-3744-48e9-88a2-d8617e9a867c",
+  "id": "aa949867-16a4-42b3-abac-089abf6a6e4d",
+  "prevId": "d090b695-c13a-4b3d-ad8f-0516d5689336",
   "version": "7",
   "dialect": "postgresql",
   "tables": {
@@ -2798,6 +2798,27 @@
         }
       },
       "indexes": {
+        "lsc_schema_check_id_linked_schema_check_id_unique": {
+          "name": "lsc_schema_check_id_linked_schema_check_id_unique",
+          "columns": [
+            {
+              "expression": "schema_check_id",
+              "isExpression": false,
+              "asc": true,
+              "nulls": "last"
+            },
+            {
+              "expression": "linked_schema_check_id",
+              "isExpression": false,
+              "asc": true,
+              "nulls": "last"
+            }
+          ],
+          "isUnique": true,
+          "concurrently": false,
+          "method": "btree",
+          "with": {}
+        },
         "lsc_schema_check_id_idx": {
           "name": "lsc_schema_check_id_idx",
           "columns": [
@@ -2858,15 +2879,7 @@
         }
       },
       "compositePrimaryKeys": {},
-      "uniqueConstraints": {
-        "linked_schema_checks_schema_check_id_unique": {
-          "name": "linked_schema_checks_schema_check_id_unique",
-          "nullsNotDistinct": false,
-          "columns": [
-            "schema_check_id"
-          ]
-        }
-      },
+      "uniqueConstraints": {},
       "checkConstraints": {}
     },
     "public.linked_subgraphs": {
@@ -8425,6 +8438,7 @@
         "graph-viewer",
         "subgraph-admin",
         "subgraph-publisher",
+        "subgraph-checker",
         "subgraph-viewer"
       ]
     },

controlplane/migrations/meta/_journal.json

@@ -918,6 +918,13 @@
       "when": 1756988174576,
       "tag": "0130_skinny_solo",
       "breakpoints": true
+    },
+    {
+      "idx": 131,
+      "version": "7",
+      "when": 1757542818295,
+      "tag": "0131_known_stepford_cuckoos",
+      "breakpoints": true
     }
   ]
 }

controlplane/src/core/bufservices/subgraph/checkSubgraphSchema.ts

@@ -129,7 +129,7 @@ export function checkSubgraphSchema(
       }
     }
 
-    if (subgraph && !authContext.rbac.hasSubGraphWriteAccess(subgraph)) {
+    if (subgraph && !authContext.rbac.hasSubGraphCheckAccess(subgraph)) {
       throw new UnauthorizedError();
     } else if (!subgraph) {
       if (!authContext.rbac.canCreateSubGraph(namespace)) {

controlplane/src/core/services/RBACEvaluator.ts

@@ -50,23 +50,31 @@ export class RBACEvaluator {
     this.isApiKey = !!isApiKey;
     this.isLegacyApiKey = this.isApiKey && groups.length === 0;
 
-    const flattenRules = groups.flatMap((group) => group.rules);
-    const rulesGroupedByRole = Object.groupBy(flattenRules, (rule) => rule.role);
-
-    const result = new Map<OrganizationRole, RuleData>();
-    for (const [role, ruleData] of Object.entries(rulesGroupedByRole)) {
-      result.set(role as OrganizationRole, {
-        namespaces: [...new Set(ruleData.flatMap((r) => r.namespaces))],
-        resources: [...new Set(ruleData.flatMap((r) => r.resources))],
-      });
-    }
+    this.roles = [];
+    this.namespaces = [];
+    this.resources = [];
+    this.rules = new Map<OrganizationRole, RuleData>();
+
+    if (!this.isLegacyApiKey) {
+      // Only evaluate the rules if the user is not a legacy API key
+      const flattenRules = groups.flatMap((group) => group.rules);
+      const rulesGroupedByRole = Object.groupBy(flattenRules, (rule) => rule.role);
+
+      const result = new Map<OrganizationRole, RuleData>();
+      for (const [role, ruleData] of Object.entries(rulesGroupedByRole)) {
+        result.set(role as OrganizationRole, {
+          namespaces: [...new Set(ruleData.flatMap((r) => r.namespaces))],
+          resources: [...new Set(ruleData.flatMap((r) => r.resources))],
+        });
+      }
 
-    this.roles = Array.from(result.keys(), (k) => k);
-    this.namespaces = [...new Set(Array.from(result.values(), (res) => res.namespaces).flat())];
-    this.resources = [...new Set(Array.from(result.values(), (res) => res.resources).flat())];
-    this.rules = result;
+      this.roles = Array.from(result.keys(), (k) => k);
+      this.namespaces = [...new Set(Array.from(result.values(), (res) => res.namespaces).flat())];
+      this.resources = [...new Set(Array.from(result.values(), (res) => res.resources).flat())];
+      this.rules = result;
+    }
 
-    this.isOrganizationAdmin = this.roles.includes('organization-admin') || this.isLegacyApiKey;
+    this.isOrganizationAdmin = this.isLegacyApiKey || this.roles.includes('organization-admin');
     this.isOrganizationAdminOrDeveloper = this.isOrganizationAdmin || this.roles.includes('organization-developer');
     this.isOrganizationApiKeyManager = this.isOrganizationAdmin || !!this.ruleFor('organization-apikey-manager');
     this.isOrganizationViewer = this.isOrganizationAdminOrDeveloper || this.roles.includes('organization-viewer');
@@ -88,68 +96,36 @@ export class RBACEvaluator {
   }
 
   hasNamespaceReadAccess(namespace: Namespace) {
-    if (this.isLegacyApiKey) {
-      // When using an API without a group, fallback to always allow (legacy implementation)
-      return true;
-    }
-
     return this.isOrganizationViewer || this.checkNamespaceAccess(namespace, ['namespace-admin', 'namespace-viewer']);
   }
 
   canCreateContract(namespace: Namespace) {
     return this.canCreateFederatedGraph(namespace);
   }
 
-  canCreateFeatureFlag(namespace: Namespace) {
+  canCreateFeatureFlag(_: Namespace) {
     return this.isOrganizationAdminOrDeveloper;
   }
 
-  hasFeatureFlagWriteAccess(featureFlag: FeatureFlag) {
+  hasFeatureFlagWriteAccess(_: FeatureFlag) {
     return this.isOrganizationAdminOrDeveloper;
   }
 
-  hasFeatureFlagReadAccess(featureFlag: FeatureFlag) {
-    if (this.isLegacyApiKey) {
-      // When using an API without a group, fallback to always allow (legacy implementation)
-      return true;
-    }
-
+  hasFeatureFlagReadAccess(_: FeatureFlag) {
     return this.isOrganizationViewer;
   }
 
   canCreateFederatedGraph(namespace: Namespace) {
-    if (this.isOrganizationAdminOrDeveloper) {
-      return true;
-    }
-
-    const rule = this.ruleFor('graph-admin');
-    if (!rule) {
-      return false;
-    }
-
-    if (rule.namespaces.length === 0 && rule.resources.length === 0) {
-      return true;
-    } else if (rule.namespaces.length > 0) {
-      return rule.namespaces.includes(namespace.id);
-    }
-
-    return false;
+    return (
+      this.isOrganizationAdminOrDeveloper || this.hasRoleWithAccessToAllOrGivenNamespace('graph-admin', namespace.id)
+    );
   }
 
   canDeleteFederatedGraph(graph: Target) {
-    if (graph.creatorUserId && this.userId && graph.creatorUserId === this.userId) {
-      // The graph creator should always have access to the provided target
-      return true;
-    }
-
-    if (this.isOrganizationAdminOrDeveloper) {
-      return true;
-    }
-
-    const rule = this.ruleFor('graph-admin');
     return (
-      !!rule &&
-      ((rule.namespaces.length === 0 && rule.resources.length === 0) || rule.namespaces.includes(graph.namespaceId))
+      this.isOrganizationAdminOrDeveloper ||
+      this.isTargetOwnedByUser(graph) ||
+      this.hasRoleWithAccessToAllOrGivenNamespace('graph-admin', graph.namespaceId)
     );
   }
 
@@ -158,51 +134,28 @@ export class RBACEvaluator {
   }
 
   hasFederatedGraphReadAccess(graph: Target) {
-    if (this.isLegacyApiKey) {
-      // When using an API without a group, fallback to always allow (legacy implementation)
-      return true;
-    }
-
-    return this.isOrganizationViewer || this.checkTargetAccess(graph, ['graph-admin', 'graph-viewer']);
+    return (
+      this.isOrganizationViewer ||
+      this.hasFederatedGraphWriteAccess(graph) ||
+      this.checkTargetAccess(graph, ['graph-viewer'])
+    );
   }
 
   canCreateSubGraph(namespace: Namespace) {
-    if (this.isOrganizationAdminOrDeveloper) {
-      return true;
-    }
-
-    const rule = this.ruleFor('subgraph-admin');
-    if (!rule) {
-      return false;
-    }
-
-    if (rule.namespaces.length === 0 && rule.resources.length === 0) {
-      return true;
-    } else if (rule.namespaces.length > 0) {
-      return rule.namespaces.includes(namespace.id);
-    }
-
-    return false;
+    return (
+      this.isOrganizationAdminOrDeveloper || this.hasRoleWithAccessToAllOrGivenNamespace('subgraph-admin', namespace.id)
+    );
   }
 
   canUpdateSubGraph(graph: Target) {
     return this.isOrganizationAdminOrDeveloper || this.checkTargetAccess(graph, ['subgraph-admin']);
   }
 
   canDeleteSubGraph(graph: Target) {
-    if (!this.isApiKey && graph.creatorUserId && this.userId && graph.creatorUserId === this.userId) {
-      // The graph creator should always have access to the provided target
-      return true;
-    }
-
-    if (this.isOrganizationAdminOrDeveloper) {
-      return true;
-    }
-
-    const rule = this.ruleFor('subgraph-admin');
     return (
-      !!rule &&
-      ((rule.namespaces.length === 0 && rule.resources.length === 0) || rule.namespaces.includes(graph.namespaceId))
+      this.isOrganizationAdminOrDeveloper ||
+      this.isTargetOwnedByUser(graph) ||
+      this.hasRoleWithAccessToAllOrGivenNamespace('subgraph-admin', graph.namespaceId)
     );
   }
 
@@ -212,15 +165,26 @@ export class RBACEvaluator {
     );
   }
 
-  hasSubGraphReadAccess(graph: Target) {
-    if (this.isLegacyApiKey) {
-      // When using an API without a group, fallback to always allow (legacy implementation)
-      return true;
-    }
+  hasSubGraphCheckAccess(graph: Target) {
+    return this.hasSubGraphWriteAccess(graph) || this.checkTargetAccess(graph, ['subgraph-checker']);
+  }
 
+  hasSubGraphReadAccess(graph: Target) {
     return (
       this.isOrganizationViewer ||
-      this.checkTargetAccess(graph, ['subgraph-admin', 'subgraph-publisher', 'subgraph-viewer'])
+      this.hasSubGraphCheckAccess(graph) ||
+      this.checkTargetAccess(graph, ['subgraph-viewer'])
+    );
+  }
+
+  private hasRoleWithAccessToAllOrGivenNamespace(role: OrganizationRole, namespaceId: string) {
+    const rule = this.ruleFor(role);
+    return (
+      !!rule &&
+      // The rule has access to every namespace
+      ((rule.namespaces.length === 0 && rule.resources.length === 0) ||
+        // The rule has access to the given namespace
+        (rule.namespaces.length > 0 && rule.namespaces.includes(namespaceId)))
     );
   }
 
@@ -237,7 +201,7 @@ export class RBACEvaluator {
       }
 
       if (
-        // The rule have access to every namespace
+        // The rule has access to every namespace
         rule.namespaces.length === 0 ||
         // The rule was given write access to the namespace
         (rule.namespaces.length > 0 && rule.namespaces.includes(ns.id))
@@ -249,6 +213,10 @@ export class RBACEvaluator {
     return false;
   }
 
+  private isTargetOwnedByUser(target: Target) {
+    return !this.isApiKey && target.creatorUserId && this.userId && target.creatorUserId === this.userId;
+  }
+
   private checkTargetAccess(target: Target, requiredRoles: OrganizationRole[]) {
     if (!this.isApiKey && target.creatorUserId && this.userId && target.creatorUserId === this.userId) {
       // The target creator should always have access to the provided target
@@ -262,9 +230,9 @@ export class RBACEvaluator {
       }
 
       if (
-        // The rule have access to every resource
+        // The rule has access to every resource
         (rule.namespaces.length === 0 && rule.resources.length === 0) ||
-        // The rule was given write access to the namespace
+        // The rule was given access to the namespace
         (rule.namespaces.length > 0 && rule.namespaces.includes(target.namespaceId)) ||
         // The rule was given write access to the resource
         (rule.resources.length > 0 && rule.resources.includes(target.targetId))

controlplane/src/db/schema.ts

@@ -1418,6 +1418,7 @@ export const organizationRoleEnum = pgEnum('organization_role', [
   'graph-viewer',
   'subgraph-admin',
   'subgraph-publisher',
+  'subgraph-checker',
   'subgraph-viewer',
 ] as const);
 

controlplane/test/check-subgraph-schema.test.ts

@@ -49,7 +49,7 @@ describe('CheckSubgraphSchema', (ctx) => {
     await afterAllSetup(dbname);
   });
 
-  test.each(['organization-admin', 'organization-developer', 'subgraph-admin', 'subgraph-publisher'])(
+  test.each(['organization-admin', 'organization-developer', 'subgraph-admin', 'subgraph-publisher', 'subgraph-checker'])(
     '%s should be able to create a subgraph, publish the schema and then check with new schema',
     async (role) => {
       const { client, server, authenticator, users } = await SetupTest({ dbname, chClient });
@@ -158,7 +158,7 @@ describe('CheckSubgraphSchema', (ctx) => {
     await server.close();
   });
 
-  test.each(['subgraph-admin', 'subgraph-publisher'])(
+  test.each(['subgraph-admin', 'subgraph-publisher', 'subgraph-checker'])(
     '%s should be able to check with new schema on allowed namespaces',
     async (role) => {
       const { client, server, authenticator, users } = await SetupTest({ dbname, chClient });