fix: Access-controlled layout (#20120) (#20149)

* fix: Accesscontrolled layout

Make Flow (a)Layout linking dynamic.
Add access checking for route Layout.

Part of #20097

* Fix tests

* format

* Take into account navigation targeting RouteAlias

* Add test for multiple RouteAliases

fix multiple aliases annotation present

* add messages for alias false checks.

* Add access tests for automatic layouts

* Log denied layout name. only note in exception that it is a layout

* Use existing access test classes

* Fix review comments.

Remove layout name from deny context.

Co-authored-by: caalador <mikael.grankvist@vaadin.com>

flow-server/src/main/java/com/vaadin/flow/router/internal/AbstractNavigationStateRenderer.java

@@ -62,11 +62,13 @@
 import com.vaadin.flow.router.PageTitle;
 import com.vaadin.flow.router.PreserveOnRefresh;
 import com.vaadin.flow.router.QueryParameters;
+import com.vaadin.flow.router.Route;
 import com.vaadin.flow.router.RouteParameters;
 import com.vaadin.flow.router.Router;
 import com.vaadin.flow.router.RouterLayout;
 import com.vaadin.flow.server.Constants;
 import com.vaadin.flow.server.HttpStatusCode;
+import com.vaadin.flow.server.RouteRegistry;
 import com.vaadin.flow.server.VaadinSession;
 import com.vaadin.flow.internal.menu.MenuRegistry;
 
@@ -155,7 +157,10 @@ public int handle(NavigationEvent event) {
         final RouteParameters parameters = navigationState.getRouteParameters();
         final RouteTarget routeTarget = navigationState.getRouteTarget();
 
-        routeLayoutTypes = routeTarget != null ? routeTarget.getParentLayouts()
+        routeLayoutTypes = routeTarget != null
+                ? getTargetParentLayouts(routeTarget,
+                        event.getSource().getRegistry(),
+                        event.getLocation().getPath())
                 : getRouterLayoutTypes(routeTargetType,
                         ui.getInternals().getRouter());
 
@@ -274,6 +279,30 @@ public int handle(NavigationEvent event) {
         return statusCode;
     }
 
+    /**
+     * Get the parentLayouts for given routeTarget or use an applicable
+     * {@code @Layout} when no parentLayouts defined and target is a Route
+     * annotated target with autoLayout enabled and no layout set.
+     *
+     * @param routeTarget
+     *            RouteTarget to get parents for
+     * @param registry
+     *            Registry in use
+     * @param path
+     *            request path
+     * @return List of parent layouts
+     */
+    protected List<Class<? extends RouterLayout>> getTargetParentLayouts(
+            RouteTarget routeTarget, RouteRegistry registry, String path) {
+        if (routeTarget.getParentLayouts().isEmpty()
+                && RouteUtil.isAutolayoutEnabled(routeTarget.getTarget(), path)
+                && registry.hasLayout(path)) {
+            return RouteUtil
+                    .collectRouteParentLayouts(registry.getLayout(path));
+        }
+        return routeTarget.getParentLayouts();
+    }
+
     private void pushHistoryStateIfNeeded(NavigationEvent event, UI ui) {
         if (event instanceof ErrorNavigationEvent) {
             ErrorNavigationEvent errorEvent = (ErrorNavigationEvent) event;

flow-server/src/main/java/com/vaadin/flow/router/internal/NavigationStateRenderer.java

@@ -65,7 +65,8 @@ protected List<Class<? extends RouterLayout>> getRouterLayoutTypes(
                 .getNavigationRouteTarget(navigationState.getResolvedPath());
 
         if (target.hasTarget()) {
-            return target.getRouteTarget().getParentLayouts();
+            return getTargetParentLayouts(target.getRouteTarget(),
+                    router.getRegistry(), target.getPath());
         } else {
             return Collections.emptyList();
         }

flow-server/src/main/java/com/vaadin/flow/router/internal/RouteUtil.java

@@ -125,11 +125,6 @@ public static List<Class<? extends RouterLayout>> getParentLayouts(
 
         if (hasRouteAndPathMatches && !route.get().layout().equals(UI.class)) {
             list.addAll(collectRouteParentLayouts(route.get().layout()));
-        } else if (route.get().autoLayout() && hasRouteAndPathMatches
-                && route.get().layout().equals(UI.class)
-                && handledRegistry.hasLayout(path)) {
-            list.addAll(
-                    collectRouteParentLayouts(handledRegistry.getLayout(path)));
         } else {
             List<RouteAlias> routeAliases = AnnotationReader
                     .getAnnotationsFor(component, RouteAlias.class);
@@ -139,14 +134,6 @@ public static List<Class<? extends RouterLayout>> getParentLayouts(
             if (matchingRoute.isPresent()) {
                 list.addAll(collectRouteParentLayouts(
                         matchingRoute.get().layout()));
-            } else {
-                matchingRoute = getMatchingRouteAliasLayout(handledRegistry,
-                        component, path, routeAliases);
-
-                if (matchingRoute.isPresent()) {
-                    list.addAll(collectRouteParentLayouts(
-                            handledRegistry.getLayout(path)));
-                }
             }
         }
 
@@ -242,17 +229,6 @@ static Optional<RouteAlias> getMatchingRouteAlias(Class<?> component,
                 .findFirst();
     }
 
-    static Optional<RouteAlias> getMatchingRouteAliasLayout(
-            RouteRegistry handledRegistry, Class<?> component, String path,
-            List<RouteAlias> routeAliases) {
-        return routeAliases.stream()
-                .filter(alias -> alias.autoLayout()
-                        && path.equals(getRouteAliasPath(component, alias))
-                        && alias.layout().equals(UI.class)
-                        && handledRegistry.hasLayout(path))
-                .findFirst();
-    }
-
     static List<Class<? extends RouterLayout>> collectRouteParentLayouts(
             Class<? extends RouterLayout> layout) {
         List<Class<? extends RouterLayout>> layouts = new ArrayList<>();
@@ -529,4 +505,42 @@ private static Stream<Class<? extends Component>> filterComponentClasses(
                 .map(clazz -> (Class<? extends Component>) clazz);
     }
 
+    /**
+     * Check if given route can get the automatic layout. Automatic layout can
+     * be used if it is a {@link Route} with no {@link Route#layout()} set and
+     * {@link Route#autoLayout()} as true.
+     *
+     * @param target
+     *            target to check for accessibility
+     * @param path
+     *            path to determine if we are targeting a {@link RouteAlias}
+     *            instead of {@link Route}
+     * @return {@code true} if auto layout can be used
+     */
+    public static boolean isAutolayoutEnabled(Class<?> target, String path) {
+        if (target.isAnnotationPresent(RouteAlias.class)
+                || target.isAnnotationPresent(RouteAlias.Container.class)) {
+            for (RouteAlias alias : target
+                    .getAnnotationsByType(RouteAlias.class)) {
+                String aliasPath = RouteUtil.getRouteAliasPath(target, alias);
+                String trimmedTemplate = PathUtil
+                        .trimPath(HasUrlParameterFormat.getTemplate(aliasPath,
+                                (Class<? extends Component>) target));
+                RouteModel routeModel = RouteModel.create(true);
+                routeModel.addRoute(trimmedTemplate,
+                        new RouteTarget((Class<? extends Component>) target));
+                NavigationRouteTarget navigationRouteTarget = routeModel
+                        .getNavigationRouteTarget(path);
+                if (navigationRouteTarget.hasTarget()) {
+                    return alias.autoLayout()
+                            && alias.layout().equals(UI.class);
+                }
+            }
+
+        }
+        return target.isAnnotationPresent(Route.class)
+                && target.getAnnotation(Route.class).autoLayout()
+                && target.getAnnotation(Route.class).layout().equals(UI.class);
+
+    }
 }

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

@@ -23,7 +23,11 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.vaadin.flow.component.Component;
 import com.vaadin.flow.router.BeforeEnterListener;
+import com.vaadin.flow.router.Layout;
+import com.vaadin.flow.router.internal.RouteUtil;
+import com.vaadin.flow.server.RouteRegistry;
 
 /**
  * Checks access to views using an {@link AccessAnnotationChecker}.
@@ -60,6 +64,33 @@ public AnnotatedViewAccessChecker(
     @Override
     public AccessCheckResult check(NavigationContext context) {
         Class<?> targetView = context.getNavigationTarget();
+        if (RouteUtil.isAutolayoutEnabled(targetView,
+                context.getLocation().getPath())) {
+            RouteRegistry registry = context.getRouter().getRegistry();
+            boolean noParents = registry.getRegisteredRoutes().stream()
+                    .filter(routeData -> routeData.getNavigationTarget()
+                            .equals(targetView))
+                    .map(data -> data.getParentLayouts().isEmpty()).findFirst()
+                    .orElse(true);
+            if (noParents
+                    && registry.hasLayout(context.getLocation().getPath())) {
+                Class<?> layout = registry
+                        .getLayout(context.getLocation().getPath());
+
+                boolean hasAccess = accessAnnotationChecker.hasAccess(layout,
+                        context.getPrincipal(), context::hasRole);
+                if (!hasAccess) {
+                    LOGGER.debug(
+                            "Denied access to view due to layout '{}' access rules",
+                            layout.getSimpleName());
+                    return context.deny(
+                            "Consider adding one of the following annotations "
+                                    + "to make the layout accessible: @AnonymousAllowed, "
+                                    + "@PermitAll, @RolesAllowed.");
+                }
+            }
+        }
+
         boolean hasAccess = accessAnnotationChecker.hasAccess(targetView,
                 context.getPrincipal(), context::hasRole);
         LOGGER.debug("Access to view '{}' with path '{}' is {}",
@@ -74,6 +105,18 @@ public AccessCheckResult check(NavigationContext context) {
             denyReason = "Consider adding one of the following annotations "
                     + "to make the view accessible: @AnonymousAllowed, "
                     + "@PermitAll, @RolesAllowed.";
+            if (targetView.isAnnotationPresent(Layout.class)
+                    && context.getRouter().getRegistry()
+                            .getTargetUrl(
+                                    (Class<? extends Component>) targetView)
+                            .isEmpty()) {
+                LOGGER.debug(
+                        "Denied access to view due to layout '{}' access rules",
+                        targetView.getSimpleName());
+                denyReason = "Consider adding one of the following annotations "
+                        + "to make the layout accessible: @AnonymousAllowed, "
+                        + "@PermitAll, @RolesAllowed.";
+            }
         } else {
             denyReason = "Access is denied by annotations on the view.";
         }

flow-server/src/test/java/com/vaadin/flow/router/internal/RouteUtilTest.java

@@ -132,6 +132,9 @@ private static class AutoLayout extends Component implements RouterLayout {
     }
 
     @Route(value = "auto")
+    @RouteAlias(value = "alias", autoLayout = false)
+    @RouteAlias(value = "mainLayout", layout = AutoLayout.class)
+    @RouteAlias(value = "autoAlias")
     @Tag(Tag.DIV)
     public static class AutoLayoutView extends Component {
     }
@@ -301,7 +304,7 @@ public void top_parent_layout_for_absolute_route_alias_parent() {
     }
 
     @Test
-    public void automaticLayoutShouldBeGottenForDefaultRoute() {
+    public void automaticLayoutShouldBeAvailableForDefaultRoute() {
 
         MockVaadinServletService service = new MockVaadinServletService() {
             @Override
@@ -316,11 +319,34 @@ public VaadinContext getContext() {
         List<Class<? extends RouterLayout>> parentLayouts = RouteUtil
                 .getParentLayouts(registry, AutoLayoutView.class, "auto");
 
-        Assert.assertEquals("Route with no layout should get automatic layout",
-                1, parentLayouts.size());
         Assert.assertEquals(
-                "Layout should be the @Layout annotated RouterLayout",
-                AutoLayout.class, parentLayouts.get(0));
+                "Route with no layout should not get automatic layout", 0,
+                parentLayouts.size());
+        Assert.assertTrue(
+                RouteUtil.isAutolayoutEnabled(AutoLayoutView.class, "auto"));
+    }
+
+    @Test
+    public void routeAliasForAutoLayoutRoute_correctAliasIsSelectedForRoute() {
+
+        MockVaadinServletService service = new MockVaadinServletService() {
+            @Override
+            public VaadinContext getContext() {
+                return new MockVaadinContext();
+            }
+        };
+        ApplicationRouteRegistry registry = ApplicationRouteRegistry
+                .getInstance(service.getContext());
+        registry.setLayout(AutoLayout.class);
+
+        Assert.assertTrue(
+                RouteUtil.isAutolayoutEnabled(AutoLayoutView.class, "auto"));
+        Assert.assertFalse("'alias' route has autolayout false",
+                RouteUtil.isAutolayoutEnabled(AutoLayoutView.class, "alias"));
+        Assert.assertFalse("'mainLayout' has a defined layout", RouteUtil
+                .isAutolayoutEnabled(AutoLayoutView.class, "mainLayout"));
+        Assert.assertTrue(RouteUtil.isAutolayoutEnabled(AutoLayoutView.class,
+                "autoAlias"));
     }
 
     @Test

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

@@ -27,6 +27,7 @@
 import com.vaadin.flow.router.ErrorParameter;
 import com.vaadin.flow.router.HasErrorParameter;
 import com.vaadin.flow.router.HasUrlParameter;
+import com.vaadin.flow.router.Layout;
 import com.vaadin.flow.router.NotFoundException;
 import com.vaadin.flow.router.Route;
 import com.vaadin.flow.router.RouteAlias;
@@ -441,4 +442,16 @@ public int setErrorParameter(BeforeEnterEvent event,
         }
     }
 
+    @Tag(Tag.DIV)
+    @Layout
+    public static class NoAuthLayout extends Component implements RouterLayout {
+    }
+
+    @Tag(Tag.DIV)
+    @Layout
+    @AnonymousAllowed
+    public static class AnonymousAllowedLayout extends Component
+            implements RouterLayout {
+    }
+
 }

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

@@ -448,6 +448,42 @@ public void rerouteToError_customNotAnnotatedErrorHandler_deny() {
         Assert.assertEquals(AccessCheckDecision.DENY, result.decision());
     }
 
+    @Test
+    public void routeWithNoAnnotationLayout_deny() {
+        NavigationContext context = setupNavigationContext(
+                AccessControlTestClasses.AnonymousAllowedView.class, null);
+        RouteRegistry registry = context.getRouter().getRegistry();
+        Mockito.when(registry.hasLayout("anon")).thenReturn(true);
+        Mockito.when(registry.getLayout("anon")).thenAnswer(
+                invocation -> AccessControlTestClasses.NoAuthLayout.class);
+        AccessCheckResult result = this.viewAccessChecker.check(context);
+        Assert.assertEquals(AccessCheckDecision.DENY, result.decision());
+    }
+
+    @Test
+    public void routeWithNoAnnotationsAllowedLayout_allowed() {
+        NavigationContext context = setupNavigationContext(
+                AccessControlTestClasses.AnonymousAllowedView.class, null);
+        RouteRegistry registry = context.getRouter().getRegistry();
+        Mockito.when(registry.hasLayout("anon")).thenReturn(true);
+        Mockito.when(registry.getLayout("anon")).thenAnswer(
+                invocation -> AccessControlTestClasses.AnonymousAllowedLayout.class);
+        AccessCheckResult result = this.viewAccessChecker.check(context);
+        Assert.assertEquals(AccessCheckDecision.ALLOW, result.decision());
+    }
+
+    @Test
+    public void routeWithNoAnnotationsAllowed_LayoutWithAllowed_denied() {
+        NavigationContext context = setupNavigationContext(
+                AccessControlTestClasses.NoAnnotationView.class, null);
+        RouteRegistry registry = context.getRouter().getRegistry();
+        Mockito.when(registry.hasLayout("noannotation")).thenReturn(true);
+        Mockito.when(registry.getLayout("noannotation")).thenAnswer(
+                invocation -> AccessControlTestClasses.AnonymousAllowedLayout.class);
+        AccessCheckResult result = this.viewAccessChecker.check(context);
+        Assert.assertEquals(AccessCheckDecision.DENY, result.decision());
+    }
+
     private AccessCheckResult checkAccess(Class<?> viewClass, User user) {
         NavigationContext context = setupNavigationContext(viewClass, user);
         return this.viewAccessChecker.check(context);

flow-server/src/test/java/com/vaadin/flow/server/startup/RouteRegistryMenuAccessTest.java

@@ -291,7 +291,9 @@ private void mockInstantiator(
         when(vaadinRequest.getService()).thenReturn(vaadinService);
         var instantiator = mock(Instantiator.class);
         when(vaadinService.getInstantiator()).thenReturn(instantiator);
-        when(vaadinService.getRouter()).thenReturn(mock(Router.class));
+        Router router = mock(Router.class);
+        when(vaadinService.getRouter()).thenReturn(router);
+        when(router.getRegistry()).thenReturn(registry);
         when(instantiator.getMenuAccessControl())
                 .thenReturn(new MenuAccessControl() {
                     @Override