Update

cthit · Jun 23, 2024 · 31fe633 · 31fe633
1 parent f42cdb8
commit 31fe633
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 64 deletions.
diff --git a/app/src/main/java/it/chalmers/gamma/adapter/primary/web/ForgotPasswordController.java b/app/src/main/java/it/chalmers/gamma/adapter/primary/web/ForgotPasswordController.java
@@ -84,11 +84,7 @@ public ModelAndView sendForgotPassword(
       return createGetForgotPassword(htmxRequest, form, bindingResult, false);
     }
 
-    try {
-      this.userResetPasswordFacade.startResetPasswordProcess(form.cidOrEmail);
-    } catch (UserResetPasswordFacade.PasswordResetProcessException e) {
-      // ignore
-    }
+    this.userResetPasswordFacade.startResetPasswordProcess(form.cidOrEmail);
 
     return createGetForgotPassword(htmxRequest, form, bindingResult, true);
   }
@@ -144,8 +140,6 @@ public ModelAndView finalizeForgotPassword(
     try {
       this.userResetPasswordFacade.finishResetPasswordProcess(
           form.token, form.password, form.confirmPassword);
-    } catch (UserResetPasswordFacade.PasswordResetProcessException e) {
-      throw new RuntimeException(e);
     } catch (IllegalArgumentException e) {
       bindingResult.addError(new ObjectError("global", e.getMessage()));
     } catch (PasswordResetRepository.TokenNotFoundRuntimeException e) {

diff --git a/app/src/main/java/it/chalmers/gamma/app/user/UserCreationFacade.java b/app/src/main/java/it/chalmers/gamma/app/user/UserCreationFacade.java
@@ -12,6 +12,7 @@
 import it.chalmers.gamma.app.user.activation.domain.UserActivationToken;
 import it.chalmers.gamma.app.user.allowlist.AllowListRepository;
 import it.chalmers.gamma.app.user.domain.*;
+import it.chalmers.gamma.security.TimerBlock;
 import jakarta.transaction.Transactional;
 import java.util.UUID;
 import org.slf4j.Logger;
@@ -51,19 +52,23 @@ public UserCreationFacade(
   public void tryToActivateUser(String cidRaw) {
     accessGuard.require(isNotSignedIn());
 
-    Cid cid = new Cid(cidRaw);
-    try {
-      if (throttlingService.canProceed(cidRaw + "-activation", 3)) {
-        UserActivationToken userActivationToken =
-            this.userActivationRepository.createActivationToken(cid);
-        sendEmail(cid, userActivationToken);
-      } else {
-        LOGGER.info("Throttling an activation and its email...");
-      }
-      LOGGER.info("Cid {} has been activated", cid);
-    } catch (UserActivationRepository.CidNotAllowedException e) {
-      LOGGER.info("Someone tried to activate the cid: {}", cid);
-    }
+    TimerBlock.minimum(3000)
+        .execute(
+            () -> {
+              Cid cid = new Cid(cidRaw);
+              try {
+                if (throttlingService.canProceed(cidRaw + "-activation", 3)) {
+                  UserActivationToken userActivationToken =
+                      this.userActivationRepository.createActivationToken(cid);
+                  sendEmail(cid, userActivationToken);
+                } else {
+                  LOGGER.info("Throttling an activation and its email...");
+                }
+                LOGGER.info("Cid {} has been activated", cid);
+              } catch (UserActivationRepository.CidNotAllowedException e) {
+                LOGGER.info("Someone tried to activate the cid: {}", cid);
+              }
+            });
   }
 
   public UUID createUser(NewUser newUser) throws EmailNotUniqueException, CidNotUniqueException {

diff --git a/app/src/main/java/it/chalmers/gamma/app/user/passwordreset/UserResetPasswordFacade.java b/app/src/main/java/it/chalmers/gamma/app/user/passwordreset/UserResetPasswordFacade.java
@@ -14,6 +14,7 @@
 import it.chalmers.gamma.app.user.passwordreset.domain.PasswordResetRepository;
 import it.chalmers.gamma.app.user.passwordreset.domain.PasswordResetToken;
 import it.chalmers.gamma.app.validation.SuccessfulValidation;
+import it.chalmers.gamma.security.TimerBlock;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
@@ -44,37 +45,42 @@ public UserResetPasswordFacade(
     this.baseUrl = baseUrl;
   }
 
-  public void startResetPasswordProcess(String cidOrEmailString)
-      throws PasswordResetProcessException {
+  public void startResetPasswordProcess(String cidOrEmailString) {
     this.accessGuard.require(isNotSignedIn());
 
-    try {
-      PasswordResetRepository.PasswordReset passwordReset;
-      if (new Email.EmailValidator().validate(cidOrEmailString) instanceof SuccessfulValidation) {
-        passwordReset = this.passwordResetRepository.createNewToken(new Email(cidOrEmailString));
-      } else if (new Cid.CidValidator().validate(cidOrEmailString)
-          instanceof SuccessfulValidation) {
-        passwordReset = this.passwordResetRepository.createNewToken(new Cid(cidOrEmailString));
-      } else {
-        throw new IllegalArgumentException("Neither an email nor a cid.");
-      }
-
-      if (throttlingService.canProceed(passwordReset.email().value() + "-password-reset", 3)) {
-        sendPasswordResetTokenMail(passwordReset.email(), passwordReset.token());
-      } else {
-        LOGGER.info("Throttling password reset process triggered.");
-      }
-    } catch (PasswordResetRepository.UserNotFoundException e) {
-      LOGGER.debug(
-          "Someone tried to reset the password for the email {} that doesn't exist",
-          cidOrEmailString);
-      throw new PasswordResetProcessException();
-    }
+    TimerBlock.minimum(3000)
+        .execute(
+            () -> {
+              try {
+                PasswordResetRepository.PasswordReset passwordReset;
+                if (new Email.EmailValidator().validate(cidOrEmailString)
+                    instanceof SuccessfulValidation) {
+                  passwordReset =
+                      this.passwordResetRepository.createNewToken(new Email(cidOrEmailString));
+                } else if (new Cid.CidValidator().validate(cidOrEmailString)
+                    instanceof SuccessfulValidation) {
+                  passwordReset =
+                      this.passwordResetRepository.createNewToken(new Cid(cidOrEmailString));
+                } else {
+                  throw new IllegalArgumentException("Neither an email nor a cid.");
+                }
+
+                if (throttlingService.canProceed(
+                    passwordReset.email().value() + "-password-reset", 3)) {
+                  sendPasswordResetTokenMail(passwordReset.email(), passwordReset.token());
+                } else {
+                  LOGGER.info("Throttling password reset process triggered.");
+                }
+              } catch (PasswordResetRepository.UserNotFoundException e) {
+                LOGGER.debug(
+                    "Someone tried to reset the password for the email {} that doesn't exist",
+                    cidOrEmailString);
+              }
+            });
   }
 
   public void finishResetPasswordProcess(
-      String inputToken, String newPassword, String confirmPassword)
-      throws PasswordResetProcessException {
+      String inputToken, String newPassword, String confirmPassword) {
     this.accessGuard.require(isNotSignedIn());
 
     if (!newPassword.equals(confirmPassword)) {
@@ -107,7 +113,4 @@ public boolean isValidToken(String token) {
 
     return this.passwordResetRepository.isTokenValid(new PasswordResetToken(token));
   }
-
-  // Vague for security reasons
-  public static class PasswordResetProcessException extends Exception {}
 }
diff --git a/app/src/main/java/it/chalmers/gamma/security/TimerBlock.java b/app/src/main/java/it/chalmers/gamma/security/TimerBlock.java
@@ -1,28 +1,26 @@
 package it.chalmers.gamma.security;
 
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
+import java.security.SecureRandom;
+import java.util.concurrent.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class TimerBlock {
-
   private static final Logger LOGGER = LoggerFactory.getLogger(TimerBlock.class);
   private static final ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1);
 
-  private final long minExecutionTimeInSeconds;
+  private final long minExecutionTimeInMillis;
 
-  private TimerBlock(long minExecutionTimeInSeconds) {
-    this.minExecutionTimeInSeconds = minExecutionTimeInSeconds;
+  private TimerBlock(long minExecutionTimeInMillis) {
+    this.minExecutionTimeInMillis = minExecutionTimeInMillis;
   }
 
-  public static TimerBlock minimum(long minExecutionTimeInSeconds) {
-    return new TimerBlock(minExecutionTimeInSeconds);
+  public static TimerBlock minimum(long minExecutionTimeInMillis) {
+    return new TimerBlock(minExecutionTimeInMillis);
   }
 
   public void execute(Runnable block) {
+    SecureRandom secureRandom = new SecureRandom();
     long startTime = System.currentTimeMillis();
     RuntimeException blockException = null;
     try {
@@ -31,18 +29,24 @@ public void execute(Runnable block) {
       blockException = e;
     } finally {
       long endTime = System.currentTimeMillis();
-      long actualExecutionTimeInSeconds = TimeUnit.MILLISECONDS.toSeconds(endTime - startTime);
+      long actualExecutionTimeInMillis = endTime - startTime;
+
+      long maxExecutionTime = minExecutionTimeInMillis + (long) (minExecutionTimeInMillis * 0.5);
+      long randomDelay =
+          minExecutionTimeInMillis
+              + secureRandom.nextInt((int) (maxExecutionTime - minExecutionTimeInMillis));
 
-      if (actualExecutionTimeInSeconds < minExecutionTimeInSeconds) {
+      if (actualExecutionTimeInMillis < randomDelay) {
         try {
-          long remainingTimeInSeconds = minExecutionTimeInSeconds - actualExecutionTimeInSeconds;
-          scheduler.schedule(() -> {}, remainingTimeInSeconds, TimeUnit.SECONDS).get();
+          long remainingTimeInMillis = randomDelay - actualExecutionTimeInMillis;
+          scheduler.schedule(() -> {}, remainingTimeInMillis, TimeUnit.MILLISECONDS).get();
         } catch (InterruptedException | ExecutionException e) {
           LOGGER.warn("Exception occurred during delay: {}", e.getMessage());
         }
       } else {
         LOGGER.warn(
-            "Block execution took longer than expected: {} seconds.", actualExecutionTimeInSeconds);
+            "Block execution took longer than expected: {} milliseconds.",
+            actualExecutionTimeInMillis);
       }
     }