[java-oidc-common] 22/28: Improve metadata response validation

[Phil Smart]

This is an automated email from the git hooks/post-receive script.

philsmart pushed a commit to branch dev/JCOMOIDC-41
in repository java-oidc-common.

View the commit online:
http://git.shibboleth.net/view/?p=java-oidc-common.git;a=commit;h=ed21e464273bf1aaa48070d5553d861780b1c06c

commit ed21e464273bf1aaa48070d5553d861780b1c06c
Author: Phil Smart <philip.smart at jisc.ac.uk>
AuthorDate: Mon Sep 12 11:16:26 2022 +0100

    Improve metadata response validation
---
 .../HTTPProviderConfigurationFetchingStrategy.java | 38 +++++++++++++++++++---
 ...PProviderConfigurationFetchingStrategyTest.java | 31 ++++++++++++++++++
 2 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategy.java b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategy.java
index 3e7c4df..526999a 100644
--- a/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategy.java
+++ b/oidc-common-metadata-impl/src/main/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategy.java
@@ -100,13 +100,14 @@ public class HTTPProviderConfigurationFetchingStrategy
      * 
      * @param strategy the strategy to set.
      */
-    public void setWellKnownLocationCompositionStrategy(@Nonnull final
+    public void setWellKnownLocationCompositionStrategy(@Nullable final
             BiFunction<Issuer, String, String> strategy) {
         ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
         ComponentSupport.ifDestroyedThrowDestroyedComponentException(this);
         
-        wellKnownLocationCompositionStrategy = 
-                Constraint.isNotNull(strategy, "Well known location composition strategy can not be null");
+        if (strategy != null) {
+            wellKnownLocationCompositionStrategy = strategy;
+        }
     }
 
     @Override
@@ -178,14 +179,41 @@ public class HTTPProviderConfigurationFetchingStrategy
             try {              
                 // this should convert the entity with the character set from the entity.
                 final String jsonDocument = EntityUtils.toString(response.getEntity());                
-                return OIDCProviderMetadata.parse(jsonDocument);
+                final OIDCProviderMetadata metadata = OIDCProviderMetadata.parse(jsonDocument);
+                if (!metadataValid(metadata, currentRequestURI)) {                    
+                    return null;
+                }
+                return metadata;
                 
             } catch (final Exception e) {
                 // catch any of the many exceptions
                 log.error("Error parsing HTTP response stream", e);
                 return null;
             }
-            
+        }
+        
+        /**
+         * Check the Issuer in the metadata is identical to the Issuer URL used to retrieve the metadata.
+         * *Note*, this just checks the Issuer URL starts with the Issuer value from the returned metadata, 
+         * otherwise these values will never be equal. It is hard to make sense of section 4.3 OpenID Discovery 1.0. 
+         * 
+         * @param metadata the OpenID Provider metadata
+         * @param issuerURL the issuerURL used to retrieve the metadata
+         * 
+         * @return true if valid, false otherwise.
+         */
+        private final boolean metadataValid(@Nonnull final OIDCProviderMetadata metadata,
+                @Nullable final String issuerURL) {
+            if (issuerURL == null || metadata.getIssuer() == null) {
+                return false;
+            }
+            final boolean valid = issuerURL.startsWith(metadata.getIssuer().getValue());
+            if (!valid) {
+                log.warn("OIDC metadata was not valid, Issuer in metadata did not match Issuer URL. Issuer "
+                        + "was '{}', IssuerURL was '{}'",metadata.getIssuer().getValue(), issuerURL);
+                return false;
+            }
+            return true;
         }
         
         /**
diff --git a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
index 3851205..ea26629 100644
--- a/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
+++ b/oidc-common-metadata-impl/src/test/java/net/shibboleth/oidc/metadata/impl/HTTPProviderConfigurationFetchingStrategyTest.java
@@ -34,6 +34,7 @@ import org.apache.http.entity.StringEntity;
 import org.apache.http.message.BasicHeader;
 import org.apache.http.protocol.HttpContext;
 import org.mockito.Mockito;
+import org.slf4j.MDC;
 import org.springframework.core.io.ClassPathResource;
 import org.testng.annotations.Test;
 
@@ -101,6 +102,9 @@ public class HTTPProviderConfigurationFetchingStrategyTest {
         final var StringEntity = new StringEntity(metadataAsString);
         StringEntity.setContentType(new BasicHeader("Content-Type", "application/json"));        
         Mockito.when(httpResponse.getEntity()).thenReturn(StringEntity);
+        
+        MDC.put(AbstractDynamicHTTPFetchingStrategy.MDC_ATTRIB_CURRENT_REQUEST_URI, 
+                "https://op.example.com/.well-known/openid-configuration");
 
         final var handledMetadata = responseHandler.handleResponse(httpResponse);
         
@@ -108,6 +112,33 @@ public class HTTPProviderConfigurationFetchingStrategyTest {
         assertEquals(handledMetadata.getIssuer().getValue(), "https://op.example.com");
     }
     
+    @Test
+    public void testResponseHandler_Failure_WrongIssuer() throws Exception {
+        
+        final OIDCProviderMetadataResponseHandler responseHandler = new OIDCProviderMetadataResponseHandler();
+
+        final HttpResponse httpResponse = Mockito.mock(HttpResponse.class);
+        final StatusLine statusLine = Mockito.mock(StatusLine.class);
+        Mockito.when(httpResponse.getStatusLine()).thenReturn(statusLine);
+        Mockito.when(statusLine.getStatusCode()).thenReturn(200);
+        
+        final var metadata = 
+                new ClassPathResource("/net/shibboleth/oidc/metadata/impl/openid-configuration.json");
+        final var metadataAsString = CharStreams.toString(new InputStreamReader(
+                metadata.getInputStream(), StandardCharsets.UTF_8));
+        
+        final var StringEntity = new StringEntity(metadataAsString);
+        StringEntity.setContentType(new BasicHeader("Content-Type", "application/json"));        
+        Mockito.when(httpResponse.getEntity()).thenReturn(StringEntity);
+        
+        MDC.put(AbstractDynamicHTTPFetchingStrategy.MDC_ATTRIB_CURRENT_REQUEST_URI, 
+                "https://wrongissuer.example.com/.well-known/openid-configuration");
+
+        final var handledMetadata = responseHandler.handleResponse(httpResponse);
+        
+        assertNull(handledMetadata);
+    }
+    
     
     @Test
     public void testResponseHandler_304Response_NoMetadata() throws Exception {

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.