This project has moved. For the latest updates, please go here.
19

Closed

Problem with cookies in Microsoft.Owin.Host.SystemWeb

description

There seems to be a problem with cookies implementation in SystemWeb hosting when mixing usage of HttpContext.GetOwinContext().Response.Cookies and HttpContext.Response.Cookies (for example default SessionStateModule). In some cases OWIN cookies are "lost" and not send to the browser.

In case of SessionStateModule this can be quite fatal. If you use session (store some values) but after the user authenticated, it is impossible for other users to login through OWIN anymore. OWIN authentication response cookies are "lost" during response processing.

More details at http://stackoverflow.com/questions/20737578/asp-net-sessionid-owin-cookies-do-not-send-to-browser

This is the code to demonstrate the problem (Home/Index method of default MVC template in VS2013). OwinCookie is not added to response.
public ActionResult Index()
{
    HttpContext.GetOwinContext()
        .Response.Cookies.Append("OwinCookie", "SomeValue");
    HttpContext.Response.Cookies["ASPCookie"].Value = "SomeValue";
    HttpContext.Response.Cookies.Remove("ASPCookie");

    return View();
}
Closed Jan 20, 2015 at 10:38 PM by Tratcher

comments

Tratcher wrote Jan 28, 2014 at 4:12 PM

Thanks for the report. We understand the problem, but there is not a clear solution. We will investigate as resources allow, but in the meantime any suggestions would be appreciated.

augustogmAraujo wrote Jun 13, 2014 at 11:15 PM

Hello,


I'm having the same problem. My aplication some times just don't login and the users are geting crazy.

I hope to get some help from you.

Thank very much.

jakkwylde wrote Jul 2, 2014 at 1:46 AM

Voting on this one... I am encountering the same issue. In the event session is not written to before attempting to authenticate the call to GetExternalLoginInfoAsync() always comes back null however inspecting the traffic indicates the third party external provides provider a token. If an asp.net session id exists before authentication all looks well. Hopefully there will be an update on this soon as it is a very significant issue and the most recent nightly build still reveals the same problem.

Tratcher wrote Jul 3, 2014 at 11:09 PM

Unfortunately it doesn't look like we'll be able to fix this until Katana v4 / ASP.NET vNext when we re-integrate all of these disparate components.

Lisaj wrote Jul 7, 2014 at 7:48 PM

Hello,
is there any solution to avoid the problem?

Tratcher wrote Jul 22, 2014 at 11:08 PM

No fix is possible when running on System.Web. This won't be a problem in v4 / ASP.NET vNext because the cookie handling will be re-integrated.

** Closed by Tratcher 07/22/2014 4:08PM

Neilski wrote Aug 1, 2014 at 5:04 AM

I have just come across this thread and it ends weeks of frustration that I have had trying to understand the problem (see http://stackoverflow.com/q/23363353/236860).

I had committed to Identity 2.0 on several projects until I discovered the problem. Waiting for vNext is not an option. Even if I could live with the immediate problem (I can't), who will pay for the time it would take to port the existing projects to vNext?

Identity 2.0 was released, promoted and demonstrated with the current release of ASP.net. Whilst I am looking forward to vNext, it shouldn't be used as the solution for existing problems - no matter how tempting that might be.

It seems that the 'old and ill-conceived' ASP.net Membership (to loosely quote the Identity 2.0 press release) is still, perhaps, a better solution.

kingdango wrote Sep 30, 2014 at 3:59 PM

^^ Unfortunately I agree with Neilski's comments. I've invested a lot of my time into Identity 2.0 and Owin OAUTH. Being multi-tentant I had to write custom provider implementations as well. Now we're doing some pre-deploy testing and come across this intermittent but deadly issue.

I don't want to beat a dead horse but this does sting a bit.

kingdango wrote Oct 1, 2014 at 1:07 PM

Something more productive than my previous comment... I think I have resolved this issue by disabling session (since we never use it) and using the CookieTempDataProvider built by @brockallen (love this guy btw).

https://github.com/brockallen/CookieTempData

I will say that it didn't "automagically" work as he suggested. I have a base controller that every controller uses and I had to override the TempDataProvider property -- a scenario he said he overcame but I didn't find that to be true.

Anyway, we have this working and I don't seem to be hitting the auth-fails-for-everyone problem anymore. Does this make sense as a workaround for people who don't need Session but do use TempData?

Cheers!

andersabel wrote Nov 16, 2014 at 3:17 PM

We got login problems because of this bug in one of our applications last week. I've looked through the source both for Katana and System.Web (thanks Microsoft for open sourcing everything).

The problem is that when a cookie is removed through the ASP.Net API, the Set-Cookie header is recreated from the HttpContext.Current.Response.Cookies collection. Doing this it completely ignores the present content of the Set-Cookie header. To System.Web, the Set-Cookies header is not the master source of cookie information; it can be overwritten any time by the response cookie collection. This is the way System.Web works and I assume we have to live with it.

However, I think it's possible to provide a workaround in Microsoft.Owin.Host.SystemWeb. I've made a small proof-of-concept hack that appears to work where I add a check to Microsoft.Owin.Host.SystemWeb.CallHeaders.AspNetResponseHeaders.Set(), to see if it is the Set-Cookie header that is updated. In that case I add the new cookie to the HttpContext.Current.Response.Cookies collection. When System.Web overwrites the header, it will include the Owin-generated cookies as they are also part of the Response.Cookies collection.

I can convert my proof-of-concept hack to some real, production-worthy code with unit tests, but before spending time on that I'd like to know if the solution looks okay to you and have any chance of being merged.

Tratcher wrote Nov 16, 2014 at 10:24 PM

It's worth a try, we already do something similar for Content-Type and Cache-Control. However, the Set-Cookie header is much more complicated. Any attempted fix will need lots of unit tests.

andersabel wrote Nov 17, 2014 at 4:07 PM

I've been working on it today in the form of a middleware that can be used with the present Katana version (source and nuget package). @Tratcher, you're right that the Set-Cookie header is more complicated, there were quite a few cases to take into consideration (and I might have missed some).

I plan to take the cookie handling code (and tests) from the middleware and move it to Microsoft.Owin.Host.SystemWeb.

Tratcher wrote Nov 24, 2014 at 3:37 PM

Here's an interesting workaround for the cookie auth middleware:

You can set the cookie manager to directly apply cookies to the SystemWeb cookie collection.
            app.UseCookieAuthentication(new CookieAuthenticationOptions
            {
                // ...
                CookieManager = new SystemWebCookieManager()
            });
    public class SystemWebCookieManager : ICookieManager
    {
        public string GetRequestCookie(IOwinContext context, string key)
        {
            if (context == null)
            {
                throw new ArgumentNullException("context");
            }

            var webContext = context.Get<HttpContextBase>(typeof(HttpContextBase).FullName);
            var cookie = webContext.Request.Cookies[key];
            return cookie == null ? null : cookie.Value;
        }

        public void AppendResponseCookie(IOwinContext context, string key, string value, CookieOptions options)
        {
            if (context == null)
            {
                throw new ArgumentNullException("context");
            }
            if (options == null)
            {
                throw new ArgumentNullException("options");
            }

            var webContext = context.Get<HttpContextBase>(typeof(HttpContextBase).FullName);

            bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
            bool pathHasValue = !string.IsNullOrEmpty(options.Path);
            bool expiresHasValue = options.Expires.HasValue;
            
            var cookie = new HttpCookie(key, value);
            if (domainHasValue)
            {
                cookie.Domain = options.Domain;
            }
            if (pathHasValue)
            {
                cookie.Path = options.Path;
            }
            if (expiresHasValue)
            {
                cookie.Expires = options.Expires.Value;
            }
            if (options.Secure)
            {
                cookie.Secure = true;
            }
            if (options.HttpOnly)
            {
                cookie.HttpOnly = true;
            }

            webContext.Response.AppendCookie(cookie);
        }

        public void DeleteCookie(IOwinContext context, string key, CookieOptions options)
        {
            if (context == null)
            {
                throw new ArgumentNullException("context");
            }
            if (options == null)
            {
                throw new ArgumentNullException("options");
            }
            
            AppendResponseCookie(
                context,
                key,
                string.Empty,
                new CookieOptions
                {
                    Path = options.Path,
                    Domain = options.Domain,
                    Expires = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc),
                });
        }
    }
Advantages:
  • This addresses the most common problem people have reported, auth cookies.
  • Simplicity, it doesn't require error-prone reparsing of cookies.
Disadvantages
  • It only applies to the cookie auth middleware, not all response cookies.
Note: this sample does not handle chunking of large cookies, but the same workaround can be applied to a copy of the chunking cookie manager.

Give it a try and let me know how it works for you.

Tratcher wrote Nov 25, 2014 at 4:21 PM

akub19 wrote Apr 28, 2015 at 9:49 PM

Tratcher, SystemWebCookieManager doesn't work. Still have ChunkingCookieManager.

AmitKarankar wrote Apr 5, 2016 at 10:22 AM

@Tratcher : app.UseCookieAuthentication(new CookieAuthenticationOptions
        {
            // ...
            CookieManager = new SystemWebCookieManager()
        });
CookieAuthenticationOptions class should have Property CookieManager of type SystemWebCookieManager.
Issue in setting it as CookieAuthenticationOptions class wont have this property.

AmitKarankar wrote Apr 5, 2016 at 10:25 AM

Tratcher , CookieManager property is not avaliable in CookieAuthenticationOptions class. Also
Please share namespace for ICookieManager

AmitKarankar wrote Apr 5, 2016 at 10:26 AM

Added comment

rikrak wrote Feb 9 at 1:57 PM

Does this need applying to the ExternalSigninCookieAuthentication logic too? Something like:
    public static void UseExternalSignInCookie(this IAppBuilder app, string externalAuthenticationType)
    {
        if (app == null)
        {
            throw new ArgumentNullException("app");
        }

        app.SetDefaultSignInAsAuthenticationType(externalAuthenticationType);
        app.UseCookieAuthentication(new CookieAuthenticationOptions
        {
            AuthenticationType = externalAuthenticationType,
            AuthenticationMode = AuthenticationMode.Passive,
            __CookieManager = new SystemWebCookieManager(),__
            CookieName = CookiePrefix + externalAuthenticationType,
            ExpireTimeSpan = TimeSpan.FromMinutes(5),
        });
    }

Tratcher wrote Feb 9 at 4:18 PM

If you have trouble with your external cookies, yes.