From 60183cb6388800524b0d6be56ad91c1ab88d223b Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Tue, 3 May 2022 20:31:39 +0300 Subject: [PATCH] refactor: Make ViewAccessControl more extensible Makes it possible to fetch the principal and roles from other sources than the HTTP request --- .../flow/server/auth/ViewAccessChecker.java | 84 ++++++++++++++----- .../server/auth/ViewAccessCheckerTest.java | 13 +-- 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java b/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java index 66c5276f23e..3a3eda6519f 100644 --- a/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java +++ b/flow-server/src/main/java/com/vaadin/flow/server/auth/ViewAccessChecker.java @@ -15,19 +15,24 @@ */ package com.vaadin.flow.server.auth; +import java.security.Principal; +import java.util.function.Function; + import javax.annotation.security.DenyAll; import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; -import javax.servlet.http.HttpServletRequest; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import javax.servlet.http.HttpSession; import com.vaadin.flow.component.Component; import com.vaadin.flow.router.BeforeEnterEvent; import com.vaadin.flow.router.BeforeEnterListener; import com.vaadin.flow.router.NotFoundException; +import com.vaadin.flow.server.VaadinRequest; import com.vaadin.flow.server.VaadinServletRequest; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** * Checks access to views using an {@link AccessAnnotationChecker}. *

@@ -141,28 +146,20 @@ public void beforeEnter(BeforeEnterEvent beforeEnterEvent) { return; } Class targetView = beforeEnterEvent.getNavigationTarget(); + VaadinRequest request = VaadinRequest.getCurrent(); - VaadinServletRequest vaadinServletRequest = VaadinServletRequest - .getCurrent(); - if (vaadinServletRequest == null) { - // This is in a background thread and we cannot access the request - // to check access - getLogger().warn("Preventing navigation to " + targetView.getName() - + " because no HTTP request is available for checking access."); - beforeEnterEvent.rerouteToError(NotFoundException.class); - return; - } + Principal principal = getPrincipal(request); + Function rolesChecker = getRolesChecker(request); - HttpServletRequest httpServletRequest = vaadinServletRequest - .getHttpServletRequest(); getLogger().debug("Checking access for view {}", targetView.getName()); if (loginView != null && targetView == loginView) { getLogger().debug("Allowing access for login view {}", targetView.getName()); return; } + boolean hasAccess = accessAnnotationChecker.hasAccess(targetView, - httpServletRequest); + principal, rolesChecker); if (hasAccess) { getLogger().debug("Allowed access to view {}", @@ -171,10 +168,23 @@ public void beforeEnter(BeforeEnterEvent beforeEnterEvent) { } getLogger().debug("Denied access to view {}", targetView.getName()); - if (httpServletRequest.getUserPrincipal() == null) { - httpServletRequest.getSession() - .setAttribute(SESSION_STORED_REDIRECT, beforeEnterEvent - .getLocation().getPathWithQueryParameters()); + if (principal == null) { + HttpSession session = (request instanceof VaadinServletRequest) + ? ((VaadinServletRequest) request).getSession() + : null; + if (session != null) { + session.setAttribute(SESSION_STORED_REDIRECT, beforeEnterEvent + .getLocation().getPathWithQueryParameters()); + } else { + if (request == null) { + getLogger().debug( + "Unable to store redirect in session because no request is available"); + } else { + getLogger().debug( + "Unable to store redirect in session because request is of type {}", + request.getClass().getName()); + } + } if (loginView != null) { beforeEnterEvent.forwardTo(loginView); } else { @@ -199,6 +209,40 @@ public void beforeEnter(BeforeEnterEvent beforeEnterEvent) { } } + /** + * Gets a function for checking roles for the currently logged in user. + * + * @param request + * the current request or {@code null} if no request is in + * progress (e.g. in a background thread) + * @return a function which takes a role name and returns {@code true} if + * the user is included in that role + */ + protected Function getRolesChecker(VaadinRequest request) { + if (request == null) { + return role -> false; + } + + return request::isUserInRole; + } + + /** + * Gets the principal for the currently logged in user. + * + * @param request + * the current request or {@code null} if no request is in + * progress (e.g. in a background thread) + * @return a representation of the currently logged in user or {@code null} + * if no user is currently logged in + * + */ + protected Principal getPrincipal(VaadinRequest request) { + if (request == null) { + return null; + } + return request.getUserPrincipal(); + } + private boolean isProductionMode(BeforeEnterEvent beforeEnterEvent) { return beforeEnterEvent.getUI().getSession().getConfiguration() .isProductionMode(); diff --git a/flow-server/src/test/java/com/vaadin/flow/server/auth/ViewAccessCheckerTest.java b/flow-server/src/test/java/com/vaadin/flow/server/auth/ViewAccessCheckerTest.java index 8b030aeec6e..bbaaa707f3d 100644 --- a/flow-server/src/test/java/com/vaadin/flow/server/auth/ViewAccessCheckerTest.java +++ b/flow-server/src/test/java/com/vaadin/flow/server/auth/ViewAccessCheckerTest.java @@ -67,14 +67,6 @@ public void init() { this.viewAccessChecker.setLoginView(TestLoginView.class); } - @Test - public void cannotUseWithoutServlet() { - Result request = setupRequest(AnonymousAllowedView.class, null, true); - CurrentInstance.clearAll(); - this.viewAccessChecker.beforeEnter(request.event); - Assert.assertEquals(NotFoundException.class, request.getRerouteError()); - } - @Test public void anonymousAccessToAnonymousViewAllowed() { Result result = checkAccess(AnonymousAllowedView.class, null); @@ -456,7 +448,8 @@ public void loggedInUserRoleAccess_to_noAnnotationAnonymousAllowedByGrandParentV public void loggedInUserRoleAccess_to_noAnnotationPermitAllByGrandParentView_allowed() { Result result = checkAccess( NoAnnotationPermitAllByGrandParentView.class, User.NORMAL_USER); - Assert.assertTrue(result.wasTargetViewRendered()); + Assert.assertTrue("Target view should have been rendered", + result.wasTargetViewRendered()); } @Test @@ -660,6 +653,8 @@ private Result setupRequest(Class navigationTarget, User user, .createRequest(principal, roles); Mockito.when(vaadinServletRequest.getHttpServletRequest()) .thenReturn(httpServletRequest); + Mockito.when(vaadinServletRequest.getUserPrincipal()) + .thenReturn(httpServletRequest.getUserPrincipal()); CurrentInstance.set(VaadinRequest.class, vaadinServletRequest); Router router = Mockito.mock(Router.class);