From 09e4a61037399242a01323952834667edecd96ee Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Mon, 13 Mar 2023 16:32:06 +1100 Subject: [PATCH 1/7] improvements to logout from the OpenIdLoginService validate Signed-off-by: Lachlan Roberts --- .../security/openid/OpenIdAuthenticator.java | 12 ++- .../openid/OpenIdAuthenticationTest.java | 85 ++++++++++++++++++- 2 files changed, 87 insertions(+), 10 deletions(-) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index b0e908a02e6c..64784d14a43c 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -464,10 +464,8 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b { if (LOG.isDebugEnabled()) LOG.debug("auth revoked {}", authentication); - synchronized (session) - { - session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED); - } + logout(req); + return Authentication.SEND_CONTINUE; } else { @@ -499,10 +497,10 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b } } } + if (LOG.isDebugEnabled()) + LOG.debug("auth {}", authentication); + return authentication; } - if (LOG.isDebugEnabled()) - LOG.debug("auth {}", authentication); - return authentication; } // If we can't send challenge. diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index 2e6d70c6efd1..26aa67f5b9a2 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -16,7 +16,9 @@ import java.io.File; import java.io.IOException; import java.security.Principal; +import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -25,17 +27,22 @@ import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.security.AbstractLoginService; import org.eclipse.jetty.security.ConstraintMapping; import org.eclipse.jetty.security.ConstraintSecurityHandler; +import org.eclipse.jetty.security.LoginService; +import org.eclipse.jetty.security.RolePrincipal; +import org.eclipse.jetty.security.UserPrincipal; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.server.session.FileSessionDataStoreFactory; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.security.Constraint; +import org.eclipse.jetty.util.security.Password; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -55,8 +62,7 @@ public class OpenIdAuthenticationTest private ServerConnector connector; private HttpClient client; - @BeforeEach - public void setup() throws Exception + public void setup(LoginService loginService) throws Exception { openIdProvider = new OpenIdProvider(CLIENT_ID, CLIENT_SECRET); openIdProvider.start(); @@ -100,6 +106,7 @@ public void setup() throws Exception securityHandler.setAuthMethod(Constraint.__OPENID_AUTH); securityHandler.setRealmName(openIdProvider.getProvider()); + securityHandler.setLoginService(loginService); securityHandler.addConstraintMapping(profileMapping); securityHandler.addConstraintMapping(loginMapping); securityHandler.addConstraintMapping(adminMapping); @@ -135,6 +142,7 @@ public void stop() throws Exception @Test public void testLoginLogout() throws Exception { + setup(null); openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); String appUriString = "http://localhost:" + connector.getLocalPort(); @@ -188,6 +196,77 @@ public void testLoginLogout() throws Exception assertThat(openIdProvider.getLoggedInUsers().getTotal(), equalTo(1L)); } + @Test + public void testNestedLoginService() throws Exception + { + AtomicBoolean loggedIn = new AtomicBoolean(true); + setup(new AbstractLoginService() + { + + @Override + protected List loadRoleInfo(UserPrincipal user) + { + return List.of(new RolePrincipal("admin")); + } + + @Override + protected UserPrincipal loadUserInfo(String username) + { + return new UserPrincipal(username, new Password("")); + } + + @Override + public boolean validate(UserIdentity user) + { + if (!loggedIn.get()) + return false; + return super.validate(user); + } + }); + + openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); + + String appUriString = "http://localhost:" + connector.getLocalPort(); + + // Initially not authenticated + ContentResponse response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + String content = response.getContentAsString(); + assertThat(content, containsString("not authenticated")); + + // Request to login is success + response = client.GET(appUriString + "/login"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("success")); + + // Now authenticated we can get info + response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("userId: 123456789")); + assertThat(content, containsString("name: Alice")); + assertThat(content, containsString("email: Alice@example.com")); + + // The nested login service has supplied the admin role. + response = client.GET(appUriString + "/admin"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + + // This causes any validation of UserIdentity in the LoginService to fail. + loggedIn.set(false); + + // This results in a logout redirect to the Provider, and then user will no longer be authenticated. + response = client.GET(appUriString + "/admin"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("not authenticated")); + + // Test that the user was logged out successfully on the openid provider. + assertThat(openIdProvider.getLoggedInUsers().getCurrent(), equalTo(0L)); + assertThat(openIdProvider.getLoggedInUsers().getMax(), equalTo(1L)); + assertThat(openIdProvider.getLoggedInUsers().getTotal(), equalTo(1L)); + } + public static class LoginPage extends HttpServlet { @Override From 12047da6d08145352f52592fe246aa2292fde5ec Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 16 Mar 2023 20:37:29 +0900 Subject: [PATCH 2/7] respect idToken expiry for lifetime of login Signed-off-by: Lachlan Roberts --- .../security/openid/OpenIdAuthenticator.java | 44 ++++++++++--- .../security/openid/OpenIdCredentials.java | 15 ++++- .../security/openid/OpenIdLoginService.java | 4 +- .../openid/OpenIdAuthenticationTest.java | 62 +++++++++++++++++-- .../jetty/security/openid/OpenIdProvider.java | 21 +++++-- 5 files changed, 123 insertions(+), 23 deletions(-) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index 64784d14a43c..6ea048eb38c3 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -248,6 +248,11 @@ public UserIdentity login(String username, Object credentials, ServletRequest re public void logout(ServletRequest request) { attemptLogoutRedirect(request); + logoutWithoutRedirect(request); + } + + private void logoutWithoutRedirect(ServletRequest request) + { super.logout(request); HttpServletRequest httpRequest = (HttpServletRequest)request; HttpSession session = httpRequest.getSession(false); @@ -265,13 +270,13 @@ public void logout(ServletRequest request) } /** - * This will attempt to redirect the request to the end_session_endpoint, and finally to the {@link #REDIRECT_PATH}. + *

This will attempt to redirect the request to the end_session_endpoint, and finally to the {@link #REDIRECT_PATH}.

* - * If end_session_endpoint is defined the request will be redirected to the end_session_endpoint, the optional - * post_logout_redirect_uri parameter will be set if {@link #REDIRECT_PATH} is non-null. + *

If end_session_endpoint is defined the request will be redirected to the end_session_endpoint, the optional + * post_logout_redirect_uri parameter will be set if {@link #REDIRECT_PATH} is non-null.

* - * If the end_session_endpoint is not defined then the request will be redirected to {@link #REDIRECT_PATH} if it is a - * non-null value, otherwise no redirection will be done. + *

If the end_session_endpoint is not defined then the request will be redirected to {@link #REDIRECT_PATH} if it is a + * non-null value, otherwise no redirection will be done.

* * @param request the request to redirect. */ @@ -366,6 +371,21 @@ public void prepareRequest(ServletRequest request) baseRequest.setMethod(method); } + private boolean hasExpiredIdToken(HttpSession session) + { + if (session != null) + { + Map claims = (Map)session.getAttribute(CLAIMS); + if (claims != null) + { + long expiry = (Long)claims.get("exp"); + long currentTimeSeconds = System.currentTimeMillis() / 1000; + return (currentTimeSeconds > expiry); + } + } + return false; + } + @Override public Authentication validateRequest(ServletRequest req, ServletResponse res, boolean mandatory) throws ServerAuthException { @@ -381,6 +401,10 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b if (uri == null) uri = URIUtil.SLASH; + HttpSession session = request.getSession(false); + if (hasExpiredIdToken(session)) + logoutWithoutRedirect(request); + mandatory |= isJSecurityCheck(uri); if (!mandatory) return new DeferredAuthentication(this); @@ -391,7 +415,9 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b try { // Get the Session. - HttpSession session = request.getSession(); + if (session == null) + session = request.getSession(true); + if (request.isRequestedSessionIdFromURL()) { sendError(request, response, "Session ID must be a cookie to support OpenID authentication"); @@ -464,8 +490,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b { if (LOG.isDebugEnabled()) LOG.debug("auth revoked {}", authentication); - logout(req); - return Authentication.SEND_CONTINUE; + logoutWithoutRedirect(request); } else { @@ -511,12 +536,11 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b return Authentication.UNAUTHENTICATED; } - // Send the the challenge. + // Send the challenge. String challengeUri = getChallengeUri(baseRequest); if (LOG.isDebugEnabled()) LOG.debug("challenge {}->{}", session.getId(), challengeUri); baseResponse.sendRedirect(challengeUri, true); - return Authentication.SEND_CONTINUE; } catch (IOException e) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index c705169d709c..648f08c355dd 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -137,12 +137,21 @@ private void validateClaims(OpenIdConfiguration configuration) throws Exception throw new AuthenticationException("Authorized party claim value should be the client_id"); // Check that the ID token has not expired by checking the exp claim. - long expiry = (Long)claims.get("exp"); - long currentTimeSeconds = (long)(System.currentTimeMillis() / 1000F); - if (currentTimeSeconds > expiry) + if (isExpired()) throw new AuthenticationException("ID Token has expired"); } + public boolean isExpired() + { + if (claims == null) + return true; + + // Check that the ID token has not expired by checking the exp claim. + long expiry = (Long)claims.get("exp"); + long currentTimeSeconds = System.currentTimeMillis() / 1000; + return (currentTimeSeconds > expiry); + } + private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException { Object aud = claims.get("aud"); diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java index f33cd40f2d36..aca737313c97 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java @@ -136,7 +136,9 @@ public boolean validate(UserIdentity user) { if (!(user.getUserPrincipal() instanceof OpenIdUserPrincipal)) return false; - + OpenIdUserPrincipal userPrincipal = (OpenIdUserPrincipal)user.getUserPrincipal(); + if (userPrincipal.getCredentials().isExpired()) + return false; return loginService == null || loginService.validate(user); } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index 26aa67f5b9a2..be43cffd3fdd 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -26,6 +26,7 @@ import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.ContentResponse; +import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.security.AbstractLoginService; import org.eclipse.jetty.security.ConstraintMapping; @@ -50,6 +51,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.startsWith; @SuppressWarnings("unchecked") public class OpenIdAuthenticationTest @@ -252,17 +254,67 @@ public boolean validate(UserIdentity user) response = client.GET(appUriString + "/admin"); assertThat(response.getStatus(), is(HttpStatus.OK_200)); - // This causes any validation of UserIdentity in the LoginService to fail. + // This causes any validation of UserIdentity in the LoginService to fail + // causing subsequent requests to be redirected to the auth endpoint for login again. loggedIn.set(false); - - // This results in a logout redirect to the Provider, and then user will no longer be authenticated. + client.setFollowRedirects(false); response = client.GET(appUriString + "/admin"); + assertThat(response.getStatus(), is(HttpStatus.SEE_OTHER_303)); + String location = response.getHeaders().get(HttpHeader.LOCATION); + assertThat(location, containsString(openIdProvider.getProvider() + "/auth")); + + // Note that we couldn't follow "OpenID Connect RP-Initiated Logout 1.0" because we redirect straight to auth endpoint. + assertThat(openIdProvider.getLoggedInUsers().getCurrent(), equalTo(1L)); + assertThat(openIdProvider.getLoggedInUsers().getMax(), equalTo(1L)); + assertThat(openIdProvider.getLoggedInUsers().getTotal(), equalTo(1L)); + } + + @Test + public void testExpiredIdToken() throws Exception + { + setup(null); + long idTokenExpiryTime = 2000; + openIdProvider.setIdTokenExpiry(idTokenExpiryTime); + openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); + + String appUriString = "http://localhost:" + connector.getLocalPort(); + + // Initially not authenticated + ContentResponse response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + String content = response.getContentAsString(); + assertThat(content, containsString("not authenticated")); + + // Request to login is success + response = client.GET(appUriString + "/login"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("success")); + + // Now authenticated we can get info + client.setFollowRedirects(false); + response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("userId: 123456789")); + assertThat(content, containsString("name: Alice")); + assertThat(content, containsString("email: Alice@example.com")); + + // After waiting past ID_Token expiry time we are no longer authenticated. + // Even though this page is non-mandatory authentication the OpenId attributes should be cleared. + Thread.sleep(idTokenExpiryTime * 2); + response = client.GET(appUriString + "/"); assertThat(response.getStatus(), is(HttpStatus.OK_200)); content = response.getContentAsString(); assertThat(content, containsString("not authenticated")); - // Test that the user was logged out successfully on the openid provider. - assertThat(openIdProvider.getLoggedInUsers().getCurrent(), equalTo(0L)); + // Attempting to access a secured page requires you to re-login. + response = client.GET(appUriString + "/profile"); + assertThat(response.getStatus(), is(HttpStatus.SEE_OTHER_303)); + assertThat(response.getHeaders().get(HttpHeader.LOCATION), startsWith(openIdProvider.getProvider() + "/auth")); + + // User was never redirected to logout page. + assertThat(openIdProvider.getLoggedInUsers().getCurrent(), equalTo(1L)); assertThat(openIdProvider.getLoggedInUsers().getMax(), equalTo(1L)); assertThat(openIdProvider.getLoggedInUsers().getTotal(), equalTo(1L)); } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java index 7e84f31d4f19..5cc60db3e224 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java @@ -61,6 +61,8 @@ public class OpenIdProvider extends ContainerLifeCycle private String provider; private User preAuthedUser; private final CounterStatistic loggedInUsers = new CounterStatistic(); + private long _idTokenExpiry = 10000; + public static void main(String[] args) throws Exception { @@ -103,6 +105,16 @@ public OpenIdProvider(String clientId, String clientSecret) addBean(server); } + public void setIdTokenExpiry(long expiry) + { + _idTokenExpiry = expiry; + } + + public long getIdTokenExpiry() + { + return _idTokenExpiry; + } + public void join() throws InterruptedException { server.join(); @@ -289,7 +301,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S long expiry = System.currentTimeMillis() + Duration.ofMinutes(10).toMillis(); String response = "{" + "\"access_token\": \"" + accessToken + "\"," + - "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId)) + "\"," + + "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId, _idTokenExpiry)) + "\"," + "\"expires_in\": " + expiry + "," + "\"token_type\": \"Bearer\"" + "}"; @@ -374,10 +386,11 @@ public String getSubject() return subject; } - public String getIdToken(String provider, String clientId) + public String getIdToken(String provider, String clientId, long expiry) { - long expiry = System.currentTimeMillis() + Duration.ofMinutes(1).toMillis(); - return JwtEncoder.createIdToken(provider, clientId, subject, name, expiry); + long currentTimeMillis = System.currentTimeMillis(); + long expiryTime = (currentTimeMillis + expiry) / 1000; + return JwtEncoder.createIdToken(provider, clientId, subject, name, expiryTime); } @Override From 93f88f19d83d6b4497dff7ad5771c00df4480849 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 17 Mar 2023 12:46:40 +0900 Subject: [PATCH 3/7] fix checkstyle error Signed-off-by: Lachlan Roberts --- .../java/org/eclipse/jetty/security/openid/OpenIdProvider.java | 1 - 1 file changed, 1 deletion(-) diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java index 5cc60db3e224..18c025e57ce0 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java @@ -63,7 +63,6 @@ public class OpenIdProvider extends ContainerLifeCycle private final CounterStatistic loggedInUsers = new CounterStatistic(); private long _idTokenExpiry = 10000; - public static void main(String[] args) throws Exception { String clientId = "CLIENT_ID123"; From aa1928828c2dc557472ff617ff44bdf76dd5de43 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 23 Mar 2023 15:32:17 +1100 Subject: [PATCH 4/7] Add respectIdTokenExpiry configuration If respectIdTokenExpiry is true always attempt re-authentication if idToken expires even for non-mandatory requests. Signed-off-by: Lachlan Roberts --- .../src/main/config/etc/jetty-openid.xml | 3 + .../src/main/config/modules/openid.mod | 3 + .../security/openid/OpenIdAuthenticator.java | 16 +++-- .../security/openid/OpenIdConfiguration.java | 11 ++++ .../security/openid/OpenIdLoginService.java | 2 +- .../openid/OpenIdAuthenticationTest.java | 61 ++++++++++++++++--- .../jetty/security/openid/OpenIdProvider.java | 5 +- 7 files changed, 84 insertions(+), 17 deletions(-) diff --git a/jetty-openid/src/main/config/etc/jetty-openid.xml b/jetty-openid/src/main/config/etc/jetty-openid.xml index 0e6c2a06a232..564b5fa316d3 100644 --- a/jetty-openid/src/main/config/etc/jetty-openid.xml +++ b/jetty-openid/src/main/config/etc/jetty-openid.xml @@ -38,6 +38,9 @@ + + + diff --git a/jetty-openid/src/main/config/modules/openid.mod b/jetty-openid/src/main/config/modules/openid.mod index c1070b29910a..5d538b0db5e9 100644 --- a/jetty-openid/src/main/config/modules/openid.mod +++ b/jetty-openid/src/main/config/modules/openid.mod @@ -45,3 +45,6 @@ etc/jetty-openid.xml ## What authentication method to use with the Token Endpoint (client_secret_post, client_secret_basic). # jetty.openid.authMethod=client_secret_post + +## Whether the user should be logged out after the idToken expiry. +# jetty.openid.respectIdTokenExpiry=false diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index 6ea048eb38c3..e7624b836453 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -402,12 +402,18 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b uri = URIUtil.SLASH; HttpSession session = request.getSession(false); - if (hasExpiredIdToken(session)) + if (_openIdConfiguration.isRespectIdTokenExpiry() && hasExpiredIdToken(session)) + { logoutWithoutRedirect(request); - - mandatory |= isJSecurityCheck(uri); - if (!mandatory) - return new DeferredAuthentication(this); + } + else + { + // If we expired a valid authentication we do not want to defer authentication, + // we want to try re-authenticate the user. + mandatory |= isJSecurityCheck(uri); + if (!mandatory) + return new DeferredAuthentication(this); + } if (isErrorPage(baseRequest.getPathInContext()) && !DeferredAuthentication.isDeferred(response)) return new DeferredAuthentication(this); diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java index 80725f52d350..917386cc90c7 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java @@ -55,6 +55,7 @@ public class OpenIdConfiguration extends ContainerLifeCycle private String tokenEndpoint; private String endSessionEndpoint; private boolean authenticateNewUsers = false; + private boolean respectIdTokenExpiry = false; /** * Create an OpenID configuration for a specific OIDC provider. @@ -275,6 +276,16 @@ public void setAuthenticateNewUsers(boolean authenticateNewUsers) this.authenticateNewUsers = authenticateNewUsers; } + public boolean isRespectIdTokenExpiry() + { + return respectIdTokenExpiry; + } + + public void setRespectIdTokenExpiry(boolean respectIdTokenExpiry) + { + this.respectIdTokenExpiry = respectIdTokenExpiry; + } + private static HttpClient newHttpClient() { ClientConnector connector = new ClientConnector(); diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java index aca737313c97..a632669c51ab 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java @@ -137,7 +137,7 @@ public boolean validate(UserIdentity user) if (!(user.getUserPrincipal() instanceof OpenIdUserPrincipal)) return false; OpenIdUserPrincipal userPrincipal = (OpenIdUserPrincipal)user.getUserPrincipal(); - if (userPrincipal.getCredentials().isExpired()) + if (configuration.isRespectIdTokenExpiry() && userPrincipal.getCredentials().isExpired()) return false; return loginService == null || loginService.validate(user); } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index be43cffd3fdd..ffe0f5e7c87f 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -65,6 +66,11 @@ public class OpenIdAuthenticationTest private HttpClient client; public void setup(LoginService loginService) throws Exception + { + setup(loginService, null); + } + + public void setup(LoginService loginService, Consumer configure) throws Exception { openIdProvider = new OpenIdProvider(CLIENT_ID, CLIENT_SECRET); openIdProvider.start(); @@ -114,7 +120,10 @@ public void setup(LoginService loginService) throws Exception securityHandler.addConstraintMapping(adminMapping); // Authentication using local OIDC Provider - server.addBean(new OpenIdConfiguration(openIdProvider.getProvider(), CLIENT_ID, CLIENT_SECRET)); + OpenIdConfiguration openIdConfiguration = new OpenIdConfiguration(openIdProvider.getProvider(), CLIENT_ID, CLIENT_SECRET); + if (configure != null) + configure.accept(openIdConfiguration); + server.addBean(openIdConfiguration); securityHandler.setInitParameter(OpenIdAuthenticator.REDIRECT_PATH, "/redirect_path"); securityHandler.setInitParameter(OpenIdAuthenticator.ERROR_PAGE, "/error"); securityHandler.setInitParameter(OpenIdAuthenticator.LOGOUT_REDIRECT_PATH, "/"); @@ -272,7 +281,7 @@ public boolean validate(UserIdentity user) @Test public void testExpiredIdToken() throws Exception { - setup(null); + setup(null, config -> config.setRespectIdTokenExpiry(true)); long idTokenExpiryTime = 2000; openIdProvider.setIdTokenExpiry(idTokenExpiryTime); openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); @@ -302,14 +311,9 @@ public void testExpiredIdToken() throws Exception // After waiting past ID_Token expiry time we are no longer authenticated. // Even though this page is non-mandatory authentication the OpenId attributes should be cleared. + // This then attempts re-authorization the first time even though it is non-mandatory page. Thread.sleep(idTokenExpiryTime * 2); response = client.GET(appUriString + "/"); - assertThat(response.getStatus(), is(HttpStatus.OK_200)); - content = response.getContentAsString(); - assertThat(content, containsString("not authenticated")); - - // Attempting to access a secured page requires you to re-login. - response = client.GET(appUriString + "/profile"); assertThat(response.getStatus(), is(HttpStatus.SEE_OTHER_303)); assertThat(response.getHeaders().get(HttpHeader.LOCATION), startsWith(openIdProvider.getProvider() + "/auth")); @@ -319,6 +323,47 @@ public void testExpiredIdToken() throws Exception assertThat(openIdProvider.getLoggedInUsers().getTotal(), equalTo(1L)); } + @Test + public void testExpiredIdTokenDisabled() throws Exception + { + setup(null); + long idTokenExpiryTime = 2000; + openIdProvider.setIdTokenExpiry(idTokenExpiryTime); + openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); + + String appUriString = "http://localhost:" + connector.getLocalPort(); + + // Initially not authenticated + ContentResponse response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + String content = response.getContentAsString(); + assertThat(content, containsString("not authenticated")); + + // Request to login is success + response = client.GET(appUriString + "/login"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("success")); + + // Now authenticated we can get info + client.setFollowRedirects(false); + response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("userId: 123456789")); + assertThat(content, containsString("name: Alice")); + assertThat(content, containsString("email: Alice@example.com")); + + // After waiting past ID_Token expiry time we are still authenticated because respectIdTokenExpiry is false by default. + Thread.sleep(idTokenExpiryTime * 2); + response = client.GET(appUriString + "/"); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + content = response.getContentAsString(); + assertThat(content, containsString("userId: 123456789")); + assertThat(content, containsString("name: Alice")); + assertThat(content, containsString("email: Alice@example.com")); + } + public static class LoginPage extends HttpServlet { @Override diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java index 18c025e57ce0..b927361ad374 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java @@ -36,7 +36,6 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; -import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.statistic.CounterStatistic; import org.slf4j.Logger; @@ -68,7 +67,7 @@ public static void main(String[] args) throws Exception String clientId = "CLIENT_ID123"; String clientSecret = "PASSWORD123"; int port = 5771; - String redirectUri = "http://localhost:8080/openid/auth"; + String redirectUri = "http://localhost:8080/j_security_check"; OpenIdProvider openIdProvider = new OpenIdProvider(clientId, clientSecret); openIdProvider.addRedirectUri(redirectUri); @@ -184,7 +183,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO } String scopeString = req.getParameter("scope"); - List scopes = (scopeString == null) ? Collections.emptyList() : Arrays.asList(StringUtil.csvSplit(scopeString)); + List scopes = (scopeString == null) ? Collections.emptyList() : Arrays.asList(scopeString.split(" ")); if (!scopes.contains("openid")) { resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no openid scope"); From 27741bedae97ecceade23c84d6fb274790638d5f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 24 Mar 2023 14:11:46 +1100 Subject: [PATCH 5/7] changes from review Signed-off-by: Lachlan Roberts --- .../security/openid/OpenIdAuthenticator.java | 19 ++++++++----------- .../security/openid/OpenIdCredentials.java | 5 +++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index e7624b836453..fc4f2c3c48fe 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -377,11 +377,7 @@ private boolean hasExpiredIdToken(HttpSession session) { Map claims = (Map)session.getAttribute(CLAIMS); if (claims != null) - { - long expiry = (Long)claims.get("exp"); - long currentTimeSeconds = System.currentTimeMillis() / 1000; - return (currentTimeSeconds > expiry); - } + return OpenIdCredentials.checkExpiry(claims); } return false; } @@ -404,17 +400,18 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b HttpSession session = request.getSession(false); if (_openIdConfiguration.isRespectIdTokenExpiry() && hasExpiredIdToken(session)) { + // After logout, fall through to the code below and send another login challenge. logoutWithoutRedirect(request); - } - else - { + // If we expired a valid authentication we do not want to defer authentication, // we want to try re-authenticate the user. - mandatory |= isJSecurityCheck(uri); - if (!mandatory) - return new DeferredAuthentication(this); + mandatory = true; } + mandatory |= isJSecurityCheck(uri); + if (!mandatory) + return new DeferredAuthentication(this); + if (isErrorPage(baseRequest.getPathInContext()) && !DeferredAuthentication.isDeferred(response)) return new DeferredAuthentication(this); diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index 648f08c355dd..7705fe27dd44 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -142,6 +142,11 @@ private void validateClaims(OpenIdConfiguration configuration) throws Exception } public boolean isExpired() + { + return checkExpiry(claims); + } + + public static boolean checkExpiry(Map claims) { if (claims == null) return true; From 2606b3fa48da095579bec78d5c18aeebec0120ea Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 24 Mar 2023 14:19:18 +1100 Subject: [PATCH 6/7] rename respectIdTokenExpiry to logoutWhenIdTokenIsExpired Signed-off-by: Lachlan Roberts --- jetty-openid/src/main/config/etc/jetty-openid.xml | 4 ++-- jetty-openid/src/main/config/modules/openid.mod | 2 +- .../jetty/security/openid/OpenIdAuthenticator.java | 2 +- .../jetty/security/openid/OpenIdConfiguration.java | 10 +++++----- .../jetty/security/openid/OpenIdLoginService.java | 2 +- .../security/openid/OpenIdAuthenticationTest.java | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/jetty-openid/src/main/config/etc/jetty-openid.xml b/jetty-openid/src/main/config/etc/jetty-openid.xml index 564b5fa316d3..acdddbf113fb 100644 --- a/jetty-openid/src/main/config/etc/jetty-openid.xml +++ b/jetty-openid/src/main/config/etc/jetty-openid.xml @@ -38,8 +38,8 @@ - - + + diff --git a/jetty-openid/src/main/config/modules/openid.mod b/jetty-openid/src/main/config/modules/openid.mod index 5d538b0db5e9..600c53cbccd4 100644 --- a/jetty-openid/src/main/config/modules/openid.mod +++ b/jetty-openid/src/main/config/modules/openid.mod @@ -47,4 +47,4 @@ etc/jetty-openid.xml # jetty.openid.authMethod=client_secret_post ## Whether the user should be logged out after the idToken expiry. -# jetty.openid.respectIdTokenExpiry=false +# jetty.openid.logoutWhenIdTokenIsExpired=false diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java index fc4f2c3c48fe..adb4c2ab1d64 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java @@ -398,7 +398,7 @@ public Authentication validateRequest(ServletRequest req, ServletResponse res, b uri = URIUtil.SLASH; HttpSession session = request.getSession(false); - if (_openIdConfiguration.isRespectIdTokenExpiry() && hasExpiredIdToken(session)) + if (_openIdConfiguration.isLogoutWhenIdTokenIsExpired() && hasExpiredIdToken(session)) { // After logout, fall through to the code below and send another login challenge. logoutWithoutRedirect(request); diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java index 917386cc90c7..76c69339d863 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdConfiguration.java @@ -55,7 +55,7 @@ public class OpenIdConfiguration extends ContainerLifeCycle private String tokenEndpoint; private String endSessionEndpoint; private boolean authenticateNewUsers = false; - private boolean respectIdTokenExpiry = false; + private boolean logoutWhenIdTokenIsExpired = false; /** * Create an OpenID configuration for a specific OIDC provider. @@ -276,14 +276,14 @@ public void setAuthenticateNewUsers(boolean authenticateNewUsers) this.authenticateNewUsers = authenticateNewUsers; } - public boolean isRespectIdTokenExpiry() + public boolean isLogoutWhenIdTokenIsExpired() { - return respectIdTokenExpiry; + return logoutWhenIdTokenIsExpired; } - public void setRespectIdTokenExpiry(boolean respectIdTokenExpiry) + public void setLogoutWhenIdTokenIsExpired(boolean logoutWhenIdTokenIsExpired) { - this.respectIdTokenExpiry = respectIdTokenExpiry; + this.logoutWhenIdTokenIsExpired = logoutWhenIdTokenIsExpired; } private static HttpClient newHttpClient() diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java index a632669c51ab..b2df98b71e82 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdLoginService.java @@ -137,7 +137,7 @@ public boolean validate(UserIdentity user) if (!(user.getUserPrincipal() instanceof OpenIdUserPrincipal)) return false; OpenIdUserPrincipal userPrincipal = (OpenIdUserPrincipal)user.getUserPrincipal(); - if (configuration.isRespectIdTokenExpiry() && userPrincipal.getCredentials().isExpired()) + if (configuration.isLogoutWhenIdTokenIsExpired() && userPrincipal.getCredentials().isExpired()) return false; return loginService == null || loginService.validate(user); } diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index ffe0f5e7c87f..fc7cf3951aa2 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -281,7 +281,7 @@ public boolean validate(UserIdentity user) @Test public void testExpiredIdToken() throws Exception { - setup(null, config -> config.setRespectIdTokenExpiry(true)); + setup(null, config -> config.setLogoutWhenIdTokenIsExpired(true)); long idTokenExpiryTime = 2000; openIdProvider.setIdTokenExpiry(idTokenExpiryTime); openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); @@ -354,7 +354,7 @@ public void testExpiredIdTokenDisabled() throws Exception assertThat(content, containsString("name: Alice")); assertThat(content, containsString("email: Alice@example.com")); - // After waiting past ID_Token expiry time we are still authenticated because respectIdTokenExpiry is false by default. + // After waiting past ID_Token expiry time we are still authenticated because logoutWhenIdTokenIsExpired is false by default. Thread.sleep(idTokenExpiryTime * 2); response = client.GET(appUriString + "/"); assertThat(response.getStatus(), is(HttpStatus.OK_200)); From b0790926eaf11082812fd2c644183beae66b1588 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 29 Mar 2023 01:08:14 +1100 Subject: [PATCH 7/7] changes from review Signed-off-by: Lachlan Roberts --- .../src/main/config/modules/openid.mod | 2 +- .../security/openid/OpenIdCredentials.java | 5 +- .../openid/OpenIdAuthenticationTest.java | 4 +- .../jetty/security/openid/OpenIdProvider.java | 28 +++--- .../distribution/openid/OpenIdProvider.java | 98 ++++++++++++++++--- 5 files changed, 105 insertions(+), 32 deletions(-) diff --git a/jetty-openid/src/main/config/modules/openid.mod b/jetty-openid/src/main/config/modules/openid.mod index 600c53cbccd4..4c4cbc18c299 100644 --- a/jetty-openid/src/main/config/modules/openid.mod +++ b/jetty-openid/src/main/config/modules/openid.mod @@ -46,5 +46,5 @@ etc/jetty-openid.xml ## What authentication method to use with the Token Endpoint (client_secret_post, client_secret_basic). # jetty.openid.authMethod=client_secret_post -## Whether the user should be logged out after the idToken expiry. +## Whether the user should be logged out after the idToken expires. # jetty.openid.logoutWhenIdTokenIsExpired=false diff --git a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java index 7705fe27dd44..df3e18af6a02 100644 --- a/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java +++ b/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdCredentials.java @@ -15,6 +15,7 @@ import java.io.Serializable; import java.net.URI; +import java.time.Instant; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -152,9 +153,7 @@ public static boolean checkExpiry(Map claims) return true; // Check that the ID token has not expired by checking the exp claim. - long expiry = (Long)claims.get("exp"); - long currentTimeSeconds = System.currentTimeMillis() / 1000; - return (currentTimeSeconds > expiry); + return Instant.ofEpochSecond((Long)claims.get("exp")).isBefore(Instant.now()); } private void validateAudience(OpenIdConfiguration configuration) throws AuthenticationException diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java index fc7cf3951aa2..e4784a0a2da3 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdAuthenticationTest.java @@ -283,7 +283,7 @@ public void testExpiredIdToken() throws Exception { setup(null, config -> config.setLogoutWhenIdTokenIsExpired(true)); long idTokenExpiryTime = 2000; - openIdProvider.setIdTokenExpiry(idTokenExpiryTime); + openIdProvider.setIdTokenDuration(idTokenExpiryTime); openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); String appUriString = "http://localhost:" + connector.getLocalPort(); @@ -328,7 +328,7 @@ public void testExpiredIdTokenDisabled() throws Exception { setup(null); long idTokenExpiryTime = 2000; - openIdProvider.setIdTokenExpiry(idTokenExpiryTime); + openIdProvider.setIdTokenDuration(idTokenExpiryTime); openIdProvider.setUser(new OpenIdProvider.User("123456789", "Alice")); String appUriString = "http://localhost:" + connector.getLocalPort(); diff --git a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java index b927361ad374..56d1ec79299d 100644 --- a/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java +++ b/jetty-openid/src/test/java/org/eclipse/jetty/security/openid/OpenIdProvider.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.io.PrintWriter; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -60,7 +61,7 @@ public class OpenIdProvider extends ContainerLifeCycle private String provider; private User preAuthedUser; private final CounterStatistic loggedInUsers = new CounterStatistic(); - private long _idTokenExpiry = 10000; + private long _idTokenDuration = Duration.ofSeconds(10).toMillis(); public static void main(String[] args) throws Exception { @@ -103,14 +104,14 @@ public OpenIdProvider(String clientId, String clientSecret) addBean(server); } - public void setIdTokenExpiry(long expiry) + public void setIdTokenDuration(long duration) { - _idTokenExpiry = expiry; + _idTokenDuration = duration; } - public long getIdTokenExpiry() + public long getIdTokenDuration() { - return _idTokenExpiry; + return _idTokenDuration; } public void join() throws InterruptedException @@ -296,11 +297,11 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S } String accessToken = "ABCDEFG"; - long expiry = System.currentTimeMillis() + Duration.ofMinutes(10).toMillis(); + long accessTokenDuration = Duration.ofMinutes(10).toSeconds(); String response = "{" + "\"access_token\": \"" + accessToken + "\"," + - "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId, _idTokenExpiry)) + "\"," + - "\"expires_in\": " + expiry + "," + + "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId, _idTokenDuration)) + "\"," + + "\"expires_in\": " + accessTokenDuration + "," + "\"token_type\": \"Bearer\"" + "}"; @@ -384,10 +385,9 @@ public String getSubject() return subject; } - public String getIdToken(String provider, String clientId, long expiry) + public String getIdToken(String provider, String clientId, long duration) { - long currentTimeMillis = System.currentTimeMillis(); - long expiryTime = (currentTimeMillis + expiry) / 1000; + long expiryTime = Instant.now().plusMillis(duration).getEpochSecond(); return JwtEncoder.createIdToken(provider, clientId, subject, name, expiryTime); } @@ -398,5 +398,11 @@ public boolean equals(Object obj) return false; return Objects.equals(subject, ((User)obj).subject) && Objects.equals(name, ((User)obj).name); } + + @Override + public int hashCode() + { + return Objects.hash(subject, name); + } } } diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/openid/OpenIdProvider.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/openid/OpenIdProvider.java index e57bf4d26fb4..8e5ad120b7f7 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/openid/OpenIdProvider.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/openid/OpenIdProvider.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.io.PrintWriter; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -37,8 +38,8 @@ import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.servlet.ServletContextHandler; import org.eclipse.jetty.servlet.ServletHolder; -import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.statistic.CounterStatistic; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -49,6 +50,7 @@ public class OpenIdProvider extends ContainerLifeCycle private static final String CONFIG_PATH = "/.well-known/openid-configuration"; private static final String AUTH_PATH = "/auth"; private static final String TOKEN_PATH = "/token"; + private static final String END_SESSION_PATH = "/end_session"; private final Map issuedAuthCodes = new HashMap<>(); protected final String clientId; @@ -59,13 +61,15 @@ public class OpenIdProvider extends ContainerLifeCycle private int port = 0; private String provider; private User preAuthedUser; + private final CounterStatistic loggedInUsers = new CounterStatistic(); + private long _idTokenDuration = Duration.ofSeconds(10).toMillis(); public static void main(String[] args) throws Exception { String clientId = "CLIENT_ID123"; String clientSecret = "PASSWORD123"; int port = 5771; - String redirectUri = "http://localhost:8080/openid/auth"; + String redirectUri = "http://localhost:8080/j_security_check"; OpenIdProvider openIdProvider = new OpenIdProvider(clientId, clientSecret); openIdProvider.addRedirectUri(redirectUri); @@ -92,14 +96,25 @@ public OpenIdProvider(String clientId, String clientSecret) ServletContextHandler contextHandler = new ServletContextHandler(); contextHandler.setContextPath("/"); - contextHandler.addServlet(new ServletHolder(new OpenIdConfigServlet()), CONFIG_PATH); - contextHandler.addServlet(new ServletHolder(new OpenIdAuthEndpoint()), AUTH_PATH); - contextHandler.addServlet(new ServletHolder(new OpenIdTokenEndpoint()), TOKEN_PATH); + contextHandler.addServlet(new ServletHolder(new ConfigServlet()), CONFIG_PATH); + contextHandler.addServlet(new ServletHolder(new AuthEndpoint()), AUTH_PATH); + contextHandler.addServlet(new ServletHolder(new TokenEndpoint()), TOKEN_PATH); + contextHandler.addServlet(new ServletHolder(new EndSessionEndpoint()), END_SESSION_PATH); server.setHandler(contextHandler); addBean(server); } + public void setIdTokenDuration(long duration) + { + _idTokenDuration = duration; + } + + public long getIdTokenDuration() + { + return _idTokenDuration; + } + public void join() throws InterruptedException { server.join(); @@ -113,6 +128,11 @@ public OpenIdConfiguration getOpenIdConfiguration() return new OpenIdConfiguration(provider, authEndpoint, tokenEndpoint, clientId, clientSecret, null); } + public CounterStatistic getLoggedInUsers() + { + return loggedInUsers; + } + @Override protected void doStart() throws Exception { @@ -145,7 +165,7 @@ public void addRedirectUri(String uri) redirectUris.add(uri); } - public class OpenIdAuthEndpoint extends HttpServlet + public class AuthEndpoint extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException @@ -165,7 +185,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO } String scopeString = req.getParameter("scope"); - List scopes = (scopeString == null) ? Collections.emptyList() : Arrays.asList(StringUtil.csvSplit(scopeString)); + List scopes = (scopeString == null) ? Collections.emptyList() : Arrays.asList(scopeString.split(" ")); if (!scopes.contains("openid")) { resp.sendError(HttpServletResponse.SC_FORBIDDEN, "no openid scope"); @@ -253,7 +273,7 @@ public void redirectUser(HttpServletRequest request, User user, String redirectU } } - public class OpenIdTokenEndpoint extends HttpServlet + private class TokenEndpoint extends HttpServlet { @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException @@ -278,20 +298,53 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S } String accessToken = "ABCDEFG"; - long expiry = System.currentTimeMillis() + Duration.ofMinutes(10).toMillis(); + long accessTokenDuration = Duration.ofMinutes(10).toSeconds(); String response = "{" + "\"access_token\": \"" + accessToken + "\"," + - "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId)) + "\"," + - "\"expires_in\": " + expiry + "," + + "\"id_token\": \"" + JwtEncoder.encode(user.getIdToken(provider, clientId, _idTokenDuration)) + "\"," + + "\"expires_in\": " + accessTokenDuration + "," + "\"token_type\": \"Bearer\"" + "}"; + loggedInUsers.increment(); resp.setContentType("text/plain"); resp.getWriter().print(response); } } - public class OpenIdConfigServlet extends HttpServlet + private class EndSessionEndpoint extends HttpServlet + { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + doPost(req, resp); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException + { + String idToken = req.getParameter("id_token_hint"); + if (idToken == null) + { + resp.sendError(HttpServletResponse.SC_BAD_REQUEST, "no id_token_hint"); + return; + } + + String logoutRedirect = req.getParameter("post_logout_redirect_uri"); + if (logoutRedirect == null) + { + resp.setStatus(HttpServletResponse.SC_OK); + resp.getWriter().println("logout success on end_session_endpoint"); + return; + } + + loggedInUsers.decrement(); + resp.setContentType("text/plain"); + resp.sendRedirect(logoutRedirect); + } + } + + private class ConfigServlet extends HttpServlet { @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException @@ -300,6 +353,7 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO "\"issuer\": \"" + provider + "\"," + "\"authorization_endpoint\": \"" + provider + AUTH_PATH + "\"," + "\"token_endpoint\": \"" + provider + TOKEN_PATH + "\"," + + "\"end_session_endpoint\": \"" + provider + END_SESSION_PATH + "\"," + "}"; resp.getWriter().write(discoveryDocument); @@ -332,10 +386,24 @@ public String getSubject() return subject; } - public String getIdToken(String provider, String clientId) + public String getIdToken(String provider, String clientId, long duration) + { + long expiryTime = Instant.now().plusMillis(duration).getEpochSecond(); + return JwtEncoder.createIdToken(provider, clientId, subject, name, expiryTime); + } + + @Override + public boolean equals(Object obj) + { + if (!(obj instanceof User)) + return false; + return Objects.equals(subject, ((User)obj).subject) && Objects.equals(name, ((User)obj).name); + } + + @Override + public int hashCode() { - long expiry = System.currentTimeMillis() + Duration.ofMinutes(1).toMillis(); - return JwtEncoder.createIdToken(provider, clientId, subject, name, expiry); + return Objects.hash(subject, name); } } }