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

Add Azure AD authentication support (aad-auth pt 3) #3755

Merged
merged 70 commits into from
Feb 24, 2023

Conversation

schmittjoseph
Copy link
Member

@schmittjoseph schmittjoseph commented Feb 22, 2023

Summary

This is the final PR in a series to add Azure Active Directory authentication support.

Notable changes:

  • When using deferred authentication validate the used configuration before applying authentication. If validation errors are found, fail fast and prevent dotnet monitor from starting and inform the user why. We do this to strictly enforce the user's deferred authentication preferences as which auth mode is used cannot be adjusted at runtime.
  • Allow the user to interactively authenticate with dotnet monitor using our in-box Swagger UI with AzureAD.
  • Introduce a new method (ConfigureSwaggerUI) on the IAuthenticationConfigurator interface.
  • Change the recommended (but not default) authentication mode to Azure AD.

There are currently no end-to-end AzureAD authentication tests as that will require using actual Azure Resources. End-to-end tests will be added as a separate piece of work.

Release Notes Entry

Add Azure Active Directory authentication support. For more details see our Azure AD documentation.

jander-msft
jander-msft previously approved these changes Feb 23, 2023
Copy link
Member

@jander-msft jander-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the crash-on-bad-configuration comment, everything else LGTM.

wiktork
wiktork previously approved these changes Feb 24, 2023
@schmittjoseph schmittjoseph dismissed stale reviews from wiktork and jander-msft via 3e20d20 February 24, 2023 17:36
jander-msft
jander-msft previously approved these changes Feb 24, 2023
@schmittjoseph schmittjoseph merged commit e3eb4a0 into dotnet:main Feb 24, 2023
@schmittjoseph schmittjoseph deleted the aad-auth-split/aad-option-rev branch February 24, 2023 21:30
@schmittjoseph
Copy link
Member Author

/backport to release/7.x

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

@schmittjoseph 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/e3eb4a053afde6b60fe90e703b4d05158a63edee.patch").Content | git am --3way --ignore-whitespace --exclude="documentation/**.md" --keep-non-patch

Bash

curl -sSL "https://github.com/dotnet/dotnet-monitor/commit/e3eb4a053afde6b60fe90e703b4d05158a63edee.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 Azure AD authentication support (aad-auth pt 3) (#3755)
.git/rebase-apply/patch:488: trailing whitespace.
        
.git/rebase-apply/patch:504: trailing whitespace.
        
.git/rebase-apply/patch:513: trailing whitespace.
        
.git/rebase-apply/patch:522: trailing whitespace.
        
.git/rebase-apply/patch:531: trailing whitespace.
        
warning: squelched 6 whitespace errors
warning: 11 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	documentation/schema.json
M	eng/dependabot/Packages.props
M	eng/dependabot/Versions.props
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
M	src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/AuthenticationTests.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
M	src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorRunner.cs
M	src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs
M	src/Tools/dotnet-monitor/Strings.Designer.cs
M	src/Tools/dotnet-monitor/Strings.resx
M	src/Tools/dotnet-monitor/dotnet-monitor.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/Tools/dotnet-monitor/dotnet-monitor.csproj
Auto-merging src/Tools/dotnet-monitor/Strings.resx
Auto-merging src/Tools/dotnet-monitor/Strings.Designer.cs
Auto-merging src/Tools/dotnet-monitor/Commands/CollectCommandHandler.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorRunner.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs
Auto-merging src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/AuthenticationTests.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
CONFLICT (content): Merge conflict in src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
Auto-merging eng/dependabot/Versions.props
Auto-merging eng/dependabot/Packages.props
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 Azure AD authentication support (aad-auth pt 3) (#3755)
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

schmittjoseph added a commit to schmittjoseph/dotnet-monitor that referenced this pull request Feb 24, 2023
@jander-msft jander-msft removed the servicing-minor Servicing fixes that is targeted for a minor release (0.x.0 version) label Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants