Disallow pointing to nonexistent buckets

Storage::insertPolicies() now cares, if bucket pointed
by inserted policies exists.

Change-Id: I113de2ead6ae17d18eb9a5928ef0181bee2f67d3

diff --git a/src/service/storage/Storage.cpp b/src/service/storage/Storage.cpp
index f8e7b8a..7f92c34 100644 (file)
--- a/src/service/storage/Storage.cpp
+++ b/src/service/storage/Storage.cpp
@@ -24,6 +24,7 @@
 #include <memory>
 #include <vector>
 
+#include <exceptions/BucketNotExistsException.h>
 #include "exceptions/DefaultBucketDeletionException.h"
 #include <types/pointers.h>
 #include <types/Policy.h>
@@ -79,6 +80,24 @@ PolicyResult Storage::minimalPolicy(const PolicyBucket &bucket, const PolicyKey
 }
 
 void Storage::insertPolicies(const std::map<PolicyBucketId, std::vector<Policy>> &policiesByBucketId) {
+
+    auto pointedBucketExists = [this] (const Policy &policy) -> void {
+        if (policy.result().policyType() == PredefinedPolicyType::BUCKET) {
+            const auto &bucketId = policy.result().metadata();
+            if (m_backend.hasBucket(bucketId) == false) {
+                throw BucketNotExistsException(bucketId);
+            }
+        }
+    };
+
+    // TODO: Rewrite, when transactions are supported
+    // Check if all of buckets exist
+    for (const auto &group : policiesByBucketId) {
+        const auto &policies = group.second;
+        std::for_each(policies.cbegin(), policies.cend(), pointedBucketExists);
+    }
+
+    // Then insert policies
     for (const auto &group : policiesByBucketId) {
         const PolicyBucketId &bucketId = group.first;
         const auto &policies = group.second;

diff --git a/test/storage/storage/policies.cpp b/test/storage/storage/policies.cpp
index fbe40fa..54e0455 100644 (file)
--- a/test/storage/storage/policies.cpp
+++ b/test/storage/storage/policies.cpp
@@ -28,6 +28,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include <exceptions/BucketNotExistsException.h>
 #include <storage/Storage.h>
 #include <storage/StorageBackend.h>
 #include <types/pointers.h>
@@ -78,6 +79,10 @@ TEST(storage, deleteBucketWithLinkedPolicies) {
 TEST(storage, insertPolicies) {
     using ::testing::Pointee;
     using ::testing::Return;
+    using PredefinedPolicyType::ALLOW;
+    using PredefinedPolicyType::BUCKET;
+    using PredefinedPolicyType::DENY;
+
     FakeStorageBackend backend;
     Storage storage(backend);
 
@@ -86,22 +91,24 @@ TEST(storage, insertPolicies) {
 
     typedef std::pair<PolicyBucketId, std::vector<Policy>> BucketPolicyPair;
 
-    auto createPolicy = [] (const std::string &keySuffix, const PolicyType &type) -> Policy {
-        return Policy(Helpers::generatePolicyKey(keySuffix), type);
+    auto createPolicy = [] (const std::string &keySuffix, const PolicyResult &result) -> Policy {
+        return Policy(Helpers::generatePolicyKey(keySuffix), result);
     };
 
     std::map<PolicyBucketId, std::vector<Policy>> policiesToInsert = {
         BucketPolicyPair(testBucket1, {
-            createPolicy("1", PredefinedPolicyType::ALLOW),
-            createPolicy("2", PredefinedPolicyType::DENY),
-            createPolicy("3", PredefinedPolicyType::DENY)
+            createPolicy("1", ALLOW),
+            createPolicy("2", DENY),
+            createPolicy("3", DENY)
         }),
         BucketPolicyPair(testBucket2, {
-            createPolicy("4", PredefinedPolicyType::ALLOW),
+            createPolicy("4", { BUCKET, testBucket1 }),
             createPolicy("5", PredefinedPolicyType::ALLOW)
         })
     };
 
+    EXPECT_CALL(backend, hasBucket(testBucket1)).WillOnce(Return(true));
+
     for (const auto &group : policiesToInsert) {
         const auto &bucketId = group.first;
         const auto &policies = group.second;
@@ -113,3 +120,30 @@ TEST(storage, insertPolicies) {
 
     storage.insertPolicies(policiesToInsert);
 }
+
+TEST(storage, insertPointingToNonexistentBucket) {
+    using ::testing::Pointee;
+    using ::testing::Return;
+    FakeStorageBackend backend;
+    Storage storage(backend);
+
+    PolicyBucketId testBucketId = "test-bucket-1";
+    PolicyBucketId nonexistentBucketId = "nonexistent";
+
+    typedef std::pair<PolicyBucketId, std::vector<Policy>> BucketPolicyPair;
+
+    auto createPolicy = [] (const std::string &keySuffix, const PolicyResult &result) -> Policy {
+        return Policy(Helpers::generatePolicyKey(keySuffix), result);
+    };
+
+    std::map<PolicyBucketId, std::vector<Policy>> policiesToInsert = {
+        BucketPolicyPair(testBucketId, {
+            createPolicy("1", { PredefinedPolicyType::DENY }),
+            createPolicy("2", { PredefinedPolicyType::BUCKET, nonexistentBucketId }),
+        }),
+    };
+
+    EXPECT_CALL(backend, hasBucket(nonexistentBucketId)).WillOnce(Return(false));
+
+    ASSERT_THROW(storage.insertPolicies(policiesToInsert), BucketNotExistsException);
+}