ACS-8048 Fix HXIAM access token validation issues (#2706)

* ACS-8047: WIP - Fixed HXIAM access token validation issues.

* ACS-8048 Accept at+jwt token + make signature algorithm configurable

* ACS-8048 Accept multiple signature algorithms

* ACS-8048 Fix PMD issue

* ACS-8048 Log using the default signature algorithm

* ACS-8048 Refactor

---------

Co-authored-by: Jamal Kaabi-Mofrad <jamal.kaabimofrad@alfresco.com>

repository/src/main/java/org/alfresco/repo/security/authentication/identityservice/IdentityServiceConfig.java

@@ -25,9 +25,14 @@
  */
 package org.alfresco.repo.security.authentication.identityservice;
 
+import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.commons.lang3.StringUtils;
+import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm;
 import org.springframework.web.util.UriComponentsBuilder;
 
 /**
@@ -35,6 +40,7 @@ import org.springframework.web.util.UriComponentsBuilder;
  *
  * @author Gavin Cornwell
  */
+@SuppressWarnings("PMD.ExcessivePublicCount")
 public class IdentityServiceConfig
 {
     private static final String REALMS = "realms";
@@ -62,6 +68,7 @@ public class IdentityServiceConfig
     private String principalAttribute;
     private boolean clientIdValidationDisabled;
     private String adminConsoleRedirectPath;
+    private String signatureAlgorithms;
 
     /**
      *
@@ -306,4 +313,18 @@ public class IdentityServiceConfig
     {
         this.adminConsoleRedirectPath = adminConsoleRedirectPath;
     }
+
+    public Set<SignatureAlgorithm> getSignatureAlgorithms()
+    {
+        return Stream.of(signatureAlgorithms.split(","))
+            .map(String::trim)
+            .map(SignatureAlgorithm::from)
+            .filter(Objects::nonNull)
+            .collect(Collectors.toUnmodifiableSet());
+    }
+
+    public void setSignatureAlgorithms(String signatureAlgorithms)
+    {
+        this.signatureAlgorithms = signatureAlgorithms;
+    }
 }

repository/src/main/java/org/alfresco/repo/security/authentication/identityservice/IdentityServiceFacadeFactoryBean.java

@@ -58,10 +58,13 @@ import java.util.function.Function;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
+import com.nimbusds.jose.JOSEObjectType;
 import com.nimbusds.jose.JWSAlgorithm;
 import com.nimbusds.jose.jwk.source.DefaultJWKSetCache;
 import com.nimbusds.jose.jwk.source.JWKSource;
 import com.nimbusds.jose.jwk.source.RemoteJWKSet;
+import com.nimbusds.jose.proc.BadJOSEException;
+import com.nimbusds.jose.proc.DefaultJOSEObjectTypeVerifier;
 import com.nimbusds.jose.proc.JWSVerificationKeySelector;
 import com.nimbusds.jose.proc.SecurityContext;
 import com.nimbusds.jose.util.ResourceRetriever;
@@ -129,6 +132,9 @@ import org.springframework.web.util.UriComponentsBuilder;
 public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentityServiceFacade>
 {
     private static final Log LOGGER = LogFactory.getLog(IdentityServiceFacadeFactoryBean.class);
+
+    private static final JOSEObjectType AT_JWT = new JOSEObjectType("at+jwt");
+
     private boolean enabled;
     private SpringBasedIdentityServiceFacadeFactory factory;
 
@@ -554,12 +560,20 @@ public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentitySer
 
     static class JwtDecoderProvider
     {
-        private static final SignatureAlgorithm SIGNATURE_ALGORITHM = SignatureAlgorithm.RS256;
+        private static final SignatureAlgorithm DEFAULT_SIGNATURE_ALGORITHM = SignatureAlgorithm.RS256;
         private final IdentityServiceConfig config;
+        private final Set<SignatureAlgorithm> signatureAlgorithms;
 
         JwtDecoderProvider(IdentityServiceConfig config)
         {
             this.config = requireNonNull(config);
+            this.signatureAlgorithms = ofNullable(config.getSignatureAlgorithms())
+                .filter(not(Set::isEmpty))
+                .orElseGet(() -> {
+                    LOGGER.warn("Unable to find any valid signature algorithms in the configuration. "
+                        + "Using the default signature algorithm: " + DEFAULT_SIGNATURE_ALGORITHM.getName() + ".");
+                    return Set.of(DEFAULT_SIGNATURE_ALGORITHM);
+                });
         }
 
         public JwtDecoder createJwtDecoder(RestOperations rest, ProviderDetails providerDetails)
@@ -587,13 +601,13 @@ public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentitySer
             {
                 final RSAPublicKey publicKey = parsePublicKey(config.getRealmKey());
                 return NimbusJwtDecoder.withPublicKey(publicKey)
-                    .signatureAlgorithm(SIGNATURE_ALGORITHM)
+                    .signatureAlgorithm(DEFAULT_SIGNATURE_ALGORITHM)
                     .build();
             }
-
             final String jwkSetUri = requireValidJwkSetUri(providerDetails);
-            return NimbusJwtDecoder.withJwkSetUri(jwkSetUri)
+            final NimbusJwtDecoder.JwkSetUriJwtDecoderBuilder decoderBuilder = NimbusJwtDecoder.withJwkSetUri(jwkSetUri);
-                .jwsAlgorithm(SIGNATURE_ALGORITHM)
+            signatureAlgorithms.forEach(decoderBuilder::jwsAlgorithm);
+            return decoderBuilder
                 .restOperations(rest)
                 .jwtProcessorCustomizer(this::reconfigureJWKSCache)
                 .build();
@@ -633,8 +647,11 @@ public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentitySer
                 resourceRetriever.get(), cache);
 
             jwtProcessor.setJWSKeySelector(new JWSVerificationKeySelector<>(
-                JWSAlgorithm.parse(SIGNATURE_ALGORITHM.getName()),
+                signatureAlgorithms.stream()
+                    .map(signatureAlgorithm -> JWSAlgorithm.parse(signatureAlgorithm.getName()))
+                    .collect(Collectors.toSet()),
                 cachingJWKSource));
+            jwtProcessor.setJWSTypeVerifier(new CustomJOSEObjectTypeVerifier(JOSEObjectType.JWT, AT_JWT));
         }
 
         private OAuth2TokenValidator<Jwt> createJwtTokenValidator(ProviderDetails providerDetails)
@@ -759,7 +776,6 @@ public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentitySer
         }
     }
 
-
     static class CustomClientHttpRequestFactory extends HttpComponentsClientHttpRequestFactory
     {
         CustomClientHttpRequestFactory(HttpClient httpClient)
@@ -781,9 +797,22 @@ public class IdentityServiceFacadeFactoryBean implements FactoryBean<IdentitySer
         }
     }
 
+    static class CustomJOSEObjectTypeVerifier extends DefaultJOSEObjectTypeVerifier<SecurityContext>
+    {
+        public CustomJOSEObjectTypeVerifier(JOSEObjectType... allowedTypes)
+        {
+            super(Set.of(allowedTypes));
+        }
+
+        @Override
+        public void verify(JOSEObjectType type, SecurityContext context) throws BadJOSEException
+        {
+            super.verify(type, context);
+        }
+    }
+
     private static boolean isDefined(String value)
     {
         return value != null && !value.isBlank();
     }
-
 }

repository/src/main/resources/alfresco/subsystems/Authentication/identity-service/identity-service-authentication-context.xml

@@ -155,6 +155,9 @@
       <property name="adminConsoleRedirectPath">
          <value>${identity-service.admin-console.redirect-path}</value>
       </property>
+      <property name="signatureAlgorithms">
+         <value>${identity-service.signature-algorithms:RS256,PS256}</value>
+      </property>
    </bean>
 
    <!-- Enable control over mapping between request and user ID -->

repository/src/main/resources/alfresco/subsystems/Authentication/identity-service/identity-service-authentication.properties

@@ -12,3 +12,4 @@ identity-service.resource=alfresco
 identity-service.credentials.secret=
 identity-service.public-client=true
 identity-service.admin-console.redirect-path=/alfresco/s/admin/admin-communitysummary
+identity-service.signature-algorithms=RS256,PS256

repository/src/test/java/org/alfresco/repo/security/authentication/identityservice/IdentityServiceFacadeFactoryBeanTest.java

@@ -25,53 +25,156 @@
  */
 package org.alfresco.repo.security.authentication.identityservice;
 
+import static com.nimbusds.jose.HeaderParameterNames.KEY_ID;
+
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.UUID;
 
+import com.nimbusds.jose.Algorithm;
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JOSEObjectType;
+import com.nimbusds.jose.JWSAlgorithm;
+import com.nimbusds.jose.JWSHeader;
+import com.nimbusds.jose.crypto.RSASSASigner;
+import com.nimbusds.jose.jwk.KeyUse;
+import com.nimbusds.jose.jwk.RSAKey;
+import com.nimbusds.jose.jwk.gen.RSAKeyGenerator;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
 import com.nimbusds.openid.connect.sdk.claims.PersonClaims;
 
 import org.alfresco.repo.security.authentication.identityservice.IdentityServiceFacadeFactoryBean.JwtAudienceValidator;
 import org.alfresco.repo.security.authentication.identityservice.IdentityServiceFacadeFactoryBean.JwtDecoderProvider;
 import org.alfresco.repo.security.authentication.identityservice.IdentityServiceFacadeFactoryBean.JwtIssuerValidator;
 import org.junit.Test;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
 import org.springframework.security.oauth2.client.registration.ClientRegistration.ProviderDetails;
 import org.springframework.security.oauth2.core.OAuth2Error;
 import org.springframework.security.oauth2.core.OAuth2TokenValidatorResult;
+import org.springframework.security.oauth2.jose.jws.SignatureAlgorithm;
+import org.springframework.security.oauth2.jwt.BadJwtException;
 import org.springframework.security.oauth2.jwt.Jwt;
 import org.springframework.security.oauth2.jwt.JwtDecoder;
+import org.springframework.web.client.RestOperations;
 
 public class IdentityServiceFacadeFactoryBeanTest
 {
     private static final String EXPECTED_ISSUER = "expected-issuer";
     private static final String EXPECTED_AUDIENCE = "expected-audience";
+    public final IdentityServiceConfig config = mock(IdentityServiceConfig.class);
+    public final RestOperations restOperations = mock(RestOperations.class);
+    public final ResponseEntity responseEntity = mock(ResponseEntity.class);
+    public final ProviderDetails providerDetails = mock(ProviderDetails.class);
 
     @Test
     public void shouldCreateJwtDecoderWithoutIDSWhenPublicKeyIsProvided()
     {
-        final IdentityServiceConfig config = mock(IdentityServiceConfig.class);
+        when(config.getRealmKey()).thenReturn(
-        when(config.getRealmKey()).thenReturn("MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAve3MabX/rp3LbE7/zNqKxuid8WT7y4qSXsNaiPvl/OVbNWW/cu5td1VndItYhH6/gL7Z5W/r4MOeTlz/fOdXfjrRJou2f3UiPQwLV9RdOH3oS4/BUe+sviD8Q3eRfWBWWz3yw8f2YNtD4bMztIMMjqthvwdEEb9S9jbxxD0o71Bsrz/FwPi7HhSDA+Z/p01Hct8m4wx13ZlKRd4YjyC12FBmi9MSgsrFuWzyQHhHTeBDoALpfuiut3rhVxUtFmVTpy6p9vil7C5J5pok4MXPH0dJCyDNQz05ww5+fD+tfksIEpFeokRpN226F+P21oQVFUWwYIaXaFlG/hfvwmnlfQIDAQAB");
+            "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAve3MabX/rp3LbE7/zNqKxuid8WT7y4qSXsNaiPvl/OVbNWW/cu5td1VndItYhH6/gL7Z5W/r4MOeTlz/fOdXfjrRJou2f3UiPQwLV9RdOH3oS4/BUe+sviD8Q3eRfWBWWz3yw8f2YNtD4bMztIMMjqthvwdEEb9S9jbxxD0o71Bsrz/FwPi7HhSDA+Z/p01Hct8m4wx13ZlKRd4YjyC12FBmi9MSgsrFuWzyQHhHTeBDoALpfuiut3rhVxUtFmVTpy6p9vil7C5J5pok4MXPH0dJCyDNQz05ww5+fD+tfksIEpFeokRpN226F+P21oQVFUWwYIaXaFlG/hfvwmnlfQIDAQAB");
         when(config.isClientIdValidationDisabled()).thenReturn(true);
-
-        final ProviderDetails providerDetails = mock(ProviderDetails.class);
         when(providerDetails.getIssuerUri()).thenReturn("https://my.issuer");
 
         final JwtDecoderProvider provider = new JwtDecoderProvider(config);
 
         final JwtDecoder decoder = provider.createJwtDecoder(null, providerDetails);
 
-        final Jwt decodedToken = decoder.decode("eyJhbGciOiJSUzI1NiJ9.eyJleHAiOjIxNDc0ODM2NDcsImp0aSI6IjEyMzQiLCJpc3MiOiJodHRwczovL215Lmlzc3VlciIsInN1YiI6ImFiYzEyMyIsInR5cCI6IkJlYXJlciIsInByZWZlcnJlZF91c2VybmFtZSI6InBpb3RyZWsifQ.k_KaOrLLh3QsT8mKphkcz2vKpulgxp92UoEDccpHJ1mxE3Pa3gFXPKTj4goUBKXieGPZRMvBDhfWNxMvRYZPiQr2NXJKapkh0bTd0qoaSWz9ICe9Nu3eg7_VA_nwUVPz_35wwmrxgVk0_kpUYQN_VtaO7ZgFE2sJzFjbkVls5aqfAMnEjEgQl837hqZvmlW2ZRWebtxXfQxAjtp0gcTg-xtAHKIINYo_1_uAtt_H9L8KqFaioxrVAEDDIlcKnb-Ks3Y62CrZauaGUJeN_aNj2gdOpdkhvCw79yJyZSGZ7okjGbidCNSAf7Bo2Y6h3dP1Gga7kRmD648ftZESrNvbyg");
+        final Jwt decodedToken = decoder.decode(
+            "eyJhbGciOiJSUzI1NiJ9.eyJleHAiOjIxNDc0ODM2NDcsImp0aSI6IjEyMzQiLCJpc3MiOiJodHRwczovL215Lmlzc3VlciIsInN1YiI6ImFiYzEyMyIsInR5cCI6IkJlYXJlciIsInByZWZlcnJlZF91c2VybmFtZSI6InBpb3RyZWsifQ.k_KaOrLLh3QsT8mKphkcz2vKpulgxp92UoEDccpHJ1mxE3Pa3gFXPKTj4goUBKXieGPZRMvBDhfWNxMvRYZPiQr2NXJKapkh0bTd0qoaSWz9ICe9Nu3eg7_VA_nwUVPz_35wwmrxgVk0_kpUYQN_VtaO7ZgFE2sJzFjbkVls5aqfAMnEjEgQl837hqZvmlW2ZRWebtxXfQxAjtp0gcTg-xtAHKIINYo_1_uAtt_H9L8KqFaioxrVAEDDIlcKnb-Ks3Y62CrZauaGUJeN_aNj2gdOpdkhvCw79yJyZSGZ7okjGbidCNSAf7Bo2Y6h3dP1Gga7kRmD648ftZESrNvbyg");
         assertThat(decodedToken).isNotNull();
 
         final Map<String, Object> claims = decodedToken.getClaims();
         assertThat(claims).isNotNull()
-                          .isNotEmpty()
+            .isNotEmpty()
-                          .containsEntry(PersonClaims.PREFERRED_USERNAME_CLAIM_NAME, "piotrek");
+            .containsEntry(PersonClaims.PREFERRED_USERNAME_CLAIM_NAME, "piotrek");
+    }
+
+    @Test
+    public void shouldAcceptAndDecodeAtJwtToken() throws JOSEException
+    {
+        when(config.isClientIdValidationDisabled()).thenReturn(true);
+        when(config.getSignatureAlgorithms()).thenReturn(Set.of(SignatureAlgorithm.PS256, SignatureAlgorithm.ES512));
+        when(restOperations.exchange(any(), any(Class.class))).thenReturn(responseEntity);
+
+        final RSAKey rsaKey = getRsaKey();
+        final RSAKey rsaPublicJWK = rsaKey.toPublicJWK();
+        when(responseEntity.getStatusCode()).thenReturn(HttpStatus.OK);
+        when(responseEntity.getBody()).thenReturn(String.format("{\"keys\": [%s]}", rsaPublicJWK.toJSONString()));
+
+        final SignedJWT signedJWT = getSignedJWT(rsaKey, "at+jwt", "userA", "https://my.issuer");
+        signedJWT.sign(new RSASSASigner(rsaKey));
+
+        when(providerDetails.getIssuerUri()).thenReturn("https://my.issuer");
+        when(providerDetails.getJwkSetUri()).thenReturn("https://my.jwkSetUri");
+
+        final JwtDecoderProvider provider = new JwtDecoderProvider(config);
+
+        final JwtDecoder decoder = provider.createJwtDecoder(restOperations, providerDetails);
+        final Jwt decodedToken = decoder.decode(signedJWT.serialize());
+        assertThat(decodedToken).isNotNull();
+
+        final Map<String, Object> claims = decodedToken.getClaims();
+        assertThat(claims).isNotNull()
+            .isNotEmpty()
+            .containsEntry(PersonClaims.PREFERRED_USERNAME_CLAIM_NAME, "userA");
+    }
+
+    @Test
+    public void shouldFailWithNotMatchingAlgorithm() throws JOSEException
+    {
+        when(config.isClientIdValidationDisabled()).thenReturn(true);
+        when(config.getSignatureAlgorithms()).thenReturn(Set.of(SignatureAlgorithm.RS256));
+
+        when(restOperations.exchange(any(), any(Class.class))).thenReturn(responseEntity);
+
+        final RSAKey rsaKey = getRsaKey();
+        final RSAKey rsaPublicJWK = rsaKey.toPublicJWK();
+        when(responseEntity.getStatusCode()).thenReturn(HttpStatus.OK);
+        when(responseEntity.getBody()).thenReturn(String.format("{\"keys\": [%s]}", rsaPublicJWK.toJSONString()));
+
+        final SignedJWT signedJWT = getSignedJWT(rsaKey, "at+jwt", "userA", "https://my.issuer");
+        signedJWT.sign(new RSASSASigner(rsaKey));
+
+        when(providerDetails.getIssuerUri()).thenReturn("https://my.issuer");
+        when(providerDetails.getJwkSetUri()).thenReturn("https://my.jwkSetUri");
+
+        final JwtDecoderProvider provider = new JwtDecoderProvider(config);
+
+        final JwtDecoder decoder = provider.createJwtDecoder(restOperations, providerDetails);
+        assertThrows(BadJwtException.class, () -> decoder.decode(signedJWT.serialize()));
+    }
+
+    @Test
+    public void shouldFailWithNotAllowedJOSEHeaderTyp() throws JOSEException
+    {
+        when(config.isClientIdValidationDisabled()).thenReturn(true);
+        when(config.getSignatureAlgorithms()).thenReturn(Set.of(SignatureAlgorithm.PS256));
+        when(restOperations.exchange(any(), any(Class.class))).thenReturn(responseEntity);
+
+        final RSAKey rsaKey = getRsaKey();
+        final RSAKey rsaPublicJWK = rsaKey.toPublicJWK();
+        when(responseEntity.getStatusCode()).thenReturn(HttpStatus.OK);
+        when(responseEntity.getBody()).thenReturn(String.format("{\"keys\": [%s]}", rsaPublicJWK.toJSONString()));
+
+        final SignedJWT signedJWT = getSignedJWT(rsaKey, "not-allowed-type", "userA", "https://my.issuer");
+        signedJWT.sign(new RSASSASigner(rsaKey));
+
+        when(providerDetails.getIssuerUri()).thenReturn("https://my.issuer");
+        when(providerDetails.getJwkSetUri()).thenReturn("https://my.jwkSetUri");
+
+        final JwtDecoderProvider provider = new JwtDecoderProvider(config);
+
+        final JwtDecoder decoder = provider.createJwtDecoder(restOperations, providerDetails);
+        assertThrows(BadJwtException.class, () -> decoder.decode(signedJWT.serialize()));
     }
 
     @Test
@@ -79,7 +182,8 @@ public class IdentityServiceFacadeFactoryBeanTest
     {
         final JwtIssuerValidator issuerValidator = new JwtIssuerValidator(EXPECTED_ISSUER);
 
-        final OAuth2TokenValidatorResult validationResult = issuerValidator.validate(tokenWithIssuer("different-issuer"));
+        final OAuth2TokenValidatorResult validationResult = issuerValidator.validate(
+            tokenWithIssuer("different-issuer"));
         assertThat(validationResult).isNotNull();
         assertThat(validationResult.hasErrors()).isTrue();
         assertThat(validationResult.getErrors()).hasSize(1);
@@ -164,15 +268,35 @@ public class IdentityServiceFacadeFactoryBeanTest
         final JwtAudienceValidator audienceValidator = new JwtAudienceValidator(EXPECTED_AUDIENCE);
 
         final Jwt token = Jwt.withTokenValue(UUID.randomUUID().toString())
-                            .claim("aud", EXPECTED_AUDIENCE)
+            .claim("aud", EXPECTED_AUDIENCE)
-                            .header("JUST", "FOR TESTING")
+            .header("JUST", "FOR TESTING")
-                            .build();
+            .build();
         final OAuth2TokenValidatorResult validationResult = audienceValidator.validate(token);
         assertThat(validationResult).isNotNull();
         assertThat(validationResult.hasErrors()).isFalse();
         assertThat(validationResult.getErrors()).isEmpty();
     }
 
+    private static RSAKey getRsaKey() throws JOSEException
+    {
+        return new RSAKeyGenerator(2048)
+            .keyUse(KeyUse.SIGNATURE)
+            .algorithm(new Algorithm("PS256"))
+            .keyID(KEY_ID)
+            .generate();
+    }
+
+    private static SignedJWT getSignedJWT(RSAKey rsaKey, String type, String usernameClaim, String issuer)
+    {
+        final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder()
+            .issuer(issuer)
+            .claim(PersonClaims.PREFERRED_USERNAME_CLAIM_NAME, usernameClaim)
+            .build();
+        return new SignedJWT(new JWSHeader.Builder(JWSAlgorithm.PS256)
+            .type(new JOSEObjectType(type))
+            .keyID(rsaKey.getKeyID()).build(), claimsSet);
+    }
+
     private Jwt tokenWithIssuer(String issuer)
     {
         return Jwt.withTokenValue(UUID.randomUUID().toString())