Skip to content

Conversation

jander-msft
Copy link
Contributor

@jander-msft jander-msft commented Apr 15, 2024

Summary

Enable validation for the iss and exp fields in the API key token. These fields are now required to be in the token when using API key authentication.

The generatekey command will produce a token with the iss field set to https://github.com/dotnet/dotnet-monitor/generatekey+MonitorApiKey (this is an existing behavior) and the exp field set to expire 7 days from when the token was generated (this is a new behavior). The expiration can be overridden with the --expiration option.

In the case where a customer is generating their own JWT keys to be used with .NET Monitor, they can override the expected issuer in configuration by setting Authentication__MonitorApiKey__Issuer to the value of the issuer that is expected. Setting this is optional; its default value is https://github.com/dotnet/dotnet-monitor/generatekey+MonitorApiKey.

This is a breaking change because both the iss and exp fields are now required and validated. This will not cause disruption to most usage of the API key authentication as it is expected that customers will use the generatekey command to create the appropriate settings and the key will have a lifetime of 7 days. Those customers who are creating their own API key must now set both iss and exp in the key and configure Authentication__MonitorApiKey__Issuer within .NET Monitor for the tool to accept the API key.

Release Notes Entry

@jander-msft jander-msft added the breaking-change Pull requests that introduce a breaking change label Apr 15, 2024
@jander-msft jander-msft force-pushed the dev/jander/key-gen-updates branch from 7ad32b8 to 9df6eae Compare April 17, 2024 20:14
@jander-msft jander-msft marked this pull request as ready for review April 17, 2024 20:24
@jander-msft jander-msft requested a review from a team as a code owner April 17, 2024 20:24
@jander-msft jander-msft added servicing-patch Servicing fixes that is targeted for a patch release (0.0.x version) servicing-minor Servicing fixes that is targeted for a minor release (0.x.0 version) servicing-major Servicing fixes that is targeted for a major release (x.0.0 version) labels Apr 17, 2024
wiktork
wiktork previously approved these changes Apr 18, 2024
@jander-msft jander-msft force-pushed the dev/jander/key-gen-updates branch from d465604 to 6549825 Compare April 20, 2024 03:28
@jander-msft jander-msft requested a review from wiktork April 22, 2024 17:14
@jander-msft jander-msft merged commit e22a67d into dotnet:main Apr 22, 2024
@jander-msft jander-msft deleted the dev/jander/key-gen-updates branch April 22, 2024 22:37
@jander-msft
Copy link
Contributor Author

/backport to release/8.0

Copy link
Contributor

Started backporting to release/8.0: https://github.com/dotnet/dotnet-monitor/actions/runs/8791855621

@jander-msft
Copy link
Contributor Author

/backport to release/7.x

Copy link
Contributor

@jander-msft backporting to release/8.0 failed, the patch most likely resulted in conflicts.

Please backport manually using one of the below commands, followed by git am --continue once the merge conflict has been resolved.

PowerShell

(Invoke-WebRequest "https://github.com/dotnet/dotnet-monitor/commit/e22a67d2d0b0cc46e5f137d3980fe9b16d61d0cc.patch").Content | git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch

Bash

curl -sSL "https://github.com/dotnet/dotnet-monitor/commit/e22a67d2d0b0cc46e5f137d3980fe9b16d61d0cc.patch" | git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch

git am error output:

$ git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch changes.patch

Applying: Add issuer and expiration validation for API keys (#6456)
.git/rebase-apply/patch:103: trailing whitespace.
        
.git/rebase-apply/patch:1138: trailing whitespace.
        
.git/rebase-apply/patch:1154: trailing whitespace.
        
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	documentation/schema.json
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
M	src/Microsoft.Diagnostics.Monitoring.WebApi/Auth/AuthConstants.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
M	src/Tools/dotnet-monitor/Strings.Designer.cs
M	src/Tools/dotnet-monitor/Strings.resx
Falling back to patching base and 3-way merge...
Auto-merging src/Tools/dotnet-monitor/Strings.resx
Auto-merging src/Tools/dotnet-monitor/Strings.Designer.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Auth/AuthConstants.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
Auto-merging src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
Auto-merging documentation/schema.json
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add issuer and expiration validation for API keys (#6456)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Copy link
Contributor

Started backporting to release/7.x: https://github.com/dotnet/dotnet-monitor/actions/runs/8791858247

Copy link
Contributor

@jander-msft backporting to release/7.x failed, the patch most likely resulted in conflicts.

Please backport manually using one of the below commands, followed by git am --continue once the merge conflict has been resolved.

PowerShell

(Invoke-WebRequest "https://github.com/dotnet/dotnet-monitor/commit/e22a67d2d0b0cc46e5f137d3980fe9b16d61d0cc.patch").Content | git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch

Bash

curl -sSL "https://github.com/dotnet/dotnet-monitor/commit/e22a67d2d0b0cc46e5f137d3980fe9b16d61d0cc.patch" | git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch

git am error output:

$ git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch changes.patch

Applying: Add issuer and expiration validation for API keys (#6456)
.git/rebase-apply/patch:103: trailing whitespace.
        
.git/rebase-apply/patch:1138: trailing whitespace.
        
.git/rebase-apply/patch:1154: trailing whitespace.
        
warning: 3 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	documentation/schema.json
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
M	src/Microsoft.Diagnostics.Monitoring.WebApi/Auth/AuthConstants.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/AuthenticationTests.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
M	src/Tools/dotnet-monitor/Auth/ApiKey/JwtBearerPostConfigure.cs
M	src/Tools/dotnet-monitor/Commands/GenerateApiKeyCommandHandler.cs
M	src/Tools/dotnet-monitor/Strings.Designer.cs
M	src/Tools/dotnet-monitor/Strings.resx
Falling back to patching base and 3-way merge...
Auto-merging src/Tools/dotnet-monitor/Strings.resx
CONFLICT (content): Merge conflict in src/Tools/dotnet-monitor/Strings.resx
Auto-merging src/Tools/dotnet-monitor/Strings.Designer.cs
CONFLICT (content): Merge conflict in src/Tools/dotnet-monitor/Strings.Designer.cs
Auto-merging src/Tools/dotnet-monitor/Commands/GenerateApiKeyCommandHandler.cs
Auto-merging src/Tools/dotnet-monitor/Auth/ApiKey/JwtBearerPostConfigure.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
CONFLICT (content): Merge conflict in src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/AuthenticationTests.cs
CONFLICT (content): Merge conflict in src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/AuthenticationTests.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.WebApi/Auth/AuthConstants.cs
Auto-merging src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
Auto-merging src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
Auto-merging documentation/schema.json
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add issuer and expiration validation for API keys (#6456)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

@jander-msft jander-msft added the update-release-notes Pull requests that should be mentioned in the release notes label May 9, 2024
@jander-msft jander-msft removed servicing-patch Servicing fixes that is targeted for a patch release (0.0.x version) servicing-minor Servicing fixes that is targeted for a minor release (0.x.0 version) servicing-major Servicing fixes that is targeted for a major release (x.0.0 version) labels Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Pull requests that introduce a breaking change update-release-notes Pull requests that should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants