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

Allow a Route to specify more than one authentication scheme #740

Closed
philproctor opened this issue Jan 8, 2019 · 14 comments · Fixed by #1870
Closed

Allow a Route to specify more than one authentication scheme #740

philproctor opened this issue Jan 8, 2019 · 14 comments · Fixed by #1870
Assignees
Labels
feature A new feature merged Issue has been merged to dev and is waiting for the next release Nov'23 November 2023 release

Comments

@philproctor
Copy link
Contributor

New Feature

Allow a reroute to specify more than one authentication scheme for a reroute. Specification could either be JSON array or comma separated.

Motivation for New Feature

Sometimes more than one authentication scheme may be allowed for an API (e.g. JWT + Client Certificates). This feature would allow more than one scheme to be use on a single reroute eithout requiring a custom override of the authentication pipeline..

@philproctor philproctor added the feature A new feature label Jan 8, 2019
@philproctor philproctor self-assigned this Jan 8, 2019
@philproctor philproctor added the needs feedback Issue is waiting on feedback before acceptance label Jan 9, 2019
@TomPallister
Copy link
Member

TomPallister commented Jan 13, 2019

@philproctor can you already do this when you provide the authentication scheme to .NET? E.g. you provide a scheme with a key that works with JWT + Client Certificates? If not I would say this is more something the .NET authentication world should support? Not something we should look to add to Ocelot?

All Ocelot does it defer to .NET here. I am reluctant to do anything else.

@feugen24
Copy link

feugen24 commented Jul 22, 2019

@TomPallister we use 3 authentications: AddCookie, AddOpenIdConnect, AddJwtBearer and some code so that they are applied automatically with [Authorize] attribute.
Link: Asp Core: Azure Ad Auth + custom JWT + custom Identity store

            services.AddMvc(options =>
            {
                var policy = new AuthorizationPolicyBuilder()
                    .RequireAuthenticatedUser()
                    .AddAuthenticationSchemes(CookieAuthenticationDefaults.AuthenticationScheme,
                        JwtBearerDefaults.AuthenticationScheme, OpenIdConnectDefaults.AuthenticationScheme)
                    .Build();
                options.Filters.Add(new AuthorizeFilter(policy));
            });

If I try to register all of them with the same scheme name in AddCookie(...), AddXXX(...), I get an error: InvalidOperationException: Scheme already exists.
So I don't know if it's possible for Ocelot to forward requests with multiple authorization types unless this feature is supported?

@MayorSheFF
Copy link
Contributor

MayorSheFF commented Apr 23, 2022

Hi Everyone.

We are using several authentication schemes too. Could you please tell if you have any info how to specify all of them for one Route.

Thank you a lot.

@samuelcoutu
Copy link

samuelcoutu commented Oct 12, 2022

Hi @raman-m

I think this is supported by .NET. We can specify multiple authentication schemes for a route:

[Authorize(AuthenticationSchemes = CookieAuthenticationDefaults.AuthenticationScheme + "," + JwtBearerDefaults.AuthenticationScheme)]
public class MyController: Controller
{
}

I think the same should be supported in Ocelot.

@michaellperry
Copy link

The method that I use in ASP.NET to accept multiple authentication schemes is to add them all to the same authorization policy. The ASP.NET Authorize attribute names the authorization policy, not the authentication scheme. If Ocelot let me specify the name of an authorization policy instead of an authentication scheme, then I could achieve this result.

Is that something that can be reasonably added?

@raman-m
Copy link
Member

raman-m commented Jun 8, 2023

@TomPallister Tom,
When Phil says specify more than one authentication scheme it looks like very abstract. But he had proposed to consider "Client Certificates" authentication scheme. Could it be concrete here? Truly speaking I don't understand what the "Client Certificates"authentication standard is? How is it implemented and supported by .NET ecosystem?
Can we accept current description?
I believe Phil needs to provide more details on "Client Certificates" in .NET.

@philproctor Hi Phil!
Do you have some preliminary solutions?
The people provides some info for CookieAuthenticationDefaults and OpenIdConnectDefaults authentication schemes.
And Ocelot has JwtBearerDefaults authentication scheme already being implemented.
Could you share more details on "Client Certificates" in .NET please? I am not familiar with that auth-scheme at all?
Is this supported in .NET world at all?
A sooner feedback is better! And we can accept this feature request.
Keep in mind that any PR with ready solution will make the issue accepted automatically.

@raman-m raman-m added question Initially seen a question could become a new feature or bug or closed ;) medium effort Likely a few days of development effort waiting Waiting for answer to question or feedback from issue raiser labels Jun 8, 2023
@raman-m raman-m changed the title Enhancement: Allow a reroute to specify more than one authentication scheme Allow a Route to specify more than one authentication scheme Jun 8, 2023
@raman-m raman-m added accepted Bug or feature would be accepted as a PR or is being worked on and removed question Initially seen a question could become a new feature or bug or closed ;) medium effort Likely a few days of development effort needs feedback Issue is waiting on feedback before acceptance waiting Waiting for answer to question or feedback from issue raiser labels Jan 7, 2024
@raman-m raman-m added this to the Nov-December'23 milestone Jan 7, 2024
@raman-m raman-m added the Nov'23 November 2023 release label Jan 7, 2024
@raman-m
Copy link
Member

raman-m commented Jan 8, 2024

@michaellperry commented on Jan 24, 2023:

Hi Michael !
Yes, authentication settings could be based either on auth-schemes or auth-policies. But here in this #740 and in #1580 people want to provide Auth settings based on auth schemes.

Could you share an example code with authorization policy approach please which you like to use?

P.S.

One of these days we are going to close this issue as implemented... The linked PR is ready on 95%.
You could give us a feedback on design 😋

@raman-m
Copy link
Member

raman-m commented Jan 13, 2024

@samuelcoutu commented on Oct 12, 2022

The same cannot be supported by Ocelot, because Ocelot has no controllers at all except very system ones.
It seems you are mixing responsibilities of entire ASP.NET pipeline and concrete controller class.
So, whole Auth feature should be useful for entire app pipeline.

And Yes, a controller can support multiple auth schemes but based on those schemes which were described for the ASP.NET pipeline. Otherwise it doesn't work. So you need to specify schemes and inject their services to DI in app start logic, and then they will be available in all controllers.

@raman-m
Copy link
Member

raman-m commented Jan 13, 2024

@feugen24 commented on Jul 22, 2019:

So I don't know if it's possible for Ocelot to forward requests with multiple authorization types unless this feature is supported?

Not auth types, but auth artifacts: URL query string parameters, request headers, and form properties.
As a far as you may know, auth token body can be sent as query param, header value and form data property.

Ocelot routes query params and headers for sure. Unfortunately Ocelot doesn't forward HttpRequest form data. This is a major design flaw that we will soon fix in a few releases.

@raman-m raman-m assigned raman-m and MayorSheFF and unassigned philproctor Jan 17, 2024
@michaellperry
Copy link

michaellperry commented Jan 25, 2024

@raman-m commented on Jan 8:

Could you share an example code with authorization policy approach please which you like to use?

Here's an example configuration that configures JWT and cookie-based authentication, and then defines an authorization policy that accepts either.

// Add authentication using JWT bearer tokens
builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
    .AddJwtBearer(options =>
    {
        options.TokenValidationParameters = new TokenValidationParameters
        {
            ValidateIssuer = true,
            ValidateAudience = true,
            ValidateLifetime = true,
            ValidateIssuerSigningKey = true,
            ValidIssuer = context.Configuration["Jwt:Issuer"],
            ValidAudience = context.Configuration["Jwt:Audience"],
            IssuerSigningKey = new SymmetricSecurityKey(Encoding.UTF8.GetBytes(context.Configuration["Jwt:Key"]))
        };
    });
// Add another using cookies
builder.Services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
    .AddCookie(options =>
    {
        options.Cookie.Name = "MyCookie";
        options.Cookie.SameSite = SameSiteMode.Strict;
        options.Cookie.SecurePolicy = CookieSecurePolicy.Always;
        options.Cookie.HttpOnly = true;
        options.Cookie.IsEssential = true;
        options.Cookie.MaxAge = TimeSpan.FromMinutes(30);
        options.LoginPath = "/Login";
        options.LogoutPath = "/Logout";
        options.AccessDeniedPath = "/AccessDenied";
        options.ReturnUrlParameter = "returnUrl";
        options.SlidingExpiration = true;
        options.ExpireTimeSpan = TimeSpan.FromMinutes(30);
    });
// Define an authorization policy named "Either" that accepts either JWT or cookie authentication
builder.Services.AddAuthorization(options =>
{
    var policy = new AuthorizationPolicyBuilder(new []
        {
            JwtBearerDefaults.AuthenticationScheme,
            CookieAuthenticationDefaults.AuthenticationScheme
        })
        .RequireAuthenticatedUser()
        .Build();
    options.AddPolicy("Either", policy);
});

After setting up this policy, I might configure my route like this:

{
    "Routes": [
        {
            "DownstreamPathTemplate": "/api/service1",
            "DownstreamScheme": "https",
            "DownstreamHostAndPorts": [
                {
                    "Host": "localhost",
                    "Port": 5001
                }
            ],
            "UpstreamPathTemplate": "/service1",
            "UpstreamHttpMethod": [ "Get" ],
            "AuthorizationPolicy": "Either"
        }
    ],
    "GlobalConfiguration": {
        "BaseUrl": "https://localhost:5000"
    }
}

That would be equivalent to placing an attribute on my controller action [Authorize(Policy = "Either")]

@raman-m
Copy link
Member

raman-m commented Jan 26, 2024

@michaellperry Thank you for the sample!

After setting up this policy, I might configure my route like this:

{
    "Routes": [
        {
            "AuthorizationPolicy": "Either"
        }
    ],
}

That would be equivalent to placing an attribute on my controller action [Authorize(Policy = "Either")]

Hmm... I'm very sorry, but Ocelot has no the AuthorizationPolicy property in it's specs and docs!
We have open issue #1159 but it is not in development now...
Did you develop custom AuthorizationMiddleware and inject it to Ocelot pipeline?

@michaellperry
Copy link

Hmm... I'm very sorry, but Ocelot has no the AuthorizationPolicy property in it's specs and docs! We have open issue #1159 but it is not in development now... Did you develop custom AuthorizationMiddleware and inject it to Ocelot pipeline?

I must have misunderstood your request for example code. My intent was to show you what code I would like to be able to write using the authorization policy approach. This would be one way to add a feature to support multiple authentication schemes. It would also have the benefit of providing additional flexibility by taking advantage of a feature already built into the ASP.NET authorization pipeline.

Thanks for pointing me to the feature. That would certainly resolve this issue. If I were to work on a PR for that, would you be able to bump the priority?

@raman-m
Copy link
Member

raman-m commented Feb 5, 2024

@michaellperry Michael,
In general, I understand your idea, but it will be new feature request. And sure this new feature is out of the scope of current #1580 and this #740 feature requests.
So, as a team, we are sure that Ocelot should not be responsible for any kind of authentication artifacts creation: tokens, headers, etc.

  • Ocelot should not sign any tokens
  • Ocelot should not sign any cookies

So, Ocelot is able to route anonymous HTTP traffic down to the target service and downstream services can sign a cookie.
So, there will be 2 routes always, if downstream is responsible for token/cookie creation, and it has an endpoint to create token/cookie.


Thanks for pointing me to the feature. That would certainly resolve this issue. If I were to work on a PR for that, would you be able to bump the priority?

Yes, we would! We welcome any contributions.
Sorry, what issue would you like to work on?

@ggnaegi FYI

@michaellperry
Copy link

Yes, we would! We welcome any contributions. Sorry, what issue would you like to work on?

That would be #1159, Add a list of AuthorizationPolicies to ReRoutes. It looks like that stalled after the initial suggestion. And it looks like it would resolve this issue. I'll take a deeper look and send you a proposal.

raman-m added a commit that referenced this issue Feb 5, 2024
* #1580. Added an opportunity to use several authentication provider keys.

* Update src/Ocelot/Configuration/Builder/AuthenticationOptionsBuilder.cs

Co-authored-by: Raynald Messié <[email protected]>

* #1580. Replaced AuthenticationProviderKeys type from the list to the array.

* #1580. Added a doc how to use AuthenticationProviderKeys in a Route.

* #1580. Amended the description how to use AuthenticationProviderKeys in a Route.

* #1580. Added an opportunity to use several authentication provider keys.

* Update src/Ocelot/Configuration/Builder/AuthenticationOptionsBuilder.cs

Co-authored-by: Raynald Messié <[email protected]>

* #1580. Replaced AuthenticationProviderKeys type from the list to the array.

* #1580. Added a doc how to use AuthenticationProviderKeys in a Route.

* #1580. Amended the description how to use AuthenticationProviderKeys in a Route.

* Quick review

* #1580. Implemented review points.

* #1580. Initialized result with AuthenticateResult.NoResult().

* #1580. Added @ggnaegi suggestions.

* #1580. Brought back the idea not to allocate AuthenticateResult instance.

* quick review

* Return Auth result of the last key in the collection

* review unit tests

* Enable parallelization of unit tests

* Fix messages

* Disable parallelization for PollyQoSProviderTests

* Switch off unstable test

* Re-enable parallelization & Isolate unstable test

* Reflection issue in middleware base: remove getting Type object

* Switch off unstable test

* Clean code

* Make MiddlewareName as public property

* Code review by @RaynaldM

* AuthenticationMiddleware: Line & Branch coverage -> 100%

* AuthenticationOptionsCreator: coverage -> 100%

* Remove private helpers with one reference

* RouteOptionsCreator: coverage -> 100%

* FileAuthenticationOptions: Refactor ToString method

* FileConfigurationFluentValidator: coverage -> 100%

* RouteFluentValidator: Branch coverage -> 100%

* TODO and Skip unstable test

* Move acceptance tests to the separate folder

* Review and refactor acceptance tests

* Add AuthenticationSteps class.
Choose inheritance over composition: less code

* Add 'GivenIHaveATokenWithScope' to 'AuthenticationSteps'

* Temporarily disable 'Should_timeout_per_default_after_90_seconds' test

* Add CreateIdentityServer method

* Add draft test

* Update route validator to support multiple auth schemes

* Acceptance tests

* Revert "TODO and Skip unstable test"

This reverts commit 1ec8564.

* Revert "Make MiddlewareName as public property"

This reverts commit 6f50c76.

* Revert "Reflection issue in middleware base: remove getting Type object"

* Clean up

* Isolate unstable test

* Mark old property as `Obsolete`

* a tiny little bit of cleanup

* Handling cases when principal or identity are null

* Update Authentication feature docs

* Convert back to block scoped namespace

---------

Co-authored-by: Igor Polishchuk <[email protected]>
Co-authored-by: Raman Maksimchuk <[email protected]>
Co-authored-by: Raynald Messié <[email protected]>
Co-authored-by: Igor Polishchuk <[email protected]>
Co-authored-by: Guillaume Gnaegi <[email protected]>
@raman-m raman-m added merged Issue has been merged to dev and is waiting for the next release and removed accepted Bug or feature would be accepted as a PR or is being worked on labels Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature merged Issue has been merged to dev and is waiting for the next release Nov'23 November 2023 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants