You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2023/07/04 09:31:17 UTC
[sling-whiteboard] branch master updated (38c8e713 -> c3fa40b4)
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
from 38c8e713 oidc-rp: replace some bespoke code with the Nimbus SDK
new 387a0f64 oidc-rp: unregister the state from the state manager only when we're done with it
new 886ac917 oidc-rp: elevate some debugs to warns
new 7cf4600b oidc-rp: parse the token response as an OIDC response
new d02f639c oidc-rp: simplify OidcCallbackServlet
new c3fa40b4 oidc-rp: store id_token from the response
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../servlets/oidc_rp/impl/OidcCallbackServlet.java | 32 ++++++++--------------
.../oidc_rp/impl/OidcConnectionFinderImpl.java | 25 +++++++++++++++--
.../oidc_rp/impl/OidcConnectionPersister.java | 6 ++--
.../servlets/oidc_rp/AuthorizationCodeFlowIT.java | 4 +++
4 files changed, 41 insertions(+), 26 deletions(-)
[sling-whiteboard] 03/05: oidc-rp: parse the token response as an OIDC response
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit 7cf4600ba37a9b8372c43f7ed4105e94bfac2d50
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jul 4 12:16:11 2023 +0300
oidc-rp: parse the token response as an OIDC response
This allows preparing for storing id tokens as well
---
.../sling/servlets/oidc_rp/impl/OidcCallbackServlet.java | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
index 29424e74..b665ead5 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
@@ -50,19 +50,19 @@ import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import com.nimbusds.oauth2.sdk.AccessTokenResponse;
import com.nimbusds.oauth2.sdk.AuthorizationCode;
import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant;
import com.nimbusds.oauth2.sdk.ParseException;
import com.nimbusds.oauth2.sdk.TokenErrorResponse;
import com.nimbusds.oauth2.sdk.TokenRequest;
-import com.nimbusds.oauth2.sdk.TokenResponse;
import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic;
import com.nimbusds.oauth2.sdk.auth.Secret;
import com.nimbusds.oauth2.sdk.id.ClientID;
import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationResponseParser;
+import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
+import com.nimbusds.openid.connect.sdk.token.OIDCTokens;
@Component(service = { Servlet.class })
@SlingServletPaths(OidcCallbackServlet.PATH)
@@ -140,7 +140,7 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
new AuthorizationCodeGrant(code, new URI(getCallbackUri(request)))
);
- TokenResponse tokenResponse = TokenResponse.parse(tokenRequest.toHTTPRequest().send());
+ OIDCTokenResponse tokenResponse = OIDCTokenResponse.parse(tokenRequest.toHTTPRequest().send());
if ( !tokenResponse.indicatesSuccess() ) {
TokenErrorResponse errorResponse = tokenResponse.toErrorResponse();
@@ -150,12 +150,12 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
return;
}
- AccessTokenResponse successResponse = tokenResponse.toSuccessResponse();
+ OIDCTokens tokens = tokenResponse.getOIDCTokens();
- String accessToken = successResponse.getTokens().getAccessToken().getValue();
- String refreshToken = successResponse.getTokens().getRefreshToken().getValue();
+ String accessToken = tokens.getAccessToken().getValue();
+ String refreshToken = tokens.getRefreshToken().getValue();
ZonedDateTime expiry = null;
- long expiresIn = successResponse.getTokens().getAccessToken().getLifetime();
+ long expiresIn = tokens.getAccessToken().getLifetime();
if ( expiresIn > 0 ) {
expiry = LocalDateTime.now().plus(expiresIn, ChronoUnit.SECONDS).atZone(ZoneId.systemDefault());
}
[sling-whiteboard] 04/05: oidc-rp: simplify OidcCallbackServlet
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit d02f639c48d7a1cfe157bcc3e0e531fb8af4500a
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jul 4 12:20:15 2023 +0300
oidc-rp: simplify OidcCallbackServlet
Instead of having the extraction code in the servlet, move it to the OidcConnectionFinderImpl where it's easier to test.
The APIs are private so it's not an issue if we use the Nimbus SDK classes in the method signatures.
---
.../servlets/oidc_rp/impl/OidcCallbackServlet.java | 17 +----------------
.../oidc_rp/impl/OidcConnectionFinderImpl.java | 18 ++++++++++++++++--
.../servlets/oidc_rp/impl/OidcConnectionPersister.java | 6 +++---
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
index b665ead5..17a21149 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
@@ -24,10 +24,6 @@ import java.net.URISyntaxException;
import java.net.URLDecoder;
import java.net.http.HttpClient;
import java.nio.charset.StandardCharsets;
-import java.time.LocalDateTime;
-import java.time.ZoneId;
-import java.time.ZonedDateTime;
-import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -62,7 +58,6 @@ import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationResponse;
import com.nimbusds.openid.connect.sdk.AuthenticationResponseParser;
import com.nimbusds.openid.connect.sdk.OIDCTokenResponse;
-import com.nimbusds.openid.connect.sdk.token.OIDCTokens;
@Component(service = { Servlet.class })
@SlingServletPaths(OidcCallbackServlet.PATH)
@@ -149,18 +144,8 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return;
}
-
- OIDCTokens tokens = tokenResponse.getOIDCTokens();
-
- String accessToken = tokens.getAccessToken().getValue();
- String refreshToken = tokens.getRefreshToken().getValue();
- ZonedDateTime expiry = null;
- long expiresIn = tokens.getAccessToken().getLifetime();
- if ( expiresIn > 0 ) {
- expiry = LocalDateTime.now().plus(expiresIn, ChronoUnit.SECONDS).atZone(ZoneId.systemDefault());
- }
- persister.persistToken(connection, request.getResourceResolver(), accessToken, refreshToken, expiry);
+ persister.persistToken(connection, request.getResourceResolver(), tokenResponse.getOIDCTokens());
if ( redirect.isEmpty() ) {
response.setStatus(HttpServletResponse.SC_OK);
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
index 3bba5b4e..38dfa251 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
@@ -19,7 +19,10 @@ package org.apache.sling.servlets.oidc_rp.impl;
import java.net.URI;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
import java.time.ZonedDateTime;
+import java.time.temporal.ChronoUnit;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.Optional;
@@ -38,6 +41,8 @@ import org.osgi.service.component.annotations.Component;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.nimbusds.openid.connect.sdk.token.OIDCTokens;
+
@Component
public class OidcConnectionFinderImpl implements OidcConnectionFinder, OidcConnectionPersister {
@@ -79,11 +84,20 @@ public class OidcConnectionFinderImpl implements OidcConnectionFinder, OidcConne
}
@Override
- public void persistToken(OidcConnection connection, ResourceResolver resolver, String tokenValue, String refreshToken, ZonedDateTime expiry) {
+ public void persistToken(OidcConnection connection, ResourceResolver resolver, OIDCTokens tokens) {
try {
User currentUser = resolver.adaptTo(User.class);
Session session = resolver.adaptTo(Session.class);
- currentUser.setProperty(propertyPath(connection, PROPERTY_NAME_ACCESS_TOKEN), session.getValueFactory().createValue(tokenValue));
+
+ String accessToken = tokens.getAccessToken().getValue();
+ String refreshToken = tokens.getRefreshToken().getValue();
+ ZonedDateTime expiry = null;
+ long expiresIn = tokens.getAccessToken().getLifetime();
+ if ( expiresIn > 0 ) {
+ expiry = LocalDateTime.now().plus(expiresIn, ChronoUnit.SECONDS).atZone(ZoneId.systemDefault());
+ }
+
+ currentUser.setProperty(propertyPath(connection, PROPERTY_NAME_ACCESS_TOKEN), session.getValueFactory().createValue(accessToken));
if ( expiry != null ) {
Calendar cal = GregorianCalendar.from(expiry);
currentUser.setProperty(propertyPath(connection, PROPERTY_NAME_EXPIRES_AT), session.getValueFactory().createValue(cal));
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionPersister.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionPersister.java
index 70ce5c65..70950345 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionPersister.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionPersister.java
@@ -16,12 +16,12 @@
*/
package org.apache.sling.servlets.oidc_rp.impl;
-import java.time.ZonedDateTime;
-
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.servlets.oidc_rp.OidcConnection;
+import com.nimbusds.openid.connect.sdk.token.OIDCTokens;
+
public interface OidcConnectionPersister {
- void persistToken(OidcConnection connection, ResourceResolver resourceResolver, String tokenValue, String refreshToken, ZonedDateTime expiry);
+ void persistToken(OidcConnection connection, ResourceResolver resourceResolver, OIDCTokens tokens);
}
[sling-whiteboard] 05/05: oidc-rp: store id_token from the response
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit c3fa40b4ca23236f90075ba974142a7322462b8b
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jul 4 12:31:07 2023 +0300
oidc-rp: store id_token from the response
---
.../apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java | 5 +++++
.../sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java | 7 +++++++
.../org/apache/sling/servlets/oidc_rp/AuthorizationCodeFlowIT.java | 4 ++++
3 files changed, 16 insertions(+)
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
index 17a21149..5c1e4673 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
@@ -145,6 +145,11 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
return;
}
+ // TODO - additional validations for the id_token
+ // - does the 'aud' claim match the client id of our connection
+ // - nonce validation (?)
+ // - iat/exp validation (?)
+
persister.persistToken(connection, request.getResourceResolver(), tokenResponse.getOIDCTokens());
if ( redirect.isEmpty() ) {
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
index 38dfa251..525621f0 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcConnectionFinderImpl.java
@@ -50,6 +50,7 @@ public class OidcConnectionFinderImpl implements OidcConnectionFinder, OidcConne
private static final String PROPERTY_NAME_EXPIRES_AT = "expiresAt";
private static final String PROPERTY_NAME_ACCESS_TOKEN = "access_token";
private static final String PROPERTY_NAME_REFRESH_TOKEN = "refresh_token";
+ private static final String PROPERTY_NAME_ID_TOKEN = "id_token";
private final Logger logger = LoggerFactory.getLogger(getClass());
@@ -91,6 +92,7 @@ public class OidcConnectionFinderImpl implements OidcConnectionFinder, OidcConne
String accessToken = tokens.getAccessToken().getValue();
String refreshToken = tokens.getRefreshToken().getValue();
+ String idToken = tokens.getIDTokenString();
ZonedDateTime expiry = null;
long expiresIn = tokens.getAccessToken().getLifetime();
if ( expiresIn > 0 ) {
@@ -109,6 +111,11 @@ public class OidcConnectionFinderImpl implements OidcConnectionFinder, OidcConne
else
currentUser.removeProperty(propertyPath(connection, PROPERTY_NAME_REFRESH_TOKEN));
+ if ( idToken != null )
+ currentUser.setProperty(propertyPath(connection, PROPERTY_NAME_ID_TOKEN), session.getValueFactory().createValue(idToken));
+ else
+ currentUser.removeProperty(propertyPath(connection, PROPERTY_NAME_ID_TOKEN));
+
session.save();
} catch (RepositoryException e) {
throw new RuntimeException(e);
diff --git a/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/AuthorizationCodeFlowIT.java b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/AuthorizationCodeFlowIT.java
index 0de766a8..fc5eac67 100644
--- a/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/AuthorizationCodeFlowIT.java
+++ b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/AuthorizationCodeFlowIT.java
@@ -181,6 +181,10 @@ class AuthorizationCodeFlowIT {
// validate that the JWT is valid; we trust what keycloak has returned but just want to ensure that
// the token was stored correctly
SignedJWT.parse(accesToken);
+
+ // in a similar manner, validate the id_token
+ String idToken = keycloakToken.get("id_token").asText();
+ SignedJWT.parse(idToken);
}
private String getUserPath(SlingClient sling, String authorizableId) throws ClientException {
[sling-whiteboard] 01/05: oidc-rp: unregister the state from the state manager only when we're done with it
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit 387a0f64a14f64a6e002361894e45310d0a94de1
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jul 4 12:14:51 2023 +0300
oidc-rp: unregister the state from the state manager only when we're done with it
---
.../org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
index 341831fc..1508c041 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
@@ -104,11 +104,11 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
throw new ServletException(authResponse.toErrorResponse().getErrorObject().toString());
Optional<String> redirect = stateManager.getStateAttribute(authResponse.getState(), OidcStateManager.PARAMETER_NAME_REDIRECT);
- stateManager.unregisterState(authResponse.getState());
String authCode = authResponse.toSuccessResponse().getAuthorizationCode().getValue();
Optional<String> desiredConnectionName = stateManager.getStateAttribute(authResponse.getState(), OidcStateManager.PARAMETER_NAME_CONNECTION);
+ stateManager.unregisterState(authResponse.getState());
if ( desiredConnectionName.isEmpty() ) {
logger.debug("Did not find any connection in stateManager");
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
[sling-whiteboard] 02/05: oidc-rp: elevate some debugs to warns
Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git
commit 886ac917f8aea89b5171a3f29df8e200c6ad250b
Author: Robert Munteanu <ro...@apache.org>
AuthorDate: Tue Jul 4 12:15:40 2023 +0300
oidc-rp: elevate some debugs to warns
---
.../org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
index 1508c041..29424e74 100644
--- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
+++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/servlets/oidc_rp/impl/OidcCallbackServlet.java
@@ -110,13 +110,13 @@ public class OidcCallbackServlet extends SlingAllMethodsServlet {
Optional<String> desiredConnectionName = stateManager.getStateAttribute(authResponse.getState(), OidcStateManager.PARAMETER_NAME_CONNECTION);
stateManager.unregisterState(authResponse.getState());
if ( desiredConnectionName.isEmpty() ) {
- logger.debug("Did not find any connection in stateManager");
+ logger.warn("Did not find any connection in stateManager");
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return;
}
OidcConnection connection = connections.get(desiredConnectionName.get());
if ( connection == null ) {
- logger.debug("Requested unknown connection {}", desiredConnectionName.get());
+ logger.warn("Requested unknown connection {}", desiredConnectionName.get());
response.sendError(HttpServletResponse.SC_BAD_REQUEST);
return;
}