-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add OAuth2 feature for Config Clients #2363
Conversation
cobar79
commented
Dec 7, 2023
- When OAuth2 token property present, call IDP for Bearer Token.
- Add Bearer Token header on config server resource requests.
- Replace deprecated Base64Utils
spring-cloud-config-client/pom.xml
Outdated
@@ -21,6 +21,11 @@ | |||
]]> | |||
</description> | |||
|
|||
<properties> | |||
<jasypt-spring-boot.version>3.0.5</jasypt-spring-boot.version> | |||
<okhttp.version>4.11.0</okhttp.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the boot-managed version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spencergibb
I attempted "boot managed" JCE but had problems with cipher in the client properties. There is so little information on JCE in a Spring Boot project and tons of examples for JASYPT that I went with the common encryption that was easier to set up and manage.
I can revisit it if I can get better documentation of getting it working the the config client.
client-secret: '{cipher}e4b1f56fb017319eb959595343d0e4f5a742e975860b723433f90e8f3e869a379db7c756e0537273aebf72cc3a4c3d48caf4955889c5e22be588523b0a88b5c8'
Property: spring.cloud.config.client-secret
Value: "{cipher}e4b1f56fb017319eb959595343d0e4f5a742e975860b723433f90e8f3e869a379db7c756e0537273aebf72cc3a4c3d48caf4955889c5e22be588523b0a88b5c8"
Origin: URL [file:config/application-local.yml] - 73:22
Reason: java.lang.UnsupportedOperationException: No decryption for FailsafeTextEncryptor. Did you configure the keystore correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does that have to do with okhttp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mock server for IDP testing. See ConfigClientRequestTemplateFactoryTest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, then back to my original comment, why can't the boot managed version of ok http be used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, looking back I didn't specify okhttp as the one to boot manage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I am on the same page, I take "boot managed version of ok http" to be the org.springframework.test.web.client.MockRestServiceServer?
Since I have never used it, I may need help getting it to work. My first attempt failed with it timing out and not responding. My guess is because it has to be bound to the same RestTemplate making the call?
If so, that is going to be the chicken and the egg dilemma since I am testing the Factory method that creates the Template.
mockRestServiceServer = MockRestServiceServer.bindTo(new RestTemplate()).build();
mockRestServiceServer.expect(requestTo(new URI(properties.getTokenUri())))
.andExpect(method(HttpMethod.POST))
.andExpect(content().contentType(MediaType.APPLICATION_FORM_URLENCODED))
.andRespond(withStatus(HttpStatus.OK)
.contentType(MediaType.APPLICATION_JSON)
.body(TOKEN_RESPONSE));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<okhttp.version>4.12.0</okhttp.version>
You should just be able to remove the version property and the version
tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Did not know it was included. Will have to update all my microservices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It didn't used to be, it was added at some point. We had the same thing in spring cloud
@@ -117,6 +117,47 @@ public class ConfigClientProperties { | |||
*/ | |||
private String password; | |||
|
|||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be cleaner to place these properties in a subclass called oauth2 so we would use spring.cloud.config.oauth2.*
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. See ConfigClientOauth2Properties
@@ -67,6 +82,17 @@ public ConfigClientProperties getProperties() { | |||
return this.properties; | |||
} | |||
|
|||
public SimplePBEByteEncryptor buildEncryptor() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be some kind of bean that the user can override
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I embedded it into the ConfigClientProperties. It got real messy trying to pass it into the pass additional configurations into the ConfigClientRequestTemplateFactory constructor.
restTemplate.setInterceptors(interceptors); | ||
} | ||
else { | ||
requestTemplateFactory.addAuthorizationToken(headers, username, password); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if this token expires?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, the Spring Security Framework handles refreshing of the token. I am still trying to set up a test for the actuator monitor endpoint to see if the security framework will append the token to the config server request to pull new updates. In theory it should work since the Spring Context is fully boot strapped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK please let us know when you have tested that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the redaction. I was looking at postman response and failed to review the client logs.
09:40:07.119 [http-nio-9440-exec-8] WARN o.s.c.c.c.ConfigServerConfigDataLoader - Could not locate PropertySource ([ConfigServerConfigDataResource@6cca236b uris = array<String>['http://localhost:8888/config-server'], optional = true, profiles = 'local']): 401 : "{"path":"/config-server/gdelt-ingest/local","message":"An error occurred while attempting to decode the Jwt: Jwt expired at 2023-12-19T15:50:42Z","propertyPath":null,"propertyValue":null,"timestamp":"2023-12-19 09:40:07.086-07"}"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ryanjbaxter @spencergibb
Is it ok to bring in a JWT Decoder to check expiration?
<dependency>
<groupId>com.auth0</groupId>
<artifactId>java-jwt</artifactId>
<version>4.4.0</version>
</dependency>
...ient/src/main/java/org/springframework/cloud/config/client/ConfigClientOauth2Properties.java
Outdated
Show resolved
Hide resolved
restTemplate.setInterceptors(interceptors); | ||
} | ||
else { | ||
requestTemplateFactory.addAuthorizationToken(headers, username, password); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK please let us know when you have tested that
We will also need to add some documentation for this |
@ryanjbaxter Can you point me to the repo that contains the Spring Cloud Config that I will need to update? |
Nervous that I didn't fork correctly. I am in the 4.0.5-SNAPSHOT and I do not see the modules directory under the docs module? Also curious how this will go since I see a 4.1.0 release of spring-cloud-starter-config on Dec 6th in Maven Central? |
Yes this will actually have to go in a feature release, most likely 4.2.x. That said its best to use |
Thanks @ryanjbaxter Can I sync the current fork in my repo or do I need to:
|
Yes I think so |
Moving to version 4.1 |