Changes to permissions with recursive operations

TimCsaky · TimCsaky · commit 941ead2aecf4 · 2025-04-07T16:55:24.000-07:00
Recursive delete requires DELETE perm on all sub-folders
Recursive Sync copies current user's permissions to all sub-folders
diff --git a/app/src/controllers/bucket.js b/app/src/controllers/bucket.js
@@ -207,7 +207,8 @@ const controller = {
 
   /**
    * @function deleteBucket
-   * Deletes the bucket
+   * Deletes a bucket
+   * Recursive option will delete all sub-folders (where current user has DELETE permission)
    * @param {object} req Express request object
    * @param {object} res Express response object
    * @param {function} next The next callback function
@@ -218,11 +219,35 @@ const controller = {
       const bucketId = addDashesToUuid(req.params.bucketId);
       const parentBucket = await bucketService.read(bucketId);
       let buckets = [parentBucket];
+
       // if doing recursive delete
       if (isTruthy(req.query.recursive)) {
-        // get sub-folders
-        const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket);
-        buckets = buckets.concat(dbChildBuckets);
+        // if current user is OIDC
+        const userId = await userService.getCurrentUserId(
+          getCurrentIdentity(req.currentUser, SYSTEM_USER), SYSTEM_USER);
+        if (userId !== SYSTEM_USER) {
+          const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket, true, userId);
+          // if there are sub-folders
+          if (dbChildBuckets.length > 0) {
+            const checkForDelete = obj => obj.permCode === 'DELETE';
+            const dbChildBucketsWithDelete = dbChildBuckets.filter(b =>
+              b.bucketPermission.some(checkForDelete));
+            // if user has DELETE on all subfolders
+            if (dbChildBucketsWithDelete.length === dbChildBuckets.length) {
+              buckets = buckets.concat(dbChildBucketsWithDelete);
+            } else {
+              throw new Problem(403, {
+                detail: 'User lacks DELETE permission for some sub-folders',
+                instance: req.originalUrl,
+              });
+            }
+          }
+        }
+        // else basic auth
+        else {
+          const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket);
+          buckets = buckets.concat(dbChildBuckets);
+        }
       }
       // do delete
       await utils.trxWrapper(async (trx) => {
diff --git a/app/src/controllers/sync.js b/app/src/controllers/sync.js
@@ -33,43 +33,44 @@ const controller = {
    */
   async syncBucketRecursive(req, res, next) {
     try {
-      // Wrap all sql operations in a single transaction
-      const response = await utils.trxWrapper(async (trx) => {
+      // current userId
+      const userId = await userService.getCurrentUserId(
+        getCurrentIdentity(req.currentUser, SYSTEM_USER),
+        SYSTEM_USER
+      );
+      // parent bucket
+      const bucketId = addDashesToUuid(req.params.bucketId);
+      const parentBucket = await bucketService.read(bucketId);
 
-        // curren userId
-        const userId = await userService.getCurrentUserId(
-          getCurrentIdentity(req.currentUser, SYSTEM_USER),
-          SYSTEM_USER
-        );
-        // parent bucket
-        const bucketId = addDashesToUuid(req.params.bucketId);
-        const parentBucket = await bucketService.read(bucketId);
+      // current user's permissions on parent bucket (folder)
+      const currentUserParentBucketPerms = userId !== SYSTEM_USER ? (await bucketPermissionService.searchPermissions({
+        bucketId: parentBucket.bucketId,
+        userId: userId
+      })).map(p => p.permCode) : [];
 
-        // current user's permissions on parent bucket (folder)
-        const currentUserParentBucketPerms = userId !== SYSTEM_USER ? (await bucketPermissionService.searchPermissions({
-          bucketId: parentBucket.bucketId,
-          userId: userId
-        })).map(p => p.permCode) : [];
+      /**
+       * sync (ie create or delete) bucket records in COMS db to match 'folders' (S3 key prefixes) that exist in S3
+      */
+      // parent + child bucket records already in COMS db
+      const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket, false, userId);
+      let dbBuckets = [parentBucket].concat(dbChildBuckets);
+      // 'folders' that exist below (and including) the parent 'folder' in S3
+      const s3Response = await storageService.listAllObjectVersions({ bucketId: bucketId, precisePath: false });
+      const s3Keys = [...new Set([
+        ...s3Response.DeleteMarkers.map(object => formatS3KeyForCompare(object.Key)),
+        ...s3Response.Versions.map(object => formatS3KeyForCompare(object.Key)),
+      ])];
 
-        /**
-         * sync (ie create or delete) bucket records in COMS db to match 'folders' (S3 key prefixes) that exist in S3
-        */
-        // parent + child bucket records already in COMS db
-        const dbChildBuckets = await bucketService.searchChildBuckets(parentBucket);
-        let dbBuckets = [parentBucket].concat(dbChildBuckets);
-        // 'folders' that exist below (and including) the parent 'folder' in S3
-        const s3Response = await storageService.listAllObjectVersions({ bucketId: bucketId, precisePath: false });
-        const s3Keys = [...new Set([
-          ...s3Response.DeleteMarkers.map(object => formatS3KeyForCompare(object.Key)),
-          ...s3Response.Versions.map(object => formatS3KeyForCompare(object.Key)),
-        ])];
+      // Wrap sync sql operations in a single transaction
+      const response = await utils.trxWrapper(async (trx) => {
 
         const syncedBuckets = await this.syncBucketRecords(
           dbBuckets,
           s3Keys,
           parentBucket,
           // assign current user's permissions on parent bucket to new sub-folders (buckets)
           currentUserParentBucketPerms,
+          userId,
           trx
         );
 
@@ -115,14 +116,16 @@ const controller = {
   /**
   * @function syncBucketRecords
   * Synchronizes (creates / prunes) COMS db bucket records for each 'directry' found in S3
+  * Adds current user's permissions to all buckets
   * @param {object[]} Array of Bucket models - bucket records already in COMS db before syncing
   * @param {string[]} s3Keys Array of key prefixes from S3 representing 'directories'
   * @param {object} Bucket model for the COMS db bucket record of parent bucket
   * @param {string[]} currentUserParentBucketPerms Array of PermCodes to add to NEW buckets
- *  @param {object} [trx] An Objection Transaction object
+  * @param {string} userId the guid of current user
+  *  @param {object} [trx] An Objection Transaction object
   * @returns {string[]} And array of bucketId's for bucket records in COMS db
   */
-  async syncBucketRecords(dbBuckets, s3Keys, parentBucket, currentUserParentBucketPerms, trx) {
+  async syncBucketRecords(dbBuckets, s3Keys, parentBucket, currentUserParentBucketPerms, userId, trx) {
     try {
       // delete buckets not found in S3 from COMS db
       const oldDbBuckets = dbBuckets.filter(b => !s3Keys.includes(b.key));
@@ -134,6 +137,17 @@ const controller = {
             })
         )
       );
+      // add current user's permissions to all buckets
+      await Promise.all(
+        dbBuckets.map(bucket => {
+          return bucketPermissionService.addPermissions(
+            bucket.bucketId,
+            currentUserParentBucketPerms.map(permCode => ({ userId, permCode })),
+            undefined,
+            trx
+          );
+        })
+      );
 
       // Create buckets only found in S3 in COMS db
       const newS3Keys = s3Keys.filter(k => !dbBuckets.map(b => b.key).includes(k));
@@ -149,8 +163,6 @@ const controller = {
             region: parentBucket.region ?? undefined,
             active: parentBucket.active,
             userId: parentBucket.createdBy ?? SYSTEM_USER,
-            // current user has MANAGE perm on parent folder (see route.hasPermission)
-            // ..so copy all their perms to NEW subfolders
             permCodes: currentUserParentBucketPerms
           };
           return bucketService.create(data, trx)
@@ -159,7 +171,6 @@ const controller = {
             });
         })
       );
-
       return dbBuckets;
     }
     catch (err) {
diff --git a/app/src/docs/v1.api-spec.yaml b/app/src/docs/v1.api-spec.yaml
@@ -199,7 +199,11 @@ paths:
       summary: Deletes a bucket
       description: >-
         Deletes the bucket record (and child objects) from the COMS database, based on bucketId.
+        When calling this endpoint using OIDC token authentication, the user requires the OIDC user
+        to have the DELETE permission for that bucket.
         Providing the 'recursive' parameter will also delete all sub-folders.
+        The recursive option also requires the OIDC user to have the DELETE permission
+        on all of these sub-folders otherwise the entire operation will fail.
         This request does not dispatch an S3 operation to request the deletion of the associated
         bucket(s) or delete any objects in object storage, it simply 'de-registers' knowledge of the bucket/folder
         and objects from the COMS system.
diff --git a/app/src/services/bucket.js b/app/src/services/bucket.js
@@ -200,18 +200,32 @@ const service = {
    * @function searchChildBuckets
    * Get db records for each bucket that acts as a sub-folder of the provided bucket
    * @param {object} parentBucket a bucket model (record) from the COMS db
+   * @param {boolean} returnPermissions also return current user's permissions for each bucket
    * @param {object} [etrx=undefined] An optional Objection Transaction object
    * @returns {Promise<object[]>} An array of bucket records
    * @throws If there are no records found
    */
-  searchChildBuckets: async (parentBucket, etrx = undefined) => {
+  searchChildBuckets: async (parentBucket, returnPermissions = false, userId, etrx = undefined) => {
     let trx;
     try {
       trx = etrx ? etrx : await Bucket.startTransaction();
-      return Bucket.query()
+      const response = Bucket.query()
+        .modify(query => {
+          if (returnPermissions) {
+            query
+              .withGraphJoined('bucketPermission')
+              .whereIn('bucketPermission.bucketId', builder => {
+                builder.distinct('bucketPermission.bucketId')
+                  .where('bucketPermission.userId', userId);
+              });
+          }
+        })
         .modify('filterKeyIsChild', parentBucket.key)
         .modify('filterEndpoint', parentBucket.endpoint)
         .where('bucket', parentBucket.bucket);
+
+      if (!etrx) await trx.commit();
+      return response;
     } catch (err) {
       if (!etrx && trx) await trx.rollback();
       throw err;
