Add OAuth2 support in spring-config-client #3129
Conversation
…ud config client Signed-off-by: prafsoni <[email protected]>
Signed-off-by: prafsoni <[email protected]>
…suer uri Signed-off-by: prafsoni <[email protected]>
Signed-off-by: prafsoni <[email protected]>
prafsoni
force-pushed
the
feature/config-client-oauth2-support
branch
from
2180ae6 to
b947caa
Compare
prafsoni
changed the title
|
|
||
| private ClientRegistrationRepository clientRegistrationRepository( | ||
| ConfigClientProperties.OAuth2Properties properties) { | ||
| OAuth2ClientProperties oauth2ClientProperties = new OAuth2ClientProperties(); |
There was a problem hiding this comment.
I would think we would just use the existing properties
https://docs.spring.io/spring-security/reference/servlet/oauth2/login/core.html
There was a problem hiding this comment.
The reason I took this approach for a few reasons.
- First, to keep things isolated and not depend on Spring Security autoconfiguration, reuse the classes provided and imitate the same here.
- Additionally, I am not sure about how it would work, as this initialization happens so early in the app lifecycle.
- Also, there are scenarios where configserver would actually serve the
spring.security.oauth2.client.*
|
|
||
| List<ClientRegistration> registrations = new ArrayList<>( | ||
| new OAuth2ClientPropertiesMapper(oauth2ClientProperties).asClientRegistrations().values()); | ||
| return new InMemoryClientRegistrationRepository(registrations); |
There was a problem hiding this comment.
This would need to be configurable. Maybe by default we use InMemoryClientRegistrationRepository but this should be a bean that can be supplied
There was a problem hiding this comment.
Sure, if there is a usecase for this, as things are a bit tricky to wire things in with all BootstrapRegistryInitializer. If I could get a little more context on what is required I could try to address this
|
Thanks. I will need to set aside some time to deep dive on this after our GA release. Also would love @jgrandja to take a look as well. |
@ryanjbaxter Thanks for the review. Since the changes are not disruptive. So, I was hoping this would make it to 5.0.0. I do understand this is a bit of an ask given that GA is so close. If there is something I could do to make it a possibility, please do let me know. |
The RC1 release is already in progress and everyone is focused on that and then the GA after. It will have to wait, I'm afraid. |
|
If I have some time I will take a look at it but as Spencer said we have a lot on our plate between now and GA and we don't want to rush this, we want to make sure we get it right. Unfortunately this major has been a challenge for the Spring Cloud team as we try to keep up with the incoming changes from Spring Boot and Spring Framework. We did not have as much time as we would have liked to put in new features into Spring Cloud. |
|
@prafsoni I took a look at the PR and it introduces the |
5 participants
Resolves #2348
spring-cloud-clientusingspring-security-oauth2-client.spring.cloud.config.oauth2.*.spring.cloud.config.oauth2.enabled: false) by default to maintain existing behavior.spring-cloud-config-client-oauth2-testsmodule for OAuth2 support integration tests