fix: Fix navigation to secured view with oauth2 without router-ignore (#14523) (#14653)

This is to fix the problem of navigating to a secured view
from a public view before login in while using the OAuth2 external
login page. Prior to this, it was needed to set "router-ignore"
attribute on the links from public pages to skip vaadin-router and
let the spring-security forward the user to the external login page.

Fixes #14253

Co-authored-by: Soroosh Taefi <taefi.soroosh@gmail.com>

Author: vaadin-bot

flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java

@@ -21,6 +21,7 @@
 import javax.annotation.security.DenyAll;
 import javax.annotation.security.PermitAll;
 import javax.annotation.security.RolesAllowed;
+import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
 import com.vaadin.flow.component.Component;

flow-server/src/test/java/com/vaadin/flow/server/auth/AccessAnnotationCheckerTest.java

@@ -57,6 +57,7 @@ public String getName() {
             return "John Doe";
         }
     };
+    static final String REQUEST_URL = "http://localhost:8080/myapp/";
 
     @Rule
     public ExpectedException exception = ExpectedException.none();
@@ -364,6 +365,8 @@ static HttpServletRequest createRequest(Principal userPrincipal,
                 .thenAnswer(query -> {
                     return roleSet.contains(query.getArguments()[0]);
                 });
+        Mockito.when(request.getRequestURL())
+                .thenReturn(new StringBuffer(REQUEST_URL));
         return request;
     }
 

flow-server/src/test/java/com/vaadin/flow/server/auth/ViewAccessCheckerTest.java

@@ -1,14 +1,18 @@
 package com.vaadin.flow.server.auth;
 
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
 import java.lang.reflect.Field;
 import java.security.Principal;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpSession;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
 
 import com.vaadin.flow.component.Component;
 import com.vaadin.flow.component.UI;
@@ -34,24 +38,19 @@
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.AnonymousAllowedView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.DenyAllView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationAnonymousAllowedByGrandParentView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationAnonymousAllowedByParentView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationDenyAllAsInterfacesIgnoredView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationDenyAllByGrandParentView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationPermitAllByGrandParentAsInterfacesIgnoredView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationPermitAllByGrandParentView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationRolesAllowedAdminByGrandParentView;
+import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationRolesAllowedUserByGrandParentView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.PermitAllView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.RolesAllowedAdminView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.RolesAllowedUserView;
 import com.vaadin.flow.server.auth.AccessControlTestClasses.TestLoginView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationAnonymousAllowedByParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationAnonymousAllowedByGrandParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationPermitAllByGrandParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationDenyAllByGrandParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationRolesAllowedUserByGrandParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationRolesAllowedAdminByGrandParentView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationDenyAllAsInterfacesIgnoredView;
-import com.vaadin.flow.server.auth.AccessControlTestClasses.NoAnnotationPermitAllByGrandParentAsInterfacesIgnoredView;
-
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-import org.mockito.Mockito;
 
 public class ViewAccessCheckerTest {
 
@@ -233,6 +232,11 @@ public void loginViewAccessAlwaysAllowed() {
     public void redirectUrlStoredForAnonymousUsers() {
         Result result = checkAccess(RolesAllowedAdminView.class, null);
         Assert.assertFalse(result.wasTargetViewRendered());
+        Assert.assertEquals(
+                AccessAnnotationCheckerTest.REQUEST_URL
+                        + getRoute(RolesAllowedAdminView.class),
+                result.sessionAttributes.get(
+                        ViewAccessChecker.SESSION_STORED_REDIRECT_ABSOLUTE));
         Assert.assertEquals(getRoute(RolesAllowedAdminView.class),
                 result.sessionAttributes
                         .get(ViewAccessChecker.SESSION_STORED_REDIRECT));
@@ -245,6 +249,8 @@ public void redirectUrlNotStoredForLoggedInUsers() {
         Assert.assertFalse(result.wasTargetViewRendered());
         Assert.assertNull(result.sessionAttributes
                 .get(ViewAccessChecker.SESSION_STORED_REDIRECT));
+        Assert.assertNull(result.sessionAttributes
+                .get(ViewAccessChecker.SESSION_STORED_REDIRECT_ABSOLUTE));
     }
 
     @Test
@@ -658,14 +664,14 @@ private Result setupRequest(Class navigationTarget, User user,
         Mockito.when(vaadinServletRequest.getHttpServletRequest())
                 .thenReturn(httpServletRequest);
         Mockito.when(vaadinServletRequest.getUserPrincipal())
-                .thenAnswer(anasert -> httpServletRequest.getUserPrincipal());
+                .thenAnswer(answer -> httpServletRequest.getUserPrincipal());
+        Mockito.when(vaadinServletRequest.getSession())
+                .thenAnswer(answer -> httpServletRequest.getSession());
         Mockito.when(vaadinServletRequest.isUserInRole(Mockito.any()))
                 .thenAnswer(answer -> httpServletRequest
                         .isUserInRole(answer.getArgument(0)));
-        Mockito.when(vaadinServletRequest.getSession())
-                .thenAnswer(answer -> httpServletRequest.getSession());
-        Mockito.when(vaadinServletRequest.getRequestURL())
-                .thenReturn(new StringBuffer("http://localhost:8080/"));
+        Mockito.when(vaadinServletRequest.getRequestURL()).thenReturn(
+                new StringBuffer(AccessAnnotationCheckerTest.REQUEST_URL));
 
         CurrentInstance.set(VaadinRequest.class, vaadinServletRequest);
 

vaadin-spring/src/main/java/com/vaadin/flow/spring/security/VaadinSavedRequestAwareAuthenticationSuccessHandler.java

@@ -22,15 +22,18 @@
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
+import com.vaadin.flow.router.BeforeEnterEvent;
 import com.vaadin.flow.server.auth.ViewAccessChecker;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.web.DefaultRedirectStrategy;
 import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler;
 import org.springframework.security.web.csrf.CsrfToken;
 import org.springframework.security.web.savedrequest.HttpSessionRequestCache;
 import org.springframework.security.web.savedrequest.RequestCache;
 import org.springframework.security.web.savedrequest.SavedRequest;
+import org.springframework.util.StringUtils;
 
 /**
  * A version of {@link SavedRequestAwareAuthenticationSuccessHandler} that
@@ -89,16 +92,9 @@ public static class RedirectStrategy extends DefaultRedirectStrategy {
         @Override
         public void sendRedirect(HttpServletRequest request,
                 HttpServletResponse response, String url) throws IOException {
-            String redirectUrl;
-            String savedRedirectUrl = response.getHeader(SAVED_URL_HEADER);
-            if (savedRedirectUrl != null) {
-                redirectUrl = savedRedirectUrl;
-            } else {
-                redirectUrl = url;
-            }
 
             if (!isTypescriptLogin(request)) {
-                super.sendRedirect(request, response, redirectUrl);
+                super.sendRedirect(request, response, url);
                 return;
             }
 
@@ -126,33 +122,111 @@ public void sendRedirect(HttpServletRequest request,
      */
     public VaadinSavedRequestAwareAuthenticationSuccessHandler() {
         setRedirectStrategy(new RedirectStrategy());
+        setTargetUrlParameter(SAVED_URL_HEADER);
     }
 
+    /**
+     * Called when a user has been successfully authenticated and finds out
+     * whether it should redirect the user back to a default success url or the
+     * originally requested url before the authentication.
+     * <p>
+     * As the user might have initiated the request to a restricted resource in
+     * different ways, this method is responsible for extracting the final
+     * target for redirection of the user and to set it on the response header,
+     * so that it can be used by the redirection strategy in a unified way. See
+     * {@link RedirectStrategy} and
+     * {@link VaadinSavedRequestAwareAuthenticationSuccessHandler#determineTargetUrl(HttpServletRequest, HttpServletResponse)}
+     * <p>
+     * If the redirection to the login page for authentication is initiated by
+     * spring security (such as entering some URI manually into the address bar
+     * and not navigating via Vaadin application), then a SavedRequest object
+     * containing the originally requested path is pushed to the request cache
+     * by the Spring Security so the redirect target url would be extracted from
+     * that.
+     * <p>
+     * Contrarily, navigating via Vaadin application router (e.g. via menus or
+     * the links within the application) will result in requests being sent to
+     * "/" or "/{app-context-root}", so the Spring Security will not intercept
+     * and the SavedRequest will be null. In this case, the target redirect url
+     * can be extracted from the session. See
+     * {@link ViewAccessChecker#beforeEnter(BeforeEnterEvent)}
+     *
+     * @param request
+     *            the request which caused the successful authentication
+     * @param response
+     *            the response
+     * @param authentication
+     *            the <tt>Authentication</tt> object which was created during
+     *            the authentication process.
+     */
     @Override
     public void onAuthenticationSuccess(HttpServletRequest request,
             HttpServletResponse response, Authentication authentication)
             throws ServletException, IOException {
-        SavedRequest savedRequest = this.requestCache.getRequest(request,
-                response);
-        String storedServerNavigation = getStoredServerNavigation(request);
-        if (storedServerNavigation != null) {
-            response.setHeader(SAVED_URL_HEADER, storedServerNavigation);
-        } else if (savedRequest != null) {
-            /*
-             * This is here instead of in sendRedirect as we do not want to
-             * fallback to the default URL but instead send that separately.
-             */
-            response.setHeader(SAVED_URL_HEADER, savedRequest.getRedirectUrl());
-        }
 
         if (isTypescriptLogin(request)) {
             response.setHeader(DEFAULT_URL_HEADER,
                     determineTargetUrl(request, response));
         }
 
+        SavedRequest savedRequest = this.requestCache.getRequest(request,
+                response);
+        String fullySavedRequestUrl = getStoredServerNavigation(request);
+
+        if (savedRequest != null) {
+            String targetUrlParameter = this.getTargetUrlParameter();
+            if (!this.isAlwaysUseDefaultTargetUrl()
+                    && (targetUrlParameter == null || !StringUtils.hasText(
+                            request.getParameter(targetUrlParameter)))) {
+                this.clearAuthenticationAttributes(request);
+                String targetUrl = savedRequest.getRedirectUrl();
+                response.setHeader(SAVED_URL_HEADER, targetUrl);
+                this.getRedirectStrategy().sendRedirect(request, response,
+                        targetUrl);
+                return;
+            } else {
+                this.requestCache.removeRequest(request, response);
+            }
+        } else if (fullySavedRequestUrl != null) {
+            response.setHeader(SAVED_URL_HEADER, fullySavedRequestUrl);
+        }
+
         super.onAuthenticationSuccess(request, response, authentication);
     }
 
+    /**
+     * Determines the originally requested path by the user before
+     * authentication by reading the target redirect url from the response
+     * header.
+     * <p>
+     * Note that if a defaultSuccessUrl has been configured on the http security
+     * configurer, or the value of {@code targetUrlParameter} is {@code null},
+     * it will fall back to the default super class implementation.
+     *
+     * @param request
+     *            the http servlet request instance
+     * @param response
+     *            the http servlet response instance
+     * @return the original requested path by the user before authentication.
+     */
+    @Override
+    protected String determineTargetUrl(HttpServletRequest request,
+            HttpServletResponse response) {
+        if (!isAlwaysUseDefaultTargetUrl()
+                && this.getTargetUrlParameter() != null) {
+            String targetUrl = response.getHeader(this.getTargetUrlParameter());
+            if (StringUtils.hasText(targetUrl)) {
+                if (this.logger.isTraceEnabled()) {
+                    this.logger.trace(LogMessage.format(
+                            "Using url %s from response header %s", targetUrl,
+                            this.getTargetUrlParameter()));
+                }
+                return targetUrl;
+            }
+        }
+        return super.determineTargetUrl(request, response);
+    }
+
     /**
      * Gets the target URL potentially stored by the server side view access
      * control.

vaadin-spring/src/main/java/com/vaadin/flow/spring/security/VaadinWebSecurity.java

@@ -28,6 +28,7 @@
 import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.http.HttpStatus;
 import org.springframework.security.access.AccessDeniedException;
@@ -96,6 +97,9 @@ public abstract class VaadinWebSecurity {
     @Autowired
     private ViewAccessChecker viewAccessChecker;
 
+    @Value("#{servletContext.contextPath}")
+    private String servletContextPath;
+
     /**
      * Registers default {@link SecurityFilterChain} bean.
      * <p>
@@ -383,6 +387,27 @@ protected void setLoginView(HttpSecurity http,
         viewAccessChecker.setLoginView(flowLoginView);
     }
 
+    /**
+     * Sets up the login page URI of the OAuth2 provider on the specified
+     * HttpSecurity instance.
+     *
+     * @param http
+     *            the http security from {@link #filterChain(HttpSecurity)}
+     * @param oauth2LoginPage
+     *            the login page of the OAuth2 provider. This Specifies the URL
+     *            to send users to if login is required.
+     * @throws Exception
+     *             Re-throws the possible exceptions while activating
+     *             OAuth2LoginConfigurer
+     */
+    protected void setOAuth2LoginPage(HttpSecurity http, String oauth2LoginPage)
+            throws Exception {
+        http.oauth2Login().loginPage(oauth2LoginPage).successHandler(
+                getVaadinSavedRequestAwareAuthenticationSuccessHandler(http))
+                .permitAll();
+        viewAccessChecker.setLoginView(servletContextPath + oauth2LoginPage);
+    }
+
     /**
      * Sets up stateless JWT authentication using cookies.
      *