Add PolicyBucketId validation

This patch introduces mechanism for checking whether new PolicyBucketId
contains forbidden characters. Now only alphanumeric characters, hyphen
and underscore can be used in PolicyBucketId. InvalidBucketIdException
is thrown and OPERATION_NOT_ALLOWED error code is returned otherwise.

Change-Id: I48b6e14d20cb62adc17560929055553df0ce1077

diff --git a/src/common/exceptions/InvalidBucketIdException.h b/src/common/exceptions/InvalidBucketIdException.h
new file mode 100644 (file)
index 0000000..f8772e4
--- /dev/null
+++ b/src/common/exceptions/InvalidBucketIdException.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd All Rights Reserved
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+/**
+ * @file        src/common/exceptions/InvalidBucketIdException.h
+ * @author      Pawel Wieczorek <p.wieczorek2@samsung.com>
+ * @version     0.1
+ * @brief       This file defines exception thrown when bucket ID contains forbidden characters
+ */
+
+#ifndef SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_
+#define SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_
+
+#include <string>
+
+#include <exceptions/Exception.h>
+
+namespace Cynara {
+
+class InvalidBucketIdException : public Exception {
+public:
+    InvalidBucketIdException(const std::string &bucketId) : m_bucketId(bucketId) {};
+    virtual ~InvalidBucketIdException() {};
+
+    const std::string message(void) const {
+        if (m_message.empty()) {
+            m_message = "Bucket ID " + bucketId() + " contains forbidden characters";
+        }
+        return m_message;
+    }
+
+    const std::string &bucketId(void) const {
+        return m_bucketId;
+    }
+
+private:
+    mutable std::string m_message;
+    std::string m_bucketId;
+};
+
+} /* namespace Cynara */
+
+#endif /* SRC_COMMON_EXCEPTIONS_INVALIDBUCKETIDEXCEPTION_H_ */

diff --git a/src/common/types/PolicyBucket.cpp b/src/common/types/PolicyBucket.cpp
index f1a45a1..622bc05 100644 (file)
--- a/src/common/types/PolicyBucket.cpp
+++ b/src/common/types/PolicyBucket.cpp
@@ -22,6 +22,7 @@
 
 #include <sstream>
 
+#include <exceptions/InvalidBucketIdException.h>
 #include <types/PolicyCollection.h>
 #include <types/PolicyKeyHelpers.h>
 
@@ -29,6 +30,25 @@
 
 namespace Cynara {
 
+const std::string PolicyBucket::m_idSeparators = "-_";
+
+PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy)
+    : m_defaultPolicy(defaultPolicy), m_id(id) {
+    idValidator(id);
+}
+
+PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyCollection &policies)
+    : m_policyCollection(makePolicyMap(policies)), m_defaultPolicy(PredefinedPolicyType::DENY),
+      m_id(id) {
+    idValidator(id);
+}
+
+PolicyBucket::PolicyBucket(const PolicyBucketId &id, const PolicyResult &defaultPolicy,
+                           const PolicyCollection &policies)
+    : m_policyCollection(makePolicyMap(policies)), m_defaultPolicy(defaultPolicy), m_id(id) {
+    idValidator(id);
+}
+
 PolicyBucket PolicyBucket::filtered(const PolicyKey &key) const {
     PolicyBucket result(m_id + "_filtered");
 
@@ -78,4 +98,18 @@ PolicyMap PolicyBucket::makePolicyMap(const PolicyCollection &policies) {
     return result;
 }
 
+void PolicyBucket::idValidator(const PolicyBucketId &id) {
+    auto isCharInvalid = [] (char c) {
+        return !(std::isalnum(c) || isIdSeparator(c));
+    };
+
+    if (id.end() != find_if(id.begin(), id.end(), isCharInvalid)) {
+        throw InvalidBucketIdException(id);
+    }
+}
+
+bool PolicyBucket::isIdSeparator(char c) {
+    return m_idSeparators.end() != find(m_idSeparators.begin(), m_idSeparators.end(), c);
+}
+
 }  // namespace Cynara

diff --git a/src/common/types/PolicyBucket.h b/src/common/types/PolicyBucket.h
index a2faf9c..df57d0f 100644 (file)
--- a/src/common/types/PolicyBucket.h
+++ b/src/common/types/PolicyBucket.h
@@ -52,20 +52,12 @@ public:
     //delete default constructor in order to prevent creation of buckets with no id
     PolicyBucket() = delete;
     PolicyBucket(const PolicyBucketId &id,
-                 const PolicyResult &defaultPolicy = PredefinedPolicyType::DENY)
-        : m_defaultPolicy(defaultPolicy),
-          m_id(id) {}
+                 const PolicyResult &defaultPolicy = PredefinedPolicyType::DENY);
     PolicyBucket(const PolicyBucketId &id,
-                 const PolicyCollection &policies)
-        : m_policyCollection(makePolicyMap(policies)),
-          m_defaultPolicy(PredefinedPolicyType::DENY),
-          m_id(id) {}
+                 const PolicyCollection &policies);
     PolicyBucket(const PolicyBucketId &id,
                  const PolicyResult &defaultPolicy,
-                 const PolicyCollection &policies)
-        : m_policyCollection(makePolicyMap(policies)),
-          m_defaultPolicy(defaultPolicy),
-          m_id(id) {}
+                 const PolicyCollection &policies);
 
     PolicyBucket filtered(const PolicyKey &key) const;
     void insertPolicy(PolicyPtr policy);
@@ -76,13 +68,6 @@ public:
 
     static PolicyMap makePolicyMap(const PolicyCollection &policies);
 
-private:
-    PolicyMap m_policyCollection;
-    PolicyResult m_defaultPolicy;
-    PolicyBucketId m_id;
-
-public:
-
     const_policy_iterator begin(void) const {
         return const_policy_iterator(m_policyCollection.begin());
     }
@@ -111,6 +96,15 @@ public:
     void setDefaultPolicy(const PolicyResult &defaultPolicy) {
         m_defaultPolicy = defaultPolicy;
     }
+
+private:
+    static void idValidator(const PolicyBucketId &id);
+    static bool isIdSeparator(char c);
+
+    PolicyMap m_policyCollection;
+    PolicyResult m_defaultPolicy;
+    PolicyBucketId m_id;
+    static const std::string m_idSeparators;
 };
 
 } /* namespace Cynara */

diff --git a/src/service/logic/Logic.cpp b/src/service/logic/Logic.cpp
index eeed387..f76ce98 100644 (file)
--- a/src/service/logic/Logic.cpp
+++ b/src/service/logic/Logic.cpp
@@ -26,6 +26,7 @@
 #include <exceptions/DatabaseException.h>
 #include <exceptions/DefaultBucketDeletionException.h>
 #include <exceptions/DefaultBucketSetNoneException.h>
+#include <exceptions/InvalidBucketIdException.h>
 #include <exceptions/PluginErrorException.h>
 #include <exceptions/PluginNotFoundException.h>
 
@@ -134,6 +135,8 @@ void Logic::execute(RequestContextPtr context, InsertOrUpdateBucketRequestPtr re
         code = CodeResponse::Code::FAILED;
     } catch (const DefaultBucketSetNoneException &ex) {
         code = CodeResponse::Code::NOT_ALLOWED;
+    } catch (const InvalidBucketIdException &ex) {
+        code = CodeResponse::Code::NOT_ALLOWED;
     }
 
     context->returnResponse(context, std::make_shared<CodeResponse>(code,

diff --git a/test/common/types/policybucket.cpp b/test/common/types/policybucket.cpp
index 983a040..6c5d7a4 100644 (file)
--- a/test/common/types/policybucket.cpp
+++ b/test/common/types/policybucket.cpp
@@ -27,6 +27,7 @@
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "exceptions/InvalidBucketIdException.h"
 #include "types/PolicyBucket.h"
 #include "types/PolicyCollection.h"
 #include "types/PolicyKey.h"
@@ -57,6 +58,15 @@ protected:
         Policy::simpleWithKey(PolicyKey("*", "*", "p1"), PredefinedPolicyType::ALLOW),
         Policy::simpleWithKey(PolicyKey("*", "*", "*"), PredefinedPolicyType::ALLOW)
     };
+
+    const std::vector<PolicyBucketId> goodIds = {
+        "_goodid", "good_id", "goodid_", "-goodid", "good-id", "goodid-"
+    };
+
+    const std::vector<PolicyBucketId> badIds = {
+        "{badid", "bad[id", "badid~", "/badid", "bad*id", "badid|", "badid;", "\tbadid", "badid\n",
+        " badid", "bad id", "badid "
+    };
 };
 
 TEST_F(PolicyBucketFixture, filtered) {
@@ -141,3 +151,29 @@ TEST_F(PolicyBucketFixture, filtered_wildcard_none) {
     auto filtered = bucket.filtered(PolicyKey("cccc", "uuuu", "pppp"));
     ASSERT_THAT(filtered, IsEmpty());
 }
+
+/**
+ * @brief   Validate PolicyBucketIds during creation - passing bucket ids
+ * @test    Scenario:
+ * - Iterate through vector of valid bucket ids and create them normally
+ * - PolicyBucket constructor should not throw any exception
+ */
+TEST_F(PolicyBucketFixture, bucket_id_validation_passes) {
+    for (auto it = goodIds.begin(); it != goodIds.end(); ++it) {
+        SCOPED_TRACE(*it);
+        ASSERT_NO_THROW(PolicyBucket(PolicyBucketId(*it)));
+    }
+}
+
+/**
+ * @brief   Validate PolicyBucketIds during creation - failing bucket ids
+ * @test    Scenario:
+ * - Iterate through vector of bucket ids containing forbidden characters
+ * - PolicyBucket constructor should throw an exception every time it is called
+ */
+TEST_F(PolicyBucketFixture, bucket_id_validation_fails) {
+    for (auto it = badIds.begin(); it != badIds.end(); ++it) {
+        SCOPED_TRACE(*it);
+        ASSERT_THROW(PolicyBucket(PolicyBucketId(*it)), InvalidBucketIdException);
+    }
+}