Fix wildcard checks

Fixed bug, where consecutive buckets were filtered with wildcard keys
instead of original key being checked

Change-Id: I97499f41c796a0262e9f8d2a57908cc8a4fc3014

diff --git a/src/service/storage/Storage.cpp b/src/service/storage/Storage.cpp
index 525bfa1..a35da0c 100644 (file)
--- a/src/service/storage/Storage.cpp
+++ b/src/service/storage/Storage.cpp
@@ -37,10 +37,10 @@ namespace Cynara {
 
 PolicyResult Storage::checkPolicy(const PolicyKey &key) {
     auto policies = m_backend.searchDefaultBucket(key);
-    return minimalPolicy(policies);
+    return minimalPolicy(policies, key);
 };
 
-PolicyResult Storage::minimalPolicy(const PolicyBucket &bucket) {
+PolicyResult Storage::minimalPolicy(const PolicyBucket &bucket, const PolicyKey &key) {
     bool hasMinimal = false;
     PolicyResult minimal = bucket.defaultPolicy();
 
@@ -63,9 +63,8 @@ PolicyResult Storage::minimalPolicy(const PolicyBucket &bucket) {
             return policyResult; // Do not expect lower value than DENY
             break;
         case PredefinedPolicyType::BUCKET: {
-                auto bucketResults = m_backend.searchBucket(policyResult.metadata(),
-                        policyRecord->key());
-                auto minimumOfBucket = minimalPolicy(bucketResults);
+                auto bucketResults = m_backend.searchBucket(policyResult.metadata(), key);
+                auto minimumOfBucket = minimalPolicy(bucketResults, key);
                 proposeMinimal(minimumOfBucket);
                 continue;
             }

diff --git a/src/service/storage/Storage.h b/src/service/storage/Storage.h
index 8309092..e16d60c 100644 (file)
--- a/src/service/storage/Storage.h
+++ b/src/service/storage/Storage.h
@@ -55,7 +55,7 @@ public:
     void deleteBucket(const PolicyBucketId &bucketId);
 
 protected:
-    PolicyResult minimalPolicy(const PolicyBucket &bucket);
+    PolicyResult minimalPolicy(const PolicyBucket &bucket, const PolicyKey &key);
 
 private:
     StorageBackend &m_backend; // backend strategy

diff --git a/test/storage/storage/check.cpp b/test/storage/storage/check.cpp
index 426c9f3..129eb8d 100644 (file)
--- a/test/storage/storage/check.cpp
+++ b/test/storage/storage/check.cpp
@@ -119,3 +119,64 @@ TEST(storage, checkBucket) {
     defaultBucket.policyCollection().push_back(Policy::simpleWithKey(pk, PredefinedPolicyType::DENY));
     ASSERT_EQ(PredefinedPolicyType::DENY, storage.checkPolicy(pk).policyType());
 }
+
+// Catch a bug, where consecutive buckets were filtered with wildcard policies' keys
+// instead of original key being checked
+TEST(storage, checkBucketWildcard) {
+    using ::testing::Return;
+    using ::testing::ReturnPointee;
+
+    const PolicyBucketId additionalBucketId = "additional-bucket";
+    const PolicyKey defaultBucketKey = PolicyKey("c", "*", "p");
+    const PolicyKey checkKey = PolicyKey("c", "u1", "p");
+
+    PolicyBucket defaultBucket(PolicyCollection({
+        Policy::bucketWithKey(defaultBucketKey, additionalBucketId)
+    }));
+
+    FakeStorageBackend backend;
+    Cynara::Storage storage(backend);
+
+    EXPECT_CALL(backend, searchDefaultBucket(checkKey))
+        .WillRepeatedly(ReturnPointee(&defaultBucket));
+
+    EXPECT_CALL(backend, searchBucket(defaultPolicyBucketId, checkKey))
+        .WillRepeatedly(ReturnPointee(&defaultBucket));
+
+    // Check, if next bucket is filtered with original key
+    EXPECT_CALL(backend, searchBucket(additionalBucketId, checkKey))
+        .WillRepeatedly(Return(PolicyBucket()));    // additional bucket would yield no records
+
+    // Should return additional bucket's default policy
+    ASSERT_EQ(PredefinedPolicyType::DENY, storage.checkPolicy(checkKey));
+}
+
+TEST(storage, checkBucketWildcardOtherDefault) {
+    using ::testing::ReturnPointee;
+
+    const PolicyBucketId additionalBucketId = "additional-bucket";
+    const PolicyKey defaultBucketKey = PolicyKey("c", "*", "p");
+    const PolicyKey checkKey = PolicyKey("c", "u1", "p");
+
+    PolicyBucket defaultBucket(PolicyCollection({
+        Policy::bucketWithKey(defaultBucketKey, additionalBucketId)
+    }));
+
+    PolicyBucket additionalBucket(additionalBucketId, PredefinedPolicyType::ALLOW);
+
+    FakeStorageBackend backend;
+    Cynara::Storage storage(backend);
+
+    EXPECT_CALL(backend, searchDefaultBucket(checkKey))
+        .WillRepeatedly(ReturnPointee(&defaultBucket));
+
+    EXPECT_CALL(backend, searchBucket(defaultPolicyBucketId, checkKey))
+        .WillRepeatedly(ReturnPointee(&defaultBucket));
+
+    // Check, if next bucket is filtered with original key
+    EXPECT_CALL(backend, searchBucket(additionalBucketId, checkKey))
+        .WillRepeatedly(ReturnPointee(&additionalBucket));
+
+    // Should return additional bucket's default policy
+    ASSERT_EQ(PredefinedPolicyType::ALLOW, storage.checkPolicy(checkKey));
+}