[cpp-sp] branch main updated: Move extension library handling to outer config class.

Scott Cantor cantor.2 at osu.edu
Mon Dec 30 19:44:36 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=16c8dc769f03c6b765b9a5c21bc73e8d576e4c96

The following commit(s) were added to refs/heads/main by this push:
     new 16c8dc76 Move extension library handling to outer config class.
16c8dc76 is described below

commit 16c8dc769f03c6b765b9a5c21bc73e8d576e4c96
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Mon Dec 30 14:44:27 2024 -0500

    Move extension library handling to outer config class.
---
 shibsp/Agent.cpp                                   |   3 +
 shibsp/Agent.h                                     |   4 +-
 shibsp/impl/AgentConfig.cpp                        | 132 +++++++++++++--------
 shibsp/impl/DefaultAgent.cpp                       |  41 -------
 tests/AgentConfigTests.cpp                         |  24 +++-
 tests/data/console-shibboleth.ini                  |   4 +
 ...le-shibboleth.ini => fatal-exts-shibboleth.ini} |   3 +
 ...shibboleth.ini => nonfatal-exts-shibboleth.ini} |   3 +
 8 files changed, 117 insertions(+), 97 deletions(-)

diff --git a/shibsp/Agent.cpp b/shibsp/Agent.cpp
index ce17dad3..54e5d327 100644
--- a/shibsp/Agent.cpp
+++ b/shibsp/Agent.cpp
@@ -51,6 +51,9 @@ Agent::~Agent()
 {
 }
 
+// TODO: we'll eventually copy/port in substantially similar versions of the old ServiceProvider
+// method impls.
+
 pair<bool,long> Agent::doAuthentication(AgentRequest& request, bool handler) const
 {
 }
diff --git a/shibsp/Agent.h b/shibsp/Agent.h
index 3175b4c1..5bc2fac3 100644
--- a/shibsp/Agent.h
+++ b/shibsp/Agent.h
@@ -57,8 +57,8 @@ namespace shibsp {
          * Loads a configuration and prepares the instance for use.
          * 
          * <p>Implemented as a separate method so that services can rely on
-         * other services while they initialize by accessing the ServiceProvider
-         * from the SPConfig singleton.
+         * other services while they initialize by accessing the Agent
+         * from the AgentConfig singleton.
          */
         virtual void init()=0;
 
diff --git a/shibsp/impl/AgentConfig.cpp b/shibsp/impl/AgentConfig.cpp
index 75ac5f52..0ff85f30 100644
--- a/shibsp/impl/AgentConfig.cpp
+++ b/shibsp/impl/AgentConfig.cpp
@@ -27,6 +27,7 @@
 #include "AgentConfig.h"
 #include "RequestMapper.h"
 #include "logging/LoggingService.h"
+#include "util/Misc.h"
 #include "util/PathResolver.h"
 #include "util/URLEncoder.h"
 
@@ -61,7 +62,6 @@ namespace shibsp {
 
         bool init(const char* inst_prefix=nullptr, const char* config_file=nullptr, bool rethrow=false);
         void term();
-        bool load_library(const char* path, void* context=nullptr);
 
         const PathResolver& getPathResolver() const {
             return m_pathResolver;
@@ -79,6 +79,8 @@ namespace shibsp {
         void _term();
 
         bool initLogging();
+        bool load_library(const char* path, void* context=nullptr);
+        void loadExtensions(Category& log);
 
         unsigned int m_initCount;
         mutex m_lock;
@@ -196,57 +198,60 @@ bool AgentInternalConfig::_init(const char* inst_prefix, const char* config_file
 
     try {
         ini_parser::read_ini(config_file_resolved, m_config);
-    } catch (const ini_parser_error& e) {
-        if (rethrow) {
-            throw;
-        }
-        return false;
-    }
 
-    registerLoggingServices();
+        registerLoggingServices();
 
-    try {
         if (!initLogging()) {
             return false;
         }
-    } catch (const std::exception& e) {
-        if (rethrow) {
-            throw;
-        }
-        return false;
-    }
 
-    // At this point, logging is active/usable.
+        // At this point, logging is active/usable.
 
-    Category& log=Category::getInstance(SHIBSP_LOGCAT ".AgentConfig");
-    log.info("%s agent initialization underway", PACKAGE_STRING);
+        Category& log=Category::getInstance(SHIBSP_LOGCAT ".AgentConfig");
+        log.info("%s agent initialization started", PACKAGE_STRING);
 
-    registerAccessControls();
-    registerRequestMappers();
+        registerAccessControls();
+        registerRequestMappers();
 
-    /*
-    XMLToolingConfig::getConfig().user_agent = string(PACKAGE_NAME) + '/' + PACKAGE_VERSION;
+        /*
+        XMLToolingConfig::getConfig().user_agent = string(PACKAGE_NAME) + '/' + PACKAGE_VERSION;
 
-    registerAttributeFactories();
+        registerAttributeFactories();
 
-    registerHandlers();
-    registerLogoutInitiators();
-    registerSessionInitiators();
-    */
+        registerHandlers();
+        registerLogoutInitiators();
+        registerSessionInitiators();
+        */
 
-    registerAgents();
+        registerAgents();
 
-    /*
-    registerListenerServices();
+        /*
+        registerListenerServices();
 
-    registerSessionCaches();
+        registerSessionCaches();
 
-    // Yes, this isn't secure, will review where we do any random generation
-    // after full code cleanup is done.
-    srand(static_cast<unsigned int>(std::time(nullptr)));
+        // Yes, this isn't secure, will review where we do any random generation
+        // after full code cleanup is done.
+        srand(static_cast<unsigned int>(std::time(nullptr)));
+        */
+
+        loadExtensions(log);
+
+        // Check for an overridden "agent-type" under the "global" subtree.
+        static const char AGENT_TYPE_PROP_PATH[] = "global.agent-type";
+        string type = m_config.get(AGENT_TYPE_PROP_PATH, DEFAULT_AGENT);
+        m_agent.reset(AgentManager.newPlugin(type, m_config, true));
+        m_agent->init();
+
+        log.info("%s agent initialization complete", PACKAGE_STRING);
+    }
+    catch (const std::exception& e) {
+        if (rethrow) {
+            throw;
+        }
+        return false;
+    }
 
-    log.info("%s library initialization complete", PACKAGE_STRING);
-    */
     return true;
 }
 
@@ -260,7 +265,7 @@ bool AgentInternalConfig::initLogging()
         SYSLOG_LOGGING_SERVICE
 #endif
         );
-    m_logging = unique_ptr<LoggingService>(LoggingServiceManager.newPlugin(type, m_config, false));
+    m_logging.reset(LoggingServiceManager.newPlugin(type, m_config, false));
     if (!m_logging->init()) {
         return false;
     }
@@ -272,7 +277,7 @@ void AgentInternalConfig::term()
     lock_guard<mutex> locker(m_lock);
 
     if (m_initCount == 0) {
-        throw runtime_error("Library terminated without initialization.");
+        throw runtime_error("Agent library terminated without initialization.");
         return;
     }
     else if (--m_initCount > 0) {
@@ -313,10 +318,8 @@ void AgentInternalConfig::_term()
     m_logging->term();
 
     /*
-    AssertionConsumerServiceManager.deregisterFactories();
     LogoutInitiatorManager.deregisterFactories();
     SessionInitiatorManager.deregisterFactories();
-    SingleLogoutServiceManager.deregisterFactories();
     HandlerManager.deregisterFactories();
     */
 
@@ -329,6 +332,41 @@ void AgentInternalConfig::_term()
     */
 }
 
+void AgentInternalConfig::loadExtensions(Category& log)
+{
+    static const char EXTENSIONS_PATH[] = "extensions";
+    const boost::optional<ptree&> exts = m_config.get_child_optional(EXTENSIONS_PATH);
+    if (!exts) {
+        return;
+    }
+
+    for (const auto& path : exts.get()) {
+        if (path.first.empty()) {
+            continue;
+        }
+
+        cout << path.first << endl;
+        
+        try {
+            if (!load_library(path.first.c_str(), const_cast<ptree*>(&path.second))) {
+                throw ConfigurationException("Extension library failed to load.");
+            }
+            log.debug("loaded extension library (%s)", path.first.c_str());
+        }
+        catch (const std::exception& e) {
+            // The value of the subtree dictates whether failure is fatal.
+            string_to_bool_translator tr;
+            if (path.second.get_value(false, tr)) {
+                log.crit("unable to load mandatory extension library %s: %s", path.first.c_str(), e.what());
+                throw;
+            }
+            else {
+                log.crit("unable to load optional extension library %s: %s", path.first.c_str(), e.what());
+            }
+        }        
+    }
+}
+
 bool AgentInternalConfig::load_library(const char* path, void* context)
 {
 #ifdef _DEBUG
@@ -337,8 +375,6 @@ bool AgentInternalConfig::load_library(const char* path, void* context)
     Category& log=Category::getInstance(SHIBSP_LOGCAT ".Config");
     log.info("loading extension: %s", path);
 
-    lock_guard<mutex> locker(m_lock);
-
     string resolved(path);
     m_pathResolver.resolve(resolved, PathResolver::SHIBSP_LIB_FILE);
 
@@ -354,12 +390,12 @@ bool AgentInternalConfig::load_library(const char* path, void* context)
         if (!handle)
              handle=LoadLibraryExA(resolved.c_str(),nullptr,0);
         if (!handle)
-            throw runtime_error(string("unable to load extension library: ") + resolved);
+            throw runtime_error(string("Unable to load extension library: ") + resolved);
         FARPROC fn=GetProcAddress(handle,"shibsp_extension_init");
         if (!fn)
-            throw runtime_error(string("unable to locate shibsp_extension_init entry point: ") + resolved);
+            throw runtime_error(string("Unable to locate shibsp_extension_init entry point: ") + resolved);
         if (reinterpret_cast<int(*)(void*)>(fn)(context)!=0)
-            throw runtime_error(string("detected error in shibsp_extension_init: ") + resolved);
+            throw runtime_error(string("Detected error in shibsp_extension_init: ") + resolved);
         SetErrorMode(em);
     }
     catch(std::exception&) {
@@ -371,18 +407,18 @@ bool AgentInternalConfig::load_library(const char* path, void* context)
 #elif defined(HAVE_DLFCN_H)
     void* handle=dlopen(resolved.c_str(),RTLD_LAZY);
     if (!handle)
-        throw runtime_error(string("unable to load extension library '") + resolved + "': " + dlerror());
+        throw runtime_error(dlerror());
     int (*fn)(void*)=(int (*)(void*))(dlsym(handle,"shibsp_extension_init"));
     if (!fn) {
         dlclose(handle);
         throw runtime_error(
-            string("unable to locate shibsp_extension_init entry point in '") + resolved + "': " +
+            string("Unable to locate shibsp_extension_init entry point in '") + resolved + "': " +
                 (dlerror() ? dlerror() : "unknown error")
             );
     }
     try {
         if (fn(context)!=0)
-            throw runtime_error(string("detected error in shibsp_extension_init in ") + resolved);
+            throw runtime_error(string("Detected error in shibsp_extension_init in ") + resolved);
     }
     catch(std::exception&) {
         if (handle)
diff --git a/shibsp/impl/DefaultAgent.cpp b/shibsp/impl/DefaultAgent.cpp
index 16555283..c145ae02 100644
--- a/shibsp/impl/DefaultAgent.cpp
+++ b/shibsp/impl/DefaultAgent.cpp
@@ -80,7 +80,6 @@ namespace {
         }
 
     private:
-        void doExtensions(const ptree&);
         //void doListener(const xercesc::DOMElement*);
         //void doCaching(const xercesc::DOMElement*);
 
@@ -100,8 +99,6 @@ namespace {
 #endif
 
     static const XMLCh applicationId[] =        UNICODE_LITERAL_13(a,p,p,l,i,c,a,t,i,o,n,I,d);
-    static const XMLCh _ArtifactMap[] =         UNICODE_LITERAL_11(A,r,t,i,f,a,c,t,M,a,p);
-    static const XMLCh _DataSealer[] =          UNICODE_LITERAL_10(D,a,t,a,S,e,a,l,e,r);
     static const XMLCh _default[] =             UNICODE_LITERAL_7(d,e,f,a,u,l,t);
     static const XMLCh _Extensions[] =          UNICODE_LITERAL_10(E,x,t,e,n,s,i,o,n,s);
     static const XMLCh _fatal[] =               UNICODE_LITERAL_5(f,a,t,a,l);
@@ -113,20 +110,12 @@ namespace {
     static const XMLCh _option[] =              UNICODE_LITERAL_6(o,p,t,i,o,n);
     static const XMLCh OutOfProcess[] =         UNICODE_LITERAL_12(O,u,t,O,f,P,r,o,c,e,s,s);
     static const XMLCh _path[] =                UNICODE_LITERAL_4(p,a,t,h);
-    static const XMLCh _ProtocolProvider[] =    UNICODE_LITERAL_16(P,r,o,t,o,c,o,l,P,r,o,v,i,d,e,r);
     static const XMLCh _provider[] =            UNICODE_LITERAL_8(p,r,o,v,i,d,e,r);
-    static const XMLCh _ReplayCache[] =         UNICODE_LITERAL_11(R,e,p,l,a,y,C,a,c,h,e);
     static const XMLCh _RequestMapper[] =       UNICODE_LITERAL_13(R,e,q,u,e,s,t,M,a,p,p,e,r);
     static const XMLCh RequestMap[] =           UNICODE_LITERAL_10(R,e,q,u,e,s,t,M,a,p);
-    static const XMLCh SecurityPolicies[] =     UNICODE_LITERAL_16(S,e,c,u,r,i,t,y,P,o,l,i,c,i,e,s);
-    static const XMLCh _SecurityPolicyProvider[] = UNICODE_LITERAL_22(S,e,c,u,r,i,t,y,P,o,l,i,c,y,P,r,o,v,i,d,e,r);
     static const XMLCh _SessionCache[] =        UNICODE_LITERAL_12(S,e,s,s,i,o,n,C,a,c,h,e);
     static const XMLCh Site[] =                 UNICODE_LITERAL_4(S,i,t,e);
-    static const XMLCh _StorageService[] =      UNICODE_LITERAL_14(S,t,o,r,a,g,e,S,e,r,v,i,c,e);
     static const XMLCh TCPListener[] =          UNICODE_LITERAL_11(T,C,P,L,i,s,t,e,n,e,r);
-    static const XMLCh tranLogFiller[] =        UNICODE_LITERAL_13(t,r,a,n,L,o,g,F,i,l,l,e,r);
-    static const XMLCh tranLogFormat[] =        UNICODE_LITERAL_13(t,r,a,n,L,o,g,F,o,r,m,a,t);
-    static const XMLCh TransportOption[] =      UNICODE_LITERAL_15(T,r,a,n,s,p,o,r,t,O,p,t,i,o,n);
     static const XMLCh _type[] =                UNICODE_LITERAL_4(t,y,p,e);
     static const XMLCh UnixListener[] =         UNICODE_LITERAL_12(U,n,i,x,L,i,s,t,e,n,e,r);
 
@@ -304,36 +293,6 @@ void DefaultAgent::init()
     */
 }
 
-void DefaultAgent::doExtensions(const ptree& pt)
-{
-    /*
-    const DOMElement* exts = XMLHelper::getFirstChildElement(e, _Extensions);
-    if (exts) {
-        exts = XMLHelper::getFirstChildElement(exts, Library);
-        while (exts) {
-            string path(XMLHelper::getAttrString(exts, nullptr, _path));
-            try {
-                if (!path.empty()) {
-                    if (!XMLToolingConfig::getConfig().load_library(path.c_str(), (void*)exts))
-                        throw ConfigurationException("XMLToolingConfig::load_library failed.");
-                    log.debug("loaded %s extension library (%s)", label, path.c_str());
-                }
-            }
-            catch (const std::exception& e) {
-                if (XMLHelper::getAttrBool(exts, false, _fatal)) {
-                    log.crit("unable to load mandatory %s extension library %s: %s", label, path.c_str(), e.what());
-                    throw;
-                }
-                else {
-                    log.crit("unable to load optional %s extension library %s: %s", label, path.c_str(), e.what());
-                }
-            }
-            exts = XMLHelper::getNextSiblingElement(exts, Library);
-        }
-    }
-    */
-}
-
 /*
 void XMLConfigImpl::doListener(const DOMElement* e, XMLConfig* conf, Category& log)
 {
diff --git a/tests/AgentConfigTests.cpp b/tests/AgentConfigTests.cpp
index 547b3b14..3388f2ce 100644
--- a/tests/AgentConfigTests.cpp
+++ b/tests/AgentConfigTests.cpp
@@ -18,13 +18,12 @@
  * Unit tests for agent config machinery and logging.
  */
 
-#include <stdexcept>
+#include "AgentConfig.h"
 
+#include <stdexcept>
 #include <boost/test/unit_test.hpp>
 #include <boost/property_tree/ini_parser.hpp>
 
-#include "AgentConfig.h"
-
 using namespace boost::property_tree::ini_parser;
 using namespace shibsp;
 using namespace std;
@@ -41,8 +40,7 @@ class exceptionCheck {
 public:
     exceptionCheck(const string& msg) : m_msg(msg) {}
     bool check_message(const exception& e) {
-        cout << e.what() << endl;
-        return m_msg.compare(e.what()) == 0;
+        return strstr(e.what(), m_msg.c_str()) != nullptr;
     }
 private:
     string m_msg;
@@ -82,7 +80,7 @@ BOOST_FIXTURE_TEST_CASE(AgentConfig_init_bad_format, AC_Fixture)
 
 BOOST_AUTO_TEST_CASE(AgentConfig_term_without_init)
 {
-    exceptionCheck checker_term("Library terminated without initialization.");
+    exceptionCheck checker_term("Agent library terminated without initialization.");
     BOOST_CHECK_EXCEPTION(AgentConfig::getConfig().term(),
         runtime_error, checker_term.check_message);
 }
@@ -98,3 +96,17 @@ BOOST_FIXTURE_TEST_CASE(AgentConfig_init_syslog, AC_Fixture)
     BOOST_CHECK(AgentConfig::getConfig().init(nullptr, (data_path + "syslog-shibboleth.ini").c_str(), true));
     AgentConfig::getConfig().term();
 }
+
+BOOST_FIXTURE_TEST_CASE(AgentConfig_init_fatal_exts, AC_Fixture)
+{
+    // TODO: this is probably going to vary by platform and require alternate validation.
+    exceptionCheck checker_fatal("dlopen(/path/to/extension.so, 0x0001): tried: ");
+    BOOST_CHECK_EXCEPTION(AgentConfig::getConfig().init(nullptr, (data_path + "fatal-exts-shibboleth.ini").c_str(), true),
+        runtime_error, checker_fatal.check_message);
+}
+
+BOOST_FIXTURE_TEST_CASE(AgentConfig_init_nonfatal_exts, AC_Fixture)
+{
+    BOOST_CHECK(AgentConfig::getConfig().init(nullptr, (data_path + "nonfatal-exts-shibboleth.ini").c_str(), true));
+    AgentConfig::getConfig().term();
+}
diff --git a/tests/data/console-shibboleth.ini b/tests/data/console-shibboleth.ini
index c8471aa8..b0c2fb47 100644
--- a/tests/data/console-shibboleth.ini
+++ b/tests/data/console-shibboleth.ini
@@ -1,3 +1,7 @@
+[global]
+# Use "partial" for partial matching
+regex-matching = full
+
 [logging]
 type = console
 default-level = WARN
diff --git a/tests/data/console-shibboleth.ini b/tests/data/fatal-exts-shibboleth.ini
similarity index 69%
copy from tests/data/console-shibboleth.ini
copy to tests/data/fatal-exts-shibboleth.ini
index c8471aa8..6eb50679 100644
--- a/tests/data/console-shibboleth.ini
+++ b/tests/data/fatal-exts-shibboleth.ini
@@ -1,3 +1,6 @@
+[extensions]
+/path/to/extension.so = true
+
 [logging]
 type = console
 default-level = WARN
diff --git a/tests/data/console-shibboleth.ini b/tests/data/nonfatal-exts-shibboleth.ini
similarity index 69%
copy from tests/data/console-shibboleth.ini
copy to tests/data/nonfatal-exts-shibboleth.ini
index c8471aa8..121af6fc 100644
--- a/tests/data/console-shibboleth.ini
+++ b/tests/data/nonfatal-exts-shibboleth.ini
@@ -1,3 +1,6 @@
+[extensions]
+/path/to/extension.so = false
+
 [logging]
 type = console
 default-level = WARN

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


More information about the commits mailing list