Revise KeyURI generation in OTPPolicy

- More idiomatic URLEncoder usage
- Replace `:` character with space in issuer if present in OTP label
- Ease testing for OTP KeyURI generation

Fixes #32833

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>

server-spi/src/main/java/org/keycloak/models/OTPPolicy.java

@@ -21,13 +21,15 @@ import org.jboss.logging.Logger;
 import org.keycloak.models.credential.OTPCredentialModel;
 import org.keycloak.models.utils.Base32;
 import org.keycloak.models.utils.HmacOTP;
+import org.keycloak.utils.StringUtil;
 
 import java.io.Serializable;
-import java.io.UnsupportedEncodingException;
 import java.net.URLEncoder;
 import java.util.HashMap;
 import java.util.Map;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+
 /**
  * @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
  * @version $Revision: 1 $
@@ -145,35 +147,49 @@ public class OTPPolicy implements Serializable {
      */
     public String getKeyURI(RealmModel realm, UserModel user, String secret) {
 
-        try {
+        String issuerName = !StringUtil.isNullOrEmpty(realm.getDisplayName()) ? realm.getDisplayName() : realm.getName();
+        String accountName = user.getUsername();
 
-            String displayName = realm.getDisplayName() != null && !realm.getDisplayName().isEmpty() ? realm.getDisplayName() : realm.getName();
+        return getKeyURI(issuerName, accountName, secret);
+    }
 
-            String accountName = URLEncoder.encode(user.getUsername(), "UTF-8");
+    /**
-            String issuerName = URLEncoder.encode(displayName, "UTF-8") .replaceAll("\\+", "%20");
+     * Constructs the <code>otpauth://</code> URI based on the <a href="https://github.com/google/google-authenticator/wiki/Key-Uri-Format">Key-Uri-Format</a>.
+     *
+     * @param rawIssuerName
+     * @param rawAccountName
+     * @param secret
+     * @return the <code>otpauth://</code> URI
+     */
+    public String getKeyURI(String rawIssuerName, String rawAccountName, String secret) {
 
-            /*
+        String accountName = URLEncoder.encode(rawAccountName, UTF_8);
-             * The issuerName component in the label is usually shown in a authenticator app, such as
+        /*
-             * Google Authenticator or FreeOTP, as a hint for the user to which system an username
+         * Replacing ':' in issuerName with a space because the ':' is not allowed in the issuer part of the label.
-             * belongs to.
+         * See: https://github.com/google/google-authenticator/wiki/Key-Uri-Format#label
-             */
+         */
-            String label = issuerName + ":" + accountName;
+        String issuerName = rawIssuerName.replaceAll("[:]", " ");
+        issuerName = URLEncoder.encode(issuerName, UTF_8).replaceAll("\\+", "%20");
 
-            String parameters = "secret=" + Base32.encode(secret.getBytes()) //
+        /*
-                                + "&digits=" + digits //
+         * The issuerName component in the label is usually shown in a authenticator app, such as
-                                + "&algorithm=" + algToKeyUriAlg.get(algorithm) //
+         * Google Authenticator or FreeOTP, as a hint for the user to which system an username
-                                + "&issuer=" + issuerName;
+         * belongs to.
+         */
+        String label = issuerName + ":" + accountName;
 
-            if (type.equals(OTPCredentialModel.HOTP)) {
+        String parameters = "secret=" + Base32.encode(secret.getBytes()) //
-                parameters += "&counter=" + initialCounter;
+                            + "&digits=" + digits //
-            } else if (type.equals(OTPCredentialModel.TOTP)) {
+                            + "&algorithm=" + algToKeyUriAlg.get(algorithm) //
-                parameters += "&period=" + period;
+                            + "&issuer=" + issuerName;
-            }
 
-            return "otpauth://" + type + "/" + label+ "?" + parameters;
+        if (type.equals(OTPCredentialModel.HOTP)) {
-        } catch (UnsupportedEncodingException e) {
+            parameters += "&counter=" + initialCounter;
-            throw new RuntimeException(e);
+        } else if (type.equals(OTPCredentialModel.TOTP)) {
+            parameters += "&period=" + period;
         }
+
+        return "otpauth://" + type + "/" + label + "?" + parameters;
     }
 
 }

server-spi/src/test/java/org/keycloak/models/OtpPolicyTest.java

@@ -0,0 +1,40 @@
+package org.keycloak.models;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.keycloak.models.credential.OTPCredentialModel;
+import org.keycloak.models.utils.HmacOTP;
+
+import java.net.URI;
+
+public class OtpPolicyTest {
+
+    OTPPolicy totpPolicy;
+
+    @Before
+    public void setup() {
+        totpPolicy = new OTPPolicy();
+        totpPolicy.setAlgorithm(HmacOTP.HMAC_SHA1);
+        totpPolicy.setDigits(6);
+        totpPolicy.setType(OTPCredentialModel.TOTP);
+    }
+
+    @Test
+    public void keyUriShouldBeValidForRealmDisplayNameWithColon() {
+
+        String keyURI = totpPolicy.getKeyURI("Test:Realm", "tester", "secret");
+        Assert.assertEquals("Test Realm", getLabelComponent(keyURI));
+    }
+
+    @Test
+    public void keyUriShouldBeValidForRealmDisplayNameWithSlash() {
+
+        String keyURI = totpPolicy.getKeyURI("Test/Realm", "tester", "secret");
+        Assert.assertEquals("Test/Realm", getLabelComponent(keyURI));
+    }
+
+    static String getLabelComponent(String keyURI) {
+        return URI.create(keyURI).getPath().substring(1).split(":")[0];
+    }
+}