Skip to content

Issuer should not support path component #1435

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

Closed
jgrandja opened this issue Nov 6, 2023 · 19 comments
Closed

Issuer should not support path component #1435

jgrandja opened this issue Nov 6, 2023 · 19 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@jgrandja
Copy link
Collaborator

jgrandja commented Nov 6, 2023

The issuer setting, if supplied via AuthorizationServerSettings.getIssuer(), should not support a path component.

With the current and all previous versions, if AuthorizationServerSettings.getIssuer() was explicitly set with https://provider.com/issuer1, the protocol endpoint URI's returned by OidcProviderConfigurationEndpointFilter and OAuth2AuthorizationServerMetadataEndpointFilter would all be incorrect. For example, token_endpoint would be https://provider.com/issuer1/oauth2/token, which would not resolve for the client since the token endpoint matches on /oauth2/token (by default) and not /issuer1/oauth2/token.

This fix should add a validation preventing a path component for issuer.

NOTE: The path component enables supporting multiple issuers per host for multi-tenant configurations. This enhancement request is being tracked in gh-1342.

Related gh-1419 gh-1416

@jgrandja jgrandja added the type: bug A general bug label Nov 6, 2023
@jgrandja jgrandja added this to the 0.4.5 milestone Nov 6, 2023
@jgrandja jgrandja self-assigned this Nov 6, 2023
@jgrandja jgrandja changed the title Issuer setting should not support path component Issuer should not support path component Nov 6, 2023
@wheredoipressnow
Copy link

Hello @jgrandja, could you please elaborate on how to deal with an authorization server that has server.servlet.context-path configured? With this change the metadata no longer contains the correct path.

@rd-marc-lehnert
Copy link

Hello @jgrandja, we just tried to upgrade our authorization server which is behind a proxy under a different path with the
issuer: https://server.domain/auth and a reverse proxy pointing https://server.domain/auth to http://backend-service. This setup now no longer works because the server does not start (Path component for issuer (...) is currently not supported). This prevents us from upgrading to Version 1.2.

@jonkjenn
Copy link

We also are affected by this and have to find a workaround to be able to upgrade. We currently hard code paths like this.

    AuthorizationServerSettings.builder()
            .issuer(issuer)
            .authorizationEndpoint("$basePath/oauth2/auth")
            .tokenEndpoint("$basePath/oauth2/token")
            .jwkSetEndpoint("$basePath/.well-known/jwks.json")

@jgrandja
Copy link
Collaborator Author

@rd-marc-lehnert @jonkjenn I'm not sure how you were even able to get it working with a path component? As mentioned in the main issue:

With the current and all previous versions, if AuthorizationServerSettings.getIssuer() was explicitly set with https://provider.com/issuer1, the protocol endpoint URI's returned by OidcProviderConfigurationEndpointFilter and OAuth2AuthorizationServerMetadataEndpointFilter would all be incorrect. For example, token_endpoint would be https://provider.com/issuer1/oauth2/token, which would not resolve for the client since the token endpoint matches on /oauth2/token (by default) and not /issuer1/oauth2/token.

If you apply the patch issuer-path.patch to main, you will see that the default-authorizationserver sample does not work. Try accessing http://localhost:9000/auth/.well-known/openid-configuration and you will get a 404.

issuer-path.patch

However, if you require a path component in your current setup, then you can configure server.servlet.context-path to achieve the same. If you apply the patch context-path.patch to main, you will be able to access http://localhost:9000/auth/.well-known/openid-configuration and the issuer identifier will dynamically resolve to http://localhost:9000/auth.

context-path.patch

Hope this helps?

@rd-marc-lehnert
Copy link

Hi @jgrandja ,thanks for the hints, I will try that on Monday. Regarding your question how we got it working: sure, standalone the uris might be inconsistent. What matters is the outside perspective with the proxy in place. And this way everything works properly.

herodotus-ecosystem added a commit to herodotus-ecosystem/dante-oss that referenced this issue Nov 26, 2023
- 主要更新
  - [升级] Spring Boot 版本升级至 3.2.0
  - [升级] Spring Cloud 版本升级至 2023.0.0-RC1
  - [升级] Spring Authorization Server 版本升级至 1.2.0
- 其它更新
  - [重构] 重构相关代码,适配 Spring Boot 3.2.0 fix: #I7W5C3
  - [重构] 重构相关代码,适配 Spring Cloud 2023.0.0-RC1 fix: #7W5C6
  - [重构] 重构 Spring Authorization Server 自定义 Provider 代码,适配最新的 Spring Authorization Server 1.2.0 版本。fix: #I7W5BY
  - [重构] 重构 Spring Authorization Server 配置代码,去除过时方法,适配最新代码。
  - [修复] 修复 Emqx 监控数据转 Influxdb2 的 Spring Integration 流程注入配置条件错误。
  - [修复] 修复 docker-compose 文件中,polaris 镜像名称不正确问题。
  - [新增] Spring Cloud Tencent Polaris 配置导入包,方便环境搭建和配置
  - [优化] 调整 Polaris 本地配置缓存目录,防止与新增配置导入包冲突和混淆
  - [修复] 调整 polaris docker-compose 默认端口,适配最新版本 Polarismesh Server。
  - [优化] 优化各个服务中,Spring Cloud Tencent 相关配置,去除无用的或者与默认参数相同的配置。
  - [新增] 新增 Spring Cloud Tencent 读取和使用本地缓存统一化配置。
  - [优化] 临时解决 SAS 1.2.0 不兼容问题,后续根据实际情况进行完善和修改。spring-projects/spring-authorization-server#1435
  - [优化] 删除 dependencies 中重复的或无用的版本控制配置,统一使用 Spring Boot Dependencies 控制依赖版本
herodotus-ecosystem added a commit to dromara/dante-cloud that referenced this issue Nov 26, 2023
- 主要更新
  - [升级] Spring Boot 版本升级至 3.2.0
  - [升级] Spring Cloud 版本升级至 2023.0.0-RC1
  - [升级] Spring Authorization Server 版本升级至 1.2.0
- 其它更新
  - [重构] 重构相关代码,适配 Spring Boot 3.2.0 fix: #I7W5C3
  - [重构] 重构相关代码,适配 Spring Cloud 2023.0.0-RC1 fix: #7W5C6
  - [重构] 重构 Spring Authorization Server 自定义 Provider 代码,适配最新的 Spring Authorization Server 1.2.0 版本。fix: #I7W5BY
  - [重构] 重构 Spring Authorization Server 配置代码,去除过时方法,适配最新代码。
  - [修复] 修复 Emqx 监控数据转 Influxdb2 的 Spring Integration 流程注入配置条件错误。
  - [修复] 修复 docker-compose 文件中,polaris 镜像名称不正确问题。
  - [新增] Spring Cloud Tencent Polaris 配置导入包,方便环境搭建和配置
  - [优化] 调整 Polaris 本地配置缓存目录,防止与新增配置导入包冲突和混淆
  - [修复] 调整 polaris docker-compose 默认端口,适配最新版本 Polarismesh Server。
  - [优化] 优化各个服务中,Spring Cloud Tencent 相关配置,去除无用的或者与默认参数相同的配置。
  - [新增] 新增 Spring Cloud Tencent 读取和使用本地缓存统一化配置。
  - [优化] 临时解决 SAS 1.2.0 不兼容问题,后续根据实际情况进行完善和修改。spring-projects/spring-authorization-server#1435
  - [优化] 删除 dependencies 中重复的或无用的版本控制配置,统一使用 Spring Boot Dependencies 控制依赖版本
@tkrah
Copy link

tkrah commented Nov 27, 2023

+1 - this one breaks our app too which runs under a custom servlet context-path which is mapped on a proxy before - this is unexpected to break in a minor/patch version upgrade here, could you revert that one on the 1.x branch or at least break it only in a new major version?

@jgrandja
Copy link
Collaborator Author

@tkrah Can you please provide more details on your setup so I can understand your issue. Did you configure AuthorizationServerSettings.getIssuer()? Did you configure server.servlet.context-path?

@tkrah
Copy link

tkrah commented Nov 27, 2023

I did configure both, the AuthorizationServerSettings.getIssuer() (including the context path) and the server.servlet.context-path and it works with 1.1.3. Trying to switch to 1.2.0 that setup breaks now.

@Prigovor
Copy link

Hello. If we use AuthorizationServerSettings.builder().issuer("https://issuer").build() and server.servlet.context-path: /path authorization happens, but from the /path/.well-known/openid-configuration we get the wrong configurations, without /path (context-path). If we try to override these configurations via AuthorizationServerSettings for example AuthorizationServerSettings.authorizationEndpoint(contextPath + defaultSettings.getAuthorizationEndpoint()).build() we get an error.

@jgrandja
Copy link
Collaborator Author

jgrandja commented Dec 1, 2023

@tkrah Since you're setting server.servlet.context-path, you should not set AuthorizationServerSettings.getIssuer()

I did configure both, the AuthorizationServerSettings.getIssuer() (including the context path)

The AuthorizationServerSettings.getIssuer() with a path component will not resolve. Please see this comment for details.

To solve this, simply remove the AuthorizationServerSettings.getIssuer() setting and preserve server.servlet.context-path and it should work.

@jonkjenn
Copy link

jonkjenn commented Dec 1, 2023

@jgrandja

I'm not sure how you were even able to get it working with a path component?
Main reason might be that we used Customizer<OidcProviderConfigurationEndpointConfigurer> to rewrite some paths.

We did manage to upgrade now though by running the same path internally as externally.

One remaining issue with the new issuer limitation though is handling an issuer with a trailing slash. I know it's horrible but it's not easy to move away from. So for us it would still be nice to be able to hard code the complete issuer.

@Prigovor
Copy link

Prigovor commented Dec 1, 2023

@jonkjenn Hello. Could you please tell us how you managed to configure the external path, and did you manage to get the correct configuration from /.well-known/openid-configuration endpoint?

@tkrah
Copy link

tkrah commented Dec 1, 2023

@tkrah Since you're setting server.servlet.context-path, you should not set AuthorizationServerSettings.getIssuer()

I have to, the external url is not the same like the one from the host which is running the auth server, so I need to set that issuer URI to get correct URLs and I need the context path to map it from the proxy from proxy X -> destination Y and using:

server.forward-headers-strategy: framework

I did configure both, the AuthorizationServerSettings.getIssuer() (including the context path)

The AuthorizationServerSettings.getIssuer() with a path component will not resolve. Please see this comment for details.

I just use 1.1.3 and I can tell you it does resolve that path component, the token endpoint is reachable with configured context path and issuer with context path, it works here without a problem.

To solve this, simply remove the AuthorizationServerSettings.getIssuer() setting and preserve server.servlet.context-path and it should work.

That does not work because I need a custom issuer URI - which all works in 1.1.3 ;)

@jonkjenn
Copy link

jonkjenn commented Dec 1, 2023

@Prigovor Just by making sure that whatever path we had publicly e.g. https://example.com/some-path/auth is the same path as the application has internally e.g. http://some-application:1234/some-path/auth so that you can take the path segment some-path/auth and apply it via server.servlet.context-path instead. I don't know how you would handle situations where you cannot have the same path.

Yes then our /.well-known/openid-configuration ends up correct with a combination of protocol and domain from the request, the context path and the settings from AuthorizationServerSettings.builder().authorizationEndpoint("/oauth2/auth").... We let issuer be configured automatically. (Except for our trailing slash issue.)

@Prigovor
Copy link

Prigovor commented Dec 1, 2023

Thanks a lot.
I finally figured it out, we don't need to specify issuer directly, we need to allow it to be configured automatically, then server.servlet.context-path will be included in the configuration. The mistake was that the issuer in our configuration was still specified.
And yes it turns out that now we are limited by the fact that we can only rely on automatic generation and use the same segments of the external and internal path.

@jgrandja
Copy link
Collaborator Author

jgrandja commented Dec 6, 2023

@rd-marc-lehnert, @jonkjenn, @tkrah, @Prigovor

Apologies for the issue this update has caused. I did not account for Proxy related settings. I went ahead and reverted this update across all branches.

@dopsun
Copy link

dopsun commented Dec 7, 2023

My authorization server also behind proxy, upgrade and hit the same issue. With help in this thread, and another one spring-projects/spring-security#5631, I got it running. Here is what I did, FYI:

  • remove issuer configuration in AuthorizationServerSettings as advised above
  • put server.forward-headers-strategy=FRAMEWORK to my Authorization server. Without this one, auto-generated issuer URI will be http instead of https behind proxy

Looking forward to get configure issuer URI capability back. Thanks.

@me-12
Copy link

me-12 commented Dec 7, 2023

@jgrandja Thank you so much for the revert!
We ran into this problem today as well. I just wrote a huge bugreport explaining why the proposed solution with context-path will not work for us. But before I hit "submit", I saw your latest reply. Very glad :)

@Mehdi-HAFID
Copy link

ok I'm confused. using spring auth server with boot 3.2.0 running in port 4002. I trying to implement the BFF implementation. so there is a reverse proxy with port 7080.

I had the error where I could not specify http://localhost:7080/auth as the issuer.
when setting the context path to /auth and setting issuer to http://localhost:7080 without /auth , the http://localhost:7080/auth/.well-known/openid-configuration shows "issuer": "http://localhost:7080" without /auth which cause the BFF server to complain about because the issuers are not the same http://localhost:7080/auth != http://localhost:7080

from reading this thread multiple time all I gather is that as @dopsun said I should remove the issuer setting

AuthorizationServerSettings.builder()
//				.issuer("http://localhost:7080")
				.build();

and add
server.forward-headers-strategy=framework
plus the context path.
and now when visiting directly through port 4002 , the issuer is issuer": "http://localhost:4002/auth",
and when visiting through the reverse proxy port 7080 , the issuer is issuer": "http://localhost:7080/auth", which is what I want and the bff no longer complains. but why?? why does it not work as normally as it should. @jgrandja said he reverted the code but using 3.2.2 does not add /auth in the .well-known/openid-configuration values, nor does the oldest version which is 3.1.0 (again using spring boot). Am I missing something. again I managed to make it work but just trying to understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

9 participants