Code Monkey home page Code Monkey logo

Comments (10)

kevinchalet avatar kevinchalet commented on September 21, 2024 1

Since that should not be needed any more given the reverse proxy forwards. #864 (comment)

I suspect the issue with HttpRequest.Scheme not being https is caused by the fact app.UseForwardedHeaders() is called too late to have an effect on OpenIddict's endpoints: try to move it before app.UseAuthentication() (ideally, even just after var app = builder.Build();).

Could this have any effect on the first issue or is that not related?

When using ASP.NET Core Data Protection tokens, there's no issuer validation (just like ASP.NET Core's cookie handler), but when using the JWT tokens, there's indeed an issuer validation that takes place and could contribute to considering a token invalid. We'll know for sure what's causing that when taking a look at the logs πŸ˜ƒ

from openiddict-core.

jeroenbai avatar jeroenbai commented on September 21, 2024 1

TL;DR: in a multi-server setting, it is required to set a encryption certificate that is the same on both servers, also when not using JWT access/refresh token encryption .DisableAccessTokenEncryption() , because there are other things that do get encrypted and can move between servers, such as the authorization code. .DisableAccessTokenEncryption() threw me a curve ball making me think I would not need a proper encryption certificate.


Looks like I was able to fix it, tested on localhost running 2 openiddict servers on different ports, Postman calling /connect/authorize on server A and calling /connect/token on server B, forcing using 2 different servers for these calls, that works now.

The issue was:

  • requirement was JWT so we disabled DataProtection
  • in the current setup, accesstokens are (for now) signed but not encrypted
  • initializing the server requires to specify both a signing and encryption certificate/key in the options
  • since we are not using JWT encryption at this stage (.DisableAccessTokenEncryption() ), we thought we didnt need an encryption certificate, but it is mandatory in the options, so we just used AddEphemeralEncryptionKey.
  • but we realized (viewing trace log) that of course the authorization code is encrypted in the authorize endpoint (server A) and then decrypted/validated in the token endpoint (server B). And those servers each use a different EphemeralEncryptionKey so that of course failed.

I still need to see it work on k8s AWS pods, but this is so obvious that I have no doubts it will work there as well with a proper encryption certificate that is the same on both pods.

Thanks for pointing me in the right direction, and keep up the good work.

from openiddict-core.

kevinchalet avatar kevinchalet commented on September 21, 2024 1

Thanks for taking the time to document it, @jeroenbai ❀️

from openiddict-core.

jeroenbai avatar jeroenbai commented on September 21, 2024 1

Confirmed on k8s as well

from openiddict-core.

kevinchalet avatar kevinchalet commented on September 21, 2024

Hi @jeroenbai,

Thanks a lot for sponsoring the project! πŸ‘πŸ»

After research, we found we probably need to AddDataProtection
https://stackoverflow.com/a/70323285
https://stackoverflow.com/a/68697338
https://stackoverflow.com/questions/70318415/how-to-properly-configure-openiddict-with-asp-net-core-dataprotection#comment124311728_70323285

Actually, calling options.UseDataProtection() is not mandatory: it's only needed when you want to generate and/or validate opaque tokens using ASP.NET Core Data Protection instead of IdentityModel (that produces JWT tokens). When using ASP.NET Core Data Protection to generate opaque tokens, the signing and encryption credentials registered via Add*Key/Certificate() are not used and ASP.NET Core Data Protection uses its own key ring, that must be kept in sync between all the instances.

What you have should work, but I'd try to uncomment //.SetApplicationName("AppName") to rule out a path difference between instances.

Any insights regarding our setup related to openiddict or asp.net would be appreciated.

Any chance you could lower the default log level to Trace and post the logs? It would help figure out what's the handler rejecting the token πŸ˜ƒ

Cheers.

from openiddict-core.

jeroenbai avatar jeroenbai commented on September 21, 2024

Thanks, I disabled UseDataProtection as we want to use specific signing certs. Waiting for retesting and logging.

One other thing that may or may not cause this, to rule this out: the openiddict server runs http (port 80) on a pod, TLS is added at the load balancer and we added ForwardedHeadersOptions in the code:

{
    ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto | ForwardedHeaders.XForwardedHost
}

Logging shows that indeed, the hostname and protocol are the external domain host name, and https, by logging HttpContext.Request.Scheme and HttpContext.Request.Host.

So I tried to remove .DisableTransportSecurityRequirement()

Since that should not be needed any more given the reverse proxy forwards. #864 (comment)

However when I remove .DisableTransportSecurityRequirement() and deploy to the server running port 80 behind load balancer that adds TLS, then https://redacted/.well-known/openid-configuration (external url) serves:

{
  "error": "invalid_request",
  "error_description": "This server only accepts HTTPS requests.",
  "error_uri": "https://documentation.openiddict.com/errors/ID2083"
}

The thing is, if I leave .DisableTransportSecurityRequirement() in the code, then the /.well-known/openid-configuration works, but has a few http urls:

"issuer": "https://redacted/",
  "authorization_endpoint": "http://redacted/connect/authorize",
  "token_endpoint": "http://redacted/connect/token",
  "userinfo_endpoint": "http://redacted/connect/userinfo",
  "jwks_uri": "http://redacted/.well-known/jwks",
  "grant_types_supported": [
    "urn:ietf:params:oauth:grant-type:saml2-bearer",
    "authorization_code",
    "refresh_token"
  ],

Could this have any effect on the first issue or is that not related?

from openiddict-core.

jeroenbai avatar jeroenbai commented on September 21, 2024

Placing the forwardedheaders code directly after the var app = builder.Build(); solved the issue regarding http urls in the .well-known/openid-configuration, thanks!

We are still encountering the same error though invalid_grant: The specified token is invalid. - https://documentation.openiddict.com/errors/ID2004 , logs are in your inbox.

from openiddict-core.

kevinchalet avatar kevinchalet commented on September 21, 2024

Placing the forwardedheaders code directly after the var app = builder.Build(); solved the issue regarding http urls in the .well-known/openid-configuration, thanks!

Great to hear πŸ‘πŸ»

We are still encountering the same error though invalid_grant: The specified token is invalid. - https://documentation.openiddict.com/errors/ID2004 , logs are in your inbox.

Just replied (TL;DR, the logs you shared are limited to errors and informational messages, which isn't enough to figure out what's going on).

from openiddict-core.

kevinchalet avatar kevinchalet commented on September 21, 2024

@jeroenbai awesome πŸ‘πŸ»

from openiddict-core.

ninety7 avatar ninety7 commented on September 21, 2024

Update2: I dug deep into aspnet core's source code and found that since dotnet 7.0, both UseAuthentication() and UseAuthorization() methods are called by default, and set at the beginning of the pipeline when using the WebApplicationBuilder, at least. I had to call them explicitly in order to be able to properly process forwarded headers. It's a heads-up for people who run into this issue! Feel free if to create a doc page with this info to share with others!


Update1: I was using the dotnet 8.0 blazor template, and just found out that it does not use app.UseAuthentication(). I found out that OpenIddict implements an authentication handler, so after adding this call, it works as expected. Now my question is, how did OpenIddict work without the UseAuthentication() call?

According to this article, since .NET 7, we don't need to call UseAuthentication() or UseAuthorization() as it will be done by the builder, but this is causing confusion, and a bug when we don't call them for OpenIddict to properly generate the baseUrl.


@kevinchalet I know this issue has been closed, but wanted to report a possible bug that is present in the v5 builds (5.0.0, 5.0.1, 5.1.0) of OpenIddict. I'm trying to set up Forwarded Headers on an Aspnet core minimal api, and tried all possible ways of achieving this according to Microsoft's Guide. The only way I have been able to make it work is to set the ASPNETCORE_FORWARDEDHEADERS_ENABLED env variable to true, and sending the X-Forwarded-Proto: https header on the request.

This env var is supposed to save devs the need to configure forwarded header options and call the UseForwardedHeaders() method, but what I suspect is that when using this env var, the host properly calling this the forwarded headers middleware properly at the beginning of the request. I don't understand how OpenIddict is bootstrapped/inserted into the request pipeline, as it is configured before the builder.Build() call and there's no app.UseOpenIddict extension methods after it.

It seems that no matter what you do or where you call the UseForwardedHeaders() method, the openiddict handlers/middleware will alwasy come first, unless you set the ASPNETCORE_FORWARDEDHEADERS_ENABLED env var.

I also tried to set a breakpoint when overriding the request scheme:

var app = builder.Build();

app.Use((context, next) =>
{
    context.Request.Scheme = "https"; // Breakpoint here
    return next(context);
});

app.UseForwardedHeaders();

But it will never hit when calling the .well-known/openid-configuration endpoint. It will hit when I call my own api endpoints.

I hope this helps you determine whether this is a bug or not, or at least help me figure out if I'm making a dumb configuration mistake.

I did not try this with OpenIddict v4

from openiddict-core.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.