Development: Improve LTI authentication (#9231)

src/main/java/de/tum/in/www1/artemis/config/lti/CustomLti13Configurer.java

@@ -5,9 +5,9 @@
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
 import org.springframework.security.web.authentication.logout.LogoutFilter;
-import org.springframework.security.web.authentication.preauth.AbstractPreAuthenticatedProcessingFilter;
 import org.springframework.stereotype.Component;
 
+import de.tum.in.www1.artemis.security.jwt.JWTFilter;
 import de.tum.in.www1.artemis.service.OnlineCourseConfigurationService;
 import de.tum.in.www1.artemis.service.connectors.lti.Lti13Service;
 import de.tum.in.www1.artemis.web.filter.Lti13LaunchFilter;
@@ -74,7 +74,7 @@ public void configure(HttpSecurity http) {
         // https://www.imsglobal.org/spec/security/v1p0/#step-3-authentication-response
         OAuth2LoginAuthenticationFilter defaultLoginFilter = configureLoginFilter(clientRegistrationRepository(http), oidcLaunchFlowAuthenticationProvider,
                 authorizationRequestRepository);
-        http.addFilterAfter(new Lti13LaunchFilter(defaultLoginFilter, "/" + LTI13_LOGIN_PATH, lti13Service(http)), AbstractPreAuthenticatedProcessingFilter.class);
+        http.addFilterAfter(new Lti13LaunchFilter(defaultLoginFilter, "/" + LTI13_LOGIN_PATH, lti13Service(http)), JWTFilter.class);
     }
 
     protected Lti13Service lti13Service(HttpSecurity http) {

src/main/java/de/tum/in/www1/artemis/service/connectors/lti/LtiService.java

@@ -11,6 +11,7 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Profile;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.ResponseCookie;
@@ -42,6 +43,9 @@
 @Profile("lti")
 public class LtiService {
 
+    @Value("${artemis.lti.trustExternalLTISystems:false}")
+    private boolean trustExternalLTISystems;
+
     public static final String LTI_GROUP_NAME = "lti";
 
     protected static final List<SimpleGrantedAuthority> SIMPLE_USER_LIST_AUTHORITY = Collections.singletonList(new SimpleGrantedAuthority(Role.STUDENT.getAuthority()));
@@ -105,6 +109,14 @@ public void authenticateLtiUser(String email, String username, String firstName,
         // 2. Case: Lookup user with the LTI email address and make sure it's not in use
         if (artemisAuthenticationProvider.getUsernameForEmail(email).isPresent() || userRepository.findOneByEmailIgnoreCase(email).isPresent()) {
             log.info("User with email {} already exists. Email is already in use.", email);
+
+            if (trustExternalLTISystems) {
+                log.info("Trusting external LTI system. Authenticating user with email: {}", email);
+                User user = userRepository.findUserWithGroupsAndAuthoritiesByEmail(email).orElseThrow();
+                SecurityContextHolder.getContext().setAuthentication(new UsernamePasswordAuthenticationToken(user.getLogin(), user.getPassword(), user.getGrantedAuthorities()));
+                return;
+            }
+
             throw new LtiEmailAlreadyInUseException();
         }
 
@@ -179,23 +191,16 @@ private void addUserToExerciseGroup(User user, Course course) {
      * @param response             the response to add the JWT cookie to
      */
     public void buildLtiResponse(UriComponentsBuilder uriComponentsBuilder, HttpServletResponse response) {
-        // TODO SK: why do we logout the user here if it was already activated?
-
         User user = userRepository.getUser();
 
         if (!user.getActivated()) {
-            log.info("User is not activated. Adding JWT cookie for activation.");
-            log.info("Add JWT cookie so the user will be logged in");
-            ResponseCookie responseCookie = jwtCookieService.buildLoginCookie(true);
-            response.addHeader(HttpHeaders.SET_COOKIE, responseCookie.toString());
-
+            log.info("User is not activated. Adding initialize parameter to query.");
             uriComponentsBuilder.queryParam("initialize", "");
         }
-        else {
-            log.info("User is activated. Adding JWT cookie for logout.");
-            prepareLogoutCookie(response);
-            uriComponentsBuilder.queryParam("ltiSuccessLoginRequired", user.getLogin());
-        }
+
+        log.info("Add/Update JWT cookie so the user will be logged in.");
+        ResponseCookie responseCookie = jwtCookieService.buildLoginCookie(true);
+        response.addHeader(HttpHeaders.SET_COOKIE, responseCookie.toString());
     }
 
     /**

src/test/java/de/tum/in/www1/artemis/connectors/LtiServiceTest.java

@@ -118,16 +118,14 @@ void addLtiQueryParamsNewUser() {
         verify(response).addHeader(any(), any());
 
         String initialize = uriComponents.getQueryParams().getFirst("initialize");
-        String ltiSuccessLoginRequired = uriComponents.getQueryParams().getFirst("ltiSuccessLoginRequired");
         assertThat(initialize).isEmpty();
-        assertThat(ltiSuccessLoginRequired).isNull();
     }
 
     @Test
     void addLtiQueryParamsExistingUser() {
         when(userRepository.getUser()).thenReturn(user);
         user.setActivated(true);
-        when(jwtCookieService.buildLogoutCookie()).thenReturn(mock(ResponseCookie.class));
+        when(jwtCookieService.buildLoginCookie(true)).thenReturn(mock(ResponseCookie.class));
 
         UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.newInstance();
         HttpServletResponse response = mock(HttpServletResponse.class);
@@ -136,12 +134,10 @@ void addLtiQueryParamsExistingUser() {
 
         UriComponents uriComponents = uriComponentsBuilder.build();
 
-        verify(jwtCookieService).buildLogoutCookie();
+        verify(jwtCookieService).buildLoginCookie(true);
         verify(response).addHeader(any(), any());
 
         String initialize = uriComponents.getQueryParams().getFirst("initialize");
-        String ltiSuccessLoginRequired = uriComponents.getQueryParams().getFirst("ltiSuccessLoginRequired");
-        assertThat(ltiSuccessLoginRequired).isEqualTo(user.getLogin());
         assertThat(initialize).isNull();
     }