From 903bfa125a8e84d94a62d835721a7cc3951ee8b9 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Thu, 12 May 2022 14:51:51 +0200 Subject: [PATCH] feat: Support view access checking in background threads with Spring (#13675) (#13754) Requires the spring context to be properly set up for the background thread. Based on #13674 Co-authored-by: Artur --- .../spring/flowsecurity/Configurator.java | 2 + .../spring/flowsecurity/views/PublicView.java | 21 +++++ .../flow/spring/flowsecurity/AppViewIT.java | 24 +++++ .../java/dev/hilla/AuthenticationUtil.java | 46 ++++++++++ .../dev/hilla/push/PushMessageHandler.java | 22 ++--- .../flow/spring/AuthenticationUtil.java | 45 +++++++++ .../SpringSecurityAutoConfiguration.java | 5 +- .../flow/spring/SpringViewAccessChecker.java | 45 +++++++++ .../spring/SpringViewAccessCheckerTest.java | 92 +++++++++++++++++++ 9 files changed, 284 insertions(+), 18 deletions(-) create mode 100644 fusion-endpoint/src/main/java/dev/hilla/AuthenticationUtil.java create mode 100644 vaadin-spring/src/main/java/com/vaadin/flow/spring/AuthenticationUtil.java create mode 100644 vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringViewAccessChecker.java create mode 100644 vaadin-spring/src/test/java/com/vaadin/flow/spring/SpringViewAccessCheckerTest.java diff --git a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/Configurator.java b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/Configurator.java index 4af8361e0cf..7b25b8bdaa3 100644 --- a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/Configurator.java +++ b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/Configurator.java @@ -1,11 +1,13 @@ package com.vaadin.flow.spring.flowsecurity; import com.vaadin.flow.component.page.AppShellConfigurator; +import com.vaadin.flow.component.page.Push; import com.vaadin.flow.server.PWA; import com.vaadin.flow.theme.Theme; @PWA(name = "Spring Security Helper Test Project", shortName = "SSH Test") @Theme("spring-security-test-app") +@Push public class Configurator implements AppShellConfigurator { } diff --git a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/PublicView.java b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/PublicView.java index 5047d013914..a969129f20d 100644 --- a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/PublicView.java +++ b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/main/java/com/vaadin/flow/spring/flowsecurity/views/PublicView.java @@ -1,5 +1,7 @@ package com.vaadin.flow.spring.flowsecurity.views; +import com.vaadin.flow.component.UI; +import com.vaadin.flow.component.button.Button; import com.vaadin.flow.component.html.H1; import com.vaadin.flow.component.html.Image; import com.vaadin.flow.component.html.Paragraph; @@ -13,6 +15,8 @@ @AnonymousAllowed public class PublicView extends FlexLayout { + public static final String BACKGROUND_NAVIGATION_ID = "backgroundNavi"; + public PublicView() { setFlexDirection(FlexDirection.COLUMN); setHeightFull(); @@ -26,6 +30,23 @@ public PublicView() { add(image); add(new Paragraph( "We are very great and have great amounts of money.")); + + Button backgroundNavigation = new Button( + "Navigate to admin view in 1 second", e -> { + UI ui = e.getSource().getUI().get(); + new Thread(() -> { + try { + Thread.sleep(1000); + } catch (InterruptedException e1) { + } + ui.access(() -> { + ui.navigate(AdminView.class); + }); + + }).start(); + }); + backgroundNavigation.setId(BACKGROUND_NAVIGATION_ID); + add(backgroundNavigation); } } diff --git a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/AppViewIT.java b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/AppViewIT.java index ab1b2281231..bcddd1cd193 100644 --- a/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/AppViewIT.java +++ b/flow-tests/vaadin-spring-tests/test-spring-security-flow/src/test/java/com/vaadin/flow/spring/flowsecurity/AppViewIT.java @@ -10,6 +10,7 @@ import com.vaadin.flow.component.button.testbench.ButtonElement; import com.vaadin.flow.component.upload.testbench.UploadElement; +import com.vaadin.flow.spring.flowsecurity.views.PublicView; import com.vaadin.testbench.TestBenchElement; import org.apache.commons.io.IOUtils; @@ -247,6 +248,29 @@ public void upload_file_in_private_view() throws IOException { .contains("/VAADIN/dynamic/resource/")); } + @Test + public void navigate_in_thread_without_access() { + open(""); + $(ButtonElement.class).id(PublicView.BACKGROUND_NAVIGATION_ID).click(); + + // This waits for longer than the delay in the UI so we do not need a + // separate + // sleep + assertLoginViewShown(); + } + + @Test + public void navigate_in_thread_with_access() { + open("login"); + loginAdmin(); + $(ButtonElement.class).id(PublicView.BACKGROUND_NAVIGATION_ID).click(); + + // This waits for longer than the delay in the UI so we do not need a + // separate + // sleep + assertAdminPageShown(ADMIN_FULLNAME); + } + private void navigateTo(String path) { navigateTo(path, true); } diff --git a/fusion-endpoint/src/main/java/dev/hilla/AuthenticationUtil.java b/fusion-endpoint/src/main/java/dev/hilla/AuthenticationUtil.java new file mode 100644 index 00000000000..600dbf19f0f --- /dev/null +++ b/fusion-endpoint/src/main/java/dev/hilla/AuthenticationUtil.java @@ -0,0 +1,46 @@ +package dev.hilla; + +import java.util.function.Function; + +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +/** + * Helpers for authentication related tasks. + */ +public class AuthenticationUtil { + + /** + * Gets the authenticated user from the Spring SecurityContextHolder. + * + * @return the authenticated user or {@code null} + */ + public static Authentication getSecurityHolderAuthentication() { + Authentication authentication = SecurityContextHolder.getContext() + .getAuthentication(); + if (authentication instanceof AnonymousAuthenticationToken) { + return null; + } + + return authentication; + + } + + /** + * Gets a function for checking if the authenticated user from the Spring + * SecurityContextHolder is in a given role. + * + * @return a function for checking if the given user has the given role + */ + public static Function getSecurityHolderRoleChecker() { + Authentication authentication = getSecurityHolderAuthentication(); + if (authentication == null) { + return role -> false; + } + return role -> authentication.getAuthorities().stream() + .anyMatch(grantedAuthority -> grantedAuthority.getAuthority() + .equals("ROLE_" + role)); + } + +} diff --git a/fusion-endpoint/src/main/java/dev/hilla/push/PushMessageHandler.java b/fusion-endpoint/src/main/java/dev/hilla/push/PushMessageHandler.java index e4c7985b3c3..f13a275468e 100644 --- a/fusion-endpoint/src/main/java/dev/hilla/push/PushMessageHandler.java +++ b/fusion-endpoint/src/main/java/dev/hilla/push/PushMessageHandler.java @@ -16,11 +16,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.security.authentication.AnonymousAuthenticationToken; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Service; +import dev.hilla.AuthenticationUtil; import dev.hilla.ConditionalOnFeatureFlag; import dev.hilla.EndpointInvocationException.EndpointAccessDeniedException; import dev.hilla.EndpointInvocationException.EndpointBadRequestException; @@ -109,19 +107,11 @@ private void handleSubscribe(SubscribeMessage message, paramsObject.set(i + "", paramsArray.get(i)); } - Authentication authentication = SecurityContextHolder.getContext() - .getAuthentication(); - Principal principal; - if (authentication instanceof AnonymousAuthenticationToken) { - principal = null; - } else { - principal = authentication; - } - Function isInRole = role -> { - return authentication.getAuthorities().stream() - .anyMatch(grantedAuthority -> grantedAuthority - .getAuthority().equals("ROLE_" + role)); - }; + Principal principal = AuthenticationUtil + .getSecurityHolderAuthentication(); + Function isInRole = AuthenticationUtil + .getSecurityHolderRoleChecker(); + try { Flux result = (Flux) endpointInvoker.invoke( message.getEndpointName(), message.getMethodName(), diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/AuthenticationUtil.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/AuthenticationUtil.java new file mode 100644 index 00000000000..c6614d4d37f --- /dev/null +++ b/vaadin-spring/src/main/java/com/vaadin/flow/spring/AuthenticationUtil.java @@ -0,0 +1,45 @@ +package com.vaadin.flow.spring; + +import java.util.function.Function; + +import org.springframework.security.authentication.AnonymousAuthenticationToken; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +/** + * Helpers for authentication related tasks. + */ +public class AuthenticationUtil { + + /** + * Gets the authenticated user from the Spring SecurityContextHolder. + * + * @return the authenticated user or {@code null} + */ + public static Authentication getSecurityHolderAuthentication() { + Authentication authentication = SecurityContextHolder.getContext() + .getAuthentication(); + if (authentication instanceof AnonymousAuthenticationToken) { + return null; + } + + return authentication; + + } + + /** + * Gets a function for checking if the authenticated user from the Spring + * SecurityContextHolder is in a given role. + * + * @return a function for checking if the given user has the given role + */ + public static Function getSecurityHolderRoleChecker() { + Authentication authentication = getSecurityHolderAuthentication(); + if (authentication == null) { + return role -> false; + } + return role -> authentication.getAuthorities().stream() + .anyMatch(grantedAuthority -> grantedAuthority.getAuthority() + .equals("ROLE_" + role)); + } +} diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringSecurityAutoConfiguration.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringSecurityAutoConfiguration.java index c5cfcbcbb51..8d24952f372 100644 --- a/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringSecurityAutoConfiguration.java +++ b/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringSecurityAutoConfiguration.java @@ -67,8 +67,9 @@ public ViewAccessCheckerInitializer viewAccessCheckerInitializer() { * @return the default view access checker */ @Bean - public ViewAccessChecker viewAccessChecker() { - return new ViewAccessChecker(false); + public ViewAccessChecker viewAccessChecker( + AccessAnnotationChecker accessAnnotationChecker) { + return new SpringViewAccessChecker(accessAnnotationChecker); } /** diff --git a/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringViewAccessChecker.java b/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringViewAccessChecker.java new file mode 100644 index 00000000000..f993491eff6 --- /dev/null +++ b/vaadin-spring/src/main/java/com/vaadin/flow/spring/SpringViewAccessChecker.java @@ -0,0 +1,45 @@ +package com.vaadin.flow.spring; + +import java.security.Principal; +import java.util.function.Function; + +import com.vaadin.flow.server.VaadinRequest; +import com.vaadin.flow.server.auth.AccessAnnotationChecker; +import com.vaadin.flow.server.auth.ViewAccessChecker; + +/** + * A Spring specific view access checker that falls back to Spring mechanisms + * when the generic mechanisms do not work. + */ +public class SpringViewAccessChecker extends ViewAccessChecker { + + /** + * Creates an instance with the given annotation checker. + * + * The created instance is disabled by default. + * + * @param accessAnnotationChecker + * the annotation checker to use + */ + public SpringViewAccessChecker( + AccessAnnotationChecker accessAnnotationChecker) { + super(accessAnnotationChecker); + } + + @Override + protected Principal getPrincipal(VaadinRequest request) { + if (request == null) { + return AuthenticationUtil.getSecurityHolderAuthentication(); + } + return super.getPrincipal(request); + } + + @Override + protected Function getRolesChecker(VaadinRequest request) { + if (request == null) { + return AuthenticationUtil.getSecurityHolderRoleChecker(); + } + return super.getRolesChecker(request); + } + +} diff --git a/vaadin-spring/src/test/java/com/vaadin/flow/spring/SpringViewAccessCheckerTest.java b/vaadin-spring/src/test/java/com/vaadin/flow/spring/SpringViewAccessCheckerTest.java new file mode 100644 index 00000000000..bc00e416641 --- /dev/null +++ b/vaadin-spring/src/test/java/com/vaadin/flow/spring/SpringViewAccessCheckerTest.java @@ -0,0 +1,92 @@ +package com.vaadin.flow.spring; + +import java.security.Principal; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Function; + +import com.vaadin.flow.component.Component; +import com.vaadin.flow.router.BeforeEnterEvent; +import com.vaadin.flow.server.auth.AccessAnnotationChecker; +import com.vaadin.flow.server.auth.ViewAccessChecker; + +import org.junit.Assert; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.GrantedAuthority; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; + +@SpringBootTest(classes = { SpringViewAccessChecker.class }) +class SpringViewAccessCheckerTest { + + @MockBean + private AccessAnnotationChecker annotationChecker; + @MockBean + private Authentication authentication; + + @Autowired + private ViewAccessChecker checker; + + public static class TestView extends Component { + + } + + @Test + void viewAccessControlWorksWithoutRequest() { + checker.enable(); + AtomicBoolean accessChecked = new AtomicBoolean(false); + + List grantedAuthorities = new ArrayList<>(); + grantedAuthorities.add(new SimpleGrantedAuthority("ROLE_fake")); + + Mockito.when(authentication.getAuthorities()) + .thenReturn((Collection) grantedAuthorities); + SecurityContextHolder.setContext(new SecurityContext() { + + @Override + public Authentication getAuthentication() { + return authentication; + } + + @Override + public void setAuthentication(Authentication authentication) { + } + + }); + Mockito.when(annotationChecker.hasAccess(Mockito.any(Class.class), + Mockito.any(), Mockito.any())).thenAnswer(answer -> { + Principal principal = answer.getArgument(1); + Function roleChecker = answer + .getArgument(2); + + Assert.assertEquals( + "Principal from security context should have been passed to checker", + authentication, principal); + + Assert.assertTrue( + "Role should have been checked from the context holder", + roleChecker.apply("fake")); + Assert.assertFalse( + "Role should have been checked from the context holder", + roleChecker.apply("fake2")); + accessChecked.set(true); + return true; + }); + + BeforeEnterEvent beforeEnterEvent = Mockito + .mock(BeforeEnterEvent.class); + Mockito.when(beforeEnterEvent.getNavigationTarget()) + .thenReturn((Class) TestView.class); + checker.beforeEnter(beforeEnterEvent); + Assert.assertTrue("Annotation checker should have been invoked", + accessChecked.get()); + } +}