NullReferenceException if request state cookie not passed to logout command

[explunit]

If the Kentor.<randomvalue> cookie is lost somewhere in the logout process (e.g. redirected via different domain), a NullReferenceException is thrown during the processing of LogoutResponse from Idp:

[NullReferenceException: Object reference not set to an instance of an object.]
Kentor.AuthServices.WebSso.LogoutCommand.HandleResponse(UnbindResult unbindResult, HttpRequestData request) +66
Kentor.AuthServices.WebSso.LogoutCommand.Run(HttpRequestData request, String returnPath, IOptions options) +190

This is because StoredRequestState is null in this line of code:

            return new CommandResult()
            {
                HttpStatusCode = HttpStatusCode.SeeOther,
                Location = request.StoredRequestState.ReturnUrl,
                ClearCookieName = "Kentor." + unbindResult.RelayState
            };

At minimum, I think we should throw a more informative exception, but I'm wondering if we can fall back to the returnPath parameter in this case? I see the note that it's only used during the start of SP-initiated logout, but I can't think of a reason why it couldn't also be used as a fallback location if the cookie does not contain it.

[AndersAbel]

Can you explain the "redirect via different domain" scenario? Usually losing the Kentor.<randomvalue> cookie indicates in incorrect setup. I just want to understand in what scenarios it would be reasonable to recover. Maybe this should be a compatibility setting to make the user aware of the cookie being lost. Then a better exception message (which we should have) could mention that there is a compatibility setting that can be used.

[explunit]

One instance of my application can be accessed over multiple public hostnames. All of these hostnames are published in the metadata as ACS urls. For the login flow, I am able to specify the return url such that it returns to the same hostname, but (if I am reading the spec correctly), there is no such option with the LogoutRequest flow. So the Idp simply returns to whatever is listed first in the metadata.

[AndersAbel]

I see, so what you really have is a situation where you know that the cookie will be lost (the sign out is initiated at abc.com and the logout response is sent to def.com).

I think that it would make more sense to add a compatibility setting "DisableLogoutStateCookie" that completely removes the cookie so that it is never set.

LogoutCommand.HandleResponse is just using the ReturnUrl content of the StoredRequestState anyways (the rest of the data that is set in the StoredRequestState in InitiateLogout could be removed).

Another option would be to send the ReturnUrl directly in the RelayState param. For short URLs that would work and if the redirect binding is used, it is even protected by a signature.

One more thing that must be taken into consideration is to ensure that if someone malicious would redirect a user directly to LogoutCommand.HandleResponse the user shouldn't get a false message that they are logged out. That can be prevented by an extra check of HttpRequestData.User in LogoutCommand.HandleResponse.

Why is everything always more complicated when one looks closer at it?

To summarize, I suggest:

• A compatibility setting that disables the logout state cookie.
• A check in LogoutCommand.HandleResponse of HttpRequestData.User that no user is logged in.
• If you need it - a compatibility setting to transfer the return url in the RelayState parameter directly. This should also lock the logout binding to Redirect both for incoming (by altering metadata) and outgoing messages to make sure that the RelayState is protected by a signature.

What do you think?

[lsherrod]

Any plans to make these changes?

[explunit]

I have no plans to tackle it at this point.