would it be possible to have the property keyValue via @ConfigurationProperties (prefix = "spring.security.oauth2.resourceserver") and a suitable decoder in OAuth2ResourceServerJwtConfiguration. For small quick tests it is easier to put the public key in the property file, as in 1.5.x.
Comment From: mbhave
I think Spring Security currently only supports obtaining the public key used for verification from the JWK Set URL.
/cc @jzheaux
Comment From: jzheaux
5.2 adds support for local public-key configuration, so it is certainly something that we can take a look at.
Comment From: Thinkenterprise
The classes for the Spring Boot OAuth2 Autoconfiguration OAuth2ResourceServerPropertiesfor configuration and OAuth2ResourceServerJwtConfiguration for the token decoder generation come from Spring Boot Autoconfiguration Packages org.springframework.boot.autoconfigure.security.oauth2.resource.servlet. So I can not fully understand why this is a Spring Security issue. Or am I wrong?
My suggestion would be to extend both classes so that public keys can be configured using a property in * .yaml * .properties.
Like this:
security:
oauth2:
resourceserver:
jwt:
keyValue: |
-----BEGIN PUBLIC KEY-----
MIGfMA0GCS ...
-----END PUBLIC KEY-----
Comment From: jzheaux
@Thinkenterprise it's a matter of supplying the conversion strategy to convert the encoded key into an instance of RSAPublicKey, for example. Something like this lives better in Spring Security.
But, yes, there would be some boot code that does the appropriate auto-configuration based on this value.
Comment From: wilkinsona
I'm not in favour of making this change. I think it would be quite YAML-specific as I don't think it would work well in a properties file, as an environment variable, as a system properties, etc. Also, we don't allow keys to be configured directly for any other similar configuration properties and consistency is important.
Comment From: Thinkenterprise
@wilkinsona If this is the basic approach of Spring Boot 2.0 to security, then I have to accept that. I think that is the responsibility of the developer. It is also a breaking change since this property was provided in version 1.5.x. For test cases, it would be a nice feature and also works with propety files. Especially as it uses it in a sample by referencing Spring Boot 2.0 from, I think @jzheaux ?
Comment From: rwinch
I'm not in favour of making this change. I think it would be quite YAML-specific as I don't think it would work well in a properties file, as an environment variable, as a system properties, etc. Also, we don't allow keys to be configured directly for any other similar configuration properties and consistency is important.
Perhaps it makes more sense to support using a separate file and pointing at that file in the properties/yml file?
Comment From: jzheaux
@Thinkenterprise The sample you are referencing is from a legacy OAuth 2.0 plugin--it's meant to illustrate how that plugin works. Please don't infer intended future capabilities for mainline Spring Security from it.
Comment From: wilkinsona
Perhaps it makes more sense to support using a separate file and pointing at that file in the properties/yml file?
Yeah, that's what I think we should do.
Comment From: michaldo
My thesis is that precedence defined in https://github.com/spring-projects/spring-boot/blob/main/module/spring-boot-security-oauth2-resource-server/src/main/java/org/springframework/boot/security/oauth2/server/resource/autoconfigure/KeyValueCondition.java is wrong.
Actually, issuer-uri is more important than public-key-location. But intension was
For small quick tests it is easier to put the public key in the property
That is my goal also. I would like to run application locally with public key JWT. But I want take closed production application and run locally. As production application is configured to use issuer uri = spring.security.oauth2.resourceserver.jwt.issuer-uri: $(JWT_HOST) - i cannot undo issuer-uri and choose public-key-location.
To conclude, public-key-location, as development and test feature, should have a precedence over production feature issuer-uri.
PS. If you decided proceed my thesis, please rename KeyValueCondition to something a bit meaningful.
Comment From: wilkinsona
We can't easily adjust the ordering as it would be a breaking change, and one that I am not sure is worth that disruption. You can separate your properties using profile-specific configuration files if necessary.
please rename KeyValueCondition to something a bit meaningful.
The class is now package-private so its name is less important. The public condition is named @ConditionalOnPublicKeyJwtDecoder.
Comment From: michaldo
I think reordering is easy. It is deprecating public-key-location, create new one with documented and well-considered priority and finally remove deprecated version. It happens almost every release. I could add a production profile but I will not do it, because my principle is default = production. It protects me against forgetting enable something important - because everything is enabled by default.
I think it is choice between true and false: is my thesis about correct priority true or is not.
And finally, it is really surprising to me thesis that naming could be more or less important. Naming is important always, and package private name is easier to improve than public name
Comment From: michaldo
A read a bit more.
Spring security documentation says that resource server may be secured by oauth2 with configured issuer-uri and public key is taken from the uri to verify JWT.
Under some circumstances public key is taken from jwk-set-uri and issuer-uri is used only verify claim iss.
That specification is implemented in IssuerUriCondition. jwk-set-uri takes priority over issuer-uri for JWT validation, issuer-uri is used to match iss claim. Perfect
And after this PR new logic is created, KeyValueCondition, not integrated with IssuerUriCondition logic, not backed by documentation requirements, without visible use case (without circumstances why public key is less important than jwk-set-uri/issuer-uri).
Even name of the implementation suggests that current behavior is not well-considered but random copy-paste from IssuerUriCondition