fix: enable API key management of workspace views + fix permission bypass vulnerability (#16768)

Fixes #16739 - Remove empty string coercion in createCoreView that caused PostgreSQL UUID errors for API keys - Add permission check allowing API keys with VIEWS permission to manage workspace views they created API keys with 'Manage Views' permission can now create, update, and delete workspace views via both GraphQL and REST APIs.
2025-12-23 02:06:47 -06:00
parent 79998ef8fc
commit 65dced14ff
26 changed files with 69 additions and 20 deletions
@@ -40,6 +40,7 @@ export class CreateViewFieldPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class CreateViewFilterGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class CreateViewFilterPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -40,6 +40,7 @@ export class CreateViewGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -34,6 +34,7 @@ export class CreateViewPermissionGuard implements CanActivate {
      visibility,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class CreateViewSortPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DeleteViewFieldPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DeleteViewFilterGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DeleteViewFilterPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DeleteViewGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class DeleteViewPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DeleteViewSortPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DestroyViewFieldPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DestroyViewFilterGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DestroyViewFilterPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DestroyViewGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class DestroyViewPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class DestroyViewSortPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class UpdateViewFieldPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class UpdateViewFilterGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class UpdateViewFilterPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class UpdateViewGroupPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -33,6 +33,7 @@ export class UpdateViewPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -45,6 +45,7 @@ export class UpdateViewSortPermissionGuard implements CanActivate {
      viewId,
      request.userWorkspaceId,
      request.workspace.id,
+      request.apiKey?.id,
    );
  }
 }
@@ -26,6 +26,7 @@ export class ViewAccessService {
    viewId: string | null,
    userWorkspaceId: string | undefined,
    workspaceId: string,
+    apiKeyId?: string,
  ): Promise<boolean> {
    // If viewId is null, the entity doesn't exist - allow the operation
    // so the service can handle the NOT_FOUND error properly
@@ -43,13 +44,14 @@ export class ViewAccessService {
      return true;
    }

-    return this.checkViewAccess(view, userWorkspaceId, workspaceId);
+    return this.checkViewAccess(view, userWorkspaceId, workspaceId, apiKeyId);
  }

  async canUserModifyViewByChildEntity(
    viewId: string | null,
    userWorkspaceId: string | undefined,
    workspaceId: string,
+    apiKeyId?: string,
  ): Promise<boolean> {
    // If viewId is null, the child entity doesn't exist
    // Allow through so the service can throw the proper entity-specific error
@@ -68,24 +70,36 @@ export class ViewAccessService {
      return true;
    }

-    return this.checkViewAccess(view, userWorkspaceId, workspaceId);
+    return this.checkViewAccess(view, userWorkspaceId, workspaceId, apiKeyId);
  }

  async canUserCreateView(
    visibility: ViewVisibility,
    userWorkspaceId: string | undefined,
    workspaceId: string,
+    apiKeyId?: string,
  ): Promise<boolean> {
-    // For WORKSPACE visibility views, user must have VIEWS permission
-    if (visibility === ViewVisibility.WORKSPACE && isDefined(userWorkspaceId)) {
-      const permissions =
-        await this.permissionsService.getUserWorkspacePermissions({
-          userWorkspaceId,
-          workspaceId,
-        });
+    // For WORKSPACE visibility views, check VIEWS permission
+    if (visibility === ViewVisibility.WORKSPACE) {
+      let hasViewsPermission = false;

-      const hasViewsPermission =
-        permissions.permissionFlags[PermissionFlagType.VIEWS] ?? false;
+      if (isDefined(userWorkspaceId)) {
+        const permissions =
+          await this.permissionsService.getUserWorkspacePermissions({
+            userWorkspaceId,
+            workspaceId,
+          });
+
+        hasViewsPermission =
+          permissions.permissionFlags[PermissionFlagType.VIEWS] ?? false;
+      } else if (isDefined(apiKeyId)) {
+        hasViewsPermission =
+          await this.permissionsService.userHasWorkspaceSettingPermission({
+            workspaceId,
+            apiKeyId,
+            setting: PermissionFlagType.VIEWS,
+          });
+      }

      if (!hasViewsPermission) {
        throw new ViewException(
@@ -102,7 +116,7 @@ export class ViewAccessService {
      }
    }

-    // For UNLISTED views or users without userWorkspaceId, allow creation
+    // For UNLISTED views, allow creation
    return true;
  }

@@ -110,18 +124,29 @@ export class ViewAccessService {
    view: ViewEntity,
    userWorkspaceId: string | undefined,
    workspaceId: string,
+    apiKeyId?: string,
  ): Promise<boolean> {
-    const permissions = isDefined(userWorkspaceId)
-      ? await this.permissionsService.getUserWorkspacePermissions({
+    let hasViewsPermission = false;
+
+    if (isDefined(userWorkspaceId)) {
+      const permissions =
+        await this.permissionsService.getUserWorkspacePermissions({
          userWorkspaceId,
          workspaceId,
-        })
-      : null;
+        });

-    const hasViewsPermission =
-      permissions?.permissionFlags[PermissionFlagType.VIEWS] ?? false;
+      hasViewsPermission =
+        permissions.permissionFlags[PermissionFlagType.VIEWS] ?? false;
+    } else if (isDefined(apiKeyId)) {
+      hasViewsPermission =
+        await this.permissionsService.userHasWorkspaceSettingPermission({
+          workspaceId,
+          apiKeyId,
+          setting: PermissionFlagType.VIEWS,
+        });
+    }

-    // Users with VIEWS permission can manipulate all views
+    // Users/API keys with VIEWS permission can manipulate all views
    if (hasViewsPermission) {
      return true;
    }
@@ -153,7 +153,7 @@ export class ViewResolver {
    return await this.viewService.createOne({
      createViewInput: input,
      workspaceId: workspace.id,
-      createdByUserWorkspaceId: userWorkspaceId ?? '',
+      createdByUserWorkspaceId: userWorkspaceId,
    });
  }