Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8.0 BlazorWebAppOidc IDX21323: RequireNonce is 'False' #53585

Closed
1 task done
travaille-dev opened this issue Jan 24, 2024 · 5 comments · Fixed by dotnet/blazor-samples#240
Closed
1 task done

8.0 BlazorWebAppOidc IDX21323: RequireNonce is 'False' #53585

travaille-dev opened this issue Jan 24, 2024 · 5 comments · Fixed by dotnet/blazor-samples#240
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation feature-blazor-server-auth Priority:1 Work that is critical for the release, but we could probably ship without
Milestone

Comments

@travaille-dev
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When running this blazor sample initially the authentication works well, but if you idle on the page (I'm assuming until the auth token needs to be refreshed) then you get this nonce error.

Since the error is pointing to the CookieOidcRefresher file I was wondering if I can get some direction on how to implement a fix for this.

Expected Behavior

I expect for the token to be refreshed in the background if it has expired.

Steps To Reproduce

  1. Set up the sample project with proper Microsoft Entra
  2. Run the project
  3. Authenticate to the weather page
  4. Wait for the token to need to be refreshed
  5. Open the counter page
  6. See error

Exceptions (if any)

No response

.NET Version

8.0.200-preview.23624.5

Anything else?

.NET SDK:
Version: 8.0.200-preview.23624.5
Commit: 8065b9770c
Workload version: 8.0.200-manifests.ba313bcd

Runtime Environment:
OS Name: Windows
OS Version: 10.0.19045
OS Platform: Windows
RID: win-x64
Base Path: C:\Program Files\dotnet\sdk\8.0.200-preview.23624.5\

.NET workloads installed:
Workload version: 8.0.200-manifests.ba313bcd
[aspire]
Installation Source: SDK 8.0.200-preview.23624, VS 17.9.34511.98
Manifest Version: 8.0.0-preview.2.23619.3/8.0.100
Manifest Path: C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.0.0-preview.2.23619.3\WorkloadManifest.json
Install Type: Msi

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

Host:
Version: 8.0.1
Architecture: x64
Commit: bf5e279d92

.NET SDKs installed:
6.0.418 [C:\Program Files\dotnet\sdk]
7.0.102 [C:\Program Files\dotnet\sdk]
8.0.101 [C:\Program Files\dotnet\sdk]
8.0.200-preview.23624.5 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.15 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 8.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.15 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 8.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.26 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.15 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 8.0.1 [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]

cc: @guardrex dotnet/blazor-samples#173

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Jan 24, 2024
@guardrex
Copy link
Contributor

guardrex commented Jan 24, 2024

Thanks @travaille-dev.

The sample app is in the Blazor samples repo ...

https://github.com/dotnet/blazor-samples/tree/main/8.0/BlazorWebAppOidc

The CookieOidcRefresher class is here ...

https://github.com/dotnet/blazor-samples/blob/main/8.0/BlazorWebAppOidc/BlazorWebAppOidc/CookieOidcRefresher.cs

Note @halter73 (if you'll be working this when you return in February), that the OIDC sample in that folder is now a non-BFF version of your sample app. Your sample app is at https://github.com/dotnet/blazor-samples/tree/main/8.0/BlazorWebAppOidcBff. The article that I'm placing will use pivots to cover them. One pivot will be focused on the non-BFF (no Aspire, no YARP) scenario, and the other pivot will be on your BFF with Aspire/YARP scenario. Also note that as of this morning (Wednesday, 1/24), I haven't even reached running either sample with Entra. The non-BFF sample is untested (by 🦖) at this time. I was happy to hear that thus far @travaille-dev had some luck with it running until this 💥 happened on the nonce. I will test with Entra here soon (Thursday, I hope 🤞🍀).

@martincostello martincostello added area-blazor Includes: Blazor, Razor Components and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Jan 24, 2024
@mkArtakMSFT mkArtakMSFT added Docs This issue tracks updating documentation feature-blazor-server-auth labels Jan 24, 2024
@mkArtakMSFT mkArtakMSFT added this to the .NET 9 Planning milestone Jan 24, 2024
@mkArtakMSFT mkArtakMSFT added the Priority:1 Work that is critical for the release, but we could probably ship without label Jan 24, 2024
@travaille-dev
Copy link
Author

travaille-dev commented Jan 25, 2024

I did want to add that I would be all right with focusing on the non BFF scenario. I'm primarily interested in aligning my auth scenario with auto rendering.

At the time, the repo didn't have a solution or project files, so I kind of fumbled my way into recreating it here https://github.com/travaille-dev/aspireSample
It should help if you still have issues running. Only setup was exposing the Weather.Get api scope in entra and setting up the client secret in the secrets.json for the blazor app proj. However, the scope doesn't /really/ matter in that solution because I hadn't gotten into protecting the api resource yet. I had the weatherapi set up as a controller api style just because I like it...

Looks like you got it up and running :)

@captnord
Copy link

captnord commented Feb 8, 2024

I have run into this snag as well which manifests as our site (deployed to an on prem server but using hosted Azure Entra ID for auth) being unavailable if left idle past the token refresh period (1 hour?). The end user sees an HTTP 500 error and we see the detailed exception that @travaille-dev posted in the submission (IDX21323).

In digging into the CookieOidcRefresher class provided in the 8.0/BlazorWebAppOidc project, I see that when the OpenIdConnectProtocolValidator is constructed it is done so with the property RequireNonce being set to false. When the method in OpenIdConnectProtocolValidator is called to validate the refresh response (starting at line 80 of the CookieOidcRefresher class) it fails on a null nonce value. It appears that the OpenIdConnectProtocolValidator is not honoring the RequireNonce = false setting.

OpenIdConnectProtocolValidator.ValidateTokenResponse() calls ValidateNonce() which checks the RequireNonce property and also checks the Nonce value of the ValidationContext being passed in as well as the ValidatedIdToken.Payload.Nonce value of the ValidationContext being passed in. What's more, once the Nonce values are validated there is a further check to see if it is expired. ValidateNonce() compares the timestamp of the ValidatedIdToken.Payload.Nonce value to the current UTC DateTime and if the nonce timestamp plus the NonceLifetime (set on the OpenIdConnectProtocolValidator) is less than UTC Now it throws OpenIdConnectProtocolInvalidNonceException (IDX21324).

TL;DR Even if RequireNonce is set to false a Nonce value still needs to be passed to the OpenIdConnectProtocolValidator and the Validator's NonceLifetime needs to be accounted for.

My proposed workaround for this is to set the Nonce value when creating the OpenIdConnectProtocolValidationContext model that is passed into OpenIdConnectProtocolValidator.ValidateTokenResponse. If it is set to the validated token's Payload.Nonce value then it should pass all of the checks in the RequireNonce method of OpenIdConnectProtocolValidator.

var validatedIdToken = JwtSecurityTokenConverter.Convert(validationResult.SecurityToken as JsonWebToken);

_oidcTokenValidator.ValidateTokenResponse(new()
{
    ProtocolMessage = message,
    Nonce = validatedIdToken.Payload.Nonce,
    ClientId = oidcOptions.ClientId,
    ValidatedIdToken = validatedIdToken,
});

Additionally, when constructing the OpenIdConnectProtocolValidator the NonceLifetime property should be set to an extended TimeSpan (I am setting it to 365 days):

private readonly OpenIdConnectProtocolValidator _oidcTokenValidator = new()
{
    // Refresh requests do not use the nonce parameter. Otherwise, we'd use oidcOptions.ProtocolValidator.
    RequireNonce = false,
    NonceLifetime = TimeSpan.FromDays(365)
};

The security implication for this is that we are essentially bypassing the Nonce check in the token refresh validation. I don't believe this is making CookieOidcRefresher any less secure than it already is since the RequireNonce property was already being set to false in order, presumably, to bypass the nonce check anyway. I am by no means an expert in any of this so please take my proposed workaround with a large grain of salt and I would appreciate those with more knowledge on the subject to offer their guidance if possible.

@guardrex
Copy link
Contributor

I just want to note here that at least one source of this (possibly THE source) is ...

Stale cookies are the devils playground! 😈😆

I just hit that while working a different samples repo issue. I already have guidance in the WASM-focused articles on using an InPrivate/incognito browser for testing, and I'm going to place those remarks in the BWA+OIDC article as well.

Anyone hitting this nonce error should first either clear cookies and try again or open an InPrivate/incognito browser window and access the app from there. There's guidance in the article on how to configure your VS to do that automatically in some of the WASM-focused security articles, which saves time while testing/debugging.

@guardrex
Copy link
Contributor

@mkArtakMSFT ... I think we've addressed this. @halter73 made the changes to the sample app on dotnet/blazor-samples#240, and I included a short section on it in the doc per dotnet/AspNetCore.Docs#32081, which states ...

Cryptographic nonce

A nonce is a string value that associates a client's session with an ID token to mitigate replay attacks.

If you receive a nonce error during authentication development and testing, use a new InPrivate/incognito browser session for each test run, no matter how small the change made to the app or test user because stale cookie data can lead to a nonce error. For more information, see the [Cookies and site data](#cookies-and-site-data) section.

A nonce isn't required or used when a refresh token is exchanged for a new access token. In the sample app, the CookieOidcRefresher (CookieOidcRefresher.cs) deliberately sets OpenIdConnectProtocolValidator.RequireNonce to false.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components Docs This issue tracks updating documentation feature-blazor-server-auth Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@halter73 @martincostello @guardrex @mkArtakMSFT @captnord @travaille-dev and others