From a06ae5aa92fe95e59d5d77ae8b0542fa61217e51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominik=20Franti=C5=A1ek=20Bu=C4=8D=C3=ADk?= <bucik@ics.muni.cz> Date: Mon, 19 Sep 2022 15:53:57 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20Sync=20Token=20endpoint?= =?UTF-8?q?=20authN=20method?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: 🧨 Needs configuration for new attributes --- .../cz/muni/ics/oidc/ToOidcSynchronizer.java | 174 ++++++++++++------ .../cz/muni/ics/oidc/props/AttrsMapping.java | 24 ++- src/main/resources/application.yml | 1 + 3 files changed, 137 insertions(+), 62 deletions(-) diff --git a/src/main/java/cz/muni/ics/oidc/ToOidcSynchronizer.java b/src/main/java/cz/muni/ics/oidc/ToOidcSynchronizer.java index 78f0154..a3e1b67 100644 --- a/src/main/java/cz/muni/ics/oidc/ToOidcSynchronizer.java +++ b/src/main/java/cz/muni/ics/oidc/ToOidcSynchronizer.java @@ -93,7 +93,6 @@ public class ToOidcSynchronizer { public static final String PKCE_TYPE_NONE = "none"; public static final String PKCE_TYPE_PLAIN = "plain code challenge"; public static final String PKCE_TYPE_SHA256 = "SHA256 code challenge"; - private final PerunAdapter perunAdapter; private final String proxyIdentifier; private final String proxyIdentifierValue; @@ -152,7 +151,8 @@ public class ToOidcSynchronizer { log.info("Removing old clients"); deleteClients(foundClientIds, res); } else { - log.warn("Script has disabled removing of old clients. This might be due to Peruns unreachability! Check previous logs for more info."); + log.warn("Script has disabled removing of old clients. " + + "This might be due to Peruns unreachability! Check previous logs for more info."); } return res; } @@ -242,7 +242,9 @@ public class ToOidcSynchronizer { } } - private void updateClient(MitreidClient original, Map<String, PerunAttributeValue> attrs, SyncResult res) + private void updateClient(MitreidClient original, + Map<String, PerunAttributeValue> attrs, + SyncResult res) throws BadPaddingException, InvalidKeyException, IllegalBlockSizeException { if (actionsProperties.getToOidc().isUpdate()) { @@ -329,69 +331,82 @@ public class ToOidcSynchronizer { } } } else { - log.warn("Deleting of clients is disabled. Following clientIDs would be deleted: {}", clientsToDelete); + log.warn("Deleting of clients is disabled. Following clientIDs would be deleted: {}", + clientsToDelete); } } private void setClientFields(MitreidClient c, Map<String, PerunAttributeValue> attrs) throws BadPaddingException, InvalidKeyException, IllegalBlockSizeException { - c.setClientId(attrs.get(perunAttrNames.getClientId()).valueAsString()); - c.setClientSecret(Utils.decrypt( - attrs.get(perunAttrNames.getClientSecret()).valueAsString(), cipher, secretKeySpec)); - c.setClientName(attrs.get(perunAttrNames.getName()).valueAsMap().get("en")); - c.setClientDescription(attrs.get(perunAttrNames.getDescription()).valueAsMap().get("en")); - c.setRedirectUris(new HashSet<>(attrs.get(perunAttrNames.getRedirectUris()).valueAsList())); - c.setAllowIntrospection(attrs.get(perunAttrNames.getIntrospection()).valueAsBoolean()); - c.setPostLogoutRedirectUris(new HashSet<>(attrs.get(perunAttrNames.getPostLogoutRedirectUris()).valueAsList())); - c.setScope(new HashSet<>(attrs.get(perunAttrNames.getScopes()).valueAsList())); - setPolicyUri(c, attrs); - setContacts(c, attrs); - setClientUri(c, attrs); + setClientId(c, attrs); + setClientSecret(c, attrs); + setClientName(c, attrs); + setClientDescription(c, attrs); + setRedirectUris(c, attrs); + setIntrospection(c, attrs); + setPostLogoutRedirectUris(c, attrs); + setScopes(c, attrs); setGrantAndResponseTypes(c, attrs); + setPKCEOptions(c, attrs); + setTokenEndpointAuthentication(c, attrs); setRefreshTokens(c, attrs); setTokenTimeouts(c, attrs); + setPolicyUri(c, attrs); + setContacts(c, attrs); + setClientUri(c, attrs); } - private void setRefreshTokens(MitreidClient c, Map<String, PerunAttributeValue> attrs) { - Set<String> grantTypes = c.getGrantTypes(); - if (grantTypes == null) { - grantTypes = new HashSet<>(); - } - if (grantAllowsRefreshTokens(grantTypes)) { - boolean requestedViaAttr = attrs.containsKey(perunAttrNames.getIssueRefreshTokens()) - && attrs.get(perunAttrNames.getIssueRefreshTokens()).valueAsBoolean(); - boolean requestedViaScopes = c.getScope().contains(OFFLINE_ACCESS); - log.debug("Refresh tokens requested via: attr({}), scopes({})", requestedViaAttr, requestedViaScopes); - if (requestedViaAttr || requestedViaScopes) { - setUpRefreshTokens(c, attrs); - } - } + private void setClientId(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + String clientId = attrs.get(perunAttrNames.getClientId()).valueAsString(); + c.setClientId(clientId); } - private void setUpRefreshTokens(MitreidClient c, Map<String, PerunAttributeValue> attrs) { - c.getScope().add(OFFLINE_ACCESS); - c.getGrantTypes().add(GRANT_REFRESH_TOKEN); - c.setClearAccessTokensOnRefresh(true); - c.setReuseRefreshToken(false); - PerunAttributeValue reuseTokens = attrs.getOrDefault(perunAttrNames.getReuseRefreshTokens(), null); - if (reuseTokens != null) { - c.setReuseRefreshToken(reuseTokens.valueAsBoolean()); - } + private void setClientSecret(MitreidClient c, Map<String, PerunAttributeValue> attrs) + throws IllegalBlockSizeException, BadPaddingException, InvalidKeyException + { + String encryptedClientSecret = attrs.get(perunAttrNames.getClientSecret()).valueAsString(); + String clientSecret = Utils.decrypt(encryptedClientSecret, cipher, secretKeySpec); + c.setClientSecret(clientSecret); } - private boolean grantAllowsRefreshTokens(Set<String> grantTypes) { - boolean res = !grantTypes.isEmpty() - && (grantTypes.contains(GRANT_DEVICE) - || grantTypes.contains(GRANT_AUTHORIZATION_CODE) - || grantTypes.contains(GRANT_HYBRID)); - log.debug("Grants '{}' {} issuing refresh tokens", grantTypes, res ? "allow" : "disallow"); - return res; + private void setClientName(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + String clientName = attrs.get(perunAttrNames.getName()).valueAsMap().get("en"); + c.setClientName(clientName); + } + + private void setClientDescription(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + String clientDescription = attrs.get(perunAttrNames.getDescription()).valueAsMap().get("en"); + c.setClientDescription(clientDescription); + } + + private void setRedirectUris(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + Set<String> redirectUris = new HashSet<>( + attrs.get(perunAttrNames.getRedirectUris()).valueAsList()); + c.setRedirectUris(redirectUris); + } + + private void setIntrospection(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + boolean introspectionAllowed = attrs.get(perunAttrNames.getIntrospection()).valueAsBoolean(); + c.setAllowIntrospection(introspectionAllowed); + } + + private void setPostLogoutRedirectUris(MitreidClient c, + Map<String, PerunAttributeValue> attrs) + { + Set<String> postLogoutRedirectUris = new HashSet<>( + attrs.get(perunAttrNames.getPostLogoutRedirectUris()).valueAsList()); + c.setPostLogoutRedirectUris(postLogoutRedirectUris); + } + + private void setScopes(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + Set<String> scopes = new HashSet<>(attrs.get(perunAttrNames.getScopes()).valueAsList()); + c.setScope(scopes); } private void setGrantAndResponseTypes(MitreidClient c, Map<String, PerunAttributeValue> attrs) { List<String> grantTypesAttrValue = attrs.get(perunAttrNames.getGrantTypes()).valueAsList().stream() - .map(String::toLowerCase).collect(Collectors.toList()); + .map(String::toLowerCase).collect(Collectors.toList()); Set<String> grantTypes = new HashSet<>(); Set<String> responseTypes = new HashSet<>(); @@ -413,7 +428,7 @@ public class ToOidcSynchronizer { grantTypes.add(GRANT_AUTHORIZATION_CODE); responseTypes.addAll(Arrays.asList(RESPONSE_TYPE_HYBRID)); log.debug("Added grants '{} {}' with response types '{}'", GRANT_HYBRID, GRANT_AUTHORIZATION_CODE, - RESPONSE_TYPE_HYBRID); + RESPONSE_TYPE_HYBRID); } if (grantTypesAttrValue.contains(DEVICE)) { @@ -421,12 +436,6 @@ public class ToOidcSynchronizer { log.debug("Added grant '{}'", GRANT_DEVICE); } - if (grantTypes.contains(GRANT_AUTHORIZATION_CODE) - || grantTypes.contains(GRANT_DEVICE)) - { - setPKCEOptions(c, attrs); - } - c.setGrantTypes(grantTypes); c.setResponseTypes(responseTypes); } @@ -439,19 +448,66 @@ public class ToOidcSynchronizer { log.debug("Code challenge requested is not equal to '{}'", PKCE_TYPE_NONE); if (PKCE_TYPE_PLAIN.equalsIgnoreCase(codeChallengeType)) { log.debug("Preparing for PKCE with challenge '{}'", PKCE_TYPE_PLAIN); - preparePkce(c); c.setCodeChallengeMethod(PKCEAlgorithm.plain); } else if (PKCE_TYPE_SHA256.equalsIgnoreCase(codeChallengeType)) { log.debug("Preparing for PKCE with challenge '{}'", PKCE_TYPE_SHA256); - preparePkce(c); c.setCodeChallengeMethod(PKCEAlgorithm.S256); } } } - private void preparePkce(MitreidClient c) { - c.setClientSecret(null); - c.setTokenEndpointAuthMethod(MitreidClient.AuthMethod.NONE); + private void setTokenEndpointAuthentication(MitreidClient c, + Map<String, PerunAttributeValue> attrs) + { + String authMethodAttrValue = attrs.get(perunAttrNames.getTokenEndpointAuthenticationMethod()) + .valueAsString(); + MitreidClient.AuthMethod authMethod = MitreidClient.AuthMethod.getByValue(authMethodAttrValue); + if (authMethod == null) { + log.debug("Failed to parse token endpoint authentication method." + + " Using client_secret_basic as default value."); + authMethod = MitreidClient.AuthMethod.SECRET_BASIC; + } + c.setTokenEndpointAuthMethod(authMethod); + if (MitreidClient.AuthMethod.NONE.equals(authMethod)) { + log.debug("NONE used as token endpoint authentication method. Removing client_secret"); + c.setClientSecret(null); + } + } + + private void setRefreshTokens(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + Set<String> grantTypes = c.getGrantTypes(); + if (grantTypes == null) { + grantTypes = new HashSet<>(); + } + if (grantAllowsRefreshTokens(grantTypes)) { + boolean requestedViaAttr = attrs.containsKey(perunAttrNames.getIssueRefreshTokens()) + && attrs.get(perunAttrNames.getIssueRefreshTokens()).valueAsBoolean(); + boolean requestedViaScopes = c.getScope().contains(OFFLINE_ACCESS); + log.debug("Refresh tokens requested via: attr({}), scopes({})", requestedViaAttr, requestedViaScopes); + if (requestedViaAttr || requestedViaScopes) { + setUpRefreshTokens(c, attrs); + } + } + } + + private void setUpRefreshTokens(MitreidClient c, Map<String, PerunAttributeValue> attrs) { + c.getScope().add(OFFLINE_ACCESS); + c.getGrantTypes().add(GRANT_REFRESH_TOKEN); + c.setClearAccessTokensOnRefresh(true); + c.setReuseRefreshToken(false); + PerunAttributeValue reuseTokens = attrs.getOrDefault(perunAttrNames.getReuseRefreshTokens(), null); + if (reuseTokens != null) { + c.setReuseRefreshToken(reuseTokens.valueAsBoolean()); + } + } + + private boolean grantAllowsRefreshTokens(Set<String> grantTypes) { + boolean res = !grantTypes.isEmpty() + && (grantTypes.contains(GRANT_DEVICE) + || grantTypes.contains(GRANT_AUTHORIZATION_CODE) + || grantTypes.contains(GRANT_HYBRID)); + log.debug("Grants '{}' {} issuing refresh tokens", grantTypes, res ? "allow" : "disallow"); + return res; } private void setTokenTimeouts(MitreidClient c, Map<String, PerunAttributeValue> attrs) { diff --git a/src/main/java/cz/muni/ics/oidc/props/AttrsMapping.java b/src/main/java/cz/muni/ics/oidc/props/AttrsMapping.java index 3085086..bdb0305 100644 --- a/src/main/java/cz/muni/ics/oidc/props/AttrsMapping.java +++ b/src/main/java/cz/muni/ics/oidc/props/AttrsMapping.java @@ -44,6 +44,8 @@ public class AttrsMapping { @NotBlank private String introspection; @NotBlank private String postLogoutRedirectUris; @NotBlank private String issueRefreshTokens; + + @NotBlank private String tokenEndpointAuthenticationMethod; private List<String> homePageUris; private String tokenTimeouts; private String reuseRefreshTokens; @@ -57,9 +59,25 @@ public class AttrsMapping { public List<String> getNames() { List<String> attrNames = new ArrayList<>( - Arrays.asList(clientId, clientSecret, name, description, redirectUris, privacyPolicy, scopes, - grantTypes, codeChallengeType, introspection, postLogoutRedirectUris, issueRefreshTokens, - masterProxyIdentifier, proxyIdentifier, isTestSp, isOidc, managersGroupId) + Arrays.asList(clientId, + clientSecret, + name, + description, + redirectUris, + privacyPolicy, + scopes, + grantTypes, + codeChallengeType, + introspection, + postLogoutRedirectUris, + issueRefreshTokens, + tokenEndpointAuthenticationMethod, + masterProxyIdentifier, + proxyIdentifier, + isTestSp, + isOidc, + managersGroupId + ) ); attrNames.addAll(contacts); diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 74aafe9..1a2924c 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -27,6 +27,7 @@ attributes: contacts: - "urn:perun:facility:attribute-def:def:administratorContact" scopes: "urn:perun:facility:attribute-def:def:requiredScopes" + token_endpoint_authentication_method: "urn:perun:facility:attribute-def:def:OIDCTokenEndpointAuthenticationMethod" grant_types: "urn:perun:facility:attribute-def:def:OIDCGrantTypes" code_challenge_type: "urn:perun:facility:attribute-def:def:OIDCCodeChallengeType" introspection: "urn:perun:facility:attribute-def:def:OIDCAllowIntrospection" -- GitLab