invalid issuer in UserPrincipalManager (#642) (#698)

* invalid issuer in UserPrincipalManager (#642)

* Braking lines in the test to make the checkstyle pass

* Moving test Junit Params version dep into parent pom

azure-spring-boot-parent/pom.xml

@@ -41,6 +41,7 @@
         <javax.jms-api.version>2.0.1</javax.jms-api.version>
         <spring-jms.version>5.1.8.RELEASE</spring-jms.version>
         <qpid-jms-client.version>0.43.0</qpid-jms-client.version>
+        <junit-params.version>1.1.1</junit-params.version>
     </properties>
 
     <profiles>
@@ -152,6 +153,12 @@
                 <version>${qpid-jms-client.version}</version>
             </dependency>
 
+            <!-- TEST -->
+            <dependency>
+                <groupId>pl.pragmatists</groupId>
+                <artifactId>JUnitParams</artifactId>
+                <version>${junit-params.version}</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
 

azure-spring-boot/pom.xml

@@ -235,6 +235,11 @@
             <artifactId>wiremock-standalone</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>pl.pragmatists</groupId>
+            <artifactId>JUnitParams</artifactId>
+            <scope>test</scope>
+        </dependency>
 
         <!--Spring JMS-->
         <dependency>

azure-spring-boot/src/main/java/com/microsoft/azure/spring/autoconfigure/aad/UserPrincipalManager.java

@@ -28,6 +28,9 @@ import java.text.ParseException;
 
 @Slf4j
 public class UserPrincipalManager {
+    private static final String LOGIN_MICROSOFT_ONLINE_ISSUER = "https://login.microsoftonline.com/";
+    private static final String STS_WINDOWS_ISSUER = "https://sts.windows.net/";
+    private static final String STS_CHINA_CLOUD_API_ISSUER = "https://sts.chinacloudapi.cn/";
 
     private final JWKSource<SecurityContext> keySource;
     private final AADAuthenticationProperties aadAuthProps;
@@ -94,13 +97,15 @@ public class UserPrincipalManager {
                 new JWSVerificationKeySelector<>(jwsAlgorithm, keySource);
         jwtProcessor.setJWSKeySelector(keySelector);
 
+         //TODO: would it make sense to inject it? and make it configurable or even allow to provide own implementation
         jwtProcessor.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier<SecurityContext>() {
             @Override
             public void verify(JWTClaimsSet claimsSet, SecurityContext ctx) throws BadJWTException {
                 super.verify(claimsSet, ctx);
                 final String issuer = claimsSet.getIssuer();
-                if (issuer == null || !issuer.contains("https://sts.windows.net/")
-                        && !issuer.contains("https://sts.chinacloudapi.cn/")) {
+                if (issuer == null || !(issuer.startsWith(LOGIN_MICROSOFT_ONLINE_ISSUER)
+                        || issuer.startsWith(STS_WINDOWS_ISSUER)
+                        || issuer.startsWith(STS_CHINA_CLOUD_API_ISSUER))) {
                     throw new BadJWTException("Invalid token issuer");
                 }
                 if (explicitAudienceCheck) {

azure-spring-boot/src/test/java/com/microsoft/azure/spring/autoconfigure/aad/UserPrincipalManagerTest.java

@@ -9,8 +9,11 @@ import com.nimbusds.jose.jwk.JWK;
 import com.nimbusds.jose.jwk.JWKSet;
 import com.nimbusds.jose.jwk.source.ImmutableJWKSet;
 import com.nimbusds.jwt.proc.BadJWTException;
+import junitparams.FileParameters;
+import junitparams.JUnitParamsRunner;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
@@ -21,6 +24,7 @@ import java.security.cert.X509Certificate;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;
 
+@RunWith(JUnitParamsRunner.class)
 public class UserPrincipalManagerTest {
 
     private static ImmutableJWKSet immutableJWKSet;
@@ -55,6 +59,15 @@ public class UserPrincipalManagerTest {
                 .isInstanceOf(BadJWTException.class);
     }
 
+    @Test
+    //TODO: add more generated tokens with other valid issuers to this file. Didn't manage to generate them
+    @FileParameters("src/test/resources/jwt-valid-issuer.txt")
+    public void validIssuer(final String token) {
+        userPrincipalManager = new UserPrincipalManager(immutableJWKSet);
+        assertThatCode(() -> userPrincipalManager.buildUserPrincipal(token))
+                .doesNotThrowAnyException();
+    }
+
     @Test
     public void nullIssuer() {
         userPrincipalManager = new UserPrincipalManager(immutableJWKSet);

azure-spring-boot/src/test/resources/jwt-valid-issuer.txt

@@ -0,0 +1 @@
+eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJodHRwczovL3N0cy53aW5kb3dzLm5ldC90ZXN0Iiwic3ViIjoidGVzdEBleGFtcGxlLmNvbSIsIm5iZiI6MTU0NTAwODE5NiwiZXhwIjo5OTk5OTk5OTk5OSwiaWF0IjoxNTQ1MDA4MTk2LCJqdGkiOiJ0ZXN0aWQiLCJ0eXAiOiJodHRwczovL2V4YW1wbGUuY29tL3JlZ2lzdGVyIn0.ZQceiSqNKiEHrNaPhKCKW2EVEnhGbyh4TjbhqB-P7E70NRS3Ad89ISBaSyhpwRS6lwdpMrwNEETFloGm8H6nv623gcWzTCnb7bqaOWKCNTV9TjvhecjIe69AkNHfvkqyopbyRktKosWm89e2nAgiGtp-Y1Pyrt1_iiwOtvahtGyaWqs82-WkFY61DFI1e4iRBI6WSIGLUUpc4vXCGdQ33OyN6wAQ2IYeHCURmB-stVT-GcoMcDZKJBqnerQsu5WDbSwkZfcVTWDK-l_sz1WSdFGTdSWATZJ_LKvxa8IPX--s0-JRmZf-0dwadjcbCNLwYtYDvtaZyczouZKGGBoWZA