[cpp-sp] branch main updated: Tests and fixes for embedded ACLs inside RequestMap.

Scott Cantor cantor.2 at osu.edu
Thu Dec 26 19:20:16 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=0dca68e9e5fed8172bf0c715ddd3932ce5e291b2

The following commit(s) were added to refs/heads/main by this push:
     new 0dca68e9 Tests and fixes for embedded ACLs inside RequestMap.
0dca68e9 is described below

commit 0dca68e9e5fed8172bf0c715ddd3932ce5e291b2
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Dec 26 14:20:12 2024 -0500

    Tests and fixes for embedded ACLs inside RequestMap.
---
 shibsp/impl/XMLRequestMapper.cpp             |  13 +--
 tests/data/impl/reqmap/acl.xml               |   3 +
 tests/data/impl/reqmap/external-with-acl.xml |   1 +
 tests/data/impl/reqmap/internal-invalid2.xml |   7 --
 tests/data/impl/reqmap/internal-with-acl.xml |  10 +++
 tests/impl/XMLRequestMapperTests.cpp         | 127 +++++++++++++++++++++++----
 6 files changed, 127 insertions(+), 34 deletions(-)

diff --git a/shibsp/impl/XMLRequestMapper.cpp b/shibsp/impl/XMLRequestMapper.cpp
index 4397a1c9..cc1cec6f 100644
--- a/shibsp/impl/XMLRequestMapper.cpp
+++ b/shibsp/impl/XMLRequestMapper.cpp
@@ -182,15 +182,14 @@ void Override::loadACL(const ptree& pt, Category& log)
         else {
             acl = pt.get_child_optional(ACCESS_CONTROL_PROP_PATH);
             if (acl) {
-                log.info("building XML-based AccessControl provider...");
-                // TODO: this is tenative, but it seems like we need to pass in the parent tree to allow it to
-                // walk down to the "expected" element, but TBD.
+                log.info("building inline XML-based AccessControl provider...");
+                // We need to pass in the parent tree to allow it to walk down to the "expected" inline element.
                 m_acl.reset(AgentConfig::getConfig().AccessControlManager.newPlugin(XML_ACCESS_CONTROL, pt, false));
             }
             else {
                 acl = pt.get_child_optional(ACCESS_CONTROL_PROVIDER_PROP_PATH);
                 if (acl) {
-                    string t(pt.get(TYPE_PROP_PATH, ""));
+                    string t(acl->get(TYPE_PROP_PATH, ""));
                     if (!t.empty()) {
                         log.info("building AccessControl provider of type %s...", t.c_str());
                         m_acl.reset(AgentConfig::getConfig().AccessControlManager.newPlugin(t.c_str(), acl.get(), false));
@@ -368,9 +367,6 @@ Override::Override(bool unicodeAware, ptree& pt, Category& log, const Override*
                 }
             }
         }
-        else if (child.first != "<xmlattr>") {
-            throw ConfigurationException(string("Unrecognized child element: ") + child.first);
-        }
     }
 }
 
@@ -634,9 +630,6 @@ XMLRequestMapperImpl::XMLRequestMapperImpl(ptree& pt, Category& log)
                 log.debug("added <Host> mapping for %s", url.c_str());
             }
         }
-        else if (child.first != "<xmlattr>") {
-            throw ConfigurationException(string("Unrecognized child element: ") + child.first);
-        }
     }
 }
 
diff --git a/tests/data/impl/reqmap/acl.xml b/tests/data/impl/reqmap/acl.xml
new file mode 100644
index 00000000..ce1c46cc
--- /dev/null
+++ b/tests/data/impl/reqmap/acl.xml
@@ -0,0 +1,3 @@
+<AccessControl>
+	<Rule require="user">jsmith</Rule>
+</AccessControl>
diff --git a/tests/data/impl/reqmap/external-with-acl.xml b/tests/data/impl/reqmap/external-with-acl.xml
new file mode 100644
index 00000000..9d2728cf
--- /dev/null
+++ b/tests/data/impl/reqmap/external-with-acl.xml
@@ -0,0 +1 @@
+<RequestMapper type="XML" path="./data/impl/reqmap/internal-with-acl.xml" />
diff --git a/tests/data/impl/reqmap/internal-invalid2.xml b/tests/data/impl/reqmap/internal-invalid2.xml
deleted file mode 100644
index 99c10271..00000000
--- a/tests/data/impl/reqmap/internal-invalid2.xml
+++ /dev/null
@@ -1,7 +0,0 @@
-<RequestMapper type="XML">
-	<RequestMap>
-		<Host name="sp.example.org">
-			<Foo/>
-		</Host>
-	</RequestMap>
-</RequestMapper>
diff --git a/tests/data/impl/reqmap/internal-with-acl.xml b/tests/data/impl/reqmap/internal-with-acl.xml
new file mode 100644
index 00000000..74eb1921
--- /dev/null
+++ b/tests/data/impl/reqmap/internal-with-acl.xml
@@ -0,0 +1,10 @@
+<RequestMap>
+	<Host name="sp.example.org">
+		<Path name="secure">
+			<AccessControl>
+				<Rule require="user">jdoe</Rule>
+			</AccessControl>
+		</Path>
+		<AccessControlProvider type="XML" path="./data/impl/reqmap/acl.xml" reloadChanges="true" />
+	</Host>
+</RequestMap>
diff --git a/tests/impl/XMLRequestMapperTests.cpp b/tests/impl/XMLRequestMapperTests.cpp
index 2dcce7e0..a57c313c 100644
--- a/tests/impl/XMLRequestMapperTests.cpp
+++ b/tests/impl/XMLRequestMapperTests.cpp
@@ -23,6 +23,8 @@
 #include "AccessControl.h"
 #include "AgentConfig.h"
 #include "RequestMapper.h"
+#include "SessionCache.h"
+#include "attribute/Attribute.h"
 #include "logging/Category.h"
 #include "util/PropertySet.h"
 
@@ -41,6 +43,64 @@ using namespace std;
 
 namespace {
 
+/** Open structure for testing manipulation. */
+struct DummySession : public Session, public NoOpBasicLockable
+{
+public:
+    DummySession() {}
+    ~DummySession() {}
+
+    const char* getID() const {
+        return nullptr;
+    }
+    const char* getApplicationID() const {
+        return nullptr;
+    }
+    time_t getExpiration() const {
+        return 0;
+    }
+    time_t getLastAccess() const {
+        return 0;
+    }
+    const char* getClientAddress() const {
+        return nullptr;
+    }
+    const char* getEntityID() const {
+        return nullptr;
+    }
+    const char* getProtocol() const {
+        return nullptr;
+    }
+    time_t getAuthnInstant() const {
+        return m_authInstant;
+    }
+    const char* getSessionIndex() const {
+        return nullptr;
+    }
+    const char* getAuthnContextClassRef() const {
+        return m_ac.c_str();
+    }
+    const vector<unique_ptr<Attribute>>& getAttributes() const {
+    }
+
+    const multimap<string,const Attribute*>& getIndexedAttributes() const {
+        if (m_attributeIndex.empty()) {
+            for (const unique_ptr<Attribute>& a : m_attributes) {
+                const vector<string>& aliases = a->getAliases();
+                for (const string& alias : a->getAliases()) {
+                    m_attributeIndex.insert(multimap<string, const Attribute*>::value_type(alias, a.get()));
+                }
+            }
+        }
+        return m_attributeIndex;
+    }
+
+    time_t m_authInstant;
+    string m_ac;
+    vector<unique_ptr<Attribute>> m_attributes;
+    mutable multimap<string,const Attribute*> m_attributeIndex;
+};
+
 class DummyRequest : public AbstractSPRequest {
 public:
     DummyRequest(const char* uri=nullptr) : AbstractSPRequest(SHIBSP_LOGCAT ".DummyRequest") {
@@ -55,7 +115,7 @@ public:
     const char* getQueryString() const { return m_query.c_str(); }
     const char* getRequestBody() const { return nullptr; }
     string getHeader(const char*) const { return nullptr; }
-    string getRemoteUser() const { return nullptr; }
+    string getRemoteUser() const { return m_user.c_str(); }
     string getAuthType() const { return nullptr; }
     long sendResponse(istream&, long status) { return status; }
     void clearHeader(const char*, const char*) {}
@@ -68,6 +128,7 @@ public:
     string m_hostname;
     int m_port;
     string m_query;
+    string m_user;
 };
 
 class exceptionCheck {
@@ -127,21 +188,6 @@ BOOST_FIXTURE_TEST_CASE(XMLRequestMapper_inline_invalid, XMLRequestMapperFixture
             ConfigurationException, checker.check_message);
 }
 
-/////////////
-// Inline content that has a bad internal element.
-/////////////
-
-BOOST_FIXTURE_TEST_CASE(XMLRequestMapper_inline_invalid_internal, XMLRequestMapperFixture)
-{
-    parse("internal-invalid2.xml");
-    BOOST_CHECK_EQUAL(tree.size(), 1);
-
-    exceptionCheck checker("Initial RequestMapper configuration was invalid.");
-    BOOST_CHECK_EXCEPTION(AgentConfig::getConfig().RequestMapperManager.newPlugin(
-        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true),
-            ConfigurationException, checker.check_message);
-}
-
 /////////////
 // Inline test to check for applicationId defaulting.
 /////////////
@@ -477,4 +523,51 @@ BOOST_FIXTURE_TEST_CASE(XMLRequestMapper_inline_Query_regex_mapping, XMLRequestM
     BOOST_CHECK(settings.first->getBool("requireSession", false));
     BOOST_CHECK_EQUAL(settings.first->getString("entityId"), "https://idp.example.org/bar");
 }
-};
\ No newline at end of file
+
+/////////////
+// External tests to check for embedded ACLs.
+/////////////
+
+BOOST_FIXTURE_TEST_CASE(XMLRequestMapper_external_ACL, XMLRequestMapperFixture)
+{
+    parse("external-with-acl.xml");
+    BOOST_CHECK_EQUAL(tree.size(), 1);
+
+    unique_ptr<RequestMapper> mapper(AgentConfig::getConfig().RequestMapperManager.newPlugin(
+        tree.front().second.get<string>("<xmlattr>.type").c_str(), tree.front().second, true));
+
+#ifdef HAVE_CXX14
+    shared_lock locker(*mapper);
+#endif
+
+    DummySession session;
+    DummyRequest request("/secure");
+    request.m_scheme = "https";
+    request.m_hostname = "sp.example.org";
+    request.m_port = 443;
+    request.m_user = "jdoe";
+
+    const RequestMapper::Settings settings = mapper->getSettings(request);
+    BOOST_CHECK(settings.second);
+#ifdef HAVE_CXX14
+    shared_lock<AccessControl> acllock(*settings.second);
+#endif
+    BOOST_CHECK_EQUAL(settings.first->getString("name"), "secure");
+    BOOST_CHECK_EQUAL(settings.second->authorized(request, &session), AccessControl::shib_acl_true);
+
+    DummyRequest request2("/secure2");
+    request2.m_scheme = "https";
+    request2.m_hostname = "sp.example.org";
+    request2.m_port = 443;
+    request2.m_user = "jsmith";
+
+    const RequestMapper::Settings settings2 = mapper->getSettings(request2);
+    BOOST_CHECK(settings2.second);
+#ifdef HAVE_CXX14
+    shared_lock<AccessControl> acllock2(*settings2.second);
+#endif
+    BOOST_CHECK_EQUAL(settings2.first->getString("name"), "sp.example.org");
+    BOOST_CHECK_EQUAL(settings2.second->authorized(request2, &session), AccessControl::shib_acl_true);
+}
+
+};

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


More information about the commits mailing list