Cleaning up attributeReleasePolicy for OIDC client apps

apache · Jul 25, 2023 · 782be0a · 782be0a
1 parent 22467ff
commit 782be0a
Show file tree

Hide file tree

Showing 4 changed files with 7 additions and 40 deletions.
diff --git a/ext/oidcc4ui/logic/src/main/java/org/apache/syncope/core/logic/OIDCC4UILogic.java b/ext/oidcc4ui/logic/src/main/java/org/apache/syncope/core/logic/OIDCC4UILogic.java
@@ -139,7 +139,6 @@ public OIDCLoginResponse login(final String redirectURI, final String authorizat
         // 2. get OpenID Connect tokens
         String idTokenHint;
         JWTClaimsSet idToken;
-        JWTClaimsSet accessToken;
         try {
             OidcCredentials credentials = new OidcCredentials();
             credentials.setCode(new AuthorizationCode(authorizationCode));
@@ -150,8 +149,6 @@ public OIDCLoginResponse login(final String redirectURI, final String authorizat
 
             idToken = credentials.getIdToken().getJWTClaimsSet();
             idTokenHint = credentials.getIdToken().serialize();
-
-            accessToken = SignedJWT.parse(credentials.getAccessToken().getValue()).getJWTClaimsSet();
         } catch (Exception e) {
             LOG.error("While validating Token Response", e);
             SyncopeClientException sce = SyncopeClientException.build(ClientExceptionType.Unknown);
@@ -170,7 +167,6 @@ public OIDCLoginResponse login(final String redirectURI, final String authorizat
             attrTO.setSchema(item.getExtAttrName());
 
             String value = Optional.ofNullable(idToken.getClaim(item.getExtAttrName())).
-                    or(() -> Optional.ofNullable(accessToken.getClaim(item.getExtAttrName()))).
                     map(Object::toString).
                     orElse(null);
             if (value != null) {

diff --git a/fit/wa-reference/src/test/java/org/apache/syncope/fit/sra/OIDCSRAITCase.java b/fit/wa-reference/src/test/java/org/apache/syncope/fit/sra/OIDCSRAITCase.java
@@ -61,6 +61,7 @@
 import org.apache.http.util.EntityUtils;
 import org.apache.syncope.common.lib.to.OIDCRPClientAppTO;
 import org.apache.syncope.common.lib.types.ClientAppType;
+import org.apache.syncope.common.lib.types.OIDCGrantType;
 import org.apache.syncope.common.lib.types.OIDCScope;
 import org.apache.syncope.common.lib.types.OIDCSubjectType;
 import org.apache.syncope.common.rest.api.RESTHeaders;
@@ -125,6 +126,7 @@ protected static void oidcClientAppSetup(
         clientApp.getScopes().add(OIDCScope.OPENID);
         clientApp.getScopes().add(OIDCScope.PROFILE);
         clientApp.getScopes().add(OIDCScope.EMAIL);
+        clientApp.getSupportedGrantTypes().add(OIDCGrantType.password);
 
         CLIENT_APP_SERVICE.update(ClientAppType.OIDCRP, clientApp);
         WA_CONFIG_SERVICE.pushToWA(WAConfigService.PushSubject.clientApps, List.of());
@@ -239,9 +241,7 @@ private void checkJWT(final String token, final boolean idToken) throws ParseExc
         assertEquals("Verdi", idTokenClaimsSet.getStringClaim("family_name"));
         assertEquals("Giuseppe", idTokenClaimsSet.getStringClaim("given_name"));
         assertEquals("Giuseppe Verdi", idTokenClaimsSet.getStringClaim("name"));
-        if (!idToken) {
-            assertEquals(Set.of("root", "child", "citizen"), Set.of(idTokenClaimsSet.getStringArrayClaim("groups")));
-        }
+        assertEquals(Set.of("root", "child", "citizen"), Set.of(idTokenClaimsSet.getStringArrayClaim("groups")));
     }
 
     protected boolean checkIdToken() {

diff --git a/pom.xml b/pom.xml
@@ -489,7 +489,7 @@ under the License.
     <tomcat.version>10.1.11</tomcat.version>
     <wildfly.version>29.0.0.Final</wildfly.version>
     <payara.version>6.2023.7</payara.version>
-    <jakarta.faces.version>4.0.2</jakarta.faces.version>
+    <jakarta.faces.version>4.0.3</jakarta.faces.version>
 
     <docker.postgresql.version>15</docker.postgresql.version>
     <docker.mysql.version>8.0</docker.mysql.version>

diff --git a/wa/starter/src/main/java/org/apache/syncope/wa/starter/mapping/OIDCRPClientAppTOMapper.java b/wa/starter/src/main/java/org/apache/syncope/wa/starter/mapping/OIDCRPClientAppTOMapper.java
@@ -29,9 +29,7 @@
 import org.apache.syncope.common.lib.types.OIDCScope;
 import org.apache.syncope.common.lib.wa.WAClientApp;
 import org.apereo.cas.oidc.claims.OidcAddressScopeAttributeReleasePolicy;
-import org.apereo.cas.oidc.claims.OidcCustomScopeAttributeReleasePolicy;
 import org.apereo.cas.oidc.claims.OidcEmailScopeAttributeReleasePolicy;
-import org.apereo.cas.oidc.claims.OidcOpenIdScopeAttributeReleasePolicy;
 import org.apereo.cas.oidc.claims.OidcPhoneScopeAttributeReleasePolicy;
 import org.apereo.cas.oidc.claims.OidcProfileScopeAttributeReleasePolicy;
 import org.apereo.cas.services.BaseMappedAttributeReleasePolicy;
@@ -93,36 +91,10 @@ public RegisteredService map(
         }
         service.setLogoutUrl(rp.getLogoutUri());
 
-        ChainingAttributeReleasePolicy chain;
-        if (attributeReleasePolicy instanceof ChainingAttributeReleasePolicy chainingAttributeReleasePolicy) {
-            chain = chainingAttributeReleasePolicy;
-        } else {
-            chain = new ChainingAttributeReleasePolicy();
-            if (attributeReleasePolicy != null) {
-                chain.addPolicies(attributeReleasePolicy);
-            }
-        }
-
         service.setScopes(rp.getScopes().stream().
                 map(s -> s.name().toLowerCase()).
                 collect(Collectors.toCollection(HashSet::new)));
 
-        if (rp.getScopes().contains(OIDCScope.OPENID)) {
-            chain.addPolicies(new OidcOpenIdScopeAttributeReleasePolicy());
-        }
-        if (rp.getScopes().contains(OIDCScope.PROFILE)) {
-            chain.addPolicies(new OidcProfileScopeAttributeReleasePolicy());
-        }
-        if (rp.getScopes().contains(OIDCScope.ADDRESS)) {
-            chain.addPolicies(new OidcAddressScopeAttributeReleasePolicy());
-        }
-        if (rp.getScopes().contains(OIDCScope.EMAIL)) {
-            chain.addPolicies(new OidcEmailScopeAttributeReleasePolicy());
-        }
-        if (rp.getScopes().contains(OIDCScope.PHONE)) {
-            chain.addPolicies(new OidcPhoneScopeAttributeReleasePolicy());
-        }
-
         Set<String> customClaims = new HashSet<>();
         if (attributeReleasePolicy instanceof BaseMappedAttributeReleasePolicy baseMapped) {
             customClaims.addAll(baseMapped.
@@ -138,7 +110,6 @@ public RegisteredService map(
                     map(p -> p.getAllowedAttributes().stream().collect(Collectors.toSet())).
                     ifPresent(customClaims::addAll);
         }
-
         if (rp.getScopes().contains(OIDCScope.PROFILE)) {
             customClaims.removeAll(OidcProfileScopeAttributeReleasePolicy.ALLOWED_CLAIMS);
         }
@@ -151,13 +122,13 @@ public RegisteredService map(
         if (rp.getScopes().contains(OIDCScope.PHONE)) {
             customClaims.removeAll(OidcPhoneScopeAttributeReleasePolicy.ALLOWED_CLAIMS);
         }
+
         if (!customClaims.isEmpty()) {
-            chain.addPolicies(new OidcCustomScopeAttributeReleasePolicy(
-                    CUSTOM_SCOPE, customClaims.stream().collect(Collectors.toList())));
             service.getScopes().add(CUSTOM_SCOPE);
         }
 
-        setPolicies(service, authPolicy, mfaPolicy, accessStrategy, chain,
+        // never set attribute relase policy for OIDC services to avoid becoming scope-free for CAS
+        setPolicies(service, authPolicy, mfaPolicy, accessStrategy, null,
                 tgtExpirationPolicy, stExpirationPolicy, tgtProxyExpirationPolicy, stProxyExpirationPolicy);
 
         return service;