relayState should be cleaned to prevent bogus redirects

Description

I started to notice errors like this:

http://blogs.sapo.pt/Shibboleth.sso/SAML/cookie
Shibboleth handler invoked at an unconfigured location.

After some detective work it seems that some of our users have bookmarked the IDP login URL with TARGET=cookie. What this means is that after they login and the Artifact is resolved by the SP, the cookie that should be present at the SP to send the browser on their way is not there. The logic at shibsp/handler/impl/AbstractHandler.cpp:recoverRelayState() should clear invalid values otherwise people will be redirected to invalid URLs. slightly smiling face

Maybe this will suffice:

// Look for StorageService-backed state of the form "ss:SSID:key".
const char* state = relayState.c_str();
+ relayState.erase();

Environment

None

Activity

Scott Cantor 
June 23, 2009 at 12:46 PM

Closing after releases.

André Cruz 
August 18, 2008 at 10:10 AM

Great. Thanks.

I'm still using IDP 1.3.3 (and I think I will for some time) but I wouldn't want to display an error page.

A redirect to the SP HomeURL is perfect.

Scott Cantor 
August 18, 2008 at 9:59 AM

This came up because the cookie-based state mechanism changed to fix a problem with frames, and the RelayState value is now cookie:key instead of just cookie. I didn't code for that and handle the old case. I can fix that, but by the time it shows up, I expect most of the bookmarks will be gone anyway.

Scott Cantor 
August 18, 2008 at 9:46 AM

If there are cases where it's not being cleared, that may be intentional, because it generally does get cleared unless it can't be for some reason. It should never be redirecting to "cookie" anyway. I think it's misinterpreting the value in some cases, that's probably the real bug.

This really needs to be prevented by the IdP anyway. That's why we supply a time parameter. The old IdP has this feature built-in, and can block stale requests if you tell it to, and display a nice error message. I don't know if the new IdP can, in which case that would be a bug there I think.

Your fix won't work, though, it cannot be cleared in every case.

Fixed

Details

Assignee

Reporter

Fix versions

Affects versions

Created August 18, 2008 at 9:34 AM
Updated June 23, 2009 at 12:46 PM
Resolved August 18, 2008 at 10:04 AM