/signin-oidc does not unset nonce and correlation cookies with param error=login_required from failed identity server prompt=none request

JeroMiya commented 10 months ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the bug

This issue has a similar symptom as other reported issues, but I believe has a different cause:

For context, we have an application that authenticates with OIDC to an identity server that is on the same domain/site as our application. We're using Duende.Bff and we have a silent login flow implemented, where an iframe is opened up to /bff/silent-login. /silent-login issues a challenge (thus setting the nonce and correlation cookies) and redirects to the identity server using the prompt=none parameter. If the user is already logged into the identity server, then the IS redirects back to /signin-oidc, which then deletes the nonce and correlation cookies. That's expected behavior. The problem occurs when the user is not already signed into the identity server. In this case, the IS redirects back to /signin-oidc with the ?error=login_required parameter. When this happens, the /signin-oidc endpoint does NOT delete the nonce and correlation cookies. And, while Duende.Bff correctly notifies the parent frame via a message that silent login was unsuccessful (so that we can notify the user and prompt for them to login manually), this leaves around extra nonce and correlation cookies until they expire.

It does not take long for these extra cookies to build up to the point that we get errors for cookies being too big. Around 4 or 5 navigations to the site without being logged in, within a 15 minute window, will cause the errors to occur. Users clicking on a handful of deep links or accidently double clicking the refresh button a couple of times while not being logged in will trigger the errors, so it's plausible this will occur outside of QA testing.

I've noticed some differences in the successful vs unsuccessful request, though I don't know if they are relevant or are the cause:

successful requests send code, scope, state, session_state, and iss parameters
unsuccessful requests send error=login_required, state, and session_state parameters (i.e. missing code and scope parameters)

Thus, I'm not sure if this is expected behavior, given that the unsuccessful request is missing the code parameter - perhaps there is no way for the OIDC middleware to associate it with the original nonce/correlation cookies? Please advise. Our options to mitigate this issue are limited, outside of eliminating silent login functionality which is an option of last resort given we are attempting to support SSO for multiple applications, and that would be a bad user experience.

Note: contrary to previously reported issues, I can verify that this is not due to a cookie SameSite issue (the identity server is on the same site, and I can confirm that the cookies are being sent correctly with the /signin-oidc request). I can also confirm that there is no login redirect loop happening, and that there is only one challenge being made (in Duende.Bff's implementation of the /bff/silent-login endpoint).

Expected Behavior

When the identity server redirects back to /signin-oidc with the ?error=login_required parameter, I expect that it will still delete the nonce and correlation cookies corresponding to the original request, just as it does when the ?error=login_required is not present in the request.

Steps To Reproduce

Here is the setup code for our API:

builder.Services.AddAuthentication(options =>
{
    options.DefaultScheme = "cookie";
    options.DefaultChallengeScheme = "oidc";
    options.DefaultSignOutScheme = "oidc";
})
.AddCookie("cookie", options =>
{
    // set session lifetime
    options.ExpireTimeSpan = TimeSpan.FromHours(8);

    // sliding or absolute
    options.SlidingExpiration = false;

    // the "__Secure" prefix is special
    // See: https://www.ietf.org/archive/id/draft-west-cookie-prefixes-05.txt
    // We are using __Secure instead of __Host because
    // the cookie will have a subpath (e.g. {subdomain.domain}/{app-subpath}) and not the root "/"
    options.Cookie.Name = "__Secure-bff";

    // strict SameSite handling
    options.Cookie.SameSite = SameSiteMode.Strict;
})
.AddOpenIdConnect("oidc", options =>
{
    if (isOptions is null)
    {
        throw new InvalidOperationException("Missing IdentityServer config section");
    }

    options.Authority = isOptions.Authority;

    // confidential client using code flow + PKCE
    options.RequireHttpsMetadata = true;
    options.ClientId = isOptions.ClientId;
    options.ClientSecret = isOptions.ClientSecret;
    options.ResponseType = OpenIdConnectResponseType.Code;

    // query response type is compatible with strict SameSite mode
    options.ResponseMode = OpenIdConnectResponseMode.Query;

    // get claims without mappings
    options.MapInboundClaims = false;
    options.GetClaimsFromUserInfoEndpoint = true;

    // save tokens into authentication session
    // to enable automatic token management
    options.SaveTokens = true;

    // request scopes
    options.Scope.Clear();
    options.Scope.Add("openid");
    options.Scope.Add("profile");

    // and refresh token
    options.Scope.Add("offline_access");
});

And here is the silent-login functionality on the frontend side (note the issue occurs with or without the redirectToLogin functionality below):

  private silentLogin(s: Session) {
    if (this.userIsAnonymous(s)) {
      console.log('not already logged in, attempting silent login...');
      const timeout = 5000;

      function redirectToLogin() {
        const returnUrl = window.location.pathname + window.location.search + window.location.hash;
        window.location.href = `/app1/bff/login?returnUrl=${returnUrl}`;
      }

      function onMessage(e: any) {
        if (e.data && e.data.source === 'bff-silent-login') {
          window.removeEventListener("message", onMessage);
          if (e.data.isLoggedIn) {
            console.log('silent login succeeded, refreshing');
            window.location.reload();
          } else {
            console.log('silent login failed due to not being logged in, redirecting to identity server login page');
            redirectToLogin();
          }
        }
      }
      window.addEventListener("message", onMessage);

      window.setTimeout(() => {
        window.removeEventListener("message", onMessage);
        console.error('silent login timed out, redirecting to login');
        redirectToLogin();
      }, timeout);

      const iframe = document.createElement('iframe');
      iframe.src = '/app1/bff/silent-login';
      iframe.setAttribute('style', 'width:0px;height:0px');
      document.body.appendChild(iframe);
    }
    return s;
  }

Exceptions (if any)

No response

.NET Version

8.0.100

Anything else?

ASP.NET Core version is 6.0.25

dotnet --info:

.NET SDK:
 Version:           8.0.100
 Commit:            57efcf1350
 Workload version:  8.0.100-manifests.8d38d0cc

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.100\

.NET workloads installed:
 Workload version: 8.0.100-manifests.8d38d0cc
 [wasm-tools]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    8.0.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.workload.mono.toolchain.current\8.0.0\WorkloadManifest.json
   Install Type:              Msi

 [android]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    34.0.43/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.43\WorkloadManifest.json
   Install Type:              Msi

 [wasm-tools-net6]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    8.0.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.workload.mono.toolchain.net6\8.0.0\WorkloadManifest.json
   Install Type:              Msi

 [maui-windows]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    8.0.3/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.3\WorkloadManifest.json
   Install Type:              Msi

 [maccatalyst]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    17.0.8478/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.0.8478\WorkloadManifest.json
   Install Type:              Msi

 [ios]
   Installation Source: VS 17.8.34330.188
   Manifest Version:    17.0.8478/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.0.8478\WorkloadManifest.json
   Install Type:              Msi

Host:
  Version:      8.0.0
  Architecture: x64
  Commit:       5535e31a71

.NET SDKs installed:
  8.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.25 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.14 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Tratcher commented 10 months ago

That makes sense, the nonce cookie isn't consumed & deleted until late in the validation process. Maybe that could happen earlier?

If there's an id token: https://github.com/dotnet/aspnetcore/blob/e6be3e95fec33ca3a3b576df4b265ead680a91a0/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs#L669

If there's a code: https://github.com/dotnet/aspnetcore/blob/e6be3e95fec33ca3a3b576df4b265ead680a91a0/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs#L752

The logic for deleting the right cookie only cross checks against the received code/token, that would have to be relaxed in the error scenario. https://github.com/dotnet/aspnetcore/blob/e6be3e95fec33ca3a3b576df4b265ead680a91a0/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs#L1062-L1090

dotnet-policy-service[bot] commented 1 week ago

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

AndyWangMSFT commented 1 week ago

@JeroMiya Thanks for posting this question. Just curious, were you able to delete the nonce/correlation cookie from server side? I'm trying to delete these two types of cookie so that they are not building up

JeroMiya commented 1 week ago

@JeroMiya Thanks for posting this question. Just curious, were you able to delete the nonce/correlation cookie from server side? I'm trying to delete these two types of cookie so that they are not building up

It's funny, we actually decided to port the Angular app to Blazor. Since we are using enhanced navigation, we removed the need to implement silent login using an iframe in the frontend, since the redirect happens on the server side before the frontend even loads (or it happens at navigation time if it happens post load). So, I didn't need to go back and implement a workaround. But the problem will still exist for anybody trying to implement the silent login flow with a more traditional frontend spa following the latest OIDC recommendations.

dotnet / aspnetcore