Merge pull request #52 from cuioss/feature/oauth-config-validation

Feature/oauth config validation

modules/authentication/portal-authentication-oauth/src/main/java/de/cuioss/portal/authentication/oauth/OAuthConfigKeys.java

@@ -71,7 +71,7 @@ public class OAuthConfigKeys {
      * <p>
      * The (cluster-internal) url to the oauth2 servers token endpoint. For
      * cluster-internal communication with the server, e.g.
-     * https://xyz/member-account-facade/oidc.
+     * {@code https://xyz/member-account-facade/oidc}.
      * </p>
      * <p>
      * If this config property is not set, the value served form the well-know
@@ -85,7 +85,7 @@ public class OAuthConfigKeys {
      * <p>
      * The (cluster-internal) url to the oauth2 servers userinfo endpoint. For
      * cluster-internal communication with the server, e.g.
-     * https://xyz/member-account-facade/oidc/userinfo.
+     * {@code https://xyz/member-account-facade/oidc/userinfo}.
      * </p>
      * <p>
      * If this config property is not set, the value served form the well-know
@@ -165,4 +165,8 @@ public class OAuthConfigKeys {
      */
     public static final String OPEN_ID_DISCOVER_PATH = OPEN_ID_SERVER_BASE + "discovery_path";
 
+    /**
+     * Ensure that the final config is valid, i.e. required attributes are present.
+     */
+    public static final String CONFIG_VALIDATION_ENABLED = OPEN_ID_BASE + "validation.enabled";
 }

modules/authentication/portal-authentication-oauth/src/main/java/de/cuioss/portal/authentication/oauth/Oauth2Configuration.java

@@ -23,9 +23,6 @@
  */
 public interface Oauth2Configuration extends Serializable {
 
-    /**
-     * @return
-     */
     String getAuthorizeUri();
 
     /**
@@ -74,4 +71,10 @@ public interface Oauth2Configuration extends Serializable {
     String getPostLogoutRedirectUri();
 
     boolean isLogoutWithIdTokenHintEnabled();
+
+    /**
+     * Validate oauth2 config.
+     * @throws IllegalStateException if e.g. a required config param is missing or invalid.
+     */
+    void validate();
 }

modules/authentication/portal-authentication-oauth/src/main/java/de/cuioss/portal/authentication/oauth/impl/Oauth2ConfigurationImpl.java

@@ -24,6 +24,8 @@
 import lombok.Data;
 import lombok.NoArgsConstructor;
 
+import static de.cuioss.tools.base.Preconditions.checkState;
+
 /**
  * Default implementation.
  *
@@ -42,10 +44,19 @@ public class Oauth2ConfigurationImpl implements Oauth2Configuration {
 
     private String clientSecret;
 
+    /**
+     * Used by the client to obtain authorization from the resource owner via user-agent redirection.
+     *
+     * @see <a href="https://datatracker.ietf.org/doc/html/rfc6749">OAuth 2.0 Authorization Request</a>
+     */
     private String authorizeUri;
 
     private String userInfoUri;
 
+    /**
+     * Used by the client to exchange an authorization grant for an access token,
+     * typically with client authentication.
+     */
     private String tokenUri;
 
     private String externalContextPath;
@@ -61,4 +72,14 @@ public class Oauth2ConfigurationImpl implements Oauth2Configuration {
     private String postLogoutRedirectUri;
 
     private boolean logoutWithIdTokenHintEnabled;
+
+    public void validate() {
+        validateRequiredAttributes();
+    }
+
+    private void validateRequiredAttributes() {
+        checkState(null != clientId, "OAuth clientId missing");
+        checkState(null != authorizeUri, "OAuth authorizeUri missing");
+        checkState(null != tokenUri, "OAuth tokenUri missing");
+    }
 }

modules/authentication/portal-authentication-oauth/src/main/java/de/cuioss/portal/authentication/oauth/impl/Oauth2DiscoveryConfigurationProducer.java

@@ -34,7 +34,9 @@
 import java.util.Map;
 import java.util.Optional;
 
-import static de.cuioss.portal.authentication.oauth.OAuthConfigKeys.*;
+import static de.cuioss.portal.authentication.oauth.OAuthConfigKeys.OPEN_ID_DISCOVER_PATH;
+import static de.cuioss.portal.authentication.oauth.OAuthConfigKeys.OPEN_ID_ROLE_MAPPER_CLAIM;
+import static de.cuioss.portal.authentication.oauth.OAuthConfigKeys.OPEN_ID_SERVER_BASE_URL;
 import static de.cuioss.tools.net.UrlHelper.addTrailingSlashToUrl;
 import static de.cuioss.tools.string.MoreStrings.isBlank;
 
@@ -102,6 +104,10 @@ public class Oauth2DiscoveryConfigurationProducer {
     @ConfigProperty(name = OAuthConfigKeys.OPEN_ID_SERVER_USER_INFO_URL)
     private Provider<Optional<String>> internalUserInfoUrl;
 
+    @Inject
+    @ConfigProperty(name = OAuthConfigKeys.CONFIG_VALIDATION_ENABLED)
+    private Provider<Boolean> configValidationEnabled;
+
     /**
      * The request to retrieve information about the current authenticated user.
      */
@@ -123,8 +129,9 @@ public void init() {
             final var builder = new CuiRestClientBuilder(LOGGER);
             final var discoveryURI = addTrailingSlashToUrl(settingServerBaseUrl) + settingOauth2discoveryUri;
             LOGGER.debug("Using discoveryURI {}", discoveryURI);
-            try {
-                final var discovery = builder.url(discoveryURI).build(RequestDiscovery.class).getDiscovery();
+            builder.url(discoveryURI);
+            try (final var discoveryEndpoint = builder.build(RequestDiscovery.class)) {
+                final var discovery = discoveryEndpoint.getDiscovery();
                 configuration = createConfiguration(discovery);
             } catch (final Exception e) {
                 LOGGER.error(e, "Auto discovery of oauth config failed, using URI: {}", discoveryURI);
@@ -135,6 +142,10 @@ public void init() {
         }
 
         LOGGER.debug("oauth config: {}", configuration);
+
+        if (null != configuration && configValidationEnabled.get()) {
+            configuration.validate();
+        }
     }
 
     private Oauth2Configuration createConfiguration(final Map<String, Object> discovery) {

modules/authentication/portal-authentication-oauth/src/main/java/de/cuioss/portal/authentication/oauth/impl/Oauth2ServiceImpl.java

@@ -45,6 +45,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 
+import static de.cuioss.tools.base.Preconditions.checkState;
 import static de.cuioss.tools.string.MoreStrings.emptyToNull;
 import static java.net.URLEncoder.encode;
 import static java.util.Objects.requireNonNull;
@@ -210,6 +211,8 @@ private static List<String> asStringList(Object value) {
     @Override
     public String retrieveClientToken(String scopes) {
         var configuration = configurationProvider.get();
+        checkState(null != configuration.getTokenUri(), "tokenUri must not be null");
+
         final var builder = new CuiRestClientBuilder(log)
             .basicAuth(configuration.getClientId(), configuration.getClientSecret())
             .register(new AcceptJsonHeaderFilter());

modules/authentication/portal-authentication-oauth/src/main/resources/META-INF/microprofile-config.properties

@@ -51,6 +51,9 @@ authentication.oidc.client.logout.params.add_id_token_hint=true
 # Must be set by the installation.
 #authentication.oidc.server.url=
 
+# If true, validates the final config, i.e. ensuring that required attributes are present.
+authentication.oidc.validation.enabled=true
+
 # The url relative to #url defining the OpenID Connect Discovery endpoint,
 # defaults to '.well-known/openid-configuration'.
 # In case of #type being "keycloak" this setting will be ignored.

modules/authentication/portal-authentication-oauth/src/test/java/de/cuioss/portal/authentication/oauth/impl/OIDCWellKnownDispatcher.java

@@ -15,6 +15,7 @@
  */
 package de.cuioss.portal.authentication.oauth.impl;
 
+import static de.cuioss.tools.io.FileLoaderUtility.toStringUnchecked;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.List;
@@ -54,10 +55,10 @@ public class OIDCWellKnownDispatcher extends Dispatcher {
     public static final FileLoader CONFIGURATION = FileLoaderUtility
             .getLoaderForPath(FileTypePrefix.CLASSPATH + "/openid-configuration.json");
 
-    public static final FileLoader TOKEN = FileLoaderUtility.getLoaderForPath(FileTypePrefix.CLASSPATH + "/token.json");
+    public static final FileLoader INVALID_CONFIGURATION = FileLoaderUtility
+            .getLoaderForPath(FileTypePrefix.CLASSPATH + "/openid-configuration-invalid.json");
 
-    public static final FileLoader CLIENT_TOKEN = FileLoaderUtility
-            .getLoaderForPath(FileTypePrefix.CLASSPATH + "/clientToken.json");
+    public static final FileLoader TOKEN = FileLoaderUtility.getLoaderForPath(FileTypePrefix.CLASSPATH + "/token.json");
 
     public static final FileLoader USER_INFO = FileLoaderUtility
             .getLoaderForPath(FileTypePrefix.CLASSPATH + "/userInfo.json");
@@ -75,13 +76,16 @@ public class OIDCWellKnownDispatcher extends Dispatcher {
     @Getter
     private String currentPort;
 
+    @Setter
+    private boolean simulateInvalidOidcConfig = false;
+
     public void reset() {
         tokenResult = new MockResponse().setResponseCode(HttpServletResponse.SC_OK)
                 .addHeader("Content-Type", MediaType.APPLICATION_JSON)
-                .setBody(FileLoaderUtility.toStringUnchecked(TOKEN));
+                .setBody(toStringUnchecked(TOKEN));
         userInfoResult = new MockResponse().setResponseCode(HttpServletResponse.SC_OK)
                 .addHeader("Content-Type", MediaType.APPLICATION_JSON)
-                .setBody(FileLoaderUtility.toStringUnchecked(USER_INFO));
+                .setBody(toStringUnchecked(USER_INFO));
     }
 
     public void assertAuthorizeURL(String actualUrl, String... parts) {
@@ -115,7 +119,7 @@ public List<RecordedRequest> nonWellKnownRequests(MockWebServer mockWebServer) t
 
         var request = mockWebServer.takeRequest();
         while (null != request) {
-            if (!request.getPath().contains(OIDC_DISCOVERY_PATH)) {
+            if (!isOidcDiscoveryPath(request.getPath())) {
                 builder.add(request);
             }
             request = mockWebServer.takeRequest(100, TimeUnit.MILLISECONDS);
@@ -124,20 +128,31 @@ public List<RecordedRequest> nonWellKnownRequests(MockWebServer mockWebServer) t
         return builder.toImmutableList();
     }
 
+    private boolean isOidcDiscoveryPath(String path) {
+        return path != null && path.contains(OIDC_DISCOVERY_PATH);
+    }
+
     @Override
-    public @NotNull MockResponse dispatch(RecordedRequest request) throws InterruptedException {
+    public @NotNull MockResponse dispatch(RecordedRequest request) {
         LOGGER.info(() -> "Serve request " + request.getPath());
-        return switch (request.getPath()) {
-        case "/" + OIDC_DISCOVERY_PATH -> new MockResponse().setResponseCode(HttpServletResponse.SC_OK)
-                .addHeader("Content-Type", MediaType.APPLICATION_JSON)
-                .setBody(FileLoaderUtility.toStringUnchecked(CONFIGURATION).replaceAll("5602", currentPort));
-        case "/auth/realms/master/protocol/openid-connect/userinfo" -> userInfoResult;
-        case "/auth/realms/master/protocol/openid-connect/token" -> tokenResult;
-        default -> {
+
+        if (null == request.getPath()) {
             LOGGER.warn(() -> "Unable to serve request " + request.getPath());
-            yield new MockResponse().setResponseCode(HttpServletResponse.SC_NOT_FOUND);
+            new MockResponse().setResponseCode(HttpServletResponse.SC_NOT_FOUND);
         }
+
+        return switch (request.getPath()) {
+            case "/" + OIDC_DISCOVERY_PATH -> new MockResponse().setResponseCode(HttpServletResponse.SC_OK)
+                .addHeader("Content-Type", MediaType.APPLICATION_JSON)
+                .setBody(simulateInvalidOidcConfig
+                    ? toStringUnchecked(INVALID_CONFIGURATION).replaceAll("5602", currentPort)
+                    : toStringUnchecked(CONFIGURATION).replaceAll("5602", currentPort));
+            case "/auth/realms/master/protocol/openid-connect/userinfo" -> userInfoResult;
+            case "/auth/realms/master/protocol/openid-connect/token" -> tokenResult;
+            default -> {
+                LOGGER.warn(() -> "Unable to serve request " + request.getPath());
+                yield new MockResponse().setResponseCode(HttpServletResponse.SC_NOT_FOUND);
+            }
         };
     }
-
 }

modules/authentication/portal-authentication-oauth/src/test/java/de/cuioss/portal/authentication/oauth/impl/Oauth2AuthenticationFacadeImplTest.java

@@ -66,7 +66,7 @@
 
 @EnableMockWebServer
 @EnableAutoWeld
-@EnablePortalConfiguration
+@EnablePortalConfiguration(configuration = "authentication.oidc.validation.enabled:false")
 @AddBeanClasses({ Oauth2AuthenticationFacadeImpl.class, Oauth2DiscoveryConfigurationProducer.class,
         RedirectorMock.class })
 @AddExtensions(ResteasyCdiExtension.class)

modules/authentication/portal-authentication-oauth/src/test/java/de/cuioss/portal/authentication/oauth/impl/Oauth2ConfigurationImplTest.java

@@ -0,0 +1,63 @@
+package de.cuioss.portal.authentication.oauth.impl;
+
+import de.cuioss.test.generator.Generators;
+import de.cuioss.test.generator.TypedGenerator;
+import org.junit.jupiter.api.Test;
+
+import java.util.Collections;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+class Oauth2ConfigurationImplTest {
+
+    private static final TypedGenerator<String> nonEmptyLetterStringGenerator = Generators.letterStrings(1, 5);
+
+    @Test
+    void validConfig() {
+        Oauth2ConfigurationImpl config = createConfig();
+
+        assertDoesNotThrow(config::validate);
+    }
+
+    @Test
+    void missingTokenUri() {
+        Oauth2ConfigurationImpl config = createConfig();
+        config.setTokenUri(null);
+
+        assertThrows(IllegalStateException.class, config::validate);
+    }
+
+    @Test
+    void missingClientId() {
+        Oauth2ConfigurationImpl config = createConfig();
+        config.setClientId(null);
+
+        assertThrows(IllegalStateException.class, config::validate);
+    }
+
+    @Test
+    void missingAuthorizeUri() {
+        Oauth2ConfigurationImpl config = createConfig();
+        config.setAuthorizeUri(null);
+
+        assertThrows(IllegalStateException.class, config::validate);
+    }
+
+    private Oauth2ConfigurationImpl createConfig() {
+        Oauth2ConfigurationImpl.Oauth2ConfigurationImplBuilder configBuilder = Oauth2ConfigurationImpl.builder();
+        configBuilder.clientId(nonEmptyLetterStringGenerator.next());
+        configBuilder.clientSecret(nonEmptyLetterStringGenerator.next());
+        configBuilder.initialScopes(nonEmptyLetterStringGenerator.next());
+        configBuilder.logoutUri(nonEmptyLetterStringGenerator.next());
+        configBuilder.externalContextPath(nonEmptyLetterStringGenerator.next());
+        configBuilder.authorizeUri(nonEmptyLetterStringGenerator.next());
+        configBuilder.logoutRedirectParamName(nonEmptyLetterStringGenerator.next());
+        configBuilder.postLogoutRedirectUri(nonEmptyLetterStringGenerator.next());
+        configBuilder.roleMapperClaims(Collections.singletonList(nonEmptyLetterStringGenerator.next()));
+        configBuilder.tokenUri(nonEmptyLetterStringGenerator.next());
+        configBuilder.externalContextPath(nonEmptyLetterStringGenerator.next());
+        configBuilder.userInfoUri(nonEmptyLetterStringGenerator.next());
+        return configBuilder.build();
+    }
+}

modules/authentication/portal-authentication-oauth/src/test/java/de/cuioss/portal/authentication/oauth/impl/Oauth2DiscoveryConfigurationProducerTest.java

@@ -28,12 +28,15 @@
 import lombok.Setter;
 import mockwebserver3.MockWebServer;
 import org.jboss.resteasy.cdi.ResteasyCdiExtension;
+import org.jboss.weld.exceptions.WeldException;
 import org.jboss.weld.junit5.auto.AddExtensions;
 import org.jboss.weld.junit5.auto.EnableAutoWeld;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 @EnableAutoWeld
 @EnablePortalConfiguration
@@ -56,12 +59,17 @@ class Oauth2DiscoveryConfigurationProducerTest
     @Getter
     private final OIDCWellKnownDispatcher dispatcher = new OIDCWellKnownDispatcher();
 
+    @BeforeEach
+    void beforeEach() {
+        configuration.fireEvent(OAuthConfigKeys.CONFIG_VALIDATION_ENABLED, "false");
+    }
+
     @Test
     void testInit() {
-
         dispatcher.configure(configuration, mockWebServer);
 
         var result = underTest.getConfiguration();
+
         assertNotNull(result);
         assertNotNull(result.getTokenUri());
         assertNotNull(result.getAuthorizeUri());
@@ -74,7 +82,8 @@ void shouldOverwriteInternalUrls() {
         dispatcher.configure(configuration, mockWebServer);
         var url1 = new URLGenerator().next().toString();
         var url2 = new URLGenerator().next().toString();
-        configuration.fireEvent(OAuthConfigKeys.OPEN_ID_SERVER_TOKEN_URL, url1,
+        configuration.fireEvent(
+            OAuthConfigKeys.OPEN_ID_SERVER_TOKEN_URL, url1,
             OAuthConfigKeys.OPEN_ID_SERVER_USER_INFO_URL, url2);
 
         var result = underTest.getConfiguration();
@@ -83,4 +92,16 @@ void shouldOverwriteInternalUrls() {
         assertEquals(url2, result.getUserInfoUri());
     }
 
+    @Test
+    void invalidOpenIdConfig() {
+        configuration.put(OAuthConfigKeys.CONFIG_VALIDATION_ENABLED, "true");
+        dispatcher.setSimulateInvalidOidcConfig(true);
+        dispatcher.configure(configuration, mockWebServer);
+
+        WeldException ex = assertThrows(WeldException.class, underTest::getConfiguration);
+
+        assertNotNull(ex.getCause());
+        assertNotNull(ex.getCause().getCause());
+        assertEquals(IllegalStateException.class, ex.getCause().getCause().getClass());
+    }
 }