[cpp-sp] branch main updated: Add proper locking to Attribute Checker.

[Scott Cantor]

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:
https://git.shibboleth.net/view/?p=cpp-sp.git;a=commit;h=347362e9594f218a2247c4d348334f2506b93ff6

The following commit(s) were added to refs/heads/main by this push:
     new 347362e9 Add proper locking to Attribute Checker.
347362e9 is described below

commit 347362e9594f218a2247c4d348334f2506b93ff6
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Thu Nov 6 09:44:04 2025 -0500

    Add proper locking to Attribute Checker.
---
 shibsp/handler/impl/AttributeCheckerHandler.cpp | 29 ++++++++++++++++---------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/shibsp/handler/impl/AttributeCheckerHandler.cpp b/shibsp/handler/impl/AttributeCheckerHandler.cpp
index fde045ba..877d9f42 100644
--- a/shibsp/handler/impl/AttributeCheckerHandler.cpp
+++ b/shibsp/handler/impl/AttributeCheckerHandler.cpp
@@ -31,7 +31,11 @@
 
 #include <algorithm>
 #include <memory>
-#include <mutex>
+#include <set>
+#ifdef HAVE_CXX14
+# include <shared_mutex>
+#endif
+#include <string>
 
 using namespace shibsp;
 using namespace boost::property_tree;
@@ -57,13 +61,13 @@ namespace shibsp {
             try {
                 request.getAgent().getSessionCache()->remove(request);
             }
-            catch (const std::exception&) {
+            catch (const exception&) {
             }
         }
 
         string m_redirectOnFailure;
         bool m_flushSession;
-        vector<string> m_attributes;
+        set<string> m_attributes;
         unique_ptr<AccessControl> m_acl;
     };
 
@@ -80,16 +84,18 @@ namespace shibsp {
 AttributeCheckerHandler::AttributeCheckerHandler(ptree& pt) : AbstractHandler(pt)
 {
     m_redirectOnFailure = getString("redirectOnFailure", "");
-    if (m_redirectOnFailure.empty())
+    if (m_redirectOnFailure.empty()) {
         throw ConfigurationException("AttributeChecker missing required redirectOnFailure setting.");
+    }
 
     m_flushSession = getBool("flushSession", false);
 
     const char* attrs = getString("attributes", "");
     if (attrs) {
         split_to_container(m_attributes, attrs);
-        if (m_attributes.empty())
+        if (m_attributes.empty()) {
             throw ConfigurationException("AttributeChecker unable to parse attributes setting.");
+        }
     }
     else if (hasProperty("path")) {
         Category::getInstance(SHIBSP_LOGCAT ".Handler.AttributeChecker").debug("attempting installation of external AccessControl rule");
@@ -104,9 +110,8 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
 {
     // If the checking passes, we route to the return URL, target URL, or homeURL in that order.
     const char* returnURL = request.getParameter("return");
-    const char* target = request.getParameter("target");
     if (!returnURL) {
-        returnURL = target;
+        returnURL = request.getParameter("target");
     }
     if (returnURL) {
         request.limitRedirect(returnURL);
@@ -118,10 +123,11 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
     unique_lock<Session> session;
     try {
         session = request.getSession();
-        if (!session)
+        if (!session) {
             request.warn("AttributeChecker found session unavailable immediately after creation");
+        }
     }
-    catch (const std::exception& ex) {
+    catch (const exception& ex) {
         request.warn(string("AttributeChecker caught exception accessing session immediately after creation: ") + ex.what());
     }
 
@@ -138,7 +144,10 @@ pair<bool,long> AttributeCheckerHandler::run(SPRequest& request, bool isHandler)
             // If that fails, the check succeeds.
             checked = find_if(m_attributes.begin(), m_attributes.end(), absent) == m_attributes.end();
         }
-        else {
+        else if (m_acl) {
+#ifdef HAVE_CXX14
+            shared_lock<AccessControl> acllock(*m_acl);
+#endif
             checked = (m_acl && m_acl->authorized(request, session.mutex()) == AccessControl::shib_acl_true);
         }
     }

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