[cpp-sp] branch main updated: Reapply some basic changes to get POST recovery ready.
Scott Cantor
cantor.2 at osu.edu
Tue Sep 30 13:00:29 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=86f91fbfee5305694de1eba6ee15f13fa348d213
The following commit(s) were added to refs/heads/main by this push:
new 86f91fbf Reapply some basic changes to get POST recovery ready.
86f91fbf is described below
commit 86f91fbfee5305694de1eba6ee15f13fa348d213
Author: Scott Cantor <cantor.2 at osu.edu>
AuthorDate: Tue Sep 30 09:00:25 2025 -0400
Reapply some basic changes to get POST recovery ready.
---
shibsp/AbstractSPRequest.cpp | 6 ++--
shibsp/RequestMapper.h | 2 ++
shibsp/handler/impl/AbstractHandler.cpp | 14 ++++++++-
shibsp/handler/impl/SessionInitiator.cpp | 6 ++--
shibsp/handler/impl/TokenConsumer.cpp | 54 ++++++++++++++++++++++++++------
shibsp/impl/XMLRequestMapper.cpp | 3 ++
6 files changed, 70 insertions(+), 15 deletions(-)
diff --git a/shibsp/AbstractSPRequest.cpp b/shibsp/AbstractSPRequest.cpp
index 5350a889..0587b519 100644
--- a/shibsp/AbstractSPRequest.cpp
+++ b/shibsp/AbstractSPRequest.cpp
@@ -132,8 +132,9 @@ string AbstractSPRequest::getRemoteAddr() const
const char* AbstractSPRequest::getParameter(const char* name) const
{
- if (!m_parser.get())
+ if (!m_parser) {
m_parser.reset(new CGIParser(*this));
+ }
pair<CGIParser::walker,CGIParser::walker> bounds = m_parser->getParameters(name);
return (bounds.first==bounds.second) ? nullptr : bounds.first->second;
@@ -141,8 +142,9 @@ const char* AbstractSPRequest::getParameter(const char* name) const
vector<const char*>::size_type AbstractSPRequest::getParameters(const char* name, vector<const char*>& values) const
{
- if (!m_parser.get())
+ if (!m_parser) {
m_parser.reset(new CGIParser(*this));
+ }
pair<CGIParser::walker,CGIParser::walker> bounds = m_parser->getParameters(name);
while (bounds.first != bounds.second) {
diff --git a/shibsp/RequestMapper.h b/shibsp/RequestMapper.h
index dd402380..10d46a09 100644
--- a/shibsp/RequestMapper.h
+++ b/shibsp/RequestMapper.h
@@ -57,6 +57,7 @@ namespace shibsp {
static const char HANDLER_CONFIG_ID_PROP_NAME[];
static const char LIFETIME_PROP_NAME[];
static const char PRESERVE_POST_DATA_PROP_NAME[];
+ static const char POST_LIMIT_PROP_NAME[];
static const char REDIRECT_ERRORS_PROP_NAME[];
static const char REDIRECT_TO_SSL_PROP_NAME[];
static const char REQUIRE_SESSION_PROP_NAME[];
@@ -73,6 +74,7 @@ namespace shibsp {
static bool EXPIRE_REDIRECTS_PROP_DEFAULT;
static unsigned int LIFETIME_PROP_DEFAULT;
static bool PRESERVE_POST_DATA_PROP_DEFAULT;
+ static unsigned int POST_LIMIT_PROP_DEFAULT;
static bool REQUIRE_SESSION_PROP_DEFAULT;
static unsigned int TIMEOUT_PROP_DEFAULT;
static bool USE_HEADERS_PROP_DEFAULT;
diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp
index b740d4c7..f509d42b 100644
--- a/shibsp/handler/impl/AbstractHandler.cpp
+++ b/shibsp/handler/impl/AbstractHandler.cpp
@@ -128,7 +128,19 @@ DDF AbstractHandler::wrapRequest(const SPRequest& request, const set<string>& he
in.addmember("port").integer(request.getPort());
in.addmember("content_type").string(request.getContentType().c_str());
if (sendBody) {
- in.addmember("body").unsafe_string(request.getRequestBody());
+ if (request.getContentType().find("application/x-www-form-urlencoded") != string::npos) {
+ unsigned int postLimit = getUnsignedInt(RequestMapper::POST_LIMIT_PROP_NAME, request,
+ RequestMapper::POST_LIMIT_PROP_DEFAULT, HANDLER_PROPERTY_FIXED | HANDLER_PROPERTY_MAP);
+ if (postLimit == 0 || request.getContentLength() <= postLimit) {
+ in.addmember("body").unsafe_string(request.getRequestBody());
+ }
+ else {
+ request.warn("POST limit exceeded, ignoring posted data");
+ }
+ }
+ else {
+ request.warn("Content type not supported, ignoring posted data");
+ }
}
in.addmember("content_length").longinteger(request.getContentLength());
in.addmember("remote_user").string(request.getRemoteUser().c_str());
diff --git a/shibsp/handler/impl/SessionInitiator.cpp b/shibsp/handler/impl/SessionInitiator.cpp
index 1646ee5d..99277f1c 100644
--- a/shibsp/handler/impl/SessionInitiator.cpp
+++ b/shibsp/handler/impl/SessionInitiator.cpp
@@ -177,8 +177,10 @@ pair<bool,long> SessionInitiator::run(SPRequest& request, bool isHandler) const
input.add(dup);
DDF wrapped = wrapRequest(request, m_remotedHeaders,
- !isHandler &&
- getBool("preservePostData", request, false, HANDLER_PROPERTY_FIXED | HANDLER_PROPERTY_MAP));
+ !isHandler && getBool(RequestMapper::PRESERVE_POST_DATA_PROP_NAME,
+ request,
+ RequestMapper::PRESERVE_POST_DATA_PROP_DEFAULT,
+ HANDLER_PROPERTY_FIXED | HANDLER_PROPERTY_MAP));
input.add(wrapped);
for (const string& propname : m_requestMapperSettings) {
diff --git a/shibsp/handler/impl/TokenConsumer.cpp b/shibsp/handler/impl/TokenConsumer.cpp
index 1d287bf3..2f1b47da 100644
--- a/shibsp/handler/impl/TokenConsumer.cpp
+++ b/shibsp/handler/impl/TokenConsumer.cpp
@@ -27,6 +27,7 @@
#include "logging/Category.h"
#include "session/SessionCache.h"
#include "remoting/RemotingService.h"
+#include "util/CGIParser.h"
#include "util/Misc.h"
#include "util/URLEncoder.h"
@@ -73,10 +74,31 @@ TokenConsumer::TokenConsumer(const ptree& pt, const char* path)
pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
{
- // TODO: check for session hook return to break loop.
-
string target;
+ // Check for a message back to the handler from a session hook.
+ if (request.getQueryString() && strstr(request.getQueryString(), "shibsp_hook=1")) {
+ // Parse the query string only, to preserve any POST data in case this is
+ // *not* a hook roundtrip but an actual token response that has that parameter
+ // for whatever odd reason.
+ CGIParser cgi(request, true);
+ pair<CGIParser::walker,CGIParser::walker> param = cgi.getParameters("shibsp_hook");
+ if (param.first != param.second && param.first->second && !strcmp(param.first->second, "1")) {
+ // This is a hook return, so we extract the target parameter and redirect to it.
+ param = cgi.getParameters("target");
+ if (param.first != param.second && param.first->second) {
+ target = param.first->second;
+ }
+ else {
+ target = getString("homeURL", request, "/", HANDLER_PROPERTY_FIXED | HANDLER_PROPERTY_MAP);
+ }
+ request.limitRedirect(target.c_str());
+ return make_pair(true, request.sendRedirect(target.c_str()));
+ }
+ }
+
+ // Not a hook response, so process as a token-consumer operation.
+
try {
DDF input("token-consumer");
DDFJanitor inputJanitor(input);
@@ -95,14 +117,28 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
if (s) {
target = s;
}
+ else if (!output.getmember("http.response.data").string()) {
+ // Shouldn't happen, but we can route ourselves to homeURL or /
+ target = getString("homeURL", request, "/", HANDLER_PROPERTY_FIXED | HANDLER_PROPERTY_MAP);
+ output.addmember("http.redirect").unsafe_string(target.c_str());
+ }
+
+ // If target is still empty, then this is a POST recovery attempt with the reesource
+ // buried in the form action.
SessionCache* cache = request.getAgent().getSessionCache();
DDF sessionData = output["session"];
- // Ownership of sessionData transfers on input to create call.
+ // Ownership of sessionData transfers on input to create call (will be detached from output).
cache->create(request, sessionData);
const char* sessionHook = request.getRequestSettings().first->getString(RequestMapper::SESSION_HOOK_PROP_NAME);
+
+ if (target.empty() && sessionHook) {
+ request.warn("response contained recovered POST data, ignoring configured sessionHook");
+ sessionHook = nullptr;
+ }
+
if (sessionHook) {
string hook(sessionHook);
request.absolutize(hook);
@@ -111,7 +147,7 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
// The target also must be included.
const URLEncoder& encoder = AgentConfig::getConfig().getURLEncoder();
string returnURL = request.getRequestURL();
- returnURL = returnURL.substr(0, returnURL.find('?')) + "?hook=1";
+ returnURL = returnURL.substr(0, returnURL.find('?')) + "?shibsp_hook=1";
string encodedTarget;
if (!target.empty()) {
@@ -134,11 +170,9 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
// Overrwrite the original redirection target and issue.
// This is necessary to ensure any Set-Cookie headers placed by the hub will reach the client.
output.addmember("http.redirect").unsafe_string(hook.c_str());
- return unwrapResponse(request, output);
}
- // TODO: POST restoration...
-
+ // Handles all normal cases, including POST recovery.
return unwrapResponse(request, output);
}
catch (exception& ex) {
@@ -148,14 +182,14 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
}
// This is a mess to allow for "ignoring" errors during passive SSO and routing back
- // to the original resource.
+ // to the original resource. Notably, we do NOT handle POST recovery here, even in the
+ // passive case, because passive SSO doesn't make any sense together with POST recovery.
+ // Passive implies requireSession is off, and POST recovery implies it's on.
const char* event = agent_ex ? agent_ex->getProperty(AgentException::EVENT_PROP_NAME) : nullptr;
if (event && !strcmp(event, "NoPassive")) {
const char* error_target = target.empty() ? agent_ex->getProperty(AgentException::TARGET_PROP_NAME) : target.c_str();
- // TODO: either recover POST data or clean up recovery state?
-
if (error_target) {
agent_ex->log(request, Priority::SHIB_WARN);
request.limitRedirect(error_target);
diff --git a/shibsp/impl/XMLRequestMapper.cpp b/shibsp/impl/XMLRequestMapper.cpp
index 8beb7f47..e0bdb08f 100644
--- a/shibsp/impl/XMLRequestMapper.cpp
+++ b/shibsp/impl/XMLRequestMapper.cpp
@@ -176,6 +176,7 @@ const char RequestMapper::HANDLER_CONFIG_ID_PROP_NAME[] = "handlerConfigId";
const char RequestMapper::EXPIRE_REDIRECTS_PROP_NAME[] = "expireRedirects";
const char RequestMapper::LIFETIME_PROP_NAME[] = "lifetime";
const char RequestMapper::PRESERVE_POST_DATA_PROP_NAME[] = "preservePostData";
+const char RequestMapper::POST_LIMIT_PROP_NAME[] = "postLimit";
const char RequestMapper::REDIRECT_ERRORS_PROP_NAME[] = "redirectErrors";
const char RequestMapper::REDIRECT_TO_SSL_PROP_NAME[] = "redirectToSSL";
const char RequestMapper::REQUIRE_LOGOUT_WITH_PROP_NAME[] = "requireLogoutWith";
@@ -191,6 +192,8 @@ const char RequestMapper::APPLICATION_ID_PROP_DEFAULT[] = "default";
bool RequestMapper::CONSISTENT_ADDRESS_PROP_DEFAULT = true;
bool RequestMapper::EXPIRE_REDIRECTS_PROP_DEFAULT = true;
unsigned int RequestMapper::LIFETIME_PROP_DEFAULT = 3600 * 8;
+bool RequestMapper::PRESERVE_POST_DATA_PROP_DEFAULT = false;
+unsigned int RequestMapper::POST_LIMIT_PROP_DEFAULT = 1024 * 1024;
bool RequestMapper::REQUIRE_SESSION_PROP_DEFAULT = false;
unsigned int RequestMapper::TIMEOUT_PROP_DEFAULT = 3600;
bool RequestMapper::USE_HEADERS_PROP_DEFAULT = false;
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list