You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cr...@apache.org on 2021/04/05 22:18:22 UTC

[sling-whiteboard] branch SLING-10193/test-coverage updated (593bea8 -> 7fc9181)

This is an automated email from the ASF dual-hosted git repository.

cris pushed a change to branch SLING-10193/test-coverage
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git.


    from 593bea8  add one IT test and clean up
     new 67b4848  updated pom.xml with dependency version ranges
     new 7fc9181  continued improving test coverage

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 saml-handler/bnd.bnd                               |   1 -
 saml-handler/pom.xml                               |  18 ++-
 .../saml2/impl/AuthenticationHandlerSAML2Impl.java |  59 +++++----
 .../org/apache/sling/auth/saml2/SamlHandlerIT.java | 133 +++++++++++++++++----
 .../impl/AuthenticationHandlerSAML2ImplTest.java   |  48 ++++++--
 .../apache/sling/auth/saml2/impl/JKSHelper.java    |  26 ++--
 .../apache/sling/auth/saml2/impl/OsgiSamlTest.java |  12 ++
 saml-handler/src/test/resources/exampleSaml2.jks   | Bin 0 -> 9461 bytes
 8 files changed, 224 insertions(+), 73 deletions(-)
 create mode 100644 saml-handler/src/test/resources/exampleSaml2.jks

[sling-whiteboard] 01/02: updated pom.xml with dependency version ranges

Posted by cr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cris pushed a commit to branch SLING-10193/test-coverage
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit 67b48480d0271bbf64df1c15e1846eb58c6e3627
Author: Cris Rockwell, College of LSA University of Michigan <cm...@umich.edu>
AuthorDate: Mon Apr 5 18:17:33 2021 -0400

    updated pom.xml with dependency version ranges
---
 saml-handler/bnd.bnd |  1 -
 saml-handler/pom.xml | 18 ++++++++++++------
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/saml-handler/bnd.bnd b/saml-handler/bnd.bnd
index f28134f..39488d7 100644
--- a/saml-handler/bnd.bnd
+++ b/saml-handler/bnd.bnd
@@ -8,7 +8,6 @@ Import-Package:!com.beust*,!antlr*,!org.apache.log.*,!org.apache.oro.text.perl*,
 !org.apache.tools.ant.*,!org.jdom*,!com.sun.org.apache.xerces.internal*,!sun.io,!org.bouncycastle.*,!javax.xml.bind.*,\
 !com.werken.xpath.*,!org.apache.xml.dtm.*,!org.apache.xml.utils.*,!org.apache.xpath.*,\
 !junit.framework*,!org.dom4j.*,!com.sun.msv.*,!sun.misc,javax.annotation;version=0.0.0,!org.relaxng.datatype.*,*
-Export-Package:org.apache.jackrabbit.oak.spi.security.authentication.external.*
 -removeheaders: Include-Resource, Private-Package
 -includeresource:\
 metrics-core-[0-9\.]+.jar;lib:=true,\
diff --git a/saml-handler/pom.xml b/saml-handler/pom.xml
index d165e2e..825f311 100644
--- a/saml-handler/pom.xml
+++ b/saml-handler/pom.xml
@@ -36,6 +36,8 @@
     <bnd.baseline.skip>true</bnd.baseline.skip>
     <powermock.version>2.0.9</powermock.version>
     <exam.version>4.13.3</exam.version>
+    <maven.compiler.source>1.11</maven.compiler.source>
+    <maven.compiler.target>1.11</maven.compiler.target>
     <pax.vm.options>
       -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5015
     </pax.vm.options>
@@ -59,8 +61,8 @@ which is licensed under the Apache-2.0 license.
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-         <source>9</source>
-         <target>9</target>
+         <source>11</source>
+         <target>11</target>
         </configuration>
       </plugin>
       <plugin>
@@ -70,6 +72,7 @@ which is licensed under the Apache-2.0 license.
           <excludes combine.children="append">
             <exclude>**/dependency-reduced-pom.xml</exclude>
             <exclude>**/*.vm</exclude>
+            <exclude>src/test/resources/*</exclude>
           </excludes>
         </configuration>
       </plugin>
@@ -318,7 +321,8 @@ which is licensed under the Apache-2.0 license.
     <dependency>
       <groupId>org.apache.sling</groupId>
       <artifactId>org.apache.sling.api</artifactId>
-      <version>[2.16.2,2.20.0]</version>
+<!--      <version>[2.16.2,2.20.0]</version>-->
+      <version>[2.16.2,2.23.0]</version>
       <scope>provided</scope>
     </dependency>
     <dependency>
@@ -329,15 +333,17 @@ which is licensed under the Apache-2.0 license.
     </dependency>
 
 <!--
-Export-Package:org.apache.jackrabbit.oak.spi.security.authentication.external.*
+this was removed from bnd.bnd
+because it became difficult manage a version range while using a specific version in IT tests
+Export-Package:org.apache.jackrabbit.oak.spi.security.authentication.external.*   
 -->
     <dependency>
       <groupId>org.apache.jackrabbit</groupId>
       <artifactId>oak-auth-external</artifactId>
-      <version>[1.32]</version>
+      <version>[1.32.0,1.38.0]</version>
+      <scope>provided</scope>
     </dependency>
 
-
 <!--    Apache Commons -->
     <dependency>
       <groupId>org.apache.commons</groupId>

[sling-whiteboard] 02/02: continued improving test coverage

Posted by cr...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

cris pushed a commit to branch SLING-10193/test-coverage
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git

commit 7fc9181478b9bf0ffbf0310d4a8bf49dc57e8add
Author: Cris Rockwell, College of LSA University of Michigan <cm...@umich.edu>
AuthorDate: Mon Apr 5 18:18:02 2021 -0400

    continued improving test coverage
---
 .../saml2/impl/AuthenticationHandlerSAML2Impl.java |  59 +++++----
 .../org/apache/sling/auth/saml2/SamlHandlerIT.java | 133 +++++++++++++++++----
 .../impl/AuthenticationHandlerSAML2ImplTest.java   |  48 ++++++--
 .../apache/sling/auth/saml2/impl/JKSHelper.java    |  26 ++--
 .../apache/sling/auth/saml2/impl/OsgiSamlTest.java |  12 ++
 saml-handler/src/test/resources/exampleSaml2.jks   | Bin 0 -> 9461 bytes
 6 files changed, 212 insertions(+), 66 deletions(-)

diff --git a/saml-handler/src/main/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2Impl.java b/saml-handler/src/main/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2Impl.java
index e4634fc..00f670b 100644
--- a/saml-handler/src/main/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2Impl.java
+++ b/saml-handler/src/main/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2Impl.java
@@ -134,16 +134,16 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
         initializeTokenStore(tokenFile);
         if (this.getSaml2SPEncryptAndSign()) {
             //      set encryption keys
+            this.idpVerificationCert = VerifySignatureCredentials.getCredential(
+                    this.getJksFileLocation(),
+                    this.getJksStorePassword().toCharArray(),
+                    this.getIdpCertAlias());
             this.spKeypair = KeyPairCredentials.getCredential(
                     this.getJksFileLocation(),
                     this.getJksStorePassword().toCharArray(),
                     this.getSpKeysAlias(),
                     this.getSpKeysPassword().toCharArray());
             //      set credential for signing
-            this.idpVerificationCert = VerifySignatureCredentials.getCredential(
-                    this.getJksFileLocation(),
-                    this.getJksStorePassword().toCharArray(),
-                    this.getIdpCertAlias());
         }
     }
 
@@ -163,6 +163,10 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
         return this.idpVerificationCert;
     }
 
+    SessionStorage getStorageAuthInfo(){
+        return this.storageAuthInfo;
+    }
+
     /**
      * Extracts session based credentials from the request. Returns
      * <code>null</code> if the secure user data is not present either in the HTTP Session.
@@ -184,7 +188,7 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
 
 // 2.  try credentials from the session
         if ( !this.getSaml2Path().isEmpty() && reqURI.startsWith(this.getSaml2Path())) {
-            final String authData = storageAuthInfo.getString(httpServletRequest);
+            final String authData = getStorageAuthInfo().getString(httpServletRequest);
             if (authData != null) {
                 if (tokenStore.isValid(authData)) {
                     return buildAuthInfo(authData);
@@ -206,7 +210,7 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
 
     private void clearSessionAttributes(final HttpServletRequest httpServletRequest,
                                         final HttpServletResponse httpServletResponse) {
-        storageAuthInfo.clear(httpServletRequest, httpServletResponse);
+        getStorageAuthInfo().clear(httpServletRequest, httpServletResponse);
     }
 
     private AuthenticationInfo processAssertionConsumerService(final HttpServletRequest httpServletRequest,
@@ -263,11 +267,12 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
     @Override
     public boolean requestCredentials(final HttpServletRequest httpServletRequest,
                                       final HttpServletResponse httpServletResponse) throws IOException {
-        // check the referrer to see if the request is for this handler
-        if (!AuthUtil.checkReferer(httpServletRequest, this.getAcsPath())) {
-            // not for this handler, so return
+        // 0. ignore this handler if an authentication handler is requested
+        if (ignoreRequestCredentials(httpServletRequest)) {
+            // consider this handler is not used
             return false;
         }
+
         if (this.getSaml2SPEnabled() ) {
             doClassloading();
             setGotoURLOnSession(httpServletRequest);
@@ -296,7 +301,17 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
         redirectUserWithRequest(httpServletRequest, httpServletResponse, authnRequest);
     }
 
-
+    /**
+     * Returns <code>true</code> if this authentication handler should ignore the
+     * call to {@link #requestCredentials(HttpServletRequest, HttpServletResponse)}.
+     * <p>
+     * This method returns <code>true</code> if the {@link #REQUEST_LOGIN_PARAMETER}
+     * is set to any value other than "SAML2" (the authentication type)
+     */
+    boolean ignoreRequestCredentials(final HttpServletRequest request) {
+        final String requestLogin = request.getParameter(REQUEST_LOGIN_PARAMETER);
+        return requestLogin != null && !AUTH_TYPE.equals(requestLogin);
+    }
 
     private void redirectUserWithRequest(final HttpServletRequest httpServletRequest ,
                      final HttpServletResponse httpServletResponse, final RequestAbstractType requestForIDP) {
@@ -309,8 +324,6 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
             setRelayStateOnSession(httpServletRequest, bindingContext);
             setRequestIDOnSession(httpServletRequest, (AuthnRequest)requestForIDP);
             endpointContext.setEndpoint(getIPDEndpoint());
-        } else if (requestForIDP instanceof LogoutRequest){
-            endpointContext.setEndpoint(getSLOEndpoint());
         }
         SignatureSigningParameters signatureSigningParameters = new SignatureSigningParameters();
         signatureSigningParameters.setSigningCredential(this.getSpKeypair());
@@ -404,9 +417,13 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
         }
     }
 
+    Credential getSPKeyPair(){
+        return this.spKeypair;
+    }
+
     private Assertion decryptAssertion(final EncryptedAssertion encryptedAssertion) {
         // Use SP Private Key to decrypt
-        StaticKeyInfoCredentialResolver keyInfoCredentialResolver = new StaticKeyInfoCredentialResolver(this.spKeypair);
+        StaticKeyInfoCredentialResolver keyInfoCredentialResolver = new StaticKeyInfoCredentialResolver(getSPKeyPair());
         Decrypter decrypter = new Decrypter(null, keyInfoCredentialResolver, new InlineEncryptedKeyResolver());
         decrypter.setRootInNewDocument(true);
         try {
@@ -694,14 +711,14 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
      * @param authInfo
      *            The authentication info used to successful log in
      */
-    private void refreshAuthData(final HttpServletRequest request, final HttpServletResponse response,
+    void refreshAuthData(final HttpServletRequest request, final HttpServletResponse response,
                                  final AuthenticationInfo authInfo) {
 
         // get current authentication data, may be missing after first login
-        String token = storageAuthInfo.getString(request);
+        String token = getStorageAuthInfo().getString(request);
 
         // check whether we have to "store" or create the data
-        final boolean refreshCookie = needsRefresh(token, this.sessionTimeout);
+        final boolean refreshCookie = needsRefresh(token);
 
         // add or refresh the stored auth hash
         if (refreshCookie) {
@@ -719,7 +736,7 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
             }
 
             if (token != null) {
-                storageAuthInfo.setString(request, token);
+                getStorageAuthInfo().setString(request, token);
             } else {
                 clearSessionAttributes(request, response);
             }
@@ -728,12 +745,11 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
 
     /**
      * Refresh the cookie periodically.
+     * Compares current time to saved expiry time
      *
-     * @param sessionTimeout
-     *            time to live for the session
      * @return true or false
      */
-    boolean needsRefresh(final String authData, final long sessionTimeout) {
+    boolean needsRefresh(final String authData) {
         boolean updateCookie = false;
         if (authData == null) {
             updateCookie = true;
@@ -741,7 +757,8 @@ public class AuthenticationHandlerSAML2Impl extends AbstractSamlHandler implemen
             String[] parts = TokenStore.split(authData);
             if (parts != null && parts.length == 3) {
                 long cookieTime = Long.parseLong(parts[1].substring(1));
-                if (System.currentTimeMillis() + (sessionTimeout / 2) > cookieTime) {
+                long timeNow = System.currentTimeMillis();
+                if (timeNow > cookieTime) {
                     updateCookie = true;
                 }
             }
diff --git a/saml-handler/src/test/java/org/apache/sling/auth/saml2/SamlHandlerIT.java b/saml-handler/src/test/java/org/apache/sling/auth/saml2/SamlHandlerIT.java
index 165ec8d..7f4283e 100644
--- a/saml-handler/src/test/java/org/apache/sling/auth/saml2/SamlHandlerIT.java
+++ b/saml-handler/src/test/java/org/apache/sling/auth/saml2/SamlHandlerIT.java
@@ -36,6 +36,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.ops4j.pax.exam.Configuration;
 import org.ops4j.pax.exam.Option;
 import org.ops4j.pax.exam.junit.PaxExam;
@@ -50,19 +51,31 @@ import org.osgi.service.cm.ConfigurationAdmin;
 import org.osgi.service.http.HttpService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import javax.inject.Inject;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.Arrays;
+import java.util.Base64;
 
+import static org.apache.sling.auth.core.spi.AuthenticationHandler.REQUEST_LOGIN_PARAMETER;
+import static org.apache.sling.auth.saml2.impl.JKSHelper.IDP_ALIAS;
+import static org.apache.sling.auth.saml2.impl.JKSHelper.SP_ALIAS;
 import static org.apache.sling.testing.paxexam.SlingOptions.logback;
 import static org.apache.sling.testing.paxexam.SlingOptions.slingAuthForm;
 import static org.apache.sling.testing.paxexam.SlingOptions.slingQuickstartOakTar;
 import static org.apache.sling.testing.paxexam.SlingOptions.versionResolver;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.when;
 import static org.ops4j.pax.exam.CoreOptions.composite;
 import static org.ops4j.pax.exam.CoreOptions.junitBundles;
 import static org.ops4j.pax.exam.CoreOptions.mavenBundle;
@@ -80,7 +93,6 @@ public class SamlHandlerIT extends TestSupport {
 
     static int HTTP_PORT = 8080;
     static int DEST_HTTP_PORT = 8484;
-    private final static int STARTUP_WAIT_SECONDS = 30;
     private static Logger logger = LoggerFactory.getLogger(SamlHandlerIT.class);
     ResourceResolver resourceResolver = null;
     Session session;
@@ -102,49 +114,67 @@ public class SamlHandlerIT extends TestSupport {
     @Inject
     ResourceResolverFactory resolverFactory;
 
-    @Filter(value = "(authtype=SAML2)")
     @Inject
+    @Filter(value = "(saml2SPEncryptAndSign=false)")
     AuthenticationHandler authHandler;
 
     @Inject
-    Saml2UserMgtService saml2UserMgtService;
+    @Filter(value = "(saml2SPEncryptAndSign=true)")
+    AuthenticationHandler authHandlerEnc;
 
+    @Inject
+    Saml2UserMgtService saml2UserMgtService;
 
     @Configuration
     public Option[] configuration() {
         // Patch versions of features provided by SlingOptions
         HTTP_PORT = findFreePort();
         DEST_HTTP_PORT = findFreePort();
+//        String OAK_VERSION = "1.32.0";
+        String OAK_VERSION = "1.38.0";
         versionResolver.setVersion("commons-codec", "commons-codec", "1.14");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-jackrabbit-api", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-auth-external", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-api", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-core-spi", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-commons", "1.32.0");
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-jackrabbit-api", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-auth-external", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-api", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-core-spi", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-commons", OAK_VERSION);
         SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "jackrabbit-jcr-commons", "2.20.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-blob-plugins", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-spi", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-core", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-blob", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-composite", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-document", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-jcr", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-lucene", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-authorization-principalbased", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-query-spi", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-security-spi", "1.32.0");
-        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-segment-tar", "1.32.0");
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-blob-plugins", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-spi", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-core", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-blob", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-composite", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-store-document", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-jcr", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-lucene", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-authorization-principalbased", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-query-spi", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-security-spi", OAK_VERSION);
+        SlingOptions.versionResolver.setVersion("org.apache.jackrabbit", "oak-segment-tar", OAK_VERSION);
         SlingOptions.versionResolver.setVersion("org.apache.tika", "tika-core", "1.24");
         SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.jcr.oak.server", "1.2.10");
-
-        return new Option[]{
+        SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.api", "2.23.0");
+        SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.resourceresolver", "1.7.2");
+        SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.scripting.core", "2.3.4");
+        SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.commons.compiler", "2.4.0");
+        SlingOptions.versionResolver.setVersion("org.apache.sling", "org.apache.sling.servlets.resolver", "2.7.12");
+        Option[] options = new Option[]{
             systemProperty("org.osgi.service.http.port").value(String.valueOf(HTTP_PORT)),
+            systemProperty("sling.home").value("./sling"),
             baseConfiguration(),
             slingQuickstart(),
             slingAuthForm(),
             failOnUnresolvedBundles(),
             mavenBundle().groupId("org.apache.jackrabbit").artifactId("oak-jackrabbit-api").version(versionResolver),
             mavenBundle().groupId("org.apache.jackrabbit").artifactId("oak-auth-external").version(versionResolver),
+            mavenBundle().groupId("org.apache.felix").artifactId("org.apache.felix.converter").version("1.0.14"),
+            mavenBundle().groupId("org.mockito").artifactId("mockito-core").version("3.3.3"),
+            mavenBundle().groupId("net.bytebuddy").artifactId("byte-buddy").version("1.10.5"),
+            mavenBundle().groupId("net.bytebuddy").artifactId("byte-buddy-agent").version("1.10.5"),
+            mavenBundle().groupId("org.objenesis").artifactId("objenesis").version("2.6"),
+            mavenBundle().groupId("org.bouncycastle").artifactId("bcprov-jdk15on").version("1.64"),
+            mavenBundle().groupId("org.bouncycastle").artifactId("bcpkix-jdk15on").version("1.64"),
+            mavenBundle().groupId("org.apache.sling").artifactId("org.apache.sling.commons.compiler").version(versionResolver),
             factoryConfiguration("org.apache.sling.jcr.repoinit.RepositoryInitializer")
                 .put("scripts", new String[]{"create service user saml2-user-mgt\n\n  set ACL for saml2-user-mgt\n\n  allow jcr:all on /home\n\n  end\n\n  create group sling-authors with path /home/groups/sling-authors"})
                 .asOption(),
@@ -182,9 +212,27 @@ public class SamlHandlerIT extends TestSupport {
                 .put("jksFileLocation", "")
                 .put("jksStorePassword", "")
                 .put("idpCertAlias","")
+                .put("spKeysAlias","")
                 .put("spKeysPassword","")
                 .asOption(),
+            factoryConfiguration("org.apache.sling.auth.saml2.AuthenticationHandlerSAML2")
+                .put("path", "/")
+                .put("entityID", "http://localhost:8080/")
+                .put("acsPath", "/sp/consumer")
+                .put("saml2userIDAttr", "username")
+                .put("saml2userHome", "/home/users/saml")
+                .put("saml2groupMembershipAttr", "groupMembership")
+                .put("syncAttrs", new String[]{"urn:oid:2.5.4.4","urn:oid:2.5.4.42","phone","urn:oid:1.2.840.113549.1.9.1"})
+                .put("saml2SPEnabled", true)
+                .put("saml2SPEncryptAndSign", true)
+                .put("jksFileLocation", "./src/test/resources/exampleSaml2.jks")
+                .put("jksStorePassword", "password")
+                .put("idpCertAlias",IDP_ALIAS)
+                .put("spKeysAlias",SP_ALIAS)
+                .put("spKeysPassword","sppassword")
+                .asOption(),
         };
+        return options;
     }
 
     /**
@@ -264,6 +312,7 @@ public class SamlHandlerIT extends TestSupport {
         assertNotNull(resolverFactory);
         assertNotNull(saml2UserMgtService);
         assertNotNull(authHandler);
+        assertNotNull(authHandlerEnc);
         logBundles();
     }
 
@@ -330,4 +379,42 @@ public class SamlHandlerIT extends TestSupport {
         }
         saml2UserMgtService.cleanUp();
     }
+
+    @Test
+    public void test_request_credentials() throws IOException {
+        HttpServletRequest requestIgnore = Mockito.mock(HttpServletRequest.class);
+        HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+        when(requestIgnore.getParameter(REQUEST_LOGIN_PARAMETER)).thenReturn("FORM");
+        // request credentials returns false when param ?sling:authRequestLogin=<something other than SAML2>
+        assertFalse(authHandler.requestCredentials(requestIgnore,response));
+        HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+        HttpSession httpSession = Mockito.mock(HttpSession.class);
+        when(request.getRequestURL()).thenReturn(new StringBuffer("/"));
+        when(request.getContextPath()).thenReturn("/");
+        when(request.getMethod()).thenReturn("POST");
+        when(request.getHeader("Referer")).thenReturn("/not/my/sp/consumer");
+        when(request.getSession()).thenReturn(httpSession);
+        // requestCredentials returns true when SAML is enabled and request is not specifying another auth handler
+        assertTrue(authHandler.requestCredentials(request,response));
+    }
+
+    @Test
+    public void test_encACSPath(){
+        String base64EndSamlResp = "PHNhbWxwOlJlc3BvbnNlIHhtbG5zOnNhbWxwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiIHhtbG5zOnNhbWw9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDphc3NlcnRpb24iIERlc3RpbmF0aW9uPSJodHRwOi8vbG9jYWxob3N0OjgwODAvc3AvY29uc3VtZXIiIElEPSJJRF9iOGZhYjUwOC1kOWM3LTQzYmEtOGM4My00Y2I1M2Y3ZjlmZmEiIEluUmVzcG9uc2VUbz0iXzRlNjhiMWIxNTk2ZDE0MmYwZTJhYWMzNjI0YzQxZDFiIiBJc3N1ZUluc3RhbnQ9IjIwMjEtMDQtMDVUMTg6MjE6MTYuMzIxWiIgVmVyc2lvbj0iMi4wIj48c2FtbDpJc3N1ZXI+aHR0cDovL2xvY2FsaG9z [...]
+        HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+        HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+        when(request.getRequestURI()).thenReturn("/sp/consumer");
+        when(request.getMethod()).thenReturn("POST");
+        when(request.getParameter("RelayState")).thenReturn("kojit9j9o1ff9q6vpeo8dnsfc9");
+        when(request.getHeader("Origin")).thenReturn("http://localhost:8484");
+        when(request.getHeader("Referer")).thenReturn("http://localhost:8484/");
+        HttpSession httpSession = Mockito.mock(HttpSession.class);
+        when(httpSession.getAttribute("saml2AuthInfo")).thenReturn("kojit9j9o1ff9q6vpeo8dnsfc9");
+        when(httpSession.getAttribute("saml2RequestID")).thenReturn("_4e68b1b1596d142f0e2aac3624c41d1b");
+        when(request.getSession(false)).thenReturn(httpSession);
+        when(request.getParameter("SAMLResponse")).thenReturn(base64EndSamlResp);
+        assertNull(authHandlerEnc.extractCredentials(request, response)) ;
+    }
+
+
 }
diff --git a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2ImplTest.java b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2ImplTest.java
index dcabb87..45da04f 100644
--- a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2ImplTest.java
+++ b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/AuthenticationHandlerSAML2ImplTest.java
@@ -20,6 +20,8 @@
 package org.apache.sling.auth.saml2.impl;
 
 import org.apache.jackrabbit.api.security.user.User;
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
 import org.apache.sling.auth.core.spi.AuthenticationInfo;
 import org.apache.sling.auth.saml2.AuthenticationHandlerSAML2;
 import org.hamcrest.core.StringStartsWith;
@@ -34,6 +36,7 @@ import org.osgi.framework.BundleContext;
 import org.jmock.api.Action;
 
 import javax.jcr.RepositoryException;
+import javax.servlet.http.HttpSession;
 import java.io.File;
 import java.io.UnsupportedEncodingException;
 import java.math.BigInteger;
@@ -41,6 +44,7 @@ import java.security.InvalidKeyException;
 import java.security.NoSuchAlgorithmException;
 import java.util.UUID;
 
+import static org.apache.sling.auth.saml2.impl.AuthenticationHandlerSAML2Impl.AUTH_TYPE;
 import static org.apache.sling.auth.saml2.impl.AuthenticationHandlerSAML2Impl.TOKEN_FILENAME;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -169,40 +173,58 @@ public class AuthenticationHandlerSAML2ImplTest {
         handler.initializeTokenStore(tokenFileActual);
         // verify token file
         assertEquals(tokenFileExpected, tokenFileActual);
-//        assertEquals(0,handler.getTokenStore().getActiveToken());
 
         // setup mock users
         User user = Mockito.mock(User.class);
         when(user.getID()).thenReturn("test-user");
+        AuthenticationInfo authenticationInfo = handler.buildAuthInfo(user);
+        long not_expired = System.currentTimeMillis() + handler.sessionTimeout;
+        String token = handler.getTokenStore().encode(not_expired , authenticationInfo.getUser());
+        String[] parts = TokenStore.split(token);
+        assertEquals(3, parts.length);
+        assertFalse(handler.needsRefresh(token));
+        assertTrue(handler.getTokenStore().isValid(token));
+
+        // expired token user
         User userExp = Mockito.mock(User.class);
         when(userExp.getID()).thenReturn("expired-user");
-        AuthenticationInfo authenticationInfo = handler.buildAuthInfo(user);
         AuthenticationInfo authenticationInfoExp = handler.buildAuthInfo(userExp);
-        long not_expired = System.currentTimeMillis() ;
-        long expired = System.currentTimeMillis() - handler.sessionTimeout - 1;
-        String token = handler.getTokenStore().encode(not_expired, authenticationInfo.getUser());
+        long expired = System.currentTimeMillis() - handler.sessionTimeout ;
         String expired_token = handler.getTokenStore().encode(expired, authenticationInfoExp.getUser());
-        // verify tokens
         String[] partsExp = TokenStore.split(expired_token);
-        String[] parts = TokenStore.split(token);
         assertEquals(3, partsExp.length);
-        assertEquals(3, parts.length);
-        assertTrue(handler.needsRefresh(expired_token, handler.sessionTimeout));
+        assertTrue(handler.needsRefresh(expired_token));
         assertFalse(handler.getTokenStore().isValid(expired_token));
-//        assertFalse(handler.needsRefresh(token, handler.sessionTimeout));
-//        assertTrue(handler.getTokenStore().isValid(token));
+
+        // test refreshAuthData
+        SlingHttpServletRequest request = Mockito.mock(SlingHttpServletRequest.class);
+        SlingHttpServletResponse response = Mockito.mock(SlingHttpServletResponse.class);
+        HttpSession session = Mockito.mock(HttpSession.class);
+        when(request.getSession()).thenReturn(session);
+        handler.getStorageAuthInfo().getString(request);
+        handler.refreshAuthData(request, response, authenticationInfo);
+
+
+        // no token user
+        SlingHttpServletRequest request2 = Mockito.mock(SlingHttpServletRequest.class);
+        HttpSession session2 = Mockito.mock(HttpSession.class);
+        when(request2.getSession()).thenReturn(session2);
+        User userNoToken = Mockito.mock(User.class);
+        when(user.getID()).thenReturn("no-token-user");
+        AuthenticationInfo authenticationInfoNT = handler.buildAuthInfo(userNoToken);
+        handler.getStorageAuthInfo().getString(request2);
+        handler.refreshAuthData(request2, response, authenticationInfoNT);
     }
 
     @Test
     public void test_buildAuthInfo() throws RepositoryException {
         final AuthenticationHandlerSAML2Impl handler = new AuthenticationHandlerSAML2Impl();
-        assertTrue(handler.needsRefresh(null,0));
+        assertTrue(handler.needsRefresh(null));
         User user = Mockito.mock(User.class);
         when(user.getID()).thenReturn("test-user");
         AuthenticationInfo authenticationInfo = handler.buildAuthInfo(user);
         assertEquals(AuthenticationHandlerSAML2Impl.AUTH_TYPE, authenticationInfo.getAuthType());
         assertEquals("test-user", authenticationInfo.getUser());
-
     }
 
     @Test
diff --git a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/JKSHelper.java b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/JKSHelper.java
index 905f71a..4e39ebc 100644
--- a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/JKSHelper.java
+++ b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/JKSHelper.java
@@ -61,14 +61,21 @@ import static org.junit.Assert.fail;
 
 public class JKSHelper {
     private static Logger logger = LoggerFactory.getLogger(JKSHelper.class);
-    static String KEYSTORE_TEST_PATH = "./target/exampleSaml2.jks";
-    static char[] KEYSTORE_TEST_PASSWORD = "password".toCharArray();
-    static String IDP_ALIAS = "idpCertAlias";
-    static String SP_ALIAS = "spAlias";
-    static char[] SP_TEST_PASSWORD = "sppassword".toCharArray();
+    public static String KEYSTORE_TEST_PATH = "./target/exampleSaml2.jks";
+    public static char[] KEYSTORE_TEST_PASSWORD = "password".toCharArray();
+    public static String IDP_ALIAS = "idpcertalias";
+    public static String SP_ALIAS = "spalias";
+    public static char[] SP_TEST_PASSWORD = "sppassword".toCharArray();
+    static String PATH = "";
 
-    static KeyStore createExampleJks(){
-        File file = new File(KEYSTORE_TEST_PATH);
+    public static KeyStore createExampleJks(){
+        PATH = KEYSTORE_TEST_PATH;
+        return createExampleJks(KEYSTORE_TEST_PATH);
+    }
+
+    public static KeyStore createExampleJks(String path){
+        PATH = path;
+        File file = new File(PATH);
         try (FileOutputStream fos = new FileOutputStream(file)){
             KeyStore ks;
             ks = KeyStore.getInstance(KeyStore.getDefaultType());
@@ -87,8 +94,8 @@ public class JKSHelper {
         return null;
     }
 
-    static void addTestingCertsToKeystore(KeyStore testKeyStore){
-        try (FileOutputStream fos = new FileOutputStream(KEYSTORE_TEST_PATH)){
+    public static void addTestingCertsToKeystore(KeyStore testKeyStore){
+        try (FileOutputStream fos = new FileOutputStream(PATH)){
             testKeyStore.load(null, KEYSTORE_TEST_PASSWORD);
             KeyPairGenerator keyPairGenerator = KeyPairGenerator.getInstance("RSA");
             keyPairGenerator.initialize(4096);
@@ -97,6 +104,7 @@ public class JKSHelper {
             final X509Certificate certIDP = JKSHelper.generate(keyPairIDP, "SHA256withRSA", "localhost", 730);
             final X509Certificate certSP = JKSHelper.generate(keyPairSP, "SHA256withRSA", "localhost", 730);
             testKeyStore.setCertificateEntry(IDP_ALIAS, certIDP);
+            testKeyStore.setKeyEntry(IDP_ALIAS+"key", keyPairIDP.getPrivate(), KEYSTORE_TEST_PASSWORD,  new Certificate[]{certIDP});
             testKeyStore.setKeyEntry(SP_ALIAS, keyPairSP.getPrivate(), SP_TEST_PASSWORD,  new Certificate[]{certSP});
             testKeyStore.store(fos, KEYSTORE_TEST_PASSWORD);
         } catch (Exception e){
diff --git a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/OsgiSamlTest.java b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/OsgiSamlTest.java
index 4696efa..09b261c 100644
--- a/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/OsgiSamlTest.java
+++ b/saml-handler/src/test/java/org/apache/sling/auth/saml2/impl/OsgiSamlTest.java
@@ -21,8 +21,10 @@
 package org.apache.sling.auth.saml2.impl;
 
 import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.auth.core.spi.AuthenticationInfo;
 import org.apache.sling.auth.saml2.Helpers;
 import org.apache.sling.auth.saml2.SAML2RuntimeException;
 import org.apache.sling.auth.saml2.Saml2User;
@@ -67,7 +69,10 @@ import java.security.cert.CertificateException;
 import java.time.Instant;
 import java.util.Dictionary;
 import java.util.Hashtable;
+
+import static org.apache.sling.auth.core.spi.AuthenticationHandler.REQUEST_LOGIN_PARAMETER;
 import static org.apache.sling.auth.saml2.Activator.initializeOpenSaml;
+import static org.apache.sling.auth.saml2.impl.AuthenticationHandlerSAML2Impl.AUTH_TYPE;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -138,6 +143,13 @@ public class OsgiSamlTest {
     }
 
     @Test
+    public void test_not_ignored_when_saml2_specified(){
+        HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+        when(request.getParameter(REQUEST_LOGIN_PARAMETER)).thenReturn("SAML2");
+        assertFalse(samlHandler.ignoreRequestCredentials(request));
+    }
+
+    @Test
     public void test_disabled_saml_handler(){
         assertFalse(samlHandler.getSaml2SPEnabled());
         final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
diff --git a/saml-handler/src/test/resources/exampleSaml2.jks b/saml-handler/src/test/resources/exampleSaml2.jks
new file mode 100644
index 0000000..d8afcee
Binary files /dev/null and b/saml-handler/src/test/resources/exampleSaml2.jks differ