From 782be0a885f8f6e872fe1ca64aef466bdadf180d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francesco=20Chicchiricc=C3=B2?= Date: Tue, 25 Jul 2023 16:40:18 +0200 Subject: [PATCH] Cleaning up attributeReleasePolicy for OIDC client apps --- .../syncope/core/logic/OIDCC4UILogic.java | 4 --- .../apache/syncope/fit/sra/OIDCSRAITCase.java | 6 ++-- pom.xml | 2 +- .../mapping/OIDCRPClientAppTOMapper.java | 35 ++----------------- 4 files changed, 7 insertions(+), 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 index e4f974bb91..29578eb994 100644 --- 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 index 220423df81..8ed895954c 100644 --- 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 index 8ee3f3dbc0..3b19b8cd03 100644 --- a/pom.xml +++ b/pom.xml @@ -489,7 +489,7 @@ under the License. 10.1.11 29.0.0.Final 6.2023.7 - 4.0.2 + 4.0.3 15 8.0 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 index f22e118ccd..d1d66bf9dd 100644 --- 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 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;