General: Only send login email notification during actual username and password login

Author: krusche

src/main/java/de/tum/cit/aet/artemis/core/security/ArtemisAuthenticationEventListener.java renamed to src/main/java/de/tum/cit/aet/artemis/core/service/ArtemisSuccessfulLoginService.java

@@ -1,4 +1,4 @@
-package de.tum.cit.aet.artemis.core.security;
+package de.tum.cit.aet.artemis.core.service;
 
 import static de.tum.cit.aet.artemis.core.config.Constants.PROFILE_CORE;
 
@@ -10,11 +10,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Value;
-import org.springframework.context.ApplicationListener;
 import org.springframework.context.annotation.Profile;
-import org.springframework.security.authentication.event.AuthenticationSuccessEvent;
-import org.springframework.security.core.Authentication;
-import org.springframework.stereotype.Component;
+import org.springframework.stereotype.Service;
 
 import de.tum.cit.aet.artemis.communication.service.notifications.MailSendingService;
 import de.tum.cit.aet.artemis.core.domain.User;
@@ -27,8 +24,8 @@
  * to users when they successfully log in.
  */
 @Profile(PROFILE_CORE)
-@Component
-public class ArtemisAuthenticationEventListener implements ApplicationListener<AuthenticationSuccessEvent> {
+@Service
+public class ArtemisSuccessfulLoginService {
 
     @Value("${artemis.user-management.password-reset.links.en:https://artemis.tum.de/account/reset/request}")
     private String passwordResetLinkEnUrl;
@@ -39,28 +36,24 @@ public class ArtemisAuthenticationEventListener implements ApplicationListener<A
     @Value("${server.url}")
     private URL artemisServerUrl;
 
-    private static final Logger log = LoggerFactory.getLogger(ArtemisAuthenticationEventListener.class);
+    private static final Logger log = LoggerFactory.getLogger(ArtemisSuccessfulLoginService.class);
 
     private final UserRepository userRepository;
 
     private final MailSendingService mailSendingService;
 
-    public ArtemisAuthenticationEventListener(UserRepository userRepository, MailSendingService mailSendingService) {
+    public ArtemisSuccessfulLoginService(UserRepository userRepository, MailSendingService mailSendingService) {
         this.userRepository = userRepository;
         this.mailSendingService = mailSendingService;
     }
 
     /**
      * Handles successful authentication events.
      * Sends a login notification email to users when they successfully authenticate.
-     *
-     * @param event The authentication success event to process
      */
-    @Override
-    public void onApplicationEvent(AuthenticationSuccessEvent event) {
-        Authentication authentication = event.getAuthentication();
+    public void sendLoginEmail(String username) {
         try {
-            User recipient = userRepository.getUserByLoginElseThrow(authentication.getName());
+            User recipient = userRepository.getUserByLoginElseThrow(username);
             var contextVariables = new HashMap<String, Object>();
             ZonedDateTime now = ZonedDateTime.now();
             contextVariables.put("loginDate", now.format(DateTimeFormatter.ofPattern("dd.MM.yyyy")));
@@ -86,17 +79,7 @@ public void onApplicationEvent(AuthenticationSuccessEvent event) {
             mailSendingService.buildAndSendAsync(recipient, "email.notification.login.title", "mail/notification/newLoginEmail", contextVariables);
         }
         catch (EntityNotFoundException ignored) {
-            log.error("User with login {} not found when trying to send newLoginEmail", authentication.getName());
+            log.error("User with login {} not found when trying to send newLoginEmail", username);
         }
     }
-
-    /**
-     * Determines whether this listener supports asynchronous execution. Async not needed since email is sent asynchronously.
-     *
-     * @return false, indicating this listener should be executed synchronously
-     */
-    @Override
-    public boolean supportsAsyncExecution() {
-        return false;
-    }
 }

src/main/java/de/tum/cit/aet/artemis/core/web/open/PublicUserJwtResource.java

@@ -38,6 +38,7 @@
 import de.tum.cit.aet.artemis.core.security.allowedTools.ToolTokenType;
 import de.tum.cit.aet.artemis.core.security.annotations.EnforceNothing;
 import de.tum.cit.aet.artemis.core.security.jwt.JWTCookieService;
+import de.tum.cit.aet.artemis.core.service.ArtemisSuccessfulLoginService;
 import de.tum.cit.aet.artemis.core.service.connectors.SAML2Service;
 
 /**
@@ -54,11 +55,15 @@ public class PublicUserJwtResource {
 
     private final AuthenticationManager authenticationManager;
 
+    private final ArtemisSuccessfulLoginService artemisSuccessfulLoginService;
+
     private final Optional<SAML2Service> saml2Service;
 
-    public PublicUserJwtResource(JWTCookieService jwtCookieService, AuthenticationManager authenticationManager, Optional<SAML2Service> saml2Service) {
+    public PublicUserJwtResource(JWTCookieService jwtCookieService, AuthenticationManager authenticationManager, ArtemisSuccessfulLoginService artemisSuccessfulLoginService,
+            Optional<SAML2Service> saml2Service) {
         this.jwtCookieService = jwtCookieService;
         this.authenticationManager = authenticationManager;
+        this.artemisSuccessfulLoginService = artemisSuccessfulLoginService;
         this.saml2Service = saml2Service;
     }
 
@@ -91,6 +96,9 @@ public ResponseEntity<Map<String, String>> authorize(@Valid @RequestBody LoginVM
             ResponseCookie responseCookie = jwtCookieService.buildLoginCookie(rememberMe, tool);
             response.addHeader(HttpHeaders.SET_COOKIE, responseCookie.toString());
 
+            // TODO: move this to the actual login implementations
+            artemisSuccessfulLoginService.sendLoginEmail(username);
+
             return ResponseEntity.ok(Map.of("access_token", responseCookie.getValue()));
         }
         catch (BadCredentialsException ex) {

src/test/java/de/tum/cit/aet/artemis/communication/notifications/service/MailServiceTest.java

@@ -25,7 +25,6 @@
 import de.tum.cit.aet.artemis.communication.service.notifications.MailSendingService;
 import de.tum.cit.aet.artemis.communication.service.notifications.MailService;
 import de.tum.cit.aet.artemis.core.domain.User;
-import de.tum.cit.aet.artemis.core.service.TimeService;
 import tech.jhipster.config.JHipsterProperties;
 
 /**
@@ -35,8 +34,6 @@
  */
 class MailServiceTest {
 
-    private MailService mailService;
-
     private MailSendingService mailSendingService;
 
     @Mock
@@ -57,13 +54,8 @@ class MailServiceTest {
     @Mock
     private SpringTemplateEngine templateEngine;
 
-    @Mock
-    private TimeService timeService;
-
     private User student1;
 
-    private User student2;
-
     private String subject;
 
     private String content;
@@ -79,7 +71,7 @@ void setUp() throws MalformedURLException, URISyntaxException {
         student1.setEmail("[email protected]");
         student1.setLangKey("de");
 
-        student2 = new User();
+        User student2 = new User();
         student2.setLogin("student2");
         student2.setId(556L);
         student2.setEmail("[email protected]");
@@ -107,7 +99,7 @@ void setUp() throws MalformedURLException, URISyntaxException {
 
         mailSendingService = new MailSendingService(jHipsterProperties, javaMailSender, messageSource, templateEngine);
 
-        mailService = new MailService(messageSource, templateEngine, mailSendingService);
+        MailService mailService = new MailService(messageSource, templateEngine, mailSendingService);
         ReflectionTestUtils.setField(mailService, "artemisServerUrl", new URI("http://localhost:8080").toURL());
     }
 

src/test/java/de/tum/cit/aet/artemis/core/security/ArtemisAuthenticationEventListenerTest.java

@@ -0,0 +1,63 @@
+package de.tum.cit.aet.artemis.core.security;
+
+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.awaitility.Awaitility.await;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import jakarta.mail.internet.MimeMessage;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.test.context.support.WithMockUser;
+
+import de.tum.cit.aet.artemis.core.domain.User;
+import de.tum.cit.aet.artemis.core.exception.EntityNotFoundException;
+import de.tum.cit.aet.artemis.core.service.ArtemisSuccessfulLoginService;
+import de.tum.cit.aet.artemis.shared.base.AbstractSpringIntegrationIndependentTest;
+
+@ExtendWith(MockitoExtension.class)
+class ArtemisSuccessfulLoginServiceTest extends AbstractSpringIntegrationIndependentTest {
+
+    private static final String TEST_PREFIX = "arsucloginst";
+
+    @Autowired
+    private ArtemisSuccessfulLoginService artemisSuccessfulLoginService;
+
+    @BeforeEach
+    void init() {
+        userUtilService.addUsers(TEST_PREFIX, 1, 2, 0, 1);
+    }
+
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "student1", roles = "INSTRUCTOR")
+    void shouldSendEmailToUserOnSuccessfulAuthentication() throws EntityNotFoundException {
+        String username = TEST_PREFIX + "student1";
+
+        User user = userTestRepository.findOneByLogin(username).get();
+
+        doNothing().when(javaMailSender).send(any(MimeMessage.class));
+        artemisSuccessfulLoginService.sendLoginEmail(username);
+        await().atMost(5, SECONDS)
+                .untilAsserted(() -> verify(mailSendingService).buildAndSendAsync(eq(user), eq("email.notification.login.title"), eq("mail/notification/newLoginEmail"), anyMap()));
+    }
+
+    @Test
+    @WithMockUser(username = TEST_PREFIX + "student1", roles = "INSTRUCTOR")
+    void shouldHandleUserNotFoundGracefully() throws EntityNotFoundException {
+        String username = "nonexistentuser";
+        doNothing().when(javaMailSender).send(any(MimeMessage.class));
+
+        artemisSuccessfulLoginService.sendLoginEmail(username);
+
+        verify(mailSendingService, never()).buildAndSendAsync(any(User.class), anyString(), anyString(), anyMap());
+    }
+}

src/test/java/de/tum/cit/aet/artemis/core/security/ArtemisSuccessfulLoginServiceTest.java

@@ -33,6 +33,7 @@
 import de.tum.cit.aet.artemis.assessment.test_repository.ResultTestRepository;
 import de.tum.cit.aet.artemis.communication.service.WebsocketMessagingService;
 import de.tum.cit.aet.artemis.communication.service.notifications.GroupNotificationService;
+import de.tum.cit.aet.artemis.communication.service.notifications.MailSendingService;
 import de.tum.cit.aet.artemis.communication.service.notifications.MailService;
 import de.tum.cit.aet.artemis.communication.service.notifications.SingleUserNotificationService;
 import de.tum.cit.aet.artemis.communication.service.notifications.push_notifications.ApplePushNotificationService;
@@ -110,6 +111,9 @@ public abstract class AbstractArtemisIntegrationTest implements MockDelegate {
     @MockitoSpyBean
     protected MailService mailService;
 
+    @MockitoSpyBean
+    protected MailSendingService mailSendingService;
+
     @MockitoSpyBean
     protected FirebasePushNotificationService firebasePushNotificationService;