dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.58k stars 25.3k forks source link

If the SUT uses a Bearer Authentication Scheme with IdentityServer, "Bearer" scheme already exists #20271

Open ghost opened 3 years ago

ghost commented 3 years ago

When looking at this document, it appears to work fine when using a basic authentication scheme, but when the Startup of the SUT contains the following, and an attempt is made to use a custom Bearer Auth scheme as per the doc, the error "Bearer" scheme already exists:

services .AddAuthentication(IdentityServerAuthenticationDefaults.AuthenticationScheme) .AddIdentityServerAuthentication(options => { options.Authority = "https://myidentityserver.com"; });

Is there a simple way to override the IdentityServer configuration from the SUT?


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

Rick-Anderson commented 3 years ago

Authentication is beyond the scope of this doc.

Reisclef commented 3 years ago

I suspected that would be the case, though seems a bit odd considering the Mock-Authentication section goes into it briefly. I understand that it seems to be a bit more complicated in this regard, but just wanted to flag it.

Rick-Anderson commented 3 years ago

If we get more responses we might reconsider. Flagging @javiercn and @guardrex to see if they'd like to invest in this area

ghost commented 3 years ago

Thanks @Rick-Anderson for your consideration.

For those that potentially stumble across this issue in the future, I can work around the issue by defining a private IWebHostEnvironment as described here, not adding the custom handler unless the environment is a custom environment, and defining said custom Environment in my test project:

IntegrationTestsWebApplicationFactory:

protected override void ConfigureWebHost(IWebHostBuilder builder)
{
    builder.UseEnvironment("IntegrationTests");
}

Startup.cs:


public Startup(........., IWebHostEnvironment env)
{
      _env = env;
}
public void ConfigureServices(IServiceCollection services)
{
    if (_env.EnvironmentName != "IntegrationTests")
    {
        services .AddAuthentication(IdentityServerAuthenticationDefaults.AuthenticationScheme) 
            .AddIdentityServerAuthentication(options => { options.Authority = "https://myidentityserver.com"; });
    }
}
Rick-Anderson commented 3 years ago

@richard-access Thanks for the update.

scottsauber commented 3 years ago

I also ran into this for what it's worth. I think the confusing part of this document is it says the following:

It's important for the Test scheme to match the scheme your app expects. Otherwise, authentication won't work.

The problem is if you match the names as it describes and register the same scheme name in the ConfigureTestServices that matches your app's ConfigureServices then you get System.InvalidOperationException : Scheme already exists: Bearer error (or instead of Bearer it will say whatever the auth scheme is). So you have to do what Richard describes where you branch the logic in the ConfigureServices.

I think it would be helpful to mention something to this effect or if there's a better way to do this I'm all ears. Happy to send a PR if there's a desired approach here.

Rick-Anderson commented 3 years ago

@blowdart are you OK with a PR with the comments @scottsauber proposes?

blowdart commented 3 years ago

Sure

scottsauber commented 3 years ago

Cool - I'll work on a PR and probably send one over tomorrow. Thanks guys!

For anyone coming here later, here's a repo with the approach. The first commit is essentially dotnet new with Azure AD and the second commit has the working integration test. The key was (as Richard described) branching in Startup.

Again - totally open to any better approaches.

I tried to unregister the specific AuthenticationOptions for that scheme in the ConfigureServices of the WebApplicationFactory but was unable to figure that out right now. I can unregister all the AuthenticationOptions such as below, but wasn't sure how to target the specific scheme. Doing the below I think would unregister all AuthenticationSchemes which may not be desired. I think it'd be nice if I could figure out this approach so the Production code doesn't need changed at all, although it definitely comes with a bit more complexity.

builder.ConfigureServices(services =>
{
    var descriptors = services.Where(x => x.ServiceType == typeof(IConfigureOptions<AuthenticationOptions>)).ToList();
    foreach (var descriptor in descriptors)
    {
        services.Remove(descriptor);
    }
}
Rick-Anderson commented 3 years ago

@scottsauber ping

scottsauber commented 3 years ago

@Rick-Anderson - Doh, sorry I lost track of tis. I'll add it to my to do list for this weekend.

Rick-Anderson commented 3 years ago

Put yourself as 2nd author

By [Javier Calvarro Nelson](https://github.com/javiercn), [Scott Sauber](https://twitter.com/scottsauber), [Steve Smith](https://ardalis.com/), and [Jos van der Til](https://jvandertil.nl)
Rick-Anderson commented 2 years ago

@scottsauber @scottsauber ping

Rick-Anderson commented 2 years ago

@scottsauber @scottsauber ping

scottsauber commented 2 years ago

@scottsauber @scottsauber ping

Doh. I'll look at this tonight @Rick-Anderson. Sorry for dropping the ball terribly on this.

scottsauber commented 2 years ago

Hitting some goofiness with 3.1 locally while I'm testing this. Will pick this up tomorrow.

Rick-Anderson commented 2 years ago

3.1 what, not .NET I hope.

scottsauber commented 2 years ago

Changing this one which is still on 3.1.

I got it sorted, had a preview of .NET 6 that was causing an issue.

Do you want me to upgrade that project while I'm at it?

Rick-Anderson commented 2 years ago

It's best to leave that and create a 6.x folder and sample. AspNetCore.Docs/aspnetcore/test/integration-tests/samples/6.x/IntegrationTestsSample/src/RazorPagesProject/

Rick-Anderson commented 2 years ago

@scottsauber take a look at https://github.com/dotnet/AspNetCore.Docs/pull/27116/files

Basic authentication tests is not a .NET repository but was written by a member of the .NET team. It provides examples of basic authentication testing.

Maybe we should just link to blowdart's sample?

Rick-Anderson commented 1 year ago

@scottsauber take a look at https://github.com/dotnet/AspNetCore.Docs/pull/27116/files

Basic authentication tests is not a .NET repository but was written by a member of the .NET team. It provides examples of basic authentication testing.

Maybe we should just link to blowdart's sample?

cremor commented 1 year ago

I can unregister all the AuthenticationOptions such as below

@scottsauber Thanks for this. I think this is a reasonable minimal change that should be included in the documentation article. It might not be the best way to do it, but it is a quick change that doesn't required any modification of production code.

As it is right now, that section in the documentation article is not useful since it will always result in an exception.