From ac29728cc382edf78e81c3b5ad60e47e23802e65 Mon Sep 17 00:00:00 2001
From: Kartheek Palla <palla.kartheekreddy@gmail.com>
Date: Wed, 22 Jul 2026 12:52:04 +0530
Subject: [PATCH] fix(security): generate OTP with SecureRandom, uniform draw,
 zero-padded (A2)

KeycloakSmsAuthenticatorUtil.getSmsCode used java.util.Random + nextFloat():
  Random r = new Random();
  long code = (long) (r.nextFloat() * maxValue);
  return Long.toString(code);
Three defects made the OTP guessable/undersized: (1) java.util.Random is a
time-seeded LCG, predictable from a few outputs; (2) nextFloat()'s ~24-bit mantissa
cannot uniformly address a 10^8 space (gaps/clustering); (3) Long.toString drops
leading zeros, shrinking effective length and leaking high-zero digits.

Replace with a shared SecureRandom, a uniform nextInt(10^n) draw, and %0Nd
zero-padding so every code is exactly nrOfDigits long. Guard nrOfDigits to 1..9
(10^9 fits in int) and throw IllegalArgumentException (still a RuntimeException, so
existing catch sites are unaffected).

Pure utility change, no Keycloak SPI contract impact. Verified: sms-provider module
test-compiles under JDK 11.

Implements A2 from implementation-designs/sunbird-auth.md. A1 (OTP expiry/first-use
invalidation), A3 (attempt lockout) and A4 (msg91 cleartext HTTP) are separate,
higher-touch changes deferred to their own PRs.
---
 .../sms/KeycloakSmsAuthenticatorUtil.java     | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/keycloak/sms-provider/src/main/java/org/sunbird/keycloak/resetcredential/sms/KeycloakSmsAuthenticatorUtil.java b/keycloak/sms-provider/src/main/java/org/sunbird/keycloak/resetcredential/sms/KeycloakSmsAuthenticatorUtil.java
index 4b248f7d..afed0bc5 100644
--- a/keycloak/sms-provider/src/main/java/org/sunbird/keycloak/resetcredential/sms/KeycloakSmsAuthenticatorUtil.java
+++ b/keycloak/sms-provider/src/main/java/org/sunbird/keycloak/resetcredential/sms/KeycloakSmsAuthenticatorUtil.java
@@ -9,9 +9,9 @@
 import org.sunbird.utils.JsonUtil;
 
 import java.io.File;
+import java.security.SecureRandom;
 import java.util.List;
 import java.util.Map;
-import java.util.Random;
 import java.util.stream.Collectors;
 
 /**
@@ -21,6 +21,8 @@ public class KeycloakSmsAuthenticatorUtil {
 
     private static Logger logger = Logger.getLogger(KeycloakSmsAuthenticatorUtil.class);
 
+    private static final SecureRandom SECURE_RANDOM = new SecureRandom();
+
     public static String getAttributeValue(UserModel user, String attributeName) {
         String result = null;
         List<String> values = user.getAttributeStream(attributeName).collect(Collectors.toList());
@@ -119,14 +121,17 @@ private static Boolean send(String mobileNumber, String code) {
     }
 
     static String getSmsCode(long nrOfDigits) {
-        if (nrOfDigits < 1) {
-            throw new RuntimeException("Number of digits must be bigger than 0");
+        if (nrOfDigits < 1 || nrOfDigits > 9) {
+            // 10^9 fits in an int; higher widths would need a long/BigInteger draw.
+            throw new IllegalArgumentException("Number of digits must be between 1 and 9");
         }
 
-        double maxValue = Math.pow(10.0, nrOfDigits); // 10 ^ nrOfDigits;
-        Random r = new Random();
-        long code = (long) (r.nextFloat() * maxValue);
-        return Long.toString(code);
+        // SecureRandom + uniform integer draw (not java.util.Random / nextFloat, which is a
+        // predictable LCG that under-samples the code space). Zero-pad so leading zeros are
+        // preserved and every OTP is exactly nrOfDigits long.
+        int bound = (int) Math.pow(10, nrOfDigits);
+        int code = SECURE_RANDOM.nextInt(bound);
+        return String.format("%0" + nrOfDigits + "d", code);
     }
 
     public static boolean validateTelephoneNumber(String telephoneNumber) {