[PM-5873 / PM-5932] Fix collection creation by users other than the O…

…rganization owner (#3721) * [AC-2106] Add check for providers and additional check for null response * [PM-5873] Separated CollectionsController.Post flexible collections logic from non-migrated orgs --------- Co-authored-by: Shane Melton <[email protected]>
bitwarden · Jan 30, 2024 · 7180a66 · 7180a66
1 parent cc2a81a
commit 7180a66
Showing 1 changed file with 41 additions and 17 deletions.
diff --git a/src/Api/Controllers/CollectionsController.cs b/src/Api/Controllers/CollectionsController.cs
@@ -213,12 +213,15 @@ public async Task<IEnumerable<SelectionReadOnlyResponseModel>> GetUsers(Guid org
     [HttpPost("")]
     public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] CollectionRequestModel model)
     {
+        if (await FlexibleCollectionsIsEnabledAsync(orgId))
+        {
+            // New flexible collections logic
+            return await Post_vNext(orgId, model);
+        }
+
         var collection = model.ToCollection(orgId);
 
-        var flexibleCollectionsIsEnabled = await FlexibleCollectionsIsEnabledAsync(orgId);
-        var authorized = flexibleCollectionsIsEnabled
-            ? (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.Create)).Succeeded
-            : await CanCreateCollection(orgId, collection.Id) || await CanEditCollectionAsync(orgId, collection.Id);
+        var authorized = await CanCreateCollection(orgId, collection.Id) || await CanEditCollectionAsync(orgId, collection.Id);
         if (!authorized)
         {
             throw new NotFoundException();
@@ -229,7 +232,6 @@ public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] Collectio
 
         // Pre-flexible collections logic assigned Managers to collections they create
         var assignUserToCollection =
-            !flexibleCollectionsIsEnabled &&
             !await _currentContext.EditAnyCollection(orgId) &&
             await _currentContext.EditAssignedCollections(orgId);
         var isNewCollection = collection.Id == default;
@@ -251,16 +253,7 @@ public async Task<CollectionResponseModel> Post(Guid orgId, [FromBody] Collectio
 
         await _collectionService.SaveAsync(collection, groups, users);
 
-        if (!_currentContext.UserId.HasValue)
-        {
-            return new CollectionResponseModel(collection);
-        }
-
-        // If we have a user, fetch the collection to get the latest permission details
-        var userCollectionDetails = await _collectionRepository.GetByIdAsync(collection.Id,
-            _currentContext.UserId.Value, await FlexibleCollectionsIsEnabledAsync(collection.OrganizationId));
-
-        return new CollectionDetailsResponseModel(userCollectionDetails);
+        return new CollectionResponseModel(collection);
     }
 
     [HttpPut("{id}")]
@@ -616,6 +609,35 @@ private async Task<IEnumerable<SelectionReadOnlyResponseModel>> GetUsers_vNext(G
         return responses;
     }
 
+    private async Task<CollectionResponseModel> Post_vNext(Guid orgId, [FromBody] CollectionRequestModel model)
+    {
+        var collection = model.ToCollection(orgId);
+
+        var authorized = (await _authorizationService.AuthorizeAsync(User, collection, BulkCollectionOperations.Create)).Succeeded;
+        if (!authorized)
+        {
+            throw new NotFoundException();
+        }
+
+        var groups = model.Groups?.Select(g => g.ToSelectionReadOnly());
+        var users = model.Users?.Select(g => g.ToSelectionReadOnly()).ToList() ?? new List<CollectionAccessSelection>();
+
+        await _collectionService.SaveAsync(collection, groups, users);
+
+        if (!_currentContext.UserId.HasValue || await _currentContext.ProviderUserForOrgAsync(orgId))
+        {
+            return new CollectionResponseModel(collection);
+        }
+
+        // If we have a user, fetch the collection to get the latest permission details
+        var userCollectionDetails = await _collectionRepository.GetByIdAsync(collection.Id,
+            _currentContext.UserId.Value, await FlexibleCollectionsIsEnabledAsync(collection.OrganizationId));
+
+        return userCollectionDetails == null
+            ? new CollectionResponseModel(collection)
+            : new CollectionDetailsResponseModel(userCollectionDetails);
+    }
+
     private async Task<CollectionResponseModel> Put_vNext(Guid id, CollectionRequestModel model)
     {
         var collection = await _collectionRepository.GetByIdAsync(id);
@@ -629,15 +651,17 @@ private async Task<CollectionResponseModel> Put_vNext(Guid id, CollectionRequest
         var users = model.Users?.Select(g => g.ToSelectionReadOnly());
         await _collectionService.SaveAsync(model.ToCollection(collection), groups, users);
 
-        if (!_currentContext.UserId.HasValue)
+        if (!_currentContext.UserId.HasValue || await _currentContext.ProviderUserForOrgAsync(collection.OrganizationId))
         {
             return new CollectionResponseModel(collection);
         }
 
         // If we have a user, fetch the collection details to get the latest permission details for the user
         var updatedCollectionDetails = await _collectionRepository.GetByIdAsync(id, _currentContext.UserId.Value, await FlexibleCollectionsIsEnabledAsync(collection.OrganizationId));
 
-        return new CollectionDetailsResponseModel(updatedCollectionDetails);
+        return updatedCollectionDetails == null
+            ? new CollectionResponseModel(collection)
+            : new CollectionDetailsResponseModel(updatedCollectionDetails);
     }
 
     private async Task PutUsers_vNext(Guid id, IEnumerable<SelectionReadOnlyRequestModel> model)