[cpp-sp] 01/02: More tests, fix OR implementation.

Scott Cantor cantor.2 at osu.edu
Thu Dec 19 01:21:21 UTC 2024


This is an automated email from the git hooks/post-receive script.

scantor pushed a commit to branch main
in repository cpp-sp.

View the commit online:
http://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=4aba83f3e41c7d9e7ec222090ec6ba4db0dbc2fe

commit 4aba83f3e41c7d9e7ec222090ec6ba4db0dbc2fe
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Wed Dec 18 20:11:27 2024 -0500

    More tests, fix OR implementation.
---
 shibsp/impl/XMLAccessControl.cpp          |   4 +-
 tests/data/impl/external-or-acl.xml       |   1 +
 tests/data/impl/inline-attr-acl.xml       |   5 +
 tests/data/impl/inline-user-acl.xml       |   5 +
 tests/data/impl/inline-valid-user-acl.xml |   5 +
 tests/data/impl/or-acl.xml                |   6 +
 tests/impl/XMLAccessControlTests.cpp      | 195 ++++++++++++++++++++----------
 tests/util/ReloadableXMLFileTests.cpp     |  74 +++---------
 8 files changed, 172 insertions(+), 123 deletions(-)

diff --git a/shibsp/impl/XMLAccessControl.cpp b/shibsp/impl/XMLAccessControl.cpp
index 25777409..104f2985 100644
--- a/shibsp/impl/XMLAccessControl.cpp
+++ b/shibsp/impl/XMLAccessControl.cpp
@@ -354,8 +354,8 @@ AccessControl::aclresult_t Operator::authorized(const SPRequest& request, const
         {
             // Look for a rule that returns true.
             for (const auto& i : m_operands) {
-                if (i->authorized(request,session) != shib_acl_true)
-                    return shib_acl_false;
+                if (i->authorized(request,session) == shib_acl_true)
+                    return shib_acl_true;
             }
             return shib_acl_false;
         }
diff --git a/tests/data/impl/external-or-acl.xml b/tests/data/impl/external-or-acl.xml
new file mode 100644
index 00000000..f0a96472
--- /dev/null
+++ b/tests/data/impl/external-or-acl.xml
@@ -0,0 +1 @@
+<AccessControlProvider type="XML" path="./data/impl/or-acl.xml" reloadChanges="1" />
diff --git a/tests/data/impl/inline-attr-acl.xml b/tests/data/impl/inline-attr-acl.xml
new file mode 100644
index 00000000..6b22d0ce
--- /dev/null
+++ b/tests/data/impl/inline-attr-acl.xml
@@ -0,0 +1,5 @@
+<AccessControlProvider type="XML">
+	<AccessControl>
+		<Rule require="affiliation" list="true">member student</Rule>
+	</AccessControl>
+</AccessControlProvider>
diff --git a/tests/data/impl/inline-user-acl.xml b/tests/data/impl/inline-user-acl.xml
new file mode 100644
index 00000000..a146b2dc
--- /dev/null
+++ b/tests/data/impl/inline-user-acl.xml
@@ -0,0 +1,5 @@
+<AccessControlProvider type="XML">
+	<AccessControl>
+		<Rule require="user">jdoe</Rule>
+	</AccessControl>
+</AccessControlProvider>
diff --git a/tests/data/impl/inline-valid-user-acl.xml b/tests/data/impl/inline-valid-user-acl.xml
new file mode 100644
index 00000000..952abec6
--- /dev/null
+++ b/tests/data/impl/inline-valid-user-acl.xml
@@ -0,0 +1,5 @@
+<AccessControlProvider type="XML">
+	<AccessControl>
+		<Rule require="valid-user" />
+	</AccessControl>
+</AccessControlProvider>
diff --git a/tests/data/impl/or-acl.xml b/tests/data/impl/or-acl.xml
new file mode 100644
index 00000000..0f398b8d
--- /dev/null
+++ b/tests/data/impl/or-acl.xml
@@ -0,0 +1,6 @@
+<AccessControl>
+	<OR>
+		<Rule require="user">jdoe</Rule>
+		<Rule require="affiliation">student</Rule>
+	</OR>
+</AccessControl>
diff --git a/tests/impl/XMLAccessControlTests.cpp b/tests/impl/XMLAccessControlTests.cpp
index e1cf144f..83d00902 100644
--- a/tests/impl/XMLAccessControlTests.cpp
+++ b/tests/impl/XMLAccessControlTests.cpp
@@ -24,8 +24,13 @@
 #include "AgentConfig.h"
 #include "SessionCache.h"
 #include "attribute/Attribute.h"
+#include "attribute/SimpleAttribute.h"
 #include "logging/Category.h"
 
+#ifdef HAVE_CXX14
+# include <shared_mutex>
+#endif
+
 #include <boost/test/unit_test.hpp>
 #include <boost/property_tree/xml_parser.hpp>
 
@@ -107,7 +112,7 @@ public:
     const char* getQueryString() const { return nullptr; }
     const char* getRequestBody() const { return nullptr; }
     string getHeader(const char*) const { return nullptr; }
-    string getRemoteUser() const { return nullptr; }
+    string getRemoteUser() const { return m_user; }
     string getAuthType() const { return nullptr; }
     long sendResponse(istream&, long status) { return status; }
     void clearHeader(const char*, const char*) {}
@@ -115,6 +120,8 @@ public:
     void setRemoteUser(const char*) {}
     long returnDecline() { return 200; }
     long returnOK() { return 200; }
+
+    string m_user;
 };
 
 class exceptionCheck {
@@ -127,15 +134,20 @@ private:
     string m_msg;
 };
 
-struct BaseFixture
+struct XMLAccessControlFixture
 {
-    BaseFixture() : data_path(DATA_PATH) {
+    XMLAccessControlFixture() : data_path(DATA_PATH) {
         AgentConfig::getConfig().init(nullptr, (data_path + "console-shibboleth.ini").c_str(), true);
     }
-    ~BaseFixture() {
+    ~XMLAccessControlFixture() {
         AgentConfig::getConfig().term();
     }
 
+    void parse(const string& filename) {
+        xml_parser::read_xml(data_path + filename, tree, xml_parser::no_comments|xml_parser::trim_whitespace);
+    }
+
+    ptree tree;
     string data_path;
 };
 
@@ -143,17 +155,9 @@ struct BaseFixture
 // File pointing to external ACL file that's invalid XML.
 /////////////
 
-struct External_Invalid_Fixture : public BaseFixture
-{
-    External_Invalid_Fixture() {
-        xml_parser::read_xml(data_path + "external-acl-badxml.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(XMLAccessControl_external_invalid, External_Invalid_Fixture)
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_external_invalid, XMLAccessControlFixture)
 {
+    parse("external-acl-badxml.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     exceptionCheck checker("Initial AccessControl configuration was invalid.");
@@ -166,17 +170,9 @@ BOOST_FIXTURE_TEST_CASE(XMLAccessControl_external_invalid, External_Invalid_Fixt
 // Inline ACL content that has the wrong child element.
 /////////////
 
-struct Inline_Invalid_Fixture : public BaseFixture
-{
-    Inline_Invalid_Fixture() {
-        xml_parser::read_xml(data_path + "internal-acl-invalid.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid, Inline_Invalid_Fixture)
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid, XMLAccessControlFixture)
 {
+    parse("internal-acl-invalid.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     exceptionCheck checker("Initial AccessControl configuration was invalid.");
@@ -189,17 +185,9 @@ BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid, Inline_Invalid_Fixture)
 // Inline ACL content that has a bad internal element.
 /////////////
 
-struct Inline_InvalidInternal_Fixture : public BaseFixture
-{
-    Inline_InvalidInternal_Fixture() {
-        xml_parser::read_xml(data_path + "internal-acl-invalid2.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid_internal, Inline_InvalidInternal_Fixture)
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid_internal, XMLAccessControlFixture)
 {
+    parse("internal-acl-invalid2.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     exceptionCheck checker("Initial AccessControl configuration was invalid.");
@@ -209,68 +197,141 @@ BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_invalid_internal, Inline_Invalid
 }
 
 /////////////
-// Inline ACL test for authnContextClassRef rule.
+// Inline ACL test for valid-user rule.
 /////////////
 
-struct Inline_ACRule_Fixture : public BaseFixture
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_ValidUserRule, XMLAccessControlFixture)
 {
-    Inline_ACRule_Fixture() {
-        xml_parser::read_xml(data_path + "inline-ac-acl.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
+    parse("inline-valid-user-acl.xml");
+    BOOST_CHECK_EQUAL(tree.size(), 1);
 
-    ptree tree;
-};
+    unique_ptr<AccessControl> acl(AgentConfig::getConfig().AccessControlManager.newPlugin(
+        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
+
+#ifdef HAVE_CXX14
+    shared_lock locker(*acl);
+#endif
 
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_ACRule, Inline_ACRule_Fixture)
+    DummyRequest request;
+    DummySession session;
+
+    BOOST_CHECK_EQUAL(acl->authorized(request, nullptr), AccessControl::shib_acl_false);
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
+}
+
+/////////////
+// Inline ACL test for user rule.
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_UserRule, XMLAccessControlFixture)
 {
+    parse("inline-user-acl.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     unique_ptr<AccessControl> acl(AgentConfig::getConfig().AccessControlManager.newPlugin(
         tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
 
-    acl->lock_shared();
+#ifdef HAVE_CXX14
+    shared_lock locker(*acl);
+#endif
 
     DummyRequest request;
     DummySession session;
-    session.m_ac = "Foo";
 
+    request.m_user = "smith";
     BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_false);
 
-    session.m_ac = "urn:oasis:names:tc:SAML:2.0:ac:classes:TimeSyncToken";
+    request.m_user = "jdoe";
     BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
-
-    acl->unlock_shared();
 }
 
-/*
-struct External_Valid_Fixture : public BaseFixture
+/////////////
+// Inline ACL test for authnContextClassRef rule.
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_ACRule, XMLAccessControlFixture)
 {
-    External_Valid_Fixture() {
-        xml_parser::read_xml(data_path + "external.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
+    parse("inline-ac-acl.xml");
+    BOOST_CHECK_EQUAL(tree.size(), 1);
 
-    ptree tree;
-};
+    unique_ptr<AccessControl> acl(AgentConfig::getConfig().AccessControlManager.newPlugin(
+        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
 
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_valid, External_Valid_Fixture)
+#ifdef HAVE_CXX14
+    shared_lock locker(*acl);
+#endif
+
+    DummyRequest request;
+    DummySession session;
+
+    session.m_ac = "Foo";
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_false);
+
+    session.m_ac = "urn:oasis:names:tc:SAML:2.0:ac:classes:TimeSyncToken";
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
+}
+
+/////////////
+// Inline ACL test for attribute rule.
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_inline_AttrRule, XMLAccessControlFixture)
 {
+    parse("inline-attr-acl.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
-    DummyXMLFile dummy(tree.front().second);
 
-    dummy.lock_shared();
-    time_t ts1 = dummy.getLastModified();
-    BOOST_CHECK_GT(ts1, 0);
-    dummy.unlock();
+    unique_ptr<AccessControl> acl(AgentConfig::getConfig().AccessControlManager.newPlugin(
+        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
+
+#ifdef HAVE_CXX14
+    shared_lock locker(*acl);
+#endif
+
+    DummyRequest request;
+    DummySession session;
 
-    dummy.forceReload();
-    sleep(2);
+    session.m_attributes.push_back(unique_ptr<Attribute>(new SimpleAttribute({"affiliation"})));
+    SimpleAttribute& attr = dynamic_cast<SimpleAttribute&>(*(session.m_attributes.back()));
 
-    dummy.lock_shared();
-    time_t ts2 = dummy.getLastModified();
-    BOOST_CHECK_GT(ts2, ts1);
-    dummy.unlock();
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_false);
+
+    attr.getValues().push_back("staff");
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_false);
+
+    attr.getValues().push_back("student");
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
 }
 
-*/
+/////////////
+// External ACL test for OR operator
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(XMLAccessControl_external_OR, XMLAccessControlFixture)
+{
+    parse("external-or-acl.xml");
+    BOOST_CHECK_EQUAL(tree.size(), 1);
+
+    unique_ptr<AccessControl> acl(AgentConfig::getConfig().AccessControlManager.newPlugin(
+        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
+
+#ifdef HAVE_CXX14
+    shared_lock locker(*acl);
+#endif
+
+    DummyRequest request;
+    DummySession session;
+
+    session.m_attributes.push_back(unique_ptr<Attribute>(new SimpleAttribute({"affiliation"})));
+    SimpleAttribute& attr = dynamic_cast<SimpleAttribute&>(*(session.m_attributes.back()));
+
+    request.m_user = "jdoe";
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
+
+    request.m_user = "smith";
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_false);
+
+    attr.getValues().push_back("student");
+    BOOST_CHECK_EQUAL(acl->authorized(request, &session), AccessControl::shib_acl_true);
+}
 
 };
\ No newline at end of file
diff --git a/tests/util/ReloadableXMLFileTests.cpp b/tests/util/ReloadableXMLFileTests.cpp
index f9e8169e..8e68266e 100644
--- a/tests/util/ReloadableXMLFileTests.cpp
+++ b/tests/util/ReloadableXMLFileTests.cpp
@@ -109,32 +109,28 @@ private:
     string m_msg;
 };
 
-struct BaseFixture
+struct ReloadableXMLFileFixture
 {
-    BaseFixture() : data_path(DATA_PATH) {
+    ReloadableXMLFileFixture() : data_path(DATA_PATH) {
         AgentConfig::getConfig().init(nullptr, (data_path + "console-shibboleth.ini").c_str(), true);
     }
-    ~BaseFixture() {
+    ~ReloadableXMLFileFixture() {
         AgentConfig::getConfig().term();
     }
-    string data_path;
-};
-
-/////////////
 
-struct External_Invalid_Fixture : public BaseFixture
-{
-    External_Invalid_Fixture() {
-        xml_parser::read_xml(data_path + "external-invalid.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-    ~External_Invalid_Fixture() {
+    void parse(const string& filename) {
+        xml_parser::read_xml(data_path + filename, tree, xml_parser::no_comments|xml_parser::trim_whitespace);
     }
 
+    string data_path;
     ptree tree;
 };
 
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_invalid, External_Invalid_Fixture)
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_invalid, ReloadableXMLFileFixture)
 {
+    parse("external-invalid.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     exceptionCheck checker("Invalid configuration.");
@@ -143,19 +139,9 @@ BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_invalid, External_Invalid_Fi
 
 /////////////
 
-struct Inline_Invalid_Fixture : public BaseFixture
-{
-    Inline_Invalid_Fixture() {
-        xml_parser::read_xml(data_path + "inline-invalid.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-    ~Inline_Invalid_Fixture() {
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_invalid, Inline_Invalid_Fixture)
+BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_invalid, ReloadableXMLFileFixture)
 {
+    parse("inline-invalid.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
 
     exceptionCheck checker("Invalid configuration.");
@@ -164,26 +150,16 @@ BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_invalid, Inline_Invalid_Fixtur
 
 /////////////
 
-struct Inline_Valid_Fixture : public BaseFixture
-{
-    Inline_Valid_Fixture() {
-        xml_parser::read_xml(data_path + "inline.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-    ~Inline_Valid_Fixture() {
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_valid, Inline_Valid_Fixture)
+BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_valid, ReloadableXMLFileFixture)
 {
+    parse("inline.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
     DummyXMLFile dummy(tree.front().second);
 
     dummy.lock_shared();
     time_t ts1 = dummy.getLastModified();
     BOOST_CHECK_EQUAL(ts1, 0);
-    dummy.unlock();
+    dummy.unlock_shared();
 
     // No-op since there's no locking internally.
     dummy.forceReload();
@@ -192,29 +168,19 @@ BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_inline_valid, Inline_Valid_Fixture)
     dummy.lock_shared();
     time_t ts2 = dummy.getLastModified();
     BOOST_CHECK_EQUAL(ts2, 0);
-    dummy.unlock();
+    dummy.unlock_shared();
 }
 
-struct External_Valid_Fixture : public BaseFixture
-{
-    External_Valid_Fixture() {
-        xml_parser::read_xml(data_path + "external.xml", tree, xml_parser::no_comments|xml_parser::trim_whitespace);
-    }
-    ~External_Valid_Fixture() {
-    }
-
-    ptree tree;
-};
-
-BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_valid, External_Valid_Fixture)
+BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_valid, ReloadableXMLFileFixture)
 {
+    parse("external.xml");
     BOOST_CHECK_EQUAL(tree.size(), 1);
     DummyXMLFile dummy(tree.front().second);
 
     dummy.lock_shared();
     time_t ts1 = dummy.getLastModified();
     BOOST_CHECK_GT(ts1, 0);
-    dummy.unlock();
+    dummy.unlock_shared();
 
     dummy.forceReload();
     sleep(2);
@@ -222,7 +188,7 @@ BOOST_FIXTURE_TEST_CASE(ReloadableFileTest_external_valid, External_Valid_Fixtur
     dummy.lock_shared();
     time_t ts2 = dummy.getLastModified();
     BOOST_CHECK_GT(ts2, ts1);
-    dummy.unlock();
+    dummy.unlock_shared();
 }
 
 };
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list