SAML2SP improvements: allow to get SP metadata as authenticated user …

…+ validate URLs in SP metadata
apache · Aug 14, 2017 · e99766a · e99766a
1 parent d31dc65
commit e99766a
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 11 deletions.
diff --git a/common/lib/pom.xml b/common/lib/pom.xml
@@ -101,6 +101,15 @@ under the License.
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-checkstyle-plugin</artifactId>
       </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <version>2.3.2</version>
+        <configuration>
+          <source>1.7</source>
+          <target>1.7</target>
+        </configuration>
+      </plugin>
     </plugins>
   </build>
 </project>
diff --git a/ext/saml2sp/agent/src/main/java/org/apache/syncope/ext/saml2lsp/agent/Metadata.java b/ext/saml2sp/agent/src/main/java/org/apache/syncope/ext/saml2lsp/agent/Metadata.java
@@ -46,11 +46,15 @@ protected void doGet(final HttpServletRequest request, final HttpServletResponse
                 getAttribute(Constants.SYNCOPE_ANONYMOUS_CLIENT);
         SAML2SPService service = anonymous.getService(SAML2SPService.class);
         WebClient.client(service).accept(MediaType.APPLICATION_XML_TYPE).type(MediaType.APPLICATION_XML_TYPE);
-        Response metadataResponse = service.getMetadata(
-                StringUtils.substringBefore(request.getRequestURL().toString(), "/saml2sp"), "saml2sp");
+        try {
+            Response metadataResponse = service.getMetadata(
+                    StringUtils.substringBefore(request.getRequestURL().toString(), "/saml2sp"), "saml2sp");
 
-        response.setContentType(metadataResponse.getMediaType().toString());
-        IOUtils.copy((InputStream) metadataResponse.getEntity(), response.getOutputStream());
-        ((InputStream) metadataResponse.getEntity()).close();
+            response.setContentType(metadataResponse.getMediaType().toString());
+            IOUtils.copy((InputStream) metadataResponse.getEntity(), response.getOutputStream());
+            ((InputStream) metadataResponse.getEntity()).close();
+        } catch (Exception e) {
+            throw new ServletException(e.getMessage());
+        }
     }
 }
diff --git a/ext/saml2sp/logic/pom.xml b/ext/saml2sp/logic/pom.xml
@@ -43,7 +43,7 @@ under the License.
       <artifactId>syncope-core-logic</artifactId>
       <version>${project.version}</version>
     </dependency>
-
+      
     <dependency>
       <groupId>org.apache.syncope.ext.saml2sp</groupId>
       <artifactId>syncope-ext-saml2sp-provisioning-java</artifactId>
@@ -64,6 +64,11 @@ under the License.
       <groupId>org.opensaml</groupId>
       <artifactId>opensaml-saml-impl</artifactId>
     </dependency>
+
+    <dependency>
+      <groupId>commons-validator</groupId>
+      <artifactId>commons-validator</artifactId>
+    </dependency>
   </dependencies>
 
   <build>

diff --git a/ext/saml2sp/logic/src/main/java/org/apache/syncope/core/logic/SAML2SPLogic.java b/ext/saml2sp/logic/src/main/java/org/apache/syncope/core/logic/SAML2SPLogic.java
@@ -37,6 +37,7 @@
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Pair;
 import org.apache.commons.lang3.tuple.Triple;
+import org.apache.commons.validator.routines.UrlValidator;
 import org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer;
 import org.apache.cxf.rs.security.jose.jws.JwsSignatureVerifier;
 import org.apache.syncope.common.lib.AbstractBaseBean;
@@ -129,6 +130,8 @@ public class SAML2SPLogic extends AbstractSAML2Logic<AbstractBaseBean> {
 
     private static final Encryptor ENCRYPTOR = Encryptor.getInstance();
 
+    private static final UrlValidator URL_VALIDATOR = new UrlValidator(new String[] { "http", "https" });
+
     @Autowired
     private AccessTokenDataBinder accessTokenDataBinder;
 
@@ -153,11 +156,29 @@ public class SAML2SPLogic extends AbstractSAML2Logic<AbstractBaseBean> {
     @Resource(name = "syncopeJWTSSOProviderDelegate")
     private JwsSignatureVerifier jwsSignatureVerifier;
 
+    private void validateUrl(final String url) {
+        boolean isValid = true;
+        if (url.contains("..")) {
+            isValid = false;
+        }
+        if (isValid) {
+            isValid = URL_VALIDATOR.isValid(url);
+        }
+
+        if (!isValid) {
+            SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.Unknown);
+            sce.getElements().add("Invalid URL: " + url);
+            throw sce;
+        }
+    }
+
     private String getAssertionConsumerURL(final String spEntityID, final String urlContext) {
-        return spEntityID + urlContext + "/assertion-consumer";
+        String assertionConsumerUrl = spEntityID + urlContext + "/assertion-consumer";
+        validateUrl(assertionConsumerUrl);
+        return assertionConsumerUrl;
     }
 
-    @PreAuthorize("hasRole('" + StandardEntitlement.ANONYMOUS + "')")
+    @PreAuthorize("isAuthenticated()")
     public void getMetadata(final String spEntityID, final String urlContext, final OutputStream os) {
         check();
 
@@ -194,10 +215,13 @@ public void getMetadata(final String spEntityID, final String urlContext, final
                 spSSODescriptor.getAssertionConsumerServices().add(assertionConsumerService);
                 spEntityDescriptor.getRoleDescriptors().add(spSSODescriptor);
 
+                String sloUrl = spEntityID + urlContext + "/logout";
+                validateUrl(sloUrl);
+
                 SingleLogoutService singleLogoutService = new SingleLogoutServiceBuilder().buildObject();
                 singleLogoutService.setBinding(bindingType.getUri());
-                singleLogoutService.setLocation(spEntityID + urlContext + "/logout");
-                singleLogoutService.setResponseLocation(spEntityID + urlContext + "/logout");
+                singleLogoutService.setLocation(sloUrl);
+                singleLogoutService.setResponseLocation(sloUrl);
                 spSSODescriptor.getSingleLogoutServices().add(singleLogoutService);
             }
 

diff --git a/pom.xml b/pom.xml
@@ -403,6 +403,7 @@ under the License.
     <commons-lang.version>3.6</commons-lang.version>
     <commons-text.version>1.1</commons-text.version>
     <commons-collection.version>4.1</commons-collection.version>
+    <commons-validator.version>1.6</commons-validator.version>
     <commons-logging.version>1.1.3</commons-logging.version>
 
     <joda.version>2.9.9</joda.version>
@@ -1001,7 +1002,12 @@ under the License.
         <artifactId>commons-collections4</artifactId>
         <version>${commons-collection.version}</version>
       </dependency>
-
+      <dependency>
+        <groupId>commons-validator</groupId>
+        <artifactId>commons-validator</artifactId>
+        <version>${commons-validator.version}</version>
+      </dependency>
+
       <dependency>
         <groupId>net.tirasa.connid</groupId>
         <artifactId>connector-framework</artifactId>