Add workaround for deprecated overload in authentication scenarios #24600

captainsafia · 2020-08-05T21:59:37Z

Description

Addresses https://github.com/aspnet/AspNetCore-ManualTests/issues/142.

This is a temporary workaround for an issue that was found when validating project templates in preview8.

In preview8, we deprecated some authentication-related overloads (#24253). Some auth scenarios use the Microsoft.Identity.Web package which takes a dependency on the preview6 version of the Microsoft.AspNetCore.Authentication.JwtBearer and Microsoft.AspNetCore.Authentication.OpenIdConnect packages (ref). This caused the application to fail during initialization with a MissingMethodException.

Unhandled exception. System.MissingMethodException: Method not found: 'Microsoft.AspNetCore.Authentication.AuthenticationBuilder Microsoft.AspNetCore.Authentication.AuthenticationBuilder.AddScheme(System.String, System.String, System.Action`2<System.__Canon,!!2>)'.
   at Microsoft.Extensions.DependencyInjection.JwtBearerExtensions.AddJwtBearer[TService](AuthenticationBuilder builder, String authenticationScheme, String displayName, Action`2 configureOptions)
   at Microsoft.Extensions.DependencyInjection.JwtBearerExtensions.AddJwtBearer(AuthenticationBuilder builder, String authenticationScheme, String displayName, Action`1 configureOptions)
   at Microsoft.Extensions.DependencyInjection.JwtBearerExtensions.AddJwtBearer(AuthenticationBuilder builder, String authenticationScheme, Action`1 configureOptions)
   at Microsoft.Identity.Web.WebApiAuthenticationBuilderExtensions.AddMicrosoftWebApi(AuthenticationBuilder builder, Action`1 configureJwtBearerOptions, Action`1 configureMicrosoftIdentityOptions, String jwtBearerScheme, Boolean subscribeToJwtBearerMiddlewareDiagnosticsEvents)
   at Microsoft.Identity.Web.WebApiAuthenticationBuilderExtensions.AddMicrosoftWebApi(AuthenticationBuilder builder, IConfiguration configuration, String configSectionName, String jwtBearerScheme, Boolean subscribeToJwtBearerMiddlewareDiagnosticsEvents)
   at Microsoft.Identity.Web.WebApiServiceCollectionExtensions.AddMicrosoftWebApiAuthentication(IServiceCollection services, IConfiguration configuration, String configSectionName, String jwtBearerScheme, Boolean subscribeToJwtBearerMiddlewareDiagnosticsEvents)
   at _0805WebApi.Startup.ConfigureServices(IServiceCollection services) in /Users/captainsafia/verifications/0805WebApi/Startup.cs:line 30
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.InvokeCore(Object instance, IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.<>c__DisplayClass9_0.<Invoke>g__Startup|0(IServiceCollection serviceCollection)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.Invoke(Object instance, IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.<>c__DisplayClass8_0.<Build>b__0(IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.UseStartup(Type startupType, HostBuilderContext context, IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass12_0.<UseStartup>b__0(HostBuilderContext context, IServiceCollection services)
   at Microsoft.Extensions.Hosting.HostBuilder.CreateServiceProvider()
   at Microsoft.Extensions.Hosting.HostBuilder.Build()
   at _0805WebApi.Program.Main(String[] args) in /Users/captainsafia/verifications/0805WebApi/Program.cs:line 16

This PR hard-codes the preview8 versions of these packages in the project template to pull in the latest version of our API. The long-term solution is for M.I.W to be updated to target >= preview8.

This change was validated on a Blazor WASM and WebAPI template.

Customer Impact

When a user creates attempts to create a template that uses some sort of AAD authentication, the application will fail to run when the user runs it using dotnet run or F5 on VS. The restore and build steps pass. The error occurs during startup at runtime.

Workaround

To resolve this issue, customers can add the correct package references themselves.

<ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="5.0.0-preview.8.20363.2"/>
    <PackageReference Include="Microsoft.AspNetCore.Authentication.OpenIdConnect" Version="5.0.0-preview.8.20363.2"/>
    <PackageReference Include="Microsoft.Identity.Web" Version="0.2.1-preview" />
  </ItemGroup>

This PR makes sure that the templates are including the correct version of the M.A.A.JwtBearer and M.A.A.OpenIdConnect packages above.

Risk

Low risk because this change has been validated in the Blazor WASM and WebAPI templates and because it's the smallest take we can change to resolve this issue.

cc: @Tratcher @HaoK @JunTaoLuo

eng/Versions.props

captainsafia · 2020-08-05T23:00:14Z

@JunTaoLuo Thanks for explaining how the versioning works for these packages. I've fixed it up. These changes should do the trick but let me know if there is something else.

dougbu

LGTM now but @JunTaoLuo should confirm

ghost · 2020-08-05T23:20:43Z

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

captainsafia · 2020-08-06T00:09:38Z

@JunTaoLuo @dougbu It looks like the template tests are failing because the publish step detects us using the local artifacts as an error during publish. What's the best way to address this?

dougbu · 2020-08-06T00:31:56Z

This issue is related to sorting and it won't impact public or rolling builds because they use real versions.

5.0.0-ci or 5.0.0-dev both sort lower than 5.0.0-preview{more}. Not sure why the temporary project isn't picking the explicit version but that might only work for upgrades. Because Microsoft.Identity.Web is an external package, it may be necessary to special-case local and PR builds i.e. use a specific public release of these two packages when the version would otherwise be 5.0.0-ci or 5.0.0-dev. Put that specific release version in eng/Versions.props and give the properties normal names if that's needed.

➕ @javiercn for his input

src/ProjectTemplates/Web.ProjectTemplates/Microsoft.DotNet.Web.ProjectTemplates.csproj

dougbu · 2020-08-06T02:52:19Z

src/ProjectTemplates/Web.ProjectTemplates/BlazorServerWeb-CSharp.csproj.in

@@ -16,6 +16,11 @@

  <!--#endif -->
  <!--#if (IndividualAuth || OrganizationalAuth) -->
+  <ItemGroup Condition="'$(VersionSuffix)' != 'ci' AND '$(VersionSuffix)' != 'dev'">


Even if it were fine to make the default for local and PR builds be whatever Microsoft.Identity.Web brings in, doesn't this condition appear in the user's project file now❔ Might need another  block.

But I doubt it's fine because it means local and PR builds will see the [Obsolete] warnings. My point was to make e.g. the ${MicrosoftAspNetCoreAuthenticationJwtBearerPackageVersion} substitution unconditional but use a property from eng/Versions.props when '$(VersionSuffix)' == 'ci' OR '$(VersionSuffix)' == 'dev'. Should just need the eng/Versions.props properties and another block in src/ProjectTemplates/Web.ProjectTemplates/Microsoft.DotNet.Web.ProjectTemplates.csproj like

<GeneratedContentProperties Condition=" '$(VersionSuffix)' == 'ci' OR '$(VersionSuffix)' == 'dev' "> $(GeneratedContentProperties); MicrosoftAspNetCoreAuthenticationJwtBearerPackageVersion=$(MicrosoftAspNetCoreAuthenticationJwtBearerPackageVersion); MicrosoftAspNetCoreAuthenticationOpenIdConnectPackageVersion=$(MicrosoftAspNetCoreAuthenticationOpenIdConnectPackageVersion); <GeneratedContentProperties>

Thanks for clarifying! I just updated this branch with a fix.

dougbu · 2020-08-06T19:04:41Z

src/ProjectTemplates/test/MvcTemplateTest.cs

@@ -247,7 +247,7 @@ private async Task MvcTemplateBuildsAndPublishes(string auth, string[] args)
            Assert.True(0 == createResult.ExitCode, ErrorMessages.GetFailedProcessMessage("create/restore", Project, createResult));

            // Verify building in debug works
-            var buildResult = await Project.RunDotNetBuildAsync();
+            var buildResult = await Project.RunDotNetBuildAsync(additionalArgs: "/p:NoWarn=NU1605");


Strange this still occurs. What was the exact warning❔

I'm trying this out as a workaround.

The exact error is:

version 16.7.0-preview-20330-02+1eab2845b for .NET\r\nCopyright (C) Microsoft Corporation. All rights reserved.\r\n\r\nF:\workspace\_work\1\s\.dotnet\sdk\5.0.100-preview.7.20330.3\MSBuild.dll -maxcpucount -property:Configuration=Release -target:Publish -verbosity:m /bl .\AspNet.pm24ssyvh1s.sln\r\n You are using a preview version of .NET. See: https://aka.ms/dotnet-core- preview\r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: Detected package downgrade: Microsoft.AspNetCore.Authentication.JwtBearer from 5.0.0-preview.6.20312.15 to 5.0.0-ci. Reference the package directly from the project to select a different version. \r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: AspNet.pm24ssyvh1s.Server -> Microsoft.Identity.Web 0.2.1-preview -> Microsoft.AspNetCore.Authentication.JwtBearer (>= 5.0.0-preview.6.20312.15) \r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: AspNet.pm24ssyvh1s.Server -> Microsoft.AspNetCore.Authentication.JwtBearer (>= 5.0.0-ci)\r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: Detected package downgrade: Microsoft.AspNetCore.Authentication.OpenIdConnect from 5.0.0-preview.6.20312.15 to 5.0.0-ci. Reference the package directly from the project to select a different version. \r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: AspNet.pm24ssyvh1s.Server -> Microsoft.Identity.Web 0.2.1-preview -> Microsoft.AspNetCore.Authentication.OpenIdConnect (>= 5.0.0-preview.6.20312.15) \r\nF:\workspace\_work\1\s\src\ProjectTemplates\BlazorTemplates.Tests\bin\Release\net5.0\TestTemplates\AspNet.pm24ssyvh1s\Server\AspNet.pm24ssyvh1s.Server.csproj : error NU1605: AspNet.pm24ssyvh1s.Server -> Microsoft.AspNetCore.Authentication.OpenIdConnect (>= 5.0.0-ci)\r\n You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview\r\n You are using a preview version of .NET. See: https://aka.ms/dotnet-core-preview\r\n AspNet.pm24ssyvh1s.Shared ->

I spoke with @pranavkm and he mentioned that the build might be failing in our public builds might be failing because they don't have access to the preview8 packages in the internal feed. When this happens, they fallback to the local verifications even though we have the VersionPrefix check in our build.

I tried this out by submitting an internal PR but it failed with the same reason above. I'm not sure what the issue is here.

I disabled the warning as a last ditch effort since this change is a workaround that we will remove after the preview8 release and I didn't want to overinvest in it.

If you have ideas of cheap fixes for this, let me know.

OIC the problem is the Conditions in https://github.com/dotnet/aspnetcore/pull/24600/files#diff-a02e9b40ed5c9908719840e31ded1de3R45-R46 See suggestion there

I'm pretty sure I had tried a similar condition at some point last night but I'll give this a go.

dougbu · 2020-08-06T20:31:41Z

src/ProjectTemplates/Web.ProjectTemplates/Microsoft.DotNet.Web.ProjectTemplates.csproj

+    <PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\JwtBearer\src\Microsoft.AspNetCore.Authentication.JwtBearer.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />
+    <PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\OpenIdConnect\src\Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />


My bad I think. $(VersionSuffix) is never equal to both ci and dev.

Suggested change

<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\JwtBearer\src\Microsoft.AspNetCore.Authentication.JwtBearer.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />

<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\OpenIdConnect\src\Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />

<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\JwtBearer\src\Microsoft.AspNetCore.Authentication.JwtBearer.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />

<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\OpenIdConnect\src\Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj" Condition=" ! ('$(VersionSuffix)' == 'ci' OR '$(VersionSuffix)' == 'dev') " />

dougbu · 2020-08-06T20:32:08Z

src/ProjectTemplates/test/MvcTemplateTest.cs

@@ -247,7 +247,7 @@ private async Task MvcTemplateBuildsAndPublishes(string auth, string[] args)
            Assert.True(0 == createResult.ExitCode, ErrorMessages.GetFailedProcessMessage("create/restore", Project, createResult));

            // Verify building in debug works
-            var buildResult = await Project.RunDotNetBuildAsync();
+            var buildResult = await Project.RunDotNetBuildAsync(additionalArgs: "/p:NoWarn=NU1605");


OIC the problem is the Conditions in https://github.com/dotnet/aspnetcore/pull/24600/files#diff-a02e9b40ed5c9908719840e31ded1de3R45-R46 See suggestion there

captainsafia · 2020-08-07T15:15:28Z

I did quite a bit of tinkering to resolve the issue we were seeing in CI around the package downgrade.

Ultimately, the only thing that worked was inlining the NoWarn="NU1605" into the package reference in the template. This means that the templates will produce this when users generate an auth-enabled template.

<PackageReference Include="Microsoft.AspNetCore.Authentication.JwtBearer" Version="5.0.0-preview8" NoWarn="NU1605" />

There's probably a way to conditionally set this depending on whether or not a template was generated in a CI-environment or not. If having the NoWarn in the template is a blocker, I can look into this.

Also, there is follow-up work to revert this PR and consume an updated version of the package. Work is tracked here: #24657

mkArtakMSFT · 2020-08-07T15:36:25Z

Thanks @captainsafia.
Given that this is a temporary workaround and the NoWarn will be gone with all the <PackageReference... which required it, when we will revert this change, we should be good here.

…arios (#24600)" This reverts commit 7eb58e0.

* Revert "Add workaround for deprecated overload in authentication scenarios (#24600)" This reverts commit 7eb58e0. * Update identity package versions

Reimplementing the changes originally included in #24600

Reimplementing the changes originally included in #24600 Co-authored-by: Safia Abdalla <[email protected]>

Add workaround for deprecated overload in authentication scenarios

8a80eb7

captainsafia requested a review from dougbu as a code owner August 5, 2020 21:59

captainsafia requested review from HaoK, Tratcher and JunTaoLuo August 5, 2020 22:01

JunTaoLuo reviewed Aug 5, 2020

View reviewed changes

eng/Versions.props Outdated Show resolved Hide resolved

dougbu reviewed Aug 5, 2020

View reviewed changes

eng/Versions.props Outdated Show resolved Hide resolved

Respond to feedback from peer review

f6d4dfa

dougbu approved these changes Aug 5, 2020

View reviewed changes

JunTaoLuo approved these changes Aug 5, 2020

View reviewed changes

mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. Servicing-consider Shiproom approval is required for the issue labels Aug 5, 2020

mkArtakMSFT added this to the 5.0.0-preview8 milestone Aug 5, 2020

mkArtakMSFT added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Aug 6, 2020

captainsafia force-pushed the safia/templ-fixes branch 2 times, most recently from 6dd6134 to 62e7a0d Compare August 6, 2020 01:06

dougbu reviewed Aug 6, 2020

View reviewed changes

src/ProjectTemplates/Web.ProjectTemplates/Microsoft.DotNet.Web.ProjectTemplates.csproj Outdated Show resolved Hide resolved

captainsafia force-pushed the safia/templ-fixes branch from ab0d740 to 2417788 Compare August 6, 2020 01:59

captainsafia requested a review from dougbu August 6, 2020 02:00

captainsafia force-pushed the safia/templ-fixes branch 2 times, most recently from 52ab407 to 65d965e Compare August 6, 2020 02:31

dougbu reviewed Aug 6, 2020

View reviewed changes

captainsafia force-pushed the safia/templ-fixes branch 2 times, most recently from 9457658 to 4f8349b Compare August 6, 2020 03:38

captainsafia force-pushed the safia/templ-fixes branch from fe62ea3 to 0a90427 Compare August 6, 2020 04:39

Use explicit version in Version.props

4a5b5de

captainsafia force-pushed the safia/templ-fixes branch from 0a90427 to 4a5b5de Compare August 6, 2020 05:29

Temporarily disable NU1065 warning

52805c3

dougbu reviewed Aug 6, 2020

View reviewed changes

captainsafia force-pushed the safia/templ-fixes branch 9 times, most recently from 22ca5e5 to 1bfdcff Compare August 7, 2020 02:44

Always use latest package version

3ea5b4e

captainsafia force-pushed the safia/templ-fixes branch from 1bfdcff to 3ea5b4e Compare August 7, 2020 03:26

captainsafia requested a review from pranavkm August 7, 2020 15:09

captainsafia mentioned this pull request Aug 7, 2020

Follow-ups to Microsoft.Identity.Web dependency workaround #24657

Closed

mkArtakMSFT merged commit 7eb58e0 into release/5.0-preview8 Aug 7, 2020

mkArtakMSFT deleted the safia/templ-fixes branch August 7, 2020 15:37

captainsafia added a commit that referenced this pull request Aug 11, 2020

Revert "Add workaround for deprecated overload in authentication scen…

c41f429

…arios (#24600)" This reverts commit 7eb58e0.

captainsafia mentioned this pull request Aug 11, 2020

Revert identity package workaround and bump versions #24819

Merged

captainsafia added a commit that referenced this pull request Aug 26, 2020

Revert "Add workaround for deprecated overload in authentication scen…

a7a9c45

…arios (#24600)" This reverts commit 7eb58e0.

JunTaoLuo pushed a commit that referenced this pull request Sep 28, 2020

Lift OIDC and JwtBearer dependencies in templates

31f92f7

Reimplementing the changes originally included in #24600

JunTaoLuo mentioned this pull request Sep 28, 2020

Lift OIDC and JwtBearer dependencies in templates #26390

Merged

Pilchie pushed a commit that referenced this pull request Sep 29, 2020

Lift OIDC and JwtBearer dependencies in templates (#26390)

e62bd65

Reimplementing the changes originally included in #24600 Co-authored-by: Safia Abdalla <[email protected]>

		<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\JwtBearer\src\Microsoft.AspNetCore.Authentication.JwtBearer.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />
		<PackageVersionVariableReference Include="$(RepoRoot)src\Security\Authentication\OpenIdConnect\src\Microsoft.AspNetCore.Authentication.OpenIdConnect.csproj" Condition="'$(VersionSuffix)' != 'ci' OR '$(VersionSuffix)' != 'dev'" />

Add workaround for deprecated overload in authentication scenarios #24600

Add workaround for deprecated overload in authentication scenarios #24600

Uh oh!

Conversation

captainsafia commented Aug 5, 2020 • edited by mkArtakMSFT Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Customer Impact

Workaround

Risk

Uh oh!

Uh oh!

Uh oh!

captainsafia commented Aug 5, 2020

Uh oh!

dougbu left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Aug 5, 2020

Uh oh!

captainsafia commented Aug 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dougbu commented Aug 6, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

captainsafia commented Aug 7, 2020

Uh oh!

mkArtakMSFT commented Aug 7, 2020

Uh oh!

Uh oh!

captainsafia commented Aug 5, 2020 •

edited by mkArtakMSFT

Loading

captainsafia commented Aug 6, 2020 •

edited

Loading