aspnet / AspNetKatana

Microsoft's OWIN implementation, the Katana project
Apache License 2.0
967 stars 334 forks source link

Random redirect loop due to expired cookie #261

Closed alamfsmb closed 4 years ago

alamfsmb commented 5 years ago

With our implementation of Azure AD B2C including the Microsoft.Owin.Security.Cookies 4.0.0 middleware together with the OpenIdConnect middleware, we were experiencing intermittent cases where users would get stuck in an infinite redirect loop between our app and Azure.

After much debugging and logging, we were seeing that in these looping cases, the repeated challenge request to Azure included the cookie that we configured in our cookie middleware and that on AuthenticateCoreAsync in the CookieAuthenticationHandler, this cookie would be found expired, therefore nullifying the AuthenticationTicket and leading to an unauthenticated request and subsequent calls to challenge for authentication and a cycle of reading the expired cookie and rechallenging.

Our workaround that seems to have worked was to ensure any expired cookies were deleted.

Besides affecting our customers, this looping had ramifications for our costs and we would like to understand if this is a true bug or if there is a documented workaround.

Thanks in advance!

Startup.Auth.cs

app.UseKentorOwinCookieSaver();

app.SetDefaultSignInAsAuthenticationType(CookieAuthenticationDefaults.AuthenticationType);

app.UseCookieAuthentication(
    new CookieAuthenticationOptions {
        CookieName = "ourCookieName",
        AuthenticationType = CookieAuthenticationDefaults.AuthenticationType,
        LoginPath = new PathString(LoginPath),
        LogoutPath = new PathString(LogoutPath),
        AuthenticationMode = AuthenticationMode.Active,
        CookieSecure = CookieSecureOption.Always,
        CookieManager = new SystemWebChunkingCookieManager(),
        SlidingExpiration = true,
        ExpireTimeSpan = TimeSpan.FromMinutes(30),
        CookieDomain = .ourDomain.com
    });
Tratcher commented 5 years ago

Do not use UseKentorOwinCookieSaver. If the documentation here is insufficient then let me know.

alamfsmb commented 5 years ago

Thank you for your quick response. We just tried removing KentorOwinCookieSaver but the redirect loop still happens occasionally so we had to keep the Delete Expired cookie hack.

Before our latest changes with removing Kentor, we were updating the cookie post login with an updated identity having additional claims via HttpContext.GetOwinContext().Authentication.SignIn(identity); but then I realized that might negatively impact the session expire and issue properties, so I changed instead to updating On SecurityTokenValidated via the notification's authenticationTicket identity.

//after cookieMiddleware
 app.UseOpenIdConnectAuthentication(
                new OpenIdConnectAuthenticationOptions {
                    MetadataAddress = Authority,
                    ClientId = ClientId,
                    RedirectUri = RedirectUri,
                    PostLogoutRedirectUri = RedirectUri,

                    Notifications = new OpenIdConnectAuthenticationNotifications {
                        RedirectToIdentityProvider = OnRedirectToIdentityProvider,
                        AuthenticationFailed = OnAuthenticationFailed,
                        SecurityTokenValidated = OnSecurityTokenValidated
                    },

                    TokenValidationParameters = new TokenValidationParameters {
                        NameClaimType = "name"
                    },

                    Scope = $"openid profile email",
                    UseTokenLifetime = false
                });

Despite this, we are still seeing extra id tokens being issued and are wondering if some cookie issues still persist to cause these extra id tokens coming from B2C.

The cookie is shared among apps on the same domain, all sharing the same machineKey and nearly the same cookie options. Essentially, one app is responsible for handling the auth and cookie, and the other apps are expected to use this cookie, and redirect to the main app for signing in and out. The other apps use this configuration

            app.UseCookieAuthentication( new CookieAuthenticationOptions {
                CookieName = "ourCookieName",
                AuthenticationType = CookieAuthenticationDefaults.AuthenticationType,
                CookieSecure = CookieSecureOption.Always,
                //LoginPath does not support redirecting to another domain for login,
                //Apply redirect to login through the provider
                LoginPath = new PathString("/"),
                Provider = new CookieAuthenticationProvider() {
                    OnApplyRedirect = ( context ) => {
                                context.RedirectUri = mainAppURL;
                        };
                        context.Response.Redirect(context.RedirectUri);
                    }
                }
            });

Do you have any ideas on what might cause these possible cookie issues?

Tratcher commented 5 years ago

Did you try the mitigation recommended in the docs? https://github.com/aspnet/AspNetKatana/wiki/System.Web-response-cookie-integration-issues

alamfsmb commented 5 years ago

Yes, we went with using "Reconfigure the CookieAuthenticationMiddleware to write directly to System.Web's cookie collection." by using SystemWebChunkingCookieManager (which seems to be the same as SystemWebCookieManager with Chunking functionality

Tratcher commented 5 years ago

Ok, it wasn't in your code above. Also, what's wrong with your CookieAuthenticationProvider code? That doesn't look like it would compile.

alamfsmb commented 5 years ago

Sorry, I took out some code in the provider and made it partially pseudocode. The CookieAuthenticationProvider for the other apps just redirects to the mainApp's signIn action which calls the HttpContext.GetOwinContext().Authentication.Challenge()