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 c55524077d1663528847f657140e242d9f8f8549 Author: Robert Munteanu <[email protected]> AuthorDate: Wed Sep 18 23:26:32 2024 +0200 oidc-rp: split up OidcClient into separate classes Clumping those concerns together did not make sense and the name of the interface gave the impression that this is an entry point to the API - which it was not. --- .../extensions/oidc_rp/OAuthTokenRefresher.java | 32 +++++++++ .../apache/sling/extensions/oidc_rp/OAuthUris.java | 58 ++++++++++++++++ .../sling/extensions/oidc_rp/OidcClient.java | 70 ------------------- ...lientImpl.java => OAuthTokenRefresherImpl.java} | 77 ++------------------ .../oidc_rp/impl/OidcEntryPointServlet.java | 60 ++++++++++++++-- .../oidc_rp/impl/OidcProviderMetadataRegistry.java | 12 +++- .../oidc_rp/support/OAuthEnabledSlingServlet.java | 11 +-- .../sling/servlets/oidc_rp/impl/OAuthUrisTest.java | 59 ++++++++++++++++ ...mplTest.java => OidcEntryPointServletTest.java} | 81 ++++++++++------------ 9 files changed, 262 insertions(+), 198 deletions(-) diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthTokenRefresher.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthTokenRefresher.java new file mode 100644 index 00000000..3b00215f --- /dev/null +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthTokenRefresher.java @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.extensions.oidc_rp; + +public interface OAuthTokenRefresher { + + /** + * Refreshes the OIDC tokens based on the supplied refresh token + * + * <p>It is the responsibility of the invoker to persist the returned tokens.</p> + * + * @param connection The connection to start the OIDC flow for + * @param refreshToken An existing refresh token + * @return OIDC tokens + * @throws OidcException in case anything goes wrong + */ + OidcTokens refreshTokens(OidcConnection connection, String refreshToken) throws OidcException; +} diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthUris.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthUris.java new file mode 100644 index 00000000..e3b7fbf4 --- /dev/null +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OAuthUris.java @@ -0,0 +1,58 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.extensions.oidc_rp; + +import java.net.URI; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; + +import org.apache.sling.api.SlingHttpServletRequest; +import org.apache.sling.extensions.oidc_rp.impl.OidcEntryPointServlet; + +public abstract class OAuthUris { + + /** + * Generates a local URI to the OIDC entry point servlet + * + * <p>The URI can be used as-is to send a redirect to the user and start the OIDC flow.</p> + * + * @param connection The connection to start the OIDC flow for + * @param request The current request + * @param redirectPath The local redirect path to use after completing the OIDC flow + * @return a local URI + * @throws OidcException in case anything goes wrong + */ + public static URI getOidcEntryPointUri(OidcConnection connection, SlingHttpServletRequest request, String redirectPath) throws OidcException { + StringBuilder uri = new StringBuilder(); + uri.append(request.getScheme()).append("://").append(request.getServerName()); + boolean needsExplicitPort = ( "https".equals(request.getScheme()) && request.getServerPort() != 443 ) + || ( "http".equals(request.getScheme()) && request.getServerPort() != 80 ) ; + + if ( needsExplicitPort ) { + uri.append(':').append(request.getServerPort()); + } + uri.append(OidcEntryPointServlet.PATH).append("?c=").append(connection.name()); + if ( redirectPath != null ) + uri.append("&redirect=").append(URLEncoder.encode(redirectPath, StandardCharsets.UTF_8)); + + return URI.create(uri.toString()); + } + + private OAuthUris() { + // prevent instantiation + } +} diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OidcClient.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OidcClient.java deleted file mode 100644 index 12cbd33d..00000000 --- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/OidcClient.java +++ /dev/null @@ -1,70 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.sling.extensions.oidc_rp; - -import java.net.URI; - -import org.apache.sling.api.SlingHttpServletRequest; - -/** - * A client for dealing with over-the-network OIDC concerns - * - * <p>This client is able to generate URLs and make network calls related to OIDC.</p> - * - */ -public interface OidcClient { - - /** - * Generates a local URI to the OIDC entry point servlet - * - * <p>The URI can be used as-is to send a redirect to the user and start the OIDC flow.</p> - * - * @param connection The connection to start the OIDC flow for - * @param request The current request - * @param redirectPath The local redirect path to use after completing the OIDC flow - * @return a local URI - * @throws OidcException in case anything goes wrong - */ - URI getOidcEntryPointUri(OidcConnection connection, SlingHttpServletRequest request, String redirectPath) throws OidcException; - - /** - * Generates a URI to the OIDC provider's authorization endpoint - * - * <p>The URI can be used as-is to start the OIDC flow directly on the identity provider's side.</p> - * - * @param connection The connection to start the OIDC flow for - * @param request The current request - * @param redirectUri The redirect path to use after completing the OIDC flow - * @return a remote URI - * @throws OidcException in case anything goes wrong - */ - URI getAuthenticationRequestUri(OidcConnection connection, SlingHttpServletRequest request, URI redirectUri) throws OidcException; - - // OidcTokens getOidcTokens(OidcConnection connection, String authenticationCode) throws OidcException; - - /** - * Refreshes the OIDC tokens based on the supplied refresh token - * - * <p>It is the responsibility of the invoker to persist the returned tokens.</p> - * - * @param connection The connection to start the OIDC flow for - * @param refreshToken An existing refresh token - * @return OIDC tokens - * @throws OidcException in case anything goes wrong - */ - OidcTokens refreshTokens(OidcConnection connection, String refreshToken) throws OidcException; -} diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcClientImpl.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OAuthTokenRefresherImpl.java similarity index 51% rename from org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcClientImpl.java rename to org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OAuthTokenRefresherImpl.java index ee8cc394..47b25bbf 100644 --- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcClientImpl.java +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OAuthTokenRefresherImpl.java @@ -16,17 +16,10 @@ */ package org.apache.sling.extensions.oidc_rp.impl; -import static org.apache.sling.extensions.oidc_rp.impl.OidcStateManager.PARAMETER_NAME_CONNECTION; -import static org.apache.sling.extensions.oidc_rp.impl.OidcStateManager.PARAMETER_NAME_REDIRECT; - import java.io.IOException; import java.net.URI; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; -import java.util.Arrays; -import org.apache.sling.api.SlingHttpServletRequest; -import org.apache.sling.extensions.oidc_rp.OidcClient; +import org.apache.sling.extensions.oidc_rp.OAuthTokenRefresher; import org.apache.sling.extensions.oidc_rp.OidcConnection; import org.apache.sling.extensions.oidc_rp.OidcException; import org.apache.sling.extensions.oidc_rp.OidcTokens; @@ -37,29 +30,23 @@ import org.osgi.service.component.annotations.Reference; import com.nimbusds.oauth2.sdk.AuthorizationGrant; import com.nimbusds.oauth2.sdk.ParseException; import com.nimbusds.oauth2.sdk.RefreshTokenGrant; -import com.nimbusds.oauth2.sdk.ResponseType; -import com.nimbusds.oauth2.sdk.Scope; import com.nimbusds.oauth2.sdk.TokenErrorResponse; import com.nimbusds.oauth2.sdk.TokenRequest; import com.nimbusds.oauth2.sdk.auth.ClientAuthentication; import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic; import com.nimbusds.oauth2.sdk.auth.Secret; import com.nimbusds.oauth2.sdk.id.ClientID; -import com.nimbusds.oauth2.sdk.id.State; import com.nimbusds.oauth2.sdk.token.RefreshToken; -import com.nimbusds.openid.connect.sdk.AuthenticationRequest; -import com.nimbusds.openid.connect.sdk.Nonce; import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; -import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; import com.nimbusds.openid.connect.sdk.token.OIDCTokens; -@Component(service = { OidcClientImpl.class, OidcClient.class }) -public class OidcClientImpl implements OidcClient { +@Component +public class OAuthTokenRefresherImpl implements OAuthTokenRefresher { private final OidcProviderMetadataRegistry providerMetadataRegistry; @Activate - public OidcClientImpl(@Reference OidcProviderMetadataRegistry providerMetadataRegistry) { + public OAuthTokenRefresherImpl(@Reference OidcProviderMetadataRegistry providerMetadataRegistry) { this.providerMetadataRegistry = providerMetadataRegistry; } @@ -102,60 +89,4 @@ public class OidcClientImpl implements OidcClient { throw new OidcException(e); } } - - @Override - public URI getOidcEntryPointUri(OidcConnection connection, SlingHttpServletRequest request, String redirectPath) { - - StringBuilder uri = new StringBuilder(); - uri.append(request.getScheme()).append("://").append(request.getServerName()); - boolean needsExplicitPort = ( "https".equals(request.getScheme()) && request.getServerPort() != 443 ) - || ( "http".equals(request.getScheme()) && request.getServerPort() != 80 ) ; - - if ( needsExplicitPort ) { - uri.append(':').append(request.getServerPort()); - } - uri.append(OidcEntryPointServlet.PATH).append("?c=").append(connection.name()); - if ( redirectPath != null ) - uri.append("&redirect=").append(URLEncoder.encode(redirectPath, StandardCharsets.UTF_8)); - - return URI.create(uri.toString()); - } - - @Override - public URI getAuthenticationRequestUri(OidcConnection connection, SlingHttpServletRequest request, URI redirectUri) { - OIDCProviderMetadata providerMetadata = providerMetadataRegistry.getProviderMetadata(connection.baseUrl()); - - // The client ID provisioned by the OpenID provider when - // the client was registered - ClientID clientID = new ClientID(connection.clientId()); - - // Generate random state string to securely pair the callback to this request - State state = new State(); - OidcStateManager stateManager = OidcStateManager.stateFor(request); - stateManager.registerState(state); - stateManager.putAttribute(state, PARAMETER_NAME_CONNECTION, connection.name()); - if ( request.getParameter(PARAMETER_NAME_REDIRECT) != null ) - stateManager.putAttribute(state, PARAMETER_NAME_REDIRECT, request.getParameter(PARAMETER_NAME_REDIRECT)); - - // Generate nonce for the ID token - Nonce nonce = new Nonce(); - - // Compose the OpenID authentication request (for the code flow) - AuthenticationRequest.Builder authRequestBuilder = new AuthenticationRequest.Builder( - new ResponseType("code"), - new Scope(connection.scopes()), - clientID, URI.create(OidcCallbackServlet.getCallbackUri(request))) - .endpointURI(providerMetadata.getAuthorizationEndpointURI()) - .state(state) - .nonce(nonce); - - if ( connection.additionalAuthorizationParameters() != null ) { - Arrays.stream(connection.additionalAuthorizationParameters()) - .map( s -> s.split("=") ) - .filter( p -> p.length == 2 ) - .forEach( p -> authRequestBuilder.customParameter(p[0], p[1])); - } - - return authRequestBuilder.build().toURI(); - } } diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcEntryPointServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcEntryPointServlet.java index 2daa0698..cdf856c0 100644 --- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcEntryPointServlet.java +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcEntryPointServlet.java @@ -16,10 +16,13 @@ */ package org.apache.sling.extensions.oidc_rp.impl; +import static org.apache.sling.extensions.oidc_rp.impl.OidcStateManager.PARAMETER_NAME_CONNECTION; +import static org.apache.sling.extensions.oidc_rp.impl.OidcStateManager.PARAMETER_NAME_REDIRECT; import static org.osgi.service.component.annotations.ReferencePolicyOption.GREEDY; import java.io.IOException; import java.net.URI; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.function.Function; @@ -33,7 +36,6 @@ import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.SlingHttpServletResponse; import org.apache.sling.api.servlets.SlingAllMethodsServlet; import org.apache.sling.auth.core.AuthConstants; -import org.apache.sling.extensions.oidc_rp.OidcClient; import org.apache.sling.extensions.oidc_rp.OidcConnection; import org.apache.sling.servlets.annotations.SlingServletPaths; import org.osgi.service.component.annotations.Activate; @@ -42,6 +44,14 @@ import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.nimbusds.oauth2.sdk.ResponseType; +import com.nimbusds.oauth2.sdk.Scope; +import com.nimbusds.oauth2.sdk.id.ClientID; +import com.nimbusds.oauth2.sdk.id.State; +import com.nimbusds.openid.connect.sdk.AuthenticationRequest; +import com.nimbusds.openid.connect.sdk.Nonce; +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; + @Component(service = { Servlet.class }, property = { AuthConstants.AUTH_REQUIREMENTS +"=" + OidcEntryPointServlet.PATH } ) @@ -49,21 +59,22 @@ import org.slf4j.LoggerFactory; public class OidcEntryPointServlet extends SlingAllMethodsServlet { private static final long serialVersionUID = 1L; - static final String PATH = "/system/sling/oidc/entry-point"; // NOSONAR + public static final String PATH = "/system/sling/oidc/entry-point"; // NOSONAR private final Logger logger = LoggerFactory.getLogger(getClass()); private final Map<String, OidcConnection> connections; - private final OidcClient oidcClient; + private final OidcProviderMetadataRegistry oidcRegistry; @Activate public OidcEntryPointServlet(@Reference(policyOption = GREEDY) List<OidcConnection> connections, - @Reference OidcClient oidcClient) { + @Reference OidcProviderMetadataRegistry oidcRegistry) { this.connections = connections.stream() .collect(Collectors.toMap( OidcConnection::name, Function.identity())); - this.oidcClient = oidcClient; + this.oidcRegistry = oidcRegistry; } + @Override protected void doGet(SlingHttpServletRequest request, SlingHttpServletResponse response) throws ServletException, IOException { @@ -85,6 +96,43 @@ public class OidcEntryPointServlet extends SlingAllMethodsServlet { if ( connection.baseUrl() == null ) throw new ServletException("Misconfigured baseUrl"); - response.sendRedirect(oidcClient.getAuthenticationRequestUri(connection, request, URI.create(OidcCallbackServlet.getCallbackUri(request))).toString()); + response.sendRedirect(getAuthenticationRequestUri(connection, request, URI.create(OidcCallbackServlet.getCallbackUri(request))).toString()); + } + + private URI getAuthenticationRequestUri(OidcConnection connection, SlingHttpServletRequest request, URI redirectUri) { + OIDCProviderMetadata providerMetadata = oidcRegistry.getProviderMetadata(connection.baseUrl()); + + // The client ID provisioned by the OpenID provider when + // the client was registered + ClientID clientID = new ClientID(connection.clientId()); + + // Generate random state string to securely pair the callback to this request + State state = new State(); + OidcStateManager stateManager = OidcStateManager.stateFor(request); + stateManager.registerState(state); + stateManager.putAttribute(state, PARAMETER_NAME_CONNECTION, connection.name()); + if ( request.getParameter(PARAMETER_NAME_REDIRECT) != null ) + stateManager.putAttribute(state, PARAMETER_NAME_REDIRECT, request.getParameter(PARAMETER_NAME_REDIRECT)); + + // Generate nonce for the ID token + Nonce nonce = new Nonce(); + + // Compose the OpenID authentication request (for the code flow) + AuthenticationRequest.Builder authRequestBuilder = new AuthenticationRequest.Builder( + new ResponseType("code"), + new Scope(connection.scopes()), + clientID, URI.create(OidcCallbackServlet.getCallbackUri(request))) + .endpointURI(providerMetadata.getAuthorizationEndpointURI()) + .state(state) + .nonce(nonce); + + if ( connection.additionalAuthorizationParameters() != null ) { + Arrays.stream(connection.additionalAuthorizationParameters()) + .map( s -> s.split("=") ) + .filter( p -> p.length == 2 ) + .forEach( p -> authRequestBuilder.customParameter(p[0], p[1])); + } + + return authRequestBuilder.build().toURI(); } } diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcProviderMetadataRegistry.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcProviderMetadataRegistry.java index 6a147a5d..66a2c4c3 100644 --- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcProviderMetadataRegistry.java +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/impl/OidcProviderMetadataRegistry.java @@ -17,6 +17,7 @@ package org.apache.sling.extensions.oidc_rp.impl; import java.io.IOException; +import java.net.URI; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -40,7 +41,8 @@ import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; public class OidcProviderMetadataRegistry { private final ConcurrentMap<String, OIDCProviderMetadata> cache = new ConcurrentHashMap<>(); - public OIDCProviderMetadata getProviderMetadata(String base) { + // visible for testing + protected OIDCProviderMetadata getProviderMetadata(String base) { return cache.computeIfAbsent(base, s -> { try { return OIDCProviderMetadata.resolve(new Issuer(s)); @@ -49,4 +51,12 @@ public class OidcProviderMetadataRegistry { } }); } + + public URI getTokenEndpoint(String base) { + return getProviderMetadata(base).getTokenEndpointURI(); + } + + public URI getAuthorizationEndpoint(String base) { + return getProviderMetadata(base).getAuthorizationEndpointURI(); + } } \ No newline at end of file diff --git a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/support/OAuthEnabledSlingServlet.java b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/support/OAuthEnabledSlingServlet.java index a4d38506..9693e60a 100644 --- a/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/support/OAuthEnabledSlingServlet.java +++ b/org.apache.sling.servlets.oidc-rp/src/main/java/org/apache/sling/extensions/oidc_rp/support/OAuthEnabledSlingServlet.java @@ -25,7 +25,8 @@ import javax.servlet.http.HttpServletResponse; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.SlingHttpServletResponse; import org.apache.sling.api.servlets.SlingSafeMethodsServlet; -import org.apache.sling.extensions.oidc_rp.OidcClient; +import org.apache.sling.extensions.oidc_rp.OAuthUris; +import org.apache.sling.extensions.oidc_rp.OAuthTokenRefresher; import org.apache.sling.extensions.oidc_rp.OidcConnection; import org.apache.sling.extensions.oidc_rp.OidcToken; import org.apache.sling.extensions.oidc_rp.OidcTokenState; @@ -45,9 +46,9 @@ public abstract class OAuthEnabledSlingServlet extends SlingSafeMethodsServlet { private final OidcTokenStore tokenStore; - private final OidcClient oidcClient; + private final OAuthTokenRefresher oidcClient; - protected OAuthEnabledSlingServlet(OidcConnection connection, OidcTokenStore tokenStore, OidcClient oidcClient) { + protected OAuthEnabledSlingServlet(OidcConnection connection, OidcTokenStore tokenStore, OAuthTokenRefresher oidcClient) { this.connection = Objects.requireNonNull(connection, "connection may not null"); this.tokenStore = Objects.requireNonNull(tokenStore, "tokenStore may not null"); this.oidcClient = Objects.requireNonNull(oidcClient, "oidcClient may not null"); @@ -74,12 +75,12 @@ public abstract class OAuthEnabledSlingServlet extends SlingSafeMethodsServlet { doGetWithToken(request, response, tokenResponse); break; case MISSING: - response.sendRedirect(oidcClient.getOidcEntryPointUri(connection, request, redirectPath).toString()); + response.sendRedirect(OAuthUris.getOidcEntryPointUri(connection, request, redirectPath).toString()); break; case EXPIRED: OidcToken refreshToken = tokenStore.getRefreshToken(connection, request.getResourceResolver()); if ( refreshToken.getState() != OidcTokenState.VALID ) { - response.sendRedirect(oidcClient.getOidcEntryPointUri(connection, request, redirectPath).toString()); + response.sendRedirect(OAuthUris.getOidcEntryPointUri(connection, request, redirectPath).toString()); return; } diff --git a/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OAuthUrisTest.java b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OAuthUrisTest.java new file mode 100644 index 00000000..27ba7325 --- /dev/null +++ b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OAuthUrisTest.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.sling.servlets.oidc_rp.impl; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.net.URI; + +import org.apache.sling.extensions.oidc_rp.OAuthUris; +import org.apache.sling.testing.mock.sling.junit5.SlingContext; +import org.apache.sling.testing.mock.sling.junit5.SlingContextExtension; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; + +@ExtendWith(SlingContextExtension.class) +class OAuthUrisTest { + + private final SlingContext context = new SlingContext(); + + @Test + void testRedirectUri() { + URI redirectUri = OAuthUris.getOidcEntryPointUri(MockOidcConnection.DEFAULT_CONNECTION, context.request(), "/foo"); + + assertThat(redirectUri).as("redirect uri") + .hasScheme("http") + .hasHost("localhost") + .hasNoPort() + .hasPath("/system/sling/oidc/entry-point") + .hasQuery("c=mock-oidc&redirect=/foo"); + } + + @Test + void testRedirectUri_customPort_noRedirect() { + context.request().setServerPort(8080); + URI redirectUri = OAuthUris.getOidcEntryPointUri(MockOidcConnection.DEFAULT_CONNECTION, context.request(), null); + + assertThat(redirectUri).as("redirect uri") + .hasScheme("http") + .hasHost("localhost") + .hasPort(8080) + .hasPath("/system/sling/oidc/entry-point") + .hasQuery("c=mock-oidc"); + } + +} diff --git a/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcClientImplTest.java b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcEntryPointServletTest.java similarity index 56% rename from org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcClientImplTest.java rename to org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcEntryPointServletTest.java index 23c01814..10c0f413 100644 --- a/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcClientImplTest.java +++ b/org.apache.sling.servlets.oidc-rp/src/test/java/org/apache/sling/servlets/oidc_rp/impl/OidcEntryPointServletTest.java @@ -18,14 +18,21 @@ package org.apache.sling.servlets.oidc_rp.impl; import static org.assertj.core.api.Assertions.assertThat; +import java.io.IOException; import java.net.URI; +import java.util.Arrays; import java.util.Collections; +import java.util.List; +import java.util.Objects; + +import javax.servlet.ServletException; import org.apache.sling.extensions.oidc_rp.OidcConnection; -import org.apache.sling.extensions.oidc_rp.impl.OidcClientImpl; +import org.apache.sling.extensions.oidc_rp.impl.OidcEntryPointServlet; import org.apache.sling.extensions.oidc_rp.impl.OidcProviderMetadataRegistry; import org.apache.sling.testing.mock.sling.junit5.SlingContext; import org.apache.sling.testing.mock.sling.junit5.SlingContextExtension; +import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletResponse; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -35,17 +42,23 @@ import com.nimbusds.openid.connect.sdk.SubjectType; import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; @ExtendWith(SlingContextExtension.class) -class OidcClientImplTest { +class OidcEntryPointServletTest { - private final SlingContext context = new SlingContext(); + private static final String MOCK_OIDC_PARAM = "mock-oidc-param"; - private OidcClientImpl clientImpl; + private final SlingContext context = new SlingContext(); + private List<OidcConnection> connections; + private OidcEntryPointServlet servlet; @BeforeEach - void initClient() { - clientImpl = new OidcClientImpl(new OidcProviderMetadataRegistry() { + void initServlet() { + connections = Arrays.asList( + MockOidcConnection.DEFAULT_CONNECTION, + new MockOidcConnection(new String[] {"openid"}, MOCK_OIDC_PARAM, "client-id", "client-secret", "http://example.com";, new String[] { "access_type=offline" } ) + ); + servlet = new OidcEntryPointServlet(connections, new OidcProviderMetadataRegistry() { @Override - public OIDCProviderMetadata getProviderMetadata(String base) { + protected OIDCProviderMetadata getProviderMetadata(String base) { return new OIDCProviderMetadata(new Issuer(base), Collections.singletonList(SubjectType.PUBLIC), URI.create("https://foo.example/jwks";)) { @Override public URI getAuthorizationEndpointURI() { @@ -57,36 +70,16 @@ class OidcClientImplTest { } @Test - void testRedirectUri() { - URI redirectUri = clientImpl.getOidcEntryPointUri(MockOidcConnection.DEFAULT_CONNECTION, context.request(), "/foo"); - - assertThat(redirectUri).as("redirect uri") - .hasScheme("http") - .hasHost("localhost") - .hasNoPort() - .hasPath("/system/sling/oidc/entry-point") - .hasQuery("c=mock-oidc&redirect=/foo"); - } + void testRedirectWithValidConnection() throws ServletException, IOException { - @Test - void testRedirectUri_customPort_noRedirect() { - context.request().setServerPort(8080); - URI redirectUri = clientImpl.getOidcEntryPointUri(MockOidcConnection.DEFAULT_CONNECTION, context.request(), null); + context.request().setQueryString("c=" + MockOidcConnection.DEFAULT_CONNECTION.name()); + MockSlingHttpServletResponse response = context.response(); - assertThat(redirectUri).as("redirect uri") - .hasScheme("http") - .hasHost("localhost") - .hasPort(8080) - .hasPath("/system/sling/oidc/entry-point") - .hasQuery("c=mock-oidc"); - } - - @Test - void testGetAuthenticationRequestUri() { - - URI requestUri = clientImpl.getAuthenticationRequestUri(MockOidcConnection.DEFAULT_CONNECTION, context.request(), URI.create("http://localhost/callback";)); - - assertThat(requestUri).as("authentication request uri") + servlet.service(context.request(), response); + + URI location = URI.create(Objects.requireNonNull(response.getHeader("Location"), "location header")); + + assertThat(location).as("authentication request uri") .hasScheme("https") .hasHost("foo.example") .hasPath("/authz") @@ -95,18 +88,20 @@ class OidcClientImplTest { .hasParameter("client_id", "client-id") .hasParameter("redirect_uri", "http://localhost/system/sling/oidc/callback";) .hasParameter("nonce") - .hasParameter("state"); + .hasParameter("state"); } @Test - void testGetAuthenticationRequestUri_customParam() { - - OidcConnection connection = new MockOidcConnection(new String[] {"openid"}, "mock-oidc", "client-id", "client-secret", "http://example.com";, new String[] { "access_type=offline" } ); + void redirectWithValidConnectionAndCustomParameter() throws ServletException, IOException { + + context.request().setQueryString("c=" + MOCK_OIDC_PARAM); + MockSlingHttpServletResponse response = context.response(); - URI requestUri = clientImpl.getAuthenticationRequestUri(connection, context.request(), URI.create("http://localhost/callback";)); - - assertThat(requestUri).as("authentication request uri") + servlet.service(context.request(), response); + + URI location = URI.create(Objects.requireNonNull(response.getHeader("Location"), "location header")); + + assertThat(location).as("authentication request uri") .hasParameter("access_type", "offline"); } - }
