You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by "nscendoni (via GitHub)" <gi...@apache.org> on 2025/08/05 07:39:36 UTC
[PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
nscendoni opened a new pull request, #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on PR #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27#issuecomment-3155386574
@nscendoni - the master build failed afterthis PR was merged, can you please double-check if it's related?
https://ci-builds.apache.org/job/Sling/job/modules/job/sling-org-apache-sling-auth-oauth-client/job/master/84/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
Posted by "rombert (via GitHub)" <gi...@apache.org>.
rombert commented on code in PR #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27#discussion_r2254451626
##########
src/test/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImplTest.java:
##########
@@ -175,6 +175,29 @@ void testStoreAccessToken() throws Exception {
verify(cryptoService).encrypt(TEST_ACCESS_TOKEN);
}
+ @Test
+ void testStoreRefreshToken() throws Exception {
+ SlingUserInfoProcessorImpl.Config cfg = Converters.standardConverter()
+ .convert(Map.of(
+ "groupsInIdToken", false,
+ "storeAccessToken", false,
+ "storeRefreshToken", true,
+ "groupsClaimName", "groups",
+ "connection", "test"))
+ .to(SlingUserInfoProcessorImpl.Config.class);
+ processor = new SlingUserInfoProcessorImpl(cryptoService, cfg);
+
+ String tokenResponse = createTokenResponse(TEST_ACCESS_TOKEN, TEST_REFRESH_TOKEN);
+
+ OidcAuthCredentials result = processor.process(null, tokenResponse, TEST_SUBJECT, TEST_IDP);
+
+ assertNotNull(result);
+ // Note: There's a bug in the original code - it uses accessToken() instead of refreshToken()
Review Comment:
I don't really understand this comment and the one on line R198. Can you please clarify @nscendoni ?
If there's still a bug in the implementation please either reopen SLING-12871 or file a new one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27#issuecomment-3153938121
## [](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_sling-org-apache-sling-auth-oauth-client&pullRequest=27)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
Posted by "anchela (via GitHub)" <gi...@apache.org>.
anchela commented on code in PR #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27#discussion_r2253635251
##########
src/main/java/org/apache/sling/auth/oauth_client/impl/SlingUserInfoProcessorImpl.java:
##########
@@ -166,7 +166,7 @@ public SlingUserInfoProcessorImpl(
// Store the Refresh Token on user node
String refreshToken = tokens.accessToken();
if (storeRefreshToken && refreshToken != null) {
- credentials.setAttribute(OAuthTokenStore.PROPERTY_NAME_ACCESS_TOKEN, cryptoService.encrypt(refreshToken));
+ credentials.setAttribute(OAuthTokenStore.PROPERTY_NAME_REFRESH_TOKEN, cryptoService.encrypt(refreshToken));
Review Comment:
in cases where oak login tokens are being created credentials attributes will be stored in the token node. given that 'refreshToken' is encrypted, i don't see a huge risk.
but this is something to be aware of.
maybe worth a documentation note....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] SLING-12871 SlingUserInfoProcessor doesn't store Refresh token [sling-org-apache-sling-auth-oauth-client]
Posted by "anchela (via GitHub)" <gi...@apache.org>.
anchela merged PR #27:
URL: https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/27
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@sling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org