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