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

Provide a mechanism for customising the auto-configured NimbusJwtDecoders #20750

Closed
wilkinsona opened this issue Mar 31, 2020 · 12 comments
Closed
Labels
type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

Inspired by #20747, it looks like it would be useful to provide a mechanism for customising the auto-configured NimbusJwtDecoder. The use case in #20747 is to provide a RestTemplate to the decoder with custom proxy settings.

@wilkinsona wilkinsona added type: enhancement A general enhancement for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Mar 31, 2020
m-kay pushed a commit to m-kay/spring-boot that referenced this issue Apr 1, 2020
…to allow configuration of a resttemplate for the autoconfigured NimbusJwtDecoder

Fixes spring-projectsgh-20750
@wilkinsona wilkinsona changed the title Provide a mechanism for customising the auto-configured NimbusJwtDecoder Provide a mechanism for customising the auto-configured NimbusJwtDecoders Apr 3, 2020
@wilkinsona wilkinsona removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Apr 3, 2020
@wilkinsona wilkinsona added this to the 2.3.x milestone Apr 3, 2020
@mbhave
Copy link
Contributor

mbhave commented Apr 7, 2020

I'd initially thought adding a JwtDecoderCustomizer would work but that's not the case. We would need a JwtDecoderBuilderCustomizer to support customizations. The problem is that the JwtDecoder built when using an issuer location isn't built using a builder so that could cause some confusion around what can be customized and what can't.

@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Apr 7, 2020
@wilkinsona
Copy link
Member Author

I've just noticed that there also isn't a restOperations(RestOperations) method on the NimbusJwtDecoder.PublicKeyJwtDecoderBuilder. That makes sense as it doesn't make any HTTP calls. Perhaps #20470 isn't such a good driver for general purpose customization after all?

@mbhave
Copy link
Contributor

mbhave commented Apr 7, 2020

Yeah, the public key version doesn't need a restOperations. The issuerUri one, however, does use one but it doesn't appear to be customizable. Given that, I don't think we should add a general purpose customizer for the JwtDecoder.

@m-kay
Copy link
Contributor

m-kay commented Apr 8, 2020

Customization is only needed in the case if the JwkSetUriJwtDecoderBuilder is used, because all the other builders are fully configurable via properties. Therefore I think only a JwkSetUriJwtDecoderBuilderCustomizer would be needed as proposed in my branch gh-20750

@mbhave
Copy link
Contributor

mbhave commented Apr 9, 2020

Thanks for the proposal @m-kay. It is true that for the customizations that Spring Security currently supports, it is only the JwkSetUriJwtDecoderBuilder that needs a customizer. However, creating a JwkSetUriJwtDecoderBuilderCustomizer would be misleading because it would not be able to customize the restOperations used by JwtDecoders which calls the JwkSetUriJwtDecoderBuilder under the hood. I have raised a Spring Security issue about that. Until then, I don't think we should do anything in Spring Boot as there's a chance that we would back ourselves into a corner.

@mbhave mbhave added status: blocked An issue that's blocked on an external project change and removed for: team-attention An issue we'd like other members of the team to review labels Apr 9, 2020
@mbhave mbhave modified the milestones: 2.3.x, 2.x Apr 9, 2020
@ThomasKasene
Copy link
Contributor

If we can't get a customizer, perhaps it's possible to inject a OAuth2TokenValidator<Jwt> into the bean so we can supply our own custom validators if we need them? This is really cumbersome to achieve with the current setup as far as I can tell, and while I feel like it's due to Spring Security's tightly coupled configuration, maybe Spring Boot can make it less painful?

There's an old issue in Spring Security that describes the problem I'm facing: spring-projects/spring-security#6909
It petitions to add a getValidator() to NimbusJwtDecoderJwkSupport to make it possible to know what validators have already been registered, but there doesn't seem to be enough traction to get it implemented.

@m-kay
Copy link
Contributor

m-kay commented Aug 17, 2021

any update on this? 👀

@wilkinsona
Copy link
Member Author

@m-kay As you can see above, @mbhave opened spring-projects/spring-security#8365. If you read through that issue, you’ll see that it was closed in favour of spring-projects/spring-security#8882. 8882 remains open and I don’t think we can do anything here without it so this issue remains blocked.

@wilkinsona
Copy link
Member Author

spring-projects/spring-security#10309 has been scheduled for Spring Security 5.8 so we may be able to do something in this area in 3.0.x. I'm going to assign this issue to 3.0.x serve as a reminder to see what becomes possible. It'll remain blocked until the changes have been made in Security.

@wilkinsona
Copy link
Member Author

wilkinsona commented Apr 14, 2023

Some changes were made in Spring Security a couple of days ago so we should be able to do something in Boot now. It may be too late now for 3.1 though.

@wilkinsona wilkinsona removed the status: blocked An issue that's blocked on an external project change label Apr 14, 2023
@wilkinsona
Copy link
Member Author

@mbhave there's a new NimbusJwtDecoder.withIssuerLocation(issuerUri) method that returns a JwkSetUriJwtDecoderBuilder. That means that both jwtDecoderByIssuerUri and jwtDecoderByJwkKeySetUri could now use a JwkSetUriJwtDecoderBuilder which allows RestOperations to be configured. We could offer a JwkSetUriJwtDecoderBuilderCustomizer now and I think it could be applied consistently. Do you think we should do so?

@wilkinsona
Copy link
Member Author

Possible implementation for discussion and review.

@wilkinsona wilkinsona modified the milestones: 3.1.x, 3.1.0-RC1 Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants