Merge pull request #26 from Aam-Digital/email_otp_improvements

Added configurability, time-to-live and improved code generation
mesutpiskin · Nov 17, 2023 · b9c6060 · b9c6060
2 parents 4ab84fa + 569f99f
commit b9c6060
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 47 deletions.
diff --git a/.gitignore b/.gitignore
@@ -23,4 +23,5 @@
 hs_err_pid*
 .DS_Store
 target/*
-.idea
+.idea
+.vscode
diff --git a/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailAuthenticatorForm.java b/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailAuthenticatorForm.java
@@ -1,35 +1,33 @@
 package com.mesutpiskin.keycloak.auth.email;
 
 import lombok.extern.jbosslog.JBossLog;
+
 import org.keycloak.authentication.AuthenticationFlowContext;
 import org.keycloak.authentication.AuthenticationFlowError;
 import org.keycloak.authentication.AuthenticationFlowException;
-import org.keycloak.authentication.Authenticator;
 import org.keycloak.email.EmailException;
 import org.keycloak.email.EmailTemplateProvider;
 import org.keycloak.events.Errors;
 import org.keycloak.forms.login.LoginFormsProvider;
+import org.keycloak.models.AuthenticationExecutionModel;
+import org.keycloak.models.AuthenticatorConfigModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserModel;
 import org.keycloak.models.utils.FormMessage;
 import org.keycloak.services.messages.Messages;
+import org.keycloak.sessions.AuthenticationSessionModel;
 import org.keycloak.authentication.authenticators.browser.AbstractUsernameFormAuthenticator;
+import org.keycloak.common.util.SecretGenerator;
 
 import jakarta.ws.rs.core.MultivaluedMap;
 import jakarta.ws.rs.core.Response;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ThreadLocalRandom;
 
 @JBossLog
-public class EmailAuthenticatorForm extends AbstractUsernameFormAuthenticator implements Authenticator {
-
-    static final String ID = "demo-email-code-form";
-
-    public static final String EMAIL_CODE = "emailCode";
-
+public class EmailAuthenticatorForm extends AbstractUsernameFormAuthenticator {
     private final KeycloakSession session;
 
     public EmailAuthenticatorForm(KeycloakSession session) {
@@ -59,15 +57,26 @@ protected Response challenge(AuthenticationFlowContext context, String error, St
     }
 
     private void generateAndSendEmailCode(AuthenticationFlowContext context) {
+        AuthenticatorConfigModel config = context.getAuthenticatorConfig();
+        AuthenticationSessionModel session = context.getAuthenticationSession();
 
-        if (context.getAuthenticationSession().getAuthNote(EMAIL_CODE) != null) {
+        if (session.getAuthNote(EmailConstants.CODE) != null) {
             // skip sending email code
             return;
         }
 
-        int emailCode = ThreadLocalRandom.current().nextInt(99999999);
-        sendEmailWithCode(context.getRealm(), context.getUser(), String.valueOf(emailCode));
-        context.getAuthenticationSession().setAuthNote(EMAIL_CODE, Integer.toString(emailCode));
+        int length = EmailConstants.DEFAULT_LENGTH;
+        int ttl = EmailConstants.DEFAULT_TTL;
+        if (config != null) {
+            // get config values
+            length = Integer.parseInt(config.getConfig().get(EmailConstants.CODE_LENGTH));
+            ttl = Integer.parseInt(config.getConfig().get(EmailConstants.CODE_TTL));
+        }
+
+        String code = SecretGenerator.getInstance().randomString(length, SecretGenerator.DIGITS);
+        sendEmailWithCode(context.getRealm(), context.getUser(), code, ttl);
+        session.setAuthNote(EmailConstants.CODE, code);
+        session.setAuthNote(EmailConstants.CODE_TTL, Long.toString(System.currentTimeMillis() + (ttl * 1000L)));
     }
 
     @Override
@@ -91,23 +100,33 @@ public void action(AuthenticationFlowContext context) {
             return;
         }
 
-        boolean valid;
-        try {
-            int givenEmailCode = Integer.parseInt(formData.getFirst(EMAIL_CODE));
-            valid = validateCode(context, givenEmailCode);
-        } catch (NumberFormatException e) {
-            valid = false;
-        }
-
-        if (!valid) {
-            context.getEvent().user(userModel).error(Errors.INVALID_USER_CREDENTIALS);
-            Response challengeResponse = challenge(context, Messages.INVALID_ACCESS_CODE);
-            context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
-            return;
+        AuthenticationSessionModel session = context.getAuthenticationSession();
+        String code = session.getAuthNote(EmailConstants.CODE);
+        String ttl = session.getAuthNote(EmailConstants.CODE_TTL);
+        String enteredCode = formData.getFirst(EmailConstants.CODE);
+
+        if (enteredCode.equals(code)) {
+            if (Long.parseLong(ttl) < System.currentTimeMillis()) {
+                // expired
+                context.getEvent().user(userModel).error(Errors.EXPIRED_CODE);
+                Response challengeResponse = challenge(context, Messages.EXPIRED_ACTION_TOKEN_SESSION_EXISTS);
+                context.failureChallenge(AuthenticationFlowError.EXPIRED_CODE, challengeResponse);
+            } else {
+                // valid
+                resetEmailCode(context);
+                context.success();
+            }
+        } else {
+            // invalid
+            AuthenticationExecutionModel execution = context.getExecution();
+            if (execution.isRequired()) {
+                context.getEvent().user(userModel).error(Errors.INVALID_USER_CREDENTIALS);
+                Response challengeResponse = challenge(context, Messages.INVALID_ACCESS_CODE);
+                context.failureChallenge(AuthenticationFlowError.INVALID_CREDENTIALS, challengeResponse);
+            } else if (execution.isConditional() || execution.isAlternative()) {
+                context.attempted();
+            }
         }
-
-        resetEmailCode(context);
-        context.success();
     }
 
     @Override
@@ -116,12 +135,7 @@ protected String disabledByBruteForceError() {
     }
 
     private void resetEmailCode(AuthenticationFlowContext context) {
-        context.getAuthenticationSession().removeAuthNote(EMAIL_CODE);
-    }
-
-    private boolean validateCode(AuthenticationFlowContext context, int givenCode) {
-        int emailCode = Integer.parseInt(context.getAuthenticationSession().getAuthNote(EMAIL_CODE));
-        return givenCode == emailCode;
+        context.getAuthenticationSession().removeAuthNote(EmailConstants.CODE);
     }
 
     @Override
@@ -144,7 +158,7 @@ public void close() {
         // NOOP
     }
 
-    private void sendEmailWithCode(RealmModel realm, UserModel user, String code) {
+    private void sendEmailWithCode(RealmModel realm, UserModel user, String code, int ttl) {
         if (user.getEmail() == null) {
             log.warnf("Could not send access code email due to missing email. realm=%s user=%s", realm.getId(), user.getUsername());
             throw new AuthenticationFlowException(AuthenticationFlowError.INVALID_USER);
@@ -153,6 +167,7 @@ private void sendEmailWithCode(RealmModel realm, UserModel user, String code) {
         Map<String, Object> mailBodyAttributes = new HashMap<>();
         mailBodyAttributes.put("username", user.getUsername());
         mailBodyAttributes.put("code", code);
+        mailBodyAttributes.put("ttl", ttl);
 
         String realmName = realm.getDisplayName() != null ? realm.getDisplayName() : realm.getName();
         List<Object> subjectParams = List.of(realmName);

diff --git a/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailAuthenticatorFormFactory.java b/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailAuthenticatorFormFactory.java
@@ -13,6 +13,10 @@
 
 @AutoService(AuthenticatorFactory.class)
 public class EmailAuthenticatorFormFactory implements AuthenticatorFactory {
+    @Override
+    public String getId() {
+        return "email-authenticator";
+    }
 
     @Override
     public String getDisplayType() {
@@ -21,12 +25,12 @@ public String getDisplayType() {
 
     @Override
     public String getReferenceCategory() {
-        return null;
+        return "otp";
     }
 
     @Override
     public boolean isConfigurable() {
-        return false;
+        return true;
     }
 
     public static final AuthenticationExecutionModel.Requirement[] REQUIREMENT_CHOICES = {
@@ -51,7 +55,13 @@ public String getHelpText() {
 
     @Override
     public List<ProviderConfigProperty> getConfigProperties() {
-        return null;
+        return List.of(
+                new ProviderConfigProperty(EmailConstants.CODE_LENGTH, "Code length",
+                        "The number of digits of the generated code.",
+                        ProviderConfigProperty.STRING_TYPE, String.valueOf(EmailConstants.DEFAULT_LENGTH)),
+                new ProviderConfigProperty(EmailConstants.CODE_TTL, "Time-to-live",
+                        "The time to live in seconds for the code to be valid.", ProviderConfigProperty.STRING_TYPE,
+                        String.valueOf(EmailConstants.DEFAULT_TTL)));
     }
 
     @Override
@@ -73,9 +83,4 @@ public void init(Config.Scope config) {
     public void postInit(KeycloakSessionFactory factory) {
         // NOOP
     }
-
-    @Override
-    public String getId() {
-        return EmailAuthenticatorForm.ID;
-    }
 }
diff --git a/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailConstants.java b/src/main/java/com/mesutpiskin/keycloak/auth/email/EmailConstants.java
@@ -0,0 +1,12 @@
+package com.mesutpiskin.keycloak.auth.email;
+
+import lombok.experimental.UtilityClass;
+
+@UtilityClass
+public class EmailConstants {
+	public String CODE = "emailCode";
+	public String CODE_LENGTH = "length";
+	public String CODE_TTL = "ttl";
+	public int DEFAULT_LENGTH = 6;
+	public int DEFAULT_TTL = 300;
+}
diff --git a/src/main/resources/theme/email-code-theme/email/html/code-email.ftl b/src/main/resources/theme/email-code-theme/email/html/code-email.ftl
@@ -1,5 +1,5 @@
 <html>
 <body>
-${kcSanitize(msg("emailCodeBody", code))?no_esc}
+${kcSanitize(msg("emailCodeBody", code, ttl))?no_esc}
 </body>
 </html>
diff --git a/src/main/resources/theme/email-code-theme/email/messages/messages_en.properties b/src/main/resources/theme/email-code-theme/email/messages/messages_en.properties
@@ -1,3 +1,3 @@
 emailCodeSubject={0} access code
-emailCodeBody=Access code: {0}
+emailCodeBody=Access code: {0}.\n\nThis code will expire within {1} seconds.
 resendCode=Resend Code
diff --git a/src/main/resources/theme/email-code-theme/email/text/code-email.ftl b/src/main/resources/theme/email-code-theme/email/text/code-email.ftl
@@ -1,2 +1,2 @@
 <#ftl output_format="plainText">
-${msg("emailCodeBody", code)}
+${msg("emailCodeBody", code, ttl)}