fix: when changing email with email based 2FA enabled, fail fast

dhis-2/dhis-api/src/main/java/org/hisp/dhis/feedback/ErrorCode.java

@@ -224,6 +224,7 @@ public enum ErrorCode {
   E3049("Sending 2FA code with email failed"),
   E3050("2FA code can not be null or empty"),
   E3051("2FA code was sent to the user's email"),
+  E3052("Email 2FA is enabled on user, can not change email. Disable 2FA first"),
 
   /* Metadata Validation */
   E4000("Missing required property `{0}`"),

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/security/spring2fa/TwoFactorAuthenticationProvider.java

@@ -30,7 +30,6 @@
 import static org.hisp.dhis.security.twofa.TwoFactorAuthService.TWO_FACTOR_AUTH_REQUIRED_RESTRICTION_NAME;
 import static org.hisp.dhis.security.twofa.TwoFactorAuthUtils.isValid2FACode;
 
-import com.google.common.base.Strings;
 import java.util.Set;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -87,96 +86,115 @@ public TwoFactorAuthenticationProvider(
 
   @Override
   public Authentication authenticate(Authentication auth) throws AuthenticationException {
-    String username = auth.getName();
-    String ip = "";
+    final String username = auth.getName();
+    final Object details = auth.getDetails();
 
-    if (auth.getDetails() instanceof ForwardedIpAwareWebAuthenticationDetails details) {
-      ip = details.getIp();
-    }
+    // Extract the forwarded IP if available
+    final String ip =
+        details instanceof ForwardedIpAwareWebAuthenticationDetails fwd ? fwd.getIp() : "";
 
     log.debug("Login attempt: {}", username);
 
-    // If enabled, temporarily block user with too many failed attempts
+    // Check for temporary lockout
+    checkLockout(username, ip);
+
+    // Authenticate via the parent method (which calls UserDetailsService#loadUserByUsername())
+    Authentication result = super.authenticate(auth);
+    UserDetails userDetails = (UserDetails) result.getPrincipal();
+
+    // Validate that the user is not configured for external auth only
+    checkExternalAuth(userDetails, username);
+
+    // If the user’s role requires 2FA enrollment but they haven’t set it up, throw an exception.
+    checkTwoFactorEnrolment(userDetails);
+
+    // Handle two-factor authentication validations.
+    checkTwoFactorAuthentication(auth, userDetails);
+
+    // Return a new authentication token with the user details.
+    return new UsernamePasswordAuthenticationToken(
+        userDetails, result.getCredentials(), result.getAuthorities());
+  }
+
+  private void checkLockout(String username, String ip) {
     if (userService.isLocked(username)) {
       log.warn("Temporary lockout for user: '{}'", username);
       throw new LockedException(String.format("IP is temporarily locked: %s", ip));
     }
+  }
 
-    // Calls the UserDetailsService#loadUserByUsername(), to create the UserDetails object,
-    // after the password is validated.
-    Authentication result = super.authenticate(auth);
-    UserDetails userDetails = (UserDetails) result.getPrincipal();
-
-    // Prevents other authentication methods (e.g., OAuth2/LDAP),
-    // to use password login.
+  private void checkExternalAuth(UserDetails userDetails, String username) {
     if (userDetails.isExternalAuth()) {
       log.info(
           "User has external authentication enabled, password login attempt aborted: '{}'",
           username);
       throw new BadCredentialsException(
           "Invalid login method, user is using external authentication");
     }
+  }
 
-    // If the user requires 2FA, and it's not enabled, redirect to
-    // the enrolment page, (via the CustomAuthFailureHandler)
-    boolean has2FARestrictionOnRole =
+  private void checkTwoFactorEnrolment(UserDetails userDetails) {
+    boolean has2FARestriction =
         userDetails.hasAnyRestrictions(Set.of(TWO_FACTOR_AUTH_REQUIRED_RESTRICTION_NAME));
-    if (!userDetails.isTwoFactorEnabled() && has2FARestrictionOnRole) {
+    if (!userDetails.isTwoFactorEnabled() && has2FARestriction) {
       throw new TwoFactorAuthenticationEnrolmentException(
           "User must setup two-factor authentication first before logging in");
     }
+  }
+
+  private void checkTwoFactorAuthentication(Authentication auth, UserDetails userDetails) {
+    if (!userDetails.isTwoFactorEnabled()) {
+      return; // Nothing to do if 2FA is not enabled.
+    }
 
-    boolean isHTTPBasicRequest = !(auth.getDetails() instanceof TwoFactorWebAuthenticationDetails);
-    if (userDetails.isTwoFactorEnabled() && isHTTPBasicRequest) {
-      // If the user has 2FA enabled and tries to authenticate with HTTP Basic
+    // Ensure the authentication details are from a form-based 2FA login
+    if (!(auth.getDetails() instanceof TwoFactorWebAuthenticationDetails authDetails)) {
       throw new PreAuthenticatedCredentialsNotFoundException(
           "User has 2FA enabled, but attempted to authenticate with a non-form based login method: "
               + userDetails.getUsername());
     }
 
-    if (userDetails.isTwoFactorEnabled()
-        && auth.getDetails() instanceof TwoFactorWebAuthenticationDetails authDetails) {
-      // Check if the user's 2FA type is enabled in config
-      TwoFactorType type = userDetails.getTwoFactorType();
-      boolean isTypeEnabled = false;
-      if (type == TwoFactorType.EMAIL_ENABLED) {
-        isTypeEnabled =
-            configurationProvider.getProperty(ConfigurationKey.EMAIL_2FA_ENABLED).equals("on");
-      } else if (type == TwoFactorType.TOTP_ENABLED) {
-        isTypeEnabled =
-            configurationProvider.getProperty(ConfigurationKey.TOTP_2FA_ENABLED).equals("on");
-      }
-
-      // Only validate 2FA code if the type is enabled
-      if (isTypeEnabled) {
-        validate2FACode(authDetails.getCode(), userDetails);
-      }
+    // Only validate the 2FA code if the configured type is enabled.
+    if (isTwoFactorTypeEnabled(userDetails.getTwoFactorType())) {
+      validate2FACode(authDetails.getCode(), userDetails);
     }
+  }
 
-    return new UsernamePasswordAuthenticationToken(
-        userDetails, result.getCredentials(), result.getAuthorities());
+  private boolean isTwoFactorTypeEnabled(TwoFactorType type) {
+    return switch (type) {
+      case EMAIL_ENABLED -> configurationProvider.isEnabled(ConfigurationKey.EMAIL_2FA_ENABLED);
+      case TOTP_ENABLED -> configurationProvider.isEnabled(ConfigurationKey.TOTP_2FA_ENABLED);
+      default -> false;
+    };
   }
 
   private void validate2FACode(@CheckForNull String code, @Nonnull UserDetails userDetails) {
     TwoFactorType type = userDetails.getTwoFactorType();
 
-    // Send 2FA code via Email if the user has email 2FA enabled and the code is empty.
-    if (TwoFactorType.EMAIL_ENABLED == type && Strings.isNullOrEmpty(code)) {
-      try {
-        twoFactorAuthService.sendEmail2FACode(userDetails.getUsername());
-      } catch (ConflictException e) {
-        throw new TwoFactorAuthenticationException(ErrorCode.E3049.getMessage());
-      }
+    // For email-based 2FA, if no code is provided, trigger sending the email code.
+    if (type == TwoFactorType.EMAIL_ENABLED && StringUtils.isBlank(code)) {
+      sendEmail2FACode(userDetails);
+      // Inform the caller that the email code has been sent.
       throw new TwoFactorAuthenticationException(ErrorCode.E3051.getMessage());
     }
 
-    if (Strings.isNullOrEmpty(code) || StringUtils.deleteWhitespace(code).isEmpty()) {
+    // If the code is blank (null, empty, or only whitespace), reject the login.
+    if (StringUtils.isBlank(code)) {
       throw new TwoFactorAuthenticationException(ErrorCode.E3023.getMessage());
     }
 
+    // Validate the provided 2FA code.
     if (!isValid2FACode(type, code, userDetails.getSecret())) {
       throw new TwoFactorAuthenticationException(ErrorCode.E3023.getMessage());
     }
-    // All good, 2FA code is valid!
+    // If no exception is thrown, the 2FA code is valid.
+  }
+
+  private void sendEmail2FACode(UserDetails userDetails) {
+    try {
+      twoFactorAuthService.sendEmail2FACode(userDetails.getUsername());
+    } catch (ConflictException e) {
+      throw new TwoFactorAuthenticationException(ErrorCode.E3049.getMessage());
+    }
   }
 }

dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/UserObjectBundleHook.java

@@ -50,6 +50,7 @@
 import org.hisp.dhis.organisationunit.OrganisationUnit;
 import org.hisp.dhis.preheat.PreheatIdentifier;
 import org.hisp.dhis.security.acl.AclService;
+import org.hisp.dhis.security.twofa.TwoFactorType;
 import org.hisp.dhis.system.util.ValidationUtils;
 import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.user.User;
@@ -132,6 +133,15 @@ public void validate(User user, ObjectBundle bundle, Consumer<ErrorReport> addRe
           new ErrorReport(User.class, ErrorCode.E4027, user.getWhatsApp(), "whatsApp")
               .setErrorProperty("whatsApp"));
     }
+
+    if (existingUser != null
+        && existingUser.getTwoFactorType() != null
+        && existingUser.getTwoFactorType().equals(TwoFactorType.EMAIL_ENABLED)
+        && existingUser.isEmailVerified()
+        && user.getEmail() != null
+        && !existingUser.getVerifiedEmail().equals(user.getEmail())) {
+      addReports.accept(new ErrorReport(User.class, ErrorCode.E3052).setErrorProperty("email"));
+    }
   }
 
   @Override

dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/TwoFactorControllerTest.java

@@ -56,13 +56,17 @@
 import lombok.extern.slf4j.Slf4j;
 import org.hisp.dhis.external.conf.DhisConfigurationProvider;
 import org.hisp.dhis.feedback.ConflictException;
+import org.hisp.dhis.feedback.ErrorCode;
 import org.hisp.dhis.http.HttpStatus;
 import org.hisp.dhis.jsontree.JsonMixed;
+import org.hisp.dhis.jsontree.JsonObject;
 import org.hisp.dhis.message.MessageSender;
 import org.hisp.dhis.outboundmessage.OutboundMessage;
 import org.hisp.dhis.security.twofa.TwoFactorAuthService;
 import org.hisp.dhis.security.twofa.TwoFactorType;
 import org.hisp.dhis.test.webapi.H2ControllerIntegrationTestBase;
+import org.hisp.dhis.test.webapi.json.domain.JsonErrorReport;
+import org.hisp.dhis.test.webapi.json.domain.JsonWebMessage;
 import org.hisp.dhis.user.CurrentUserUtil;
 import org.hisp.dhis.user.SystemUser;
 import org.hisp.dhis.user.User;
@@ -99,7 +103,6 @@ void tearDown() {
   @Test
   void testEnrollTOTP2FA()
       throws ChecksumException, NotFoundException, DecodingException, IOException, FormatException {
-
     User user = createUserAndInjectSecurityContext(false);
 
     assertStatus(HttpStatus.OK, POST("/2fa/enrollTOTP2FA"));
@@ -449,4 +452,63 @@ void testDisable2FAWhenTypeDisabled() {
     assertNull(updatedUser.getSecret());
     assertEquals(TwoFactorType.NOT_ENABLED, updatedUser.getTwoFactorType());
   }
+
+  @Test
+  void testChangeEmailWhenEmail2FAIsEnabledWithMeEndpoint() {
+    // Given user has Email 2FA enabled
+    config.getProperties().put(EMAIL_2FA_ENABLED.getKey(), "on");
+
+    User user = createUserAndInjectSecurityContext(false);
+    user.setEmail("[email protected]");
+    user.setVerifiedEmail("[email protected]");
+    userService.encodeAndSetPassword(user, "Test123###...");
+
+    assertStatus(HttpStatus.OK, POST("/2fa/enrollEmail2FA"));
+    User enrolledUser = userService.getUserByUsername(user.getUsername());
+    String code = enrolledUser.getSecret().split("\\|")[0];
+    assertStatus(HttpStatus.OK, POST("/2fa/enable", "{'code':'" + code + "'}"));
+    assertEquals(TwoFactorType.EMAIL_ENABLED, user.getTwoFactorType());
+
+    // When trying to change email
+    assertWebMessage(
+        "Conflict",
+        409,
+        "ERROR",
+        "Email address cannot be changed, when email-based 2FA is enabled, please disable 2FA first",
+        PUT("/me", "{'email':'[email protected]'}").content(HttpStatus.CONFLICT));
+  }
+
+  @Test
+  void testChangeEmailWhenEmail2FAIsEnabledWithUserEndpoint() {
+    // Given user has Email 2FA enabled
+    config.getProperties().put(EMAIL_2FA_ENABLED.getKey(), "on");
+
+    User user = createUserAndInjectSecurityContext(false);
+    user.setEmail("[email protected]");
+    user.setVerifiedEmail("[email protected]");
+    userService.encodeAndSetPassword(user, "Test123###...");
+
+    assertStatus(HttpStatus.OK, POST("/2fa/enrollEmail2FA"));
+    User enrolledUser = userService.getUserByUsername(user.getUsername());
+    String code = enrolledUser.getSecret().split("\\|")[0];
+    assertStatus(HttpStatus.OK, POST("/2fa/enable", "{'code':'" + code + "'}"));
+    assertEquals(TwoFactorType.EMAIL_ENABLED, user.getTwoFactorType());
+
+    switchContextToUser(getAdminUser());
+
+    // When trying to change email
+    JsonObject jsonUser = GET("/users/{id}", user.getUid()).content();
+    String jsonUserString = jsonUser.toString();
+    jsonUserString = jsonUserString.replace("[email protected]", "another.email@com");
+
+    JsonWebMessage msg =
+        assertWebMessage(
+            "Conflict",
+            409,
+            "WARNING",
+            "One or more errors occurred, please see full details in import report.",
+            PUT("/users/" + user.getUid(), jsonUserString).content(HttpStatus.CONFLICT));
+
+    msg.getResponse().find(JsonErrorReport.class, error -> error.getErrorCode() == ErrorCode.E3052);
+  }
 }

dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/MeController.java

@@ -72,6 +72,7 @@
 import org.hisp.dhis.security.acl.AclService;
 import org.hisp.dhis.security.apikey.ApiToken;
 import org.hisp.dhis.security.apikey.ApiTokenService;
+import org.hisp.dhis.security.twofa.TwoFactorType;
 import org.hisp.dhis.setting.UserSettings;
 import org.hisp.dhis.system.util.ValidationUtils;
 import org.hisp.dhis.user.CredentialsInfo;
@@ -226,6 +227,15 @@ public void updateCurrentUser(
 
     User user = renderService.fromJson(request.getInputStream(), User.class);
 
+    if (currentUser.getTwoFactorType() != null
+        && currentUser.getTwoFactorType().equals(TwoFactorType.EMAIL_ENABLED)
+        && currentUser.isEmailVerified()
+        && user.getEmail() != null
+        && !currentUser.getVerifiedEmail().equals(user.getEmail())) {
+      throw new ConflictException(
+          "Email address cannot be changed, when email-based 2FA is enabled, please disable 2FA first");
+    }
+
     merge(currentUser, user);
 
     if (user.getWhatsApp() != null && !ValidationUtils.validateWhatsApp(user.getWhatsApp())) {