[cpp-sp] branch main updated: Flesh out TokenConsumer handler.
Scott Cantor
cantor.2 at osu.edu
Tue Jun 17 13:38:09 UTC 2025
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=2fa0f6e41d51da568131f51d71573fa471c1f2ce
The following commit(s) were added to refs/heads/main by this push:
new 2fa0f6e4 Flesh out TokenConsumer handler.
2fa0f6e4 is described below
commit 2fa0f6e41d51da568131f51d71573fa471c1f2ce
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Jun 17 09:38:05 2025 -0400
Flesh out TokenConsumer handler.
---
.../impl/DefaultAttributeConfiguration.cpp | 9 ++-
shibsp/handler/impl/TokenConsumer.cpp | 64 +++++++++++++++-------
shibsp/session/SessionCache.h | 2 +
shibsp/session/impl/AbstractSessionCache.cpp | 21 ++++++-
.../session/impl/bogus-filesystem-shibboleth.ini | 1 -
tests/data/session/impl/filesystem-shibboleth.ini | 1 -
tests/data/session/impl/memory-shibboleth.ini | 1 -
tests/session/impl/FilesystemSessionCacheTests.cpp | 19 +++++++
8 files changed, 91 insertions(+), 27 deletions(-)
diff --git a/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp b/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
index 1465ed40..7011f780 100644
--- a/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
+++ b/shibsp/attribute/impl/DefaultAttributeConfiguration.cpp
@@ -194,9 +194,14 @@ bool DefaultAttributeConfiguration::processAttributes(DDF& attributes) const
value.destroy();
m_log.warn("attribute '%s' value was not a supported type", attr.name());
}
+
+ value = attr.next();
+ }
+
+ if (attr.integer() == 0) {
+ m_log.info("no values remain in attribute (%s) after processing", attr.name());
+ attr.destroy();
}
-
- value = attr.next();
}
attr = attributes.next();
diff --git a/shibsp/handler/impl/TokenConsumer.cpp b/shibsp/handler/impl/TokenConsumer.cpp
index 972c1b23..4f5b381f 100644
--- a/shibsp/handler/impl/TokenConsumer.cpp
+++ b/shibsp/handler/impl/TokenConsumer.cpp
@@ -25,12 +25,14 @@
#include "SPRequest.h"
#include "handler/AbstractHandler.h"
#include "logging/Category.h"
+#include "session/SessionCache.h"
#include "remoting/RemotingService.h"
#include "util/URLEncoder.h"
#include <ctime>
#include <sstream>
#include <boost/property_tree/ptree.hpp>
+#include <boost/algorithm/string.hpp>>
using namespace shibsp;
using namespace boost::property_tree;
@@ -64,12 +66,16 @@ TokenConsumer::TokenConsumer(const ptree& pt, const char* path)
pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
{
+ bool wasPassive;
+ string target;
+
try {
DDF input("token-consumer");
DDFJanitor inputJanitor(input);
input.structure();
input.addmember("application").string(
- request.getRequestSettings().first->getString("applicationId", "default"));
+ request.getRequestSettings().first->getString(
+ RequestMapper::APPLICATION_ID_PROP_NAME, RequestMapper::APPLICATION_ID_PROP_DEFAULT));
DDF wrapped = wrapRequest(request, m_remotedHeaders);
input.add(wrapped);
@@ -77,9 +83,19 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
DDF output = request.getAgent().getRemotingService()->send(input);
DDFJanitor outputJanitor(output);
- // TODO: process outbound session data
+ wasPassive = output["passive"].integer() == 1;
+ const char* s = output.getmember("http.redirect").string();
+ if (s) {
+ target = s;
+ }
+
- const char* sessionHook = request.getRequestSettings().first->getString("sessionHook");
+ SessionCache* cache = request.getAgent().getSessionCache();
+ DDF sessionData = output["session"];
+ // Ownership of sessionData transfers on input to create call.
+ cache->create(request, sessionData);
+
+ const char* sessionHook = request.getRequestSettings().first->getString(RequestMapper::SESSION_HOOK_PROP_NAME);
if (sessionHook) {
string hook(sessionHook);
request.absolutize(hook);
@@ -90,10 +106,9 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
string returnURL = request.getRequestURL();
returnURL = returnURL.substr(0, returnURL.find('?')) + "?hook=1";
- const char* target = output.getmember("http.redirect").string();
string encodedTarget;
- if (target) {
- encodedTarget = encoder.encode(target);
+ if (!target.empty()) {
+ encodedTarget = encoder.encode(target.c_str());
returnURL += "&target=" + encodedTarget;
}
if (hook.find('?') == string::npos) {
@@ -115,12 +130,9 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
return unwrapResponse(request, output);
}
- // TODO: remove POC debugging code...
- stringstream dump;
- dump << output;
- return make_pair(true,request.sendResponse(dump));
-
- //return unwrapResponse(request, output);
+ // TODO: POST restoration...
+
+ return unwrapResponse(request, output);
}
catch (exception& ex) {
AgentException* agent_ex = dynamic_cast<AgentException*>(&ex);
@@ -128,18 +140,32 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
agent_ex->addProperty("handlerType", TOKEN_CONSUMER_HANDLER);
}
+ // THis is a mess to allow for "ignoring" errors during passive SSO and routing back
+ // to the original resource.
+
+ // The passive and target values can come from the output message or the exception.
+ // When the cache throws, the error typically would not carry that information but the
+ // output would have.
+
const char* passive = agent_ex ? agent_ex->getProperty("passive") : nullptr;
- if (passive && !strcmp(passive, "1")) {
+ if (wasPassive || (passive && !strcmp(passive, "1"))) {
agent_ex->log(request, Priority::SHIB_WARN);
- const char* error_target = agent_ex->getProperty("target");
+ const char* error_target = target.empty() ? agent_ex->getProperty("target") : target.c_str();
// TODO: either recover POST data or clean up recovery state?
- // Make sure the target isn't the same as this handler, so avoid a loop.
- request.log(Priority::SHIB_INFO,
- "trapping TokenConsumer failure and returning to target location for passive request");
- request.limitRedirect(error_target);
- return make_pair(true, request.sendRedirect(error_target));
+ if (error_target) {
+ request.limitRedirect(error_target);
+ // Make sure the target isn't a prefix of this handler, to avoid a loop.
+ if (boost::starts_with(error_target, request.getRequestURL())) {
+ request.log(Priority::SHIB_WARN,
+ "TokenConsumer target location matched handler, not trapping passive request error");
+ } else {
+ request.log(Priority::SHIB_INFO,
+ "trapping TokenConsumer failure and returning to target location for passive request");
+ return make_pair(true, request.sendRedirect(error_target));
+ }
+ }
}
throw;
}
diff --git a/shibsp/session/SessionCache.h b/shibsp/session/SessionCache.h
index eb3e328c..dd57675c 100644
--- a/shibsp/session/SessionCache.h
+++ b/shibsp/session/SessionCache.h
@@ -126,6 +126,8 @@ namespace shibsp {
* <p>The second parameter's ownership is assumed by this method regardless of the
* outcome.</p>
*
+ * <p>An exception is raised in the event of an error.</p>
+ *
* @param request request to bind the session to
* @param session session data obtained from the hub
*
diff --git a/shibsp/session/impl/AbstractSessionCache.cpp b/shibsp/session/impl/AbstractSessionCache.cpp
index 37611277..9d743321 100644
--- a/shibsp/session/impl/AbstractSessionCache.cpp
+++ b/shibsp/session/impl/AbstractSessionCache.cpp
@@ -20,8 +20,10 @@
#include "internal.h"
#include "exceptions.h"
+#include "Agent.h"
#include "AgentConfig.h"
#include "SPRequest.h"
+#include "attribute/AttributeConfiguration.h"
#include "io/CookieManager.h"
#include "logging/Category.h"
#include "session/AbstractSessionCache.h"
@@ -121,9 +123,13 @@ bool AbstractSessionCache::isSessionDataValid(DDF& sessionData)
// non-null string value, and no other value types.
DDF attrs = sessionData["attributes"];
- if (!attrs.islist() || attrs.integer() == 0) {
+ if (!attrs.islist()) {
return false;
}
+ else if (attrs.integer() == 0) {
+ // Empty list.
+ return true;
+ }
DDF attr = attrs.first();
@@ -244,12 +250,21 @@ string AbstractSessionCache::create(SPRequest& request, DDF& session)
session.remove();
// Add additional fields managed by agent.
- // attributes and data members should be present from hub.
+ // The attributes member should be present from hub.
session.addmember("ts").longinteger(time(nullptr));
session.addmember("app_id").string(request.getRequestSettings().first->getString(
RequestMapper::APPLICATION_ID_PROP_NAME, RequestMapper::APPLICATION_ID_PROP_DEFAULT));
session.addmember("addr").string(request.getRemoteAddr());
+ const AttributeConfiguration& attrConfig = request.getAgent().getAttributeConfiguration(
+ request.getRequestSettings().first->getString(RequestMapper::ATTRIBUTE_CONFIG_ID_PROP_NAME));
+ DDF attrs = session["attributes"];
+ if (!attrConfig.processAttributes(attrs)) {
+ m_log.warn("error processing session attributes for storage/use");
+ session.destroy();
+ throw SessionException("Error while processing session attributes for storage.");
+ }
+
// Write the data to the back-end, obtaining a key.
string key;
try {
@@ -259,7 +274,7 @@ string AbstractSessionCache::create(SPRequest& request, DDF& session)
catch (const exception& ex) {
// Should be logged by the SPI.
session.destroy();
- return string();
+ throw;
}
session.name(key.c_str());
diff --git a/tests/data/session/impl/bogus-filesystem-shibboleth.ini b/tests/data/session/impl/bogus-filesystem-shibboleth.ini
index 0e71f6ee..2c87daf5 100644
--- a/tests/data/session/impl/bogus-filesystem-shibboleth.ini
+++ b/tests/data/session/impl/bogus-filesystem-shibboleth.ini
@@ -1,7 +1,6 @@
[global]
agentID = sp.example.org
skipHandlers = true
-skipAttributes = true
[logging]
type = console
diff --git a/tests/data/session/impl/filesystem-shibboleth.ini b/tests/data/session/impl/filesystem-shibboleth.ini
index 27a58dc1..0ca6e801 100644
--- a/tests/data/session/impl/filesystem-shibboleth.ini
+++ b/tests/data/session/impl/filesystem-shibboleth.ini
@@ -1,7 +1,6 @@
[global]
agentID = sp.example.org
skipHandlers = true
-skipAttributes = true
[logging]
type = console
diff --git a/tests/data/session/impl/memory-shibboleth.ini b/tests/data/session/impl/memory-shibboleth.ini
index 548a780f..32e52f88 100644
--- a/tests/data/session/impl/memory-shibboleth.ini
+++ b/tests/data/session/impl/memory-shibboleth.ini
@@ -1,7 +1,6 @@
[global]
agentID = sp.example.org
skipHandlers = true
-skipAttributes = true
[logging]
type = console
diff --git a/tests/session/impl/FilesystemSessionCacheTests.cpp b/tests/session/impl/FilesystemSessionCacheTests.cpp
index a161cc98..66fd86db 100644
--- a/tests/session/impl/FilesystemSessionCacheTests.cpp
+++ b/tests/session/impl/FilesystemSessionCacheTests.cpp
@@ -75,6 +75,25 @@ struct FilesystemFixture
string data_path;
};
+BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_invalid_attributes, FilesystemFixture)
+{
+ bool started = AgentConfig::getConfig().start();
+ BOOST_CHECK(started);
+
+ DDF obj(nullptr);
+ DDFJanitor janitor(obj);
+
+ obj.addmember("session.attributes"); // not a list
+
+ DummyRequest request("https://sp.example.org/secure/index.html");
+ DDF child = obj["session"];
+
+ SessionCache* cache = AgentConfig::getConfig().getAgent().getSessionCache();
+
+ exceptionCheck checker("Error while processing session attributes for storage.");
+ BOOST_CHECK_EXCEPTION(cache->create(request, child), SessionException, checker.check_message);
+}
+
BOOST_FIXTURE_TEST_CASE(FilesystemSessionCache_tests, FilesystemFixture)
{
bool started = AgentConfig::getConfig().start();
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list