-
-
Notifications
You must be signed in to change notification settings - Fork 527
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
Breaking changes in OpenIddict 4.0 impacting how URIs are handled #1613
Comments
Note: if you want to give it a try, the new behavior is present in the 4.0 rc1 bits, whose nightly builds can be found on the MyGet feed. It should land on NuGet.org very soon. |
Hi, we have tried to migrate our cloud instance from v4.0.0-preview4.22517.68 to v4.0.0 release. I have checked the HttpRequest and I think the problem is that the endpoint is configured to be "https://ourwebsite.com/connect/token" however the request contains "http://ourwebsite.com/connect/token". Our service is behind an NGINX reverse proxy and we use TLS termination for every site. The client is a SPA. We configure our endpoints using the Header forwarding did not help as well. How is it possible to configure the OpenIddict provider to match the request regardless of the schema? Thank you in advance. EDIT: forgot to mention that the preview worked without a problem for the same configuration. |
@danielleiszen hi. The change you describe is expected and is caused by an invalid configuration of your ASP.NET application (if you check |
thanks for the prompt response. if I understand correctly:
since https is changed to http due to TLS termination by NGINX, we can explicitly use a custom middleware to replace the schema in every request to https. am I right or do you see any other option? thank you |
@kevinchalet @danielleiszen I have exactly the same issue and I'm not sure how to solve it. As SetIssuer doesn't set the base URI anymore, I don't know how to override let's say SetTokenEndpointUris. Any idea what I'm doing wrong? Current code that was working in version 4 preview:
|
@AmbroiseCouissin the root cause is the same: your ASP.NET Core application or the proxy is/are not correctly configured to flow the right headers up to ASP.NET Core. Read https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-7.0 for more information. I'm going to close this ticket. If you still need help, consider sponsoring the project and a create a new dedicated ticket. |
How will this work for instance where the default port isn't being used? Like if we are running locally in IIS Express all apps trying to use this would fail because the .well-known/openid-configuration endpoint is missing the port for all the endpoints making running all apps locally fail. |
Non-default ports are supported. If you’re seeing a different behavior, open a support ticket (sponsors-only). |
Initially planned for OpenIddict 5.0, the modifications introducing behavior changes in the URIs handling will finally ship as part of 4.0.
Before detailing these changes, here's a bit of history to understand the motivations for changing the existing behavior:
OpenIddict 1.x/2.x exclusively used ASP.NET Core's
PathString
primitive type as a way to represent endpoint addresses. As a consequence, only relative paths were supported and, as required byPathString
's constructor, paths had to start with a leading/
(e.g/connect/authorize
). This was simple and unambiguous, but sadly quite limited.As part of the ASP.NET Core decoupling, OpenIddict 3.0 stopped using the ASP.NET Core-specific
PathString
type to represent endpoint addresses and started usingSystem.Uri
, which was also a good opportunity to support absolute URIs (e.g if you want to separate the userinfo endpoint from your main authorization server, you can simply calloptions.SetUserinfoEndpointUris("https://api.domain.com/userinfo")
with an absolute URI and OpenIddict will return it as-is in the discovery document (userinfo requests won't be served by OpenIddict itself when the host of the specified address differs from the one specified in the received requests).To avoid a breaking change at the time, the same semantics as the ones defined by
PathString
were preserved, so things worked similarly as in previous versions: relative URIs still had to start with a leading/
and none of the typical canonicalization rules of URIs applied (e.g you couldn't start your endpoint address with./
or have..
in them).Retrospectively, not adopting that breaking change was not a good idea, as relative
System.Uri
instances starting with a/
don't have the same meaning asPathString
(that always starts with a/
): unlikePathString
, a leading/
in aSystem.Uri
always indicates a site/root-relative URI, which is the typical behavior you also see with URLs in a HTML document (e.g if the current URL ishttps://domain.com/path/
, a relative link todocument.html
will point tohttps://domain.com/path/document.html
while a relative link to/document.html
will point tohttps://domain.com/document.html
).These inconsistencies contributed to confusing users - who often asked why things didn't work when they used
options.SetAuthorizationEndpointUris("authorize")
instead ofoptions.SetAuthorizationEndpointUris("/authorize")
- so the behavior in OpenIddict 4.0 will change to fully embrace theSystem.Uri
rules for relative URIs and reduce the confusion:Relative endpoint URIs will now follow the
System.Uri
rulesTwo cases exist:
HttpRequest.PathBase
is equal tostring.Empty
It's the most common scenario. In this case, the application root is
http://domain.com/
so if you keep using/connect/authorize
as an endpoint address, nothing will change compared to 3.x as the resulting absolute URI will behttp://domain.com/connect/authorize
.If you decide to use
./connect/authorize
orconnect/authorize
or even../connect/authorize
, the resulting address will still behttp://domain.com/connect/authorize
.HttpRequest.PathBase
is not equal tostring.Empty
It's typically the case when using IIS virtual application paths or when using certain ASP.NET Core middleware (built-in or not) that change
HttpRequest.PathBase
to a specific value (e.g for multi-tenant scenarios). In this case, the application root ishttp://domain.com/pathbase
so if you use/endpoint
(which is a root-relative address), the resulting address will behttp://domain.com/endpoint
and nothttp://domain.com/pathbase/endpoint
.To keep the same final address, users will have to change the endpoint address to
endpoint
or./endpoint
instead of/endpoint
.As a positive consequence, users are now able to specify relative addresses that are outside the current application. For instance, an
HttpRequest.PathBase
of/tenantA/authorization-server
with an address of../userinfo
will result in an absolute URI ofhttp://domain.com/tenantA/userinfo
, which can be useful when the userinfo API is not in the same application as the authorization server.The configuration endpoint (
.well-known/openid-configuration
) no longer uses theOpenIddictServerOptions.Issuer
property as the base URI to compute absolute URIsIn 4.0, the absolute endpoints URIs returned by
.well-known/openid-configuration
will no longer useOpenIddictServerOptions.Issuer
as the base URI. Instead, the computed value ofHttpRequest.Scheme + Uri.SchemeDelimiter + HttpRequest.Host + HttpRequest.PathBase
- which was already used when no explicitOpenIddictServerOptions.Issuer
was set - will always be used, so that the addresses reflect the request details exposed by ASP.NET Core.I took a look at the community projects that sponsor OpenIddict and here's how it will affect them:
ABP Framework: I'm not super familiar with the ABP internals, but it doesn't seem to directly set
HttpRequest.PathBase
so the impact should be limited to scenarios where it's set elsewhere (e.g when using IIS virtual application paths). The recommended approach is to remove the leading/
from the endpoint paths: https://github.com/abpframework/abp/blob/1648f1b6cc3b54b1f335b17da79a56c37fa0c1bf/modules/openiddict/src/Volo.Abp.OpenIddict.AspNetCore/Volo/Abp/OpenIddict/AbpOpenIddictAspNetCoreModule.cs#L56-L69 /cc @maliming @gterdemOrchardCore: OC uses
HttpRequest.PathBase
for multi-tenancy and represents the OpenIddict endpoints usingPathString
so it will be directly affected. The recommended approach is to remove the leading/
from the endpoint paths: https://github.com/OrchardCMS/OrchardCore/blob/231cb192037d3702b934633fca3e06710b4a6b7b/src/OrchardCore.Modules/OrchardCore.OpenId/Configuration/OpenIdServerConfiguration.cs#L78-L106Squidex: Squidex seems to always use absolute endpoint URIs. If so, it shouldn't be affected at all: https://github.com/Squidex/squidex/blob/b607c8606249b52c4e695d2edfe3a999e1ad7e80/backend/src/Squidex/Areas/IdentityServer/Config/IdentityServerServices.cs#L148-L167 /cc @SebastianStehle
Feel free if you have any question or remark.
The text was updated successfully, but these errors were encountered: