Vaadin security enhancements (#1003)

* Removed VaadinManagedSecurity configuration from MgmtUiAutoConfiguration
* added SessionFixationProtectionStrategy for additional protection of UI session authentication
* added VaadinSessionClosingLogoutHandler to logout from all UI sessions
* added AccessDecisionManager to UI security configuration in order to support method security in UI in context of VaadinSharedSecurity
* Changed UI push transport from WEBSOCKET to WEBSOCKET_XHR to solve problems with Spring Security Context
* Suppressed atmosphere IOUtils false-positive warning
* Removed obsolete AsyncVaadinServletConfiguration
* Defined Vaadin4SpringServlet bean instead of plain SpringVaadinServlet for configuration flexibility
* Removed obsolete SpringSecurityAtmosphereInterceptor because the client does not communicate with the server using websocket protocol anymore
* Removed unit test for SpringSecurityAtmosphereInterceptor
* Removed obsolete AuthenticationManagerConfigurer coming from Vaadin Managed Security in InMemoryUserManagementAutoConfiguration
* Removed SessionFixationProtectionStrategy and VaadinSessionClosingLogoutHandler because all wrapper sessions are invalidated when the session managed by Spring gets invalidated together with configured HttpSessionEventPublisher events
* Added call to close the current session before logout redirect
* added comment why we used WEBSOCKET_XHR instead of WEBSOCKET

Signed-off-by: Bogdan Bondar <Bogdan.Bondar@bosch.io>

hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/mgmt/ui/MgmtUiAutoConfiguration.java

@@ -29,7 +29,6 @@ import org.springframework.context.annotation.Import;
 import org.vaadin.spring.annotation.EnableVaadinExtensions;
 import org.vaadin.spring.events.EventBus.UIEventBus;
 import org.vaadin.spring.events.annotation.EnableEventBus;
-import org.vaadin.spring.security.annotation.EnableVaadinManagedSecurity;
 
 import com.vaadin.spring.annotation.UIScope;
 
@@ -37,7 +36,6 @@ import com.vaadin.spring.annotation.UIScope;
  * The Management UI auto configuration.
  */
 @Configuration
-@EnableVaadinManagedSecurity
 @EnableVaadinExtensions
 @EnableEventBus
 @ConditionalOnClass(MgmtUiConfiguration.class)
@@ -114,5 +112,4 @@ public class MgmtUiAutoConfiguration {
 
         return delayedEventBusPushStrategy;
     }
-
 }

hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/InMemoryUserManagementAutoConfiguration.java

@@ -32,7 +32,6 @@ import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
-import org.vaadin.spring.security.config.AuthenticationManagerConfigurer;
 
 /**
  * Auto-configuration for the in-memory-user-management.
@@ -41,8 +40,7 @@ import org.vaadin.spring.security.config.AuthenticationManagerConfigurer;
 @Configuration
 @ConditionalOnMissingBean(UserDetailsService.class)
 @EnableConfigurationProperties({ MultiUserProperties.class })
-public class InMemoryUserManagementAutoConfiguration extends GlobalAuthenticationConfigurerAdapter
-        implements AuthenticationManagerConfigurer {
+public class InMemoryUserManagementAutoConfiguration extends GlobalAuthenticationConfigurerAdapter {
 
     private static final String DEFAULT_TENANT = "DEFAULT";
 
@@ -93,8 +91,7 @@ public class InMemoryUserManagementAutoConfiguration extends GlobalAuthenticatio
             final String name = securityProperties.getUser().getName();
             final String password = securityProperties.getUser().getPassword();
             final List<String> roles = securityProperties.getUser().getRoles();
-            List<GrantedAuthority> authorityList = roles.isEmpty()
-                    ? PermissionUtils.createAllAuthorityList()
+            final List<GrantedAuthority> authorityList = roles.isEmpty() ? PermissionUtils.createAllAuthorityList()
                     : createAuthoritiesFromList(roles);
             userPrincipals
                     .add(new UserPrincipal(name, password, name, name, name, null, DEFAULT_TENANT, authorityList));
@@ -104,7 +101,7 @@ public class InMemoryUserManagementAutoConfiguration extends GlobalAuthenticatio
     }
 
     private static List<GrantedAuthority> createAuthoritiesFromList(final List<String> userAuthorities) {
-        List<GrantedAuthority> grantedAuthorityList = new ArrayList<>(userAuthorities.size());
+        final List<GrantedAuthority> grantedAuthorityList = new ArrayList<>(userAuthorities.size());
         for (final String permission : userAuthorities) {
             grantedAuthorityList.add(new SimpleGrantedAuthority(permission));
             grantedAuthorityList.add(new SimpleGrantedAuthority("ROLE_" + permission));
@@ -154,7 +151,7 @@ public class InMemoryUserManagementAutoConfiguration extends GlobalAuthenticatio
                 final Authentication authentication, final UserDetails user) {
             final UsernamePasswordAuthenticationToken result = new UsernamePasswordAuthenticationToken(principal,
                     authentication.getCredentials(), user.getAuthorities());
-            result.setDetails(new TenantAwareAuthenticationDetails("DEFAULT", false));
+            result.setDetails(new TenantAwareAuthenticationDetails(DEFAULT_TENANT, false));
             return result;
         }
     }

hawkbit-autoconfigure/src/main/java/org/eclipse/hawkbit/autoconfigure/security/SecurityManagedConfiguration.java

@@ -60,11 +60,13 @@ import org.springframework.context.annotation.PropertySource;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 import org.springframework.http.HttpStatus;
+import org.springframework.security.access.AccessDecisionManager;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.InsufficientAuthenticationException;
 import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
 import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration;
 import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
+import org.springframework.security.config.annotation.method.configuration.GlobalMethodSecurityConfiguration;
 import org.springframework.security.config.annotation.web.WebSecurityConfigurer;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.builders.WebSecurity;
@@ -595,6 +597,7 @@ public class SecurityManagedConfiguration {
      */
     @Configuration
     @Order(400)
+    @EnableWebSecurity
     @EnableVaadinSharedSecurity
     @ConditionalOnClass(MgmtUiConfiguration.class)
     public static class UISecurityConfigurationAdapter extends WebSecurityConfigurerAdapter {
@@ -714,7 +717,6 @@ public class SecurityManagedConfiguration {
             // UI logout
             httpSec.logout().logoutUrl("/UI/logout*").addLogoutHandler(logoutHandler)
                     .logoutSuccessHandler(logoutSuccessHandler);
-
         }
 
         @Override
@@ -722,8 +724,25 @@ public class SecurityManagedConfiguration {
             // No security for static content
             webSecurity.ignoring().antMatchers("/documentation/**", "/VAADIN/**", "/*.*", "/docs/**");
         }
-    }
 
+        /**
+         * Configuration that defines the {@link AccessDecisionManager} bean for
+         * UI method security used by the Vaadin Servlet. Notice: we can not use
+         * the top-level method security configuration because
+         * {@link AdviceMode.ASPECTJ} is not supported.
+         */
+        @Configuration
+        @EnableGlobalMethodSecurity(securedEnabled = true, prePostEnabled = true, proxyTargetClass = true)
+        @ConditionalOnClass(MgmtUiConfiguration.class)
+        static class UIMethodSecurity extends GlobalMethodSecurityConfiguration {
+
+            @Bean(name = VaadinSharedSecurityConfiguration.ACCESS_DECISION_MANAGER_BEAN)
+            @Override
+            protected AccessDecisionManager accessDecisionManager() {
+                return super.accessDecisionManager();
+            }
+        }
+    }
 }
 
 /**

hawkbit-runtime/hawkbit-update-server/src/main/java/org/eclipse/hawkbit/app/MyUI.java

@@ -32,11 +32,13 @@ import com.vaadin.spring.navigator.SpringViewProvider;
  * A {@link SpringUI} annotated class must be present in the classpath. The
  * easiest way to get an hawkBit UI running is to extend the
  * {@link AbstractHawkbitUI} and to annotated it with {@link SpringUI} as in
- * this example.
+ * this example. WEBSOCKET_XHR transport is used instead of WEBSOCKET in order
+ * to preserve Spring Security Context, that does not work using websocket
+ * communication with Vaadin Shared Security.
  *
  */
 @SpringUI
-@Push(value = PushMode.AUTOMATIC, transport = Transport.WEBSOCKET)
+@Push(value = PushMode.AUTOMATIC, transport = Transport.WEBSOCKET_XHR)
 // Exception squid:MaximumInheritanceDepth - Most of the inheritance comes from
 // Vaadin.
 @SuppressWarnings({ "squid:MaximumInheritanceDepth" })

hawkbit-runtime/hawkbit-update-server/src/main/resources/logback-spring.xml

@@ -19,6 +19,11 @@
 
    <!-- Security Log with hints on potential attacks -->
    <logger name="server-security" level="INFO" />
+   
+   <!-- Suppressing "More than one Servlet Mapping defined. WebSocket may not work" 
+   error due to the way VaadinServletConfiguration configures the endpoints mapping ("/UI" and "/UI/*"). 
+   At the end only the first "/UI" is taken for websocket communication.  -->
+   <logger name="org.atmosphere.util.IOUtils" level="OFF" />
 
    <Root level="INFO">
       <appender-ref ref="CONSOLE" />

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/AsyncVaadinServletConfiguration.java

@@ -1,87 +0,0 @@
-/**
- * Copyright (c) 2015 Bosch Software Innovations GmbH and others.
- *
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- */
-package org.eclipse.hawkbit.ui;
-
-import javax.servlet.ServletException;
-
-import org.atmosphere.container.JSR356AsyncSupport;
-import org.atmosphere.cpr.ApplicationConfig;
-import org.eclipse.hawkbit.ui.utils.VaadinMessageSource;
-import org.springframework.boot.context.properties.EnableConfigurationProperties;
-import org.springframework.boot.web.servlet.ServletRegistrationBean;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
-
-import com.vaadin.server.VaadinServlet;
-import com.vaadin.spring.boot.internal.VaadinServletConfiguration;
-import com.vaadin.spring.boot.internal.VaadinServletConfigurationProperties;
-import com.vaadin.spring.server.SpringVaadinServlet;
-
-/**
- * {@link VaadinServletConfiguration} that sets the context path for
- * {@link JSR356AsyncSupport} that registers
- * {@link SpringSecurityAtmosphereInterceptor} for spring security integration.
- */
-@Configuration
-@EnableConfigurationProperties(VaadinServletConfigurationProperties.class)
-@Import(VaadinServletConfiguration.class)
-public class AsyncVaadinServletConfiguration extends VaadinServletConfiguration {
-
-    /**
-     * Localized system message provider bean.
-     * 
-     * @param uiProperties
-     *            UiProperties
-     * @param i18n
-     *            VaadinMessageSource
-     *
-     * @return Localized system message provider
-     */
-    @Bean
-    public LocalizedSystemMessagesProvider localizedSystemMessagesProvider(final UiProperties uiProperties,
-            final VaadinMessageSource i18n) {
-        return new LocalizedSystemMessagesProvider(uiProperties, i18n);
-    }
-
-    /**
-     * Vaadin servlet bean.
-     * 
-     * @param localizedSystemMessagesProvider
-     *            LocalizedSystemMessagesProvider
-     *
-     * @return Vaadin servlet service
-     */
-    @Bean
-    public VaadinServlet vaadinServlet(final LocalizedSystemMessagesProvider localizedSystemMessagesProvider) {
-        return new SpringVaadinServlet() {
-            @Override
-            public void servletInitialized() throws ServletException {
-                super.servletInitialized();
-                getService().setSystemMessagesProvider(localizedSystemMessagesProvider);
-            }
-        };
-    }
-
-    @Override
-    @Bean
-    protected ServletRegistrationBean vaadinServletRegistration() {
-        return createServletRegistrationBean();
-    }
-
-    @Override
-    protected void addInitParameters(final ServletRegistrationBean servletRegistrationBean) {
-        super.addInitParameters(servletRegistrationBean);
-
-        servletRegistrationBean.addInitParameter(ApplicationConfig.JSR356_MAPPING_PATH, "/UI");
-        servletRegistrationBean.addInitParameter(ApplicationConfig.ATMOSPHERE_INTERCEPTORS,
-                SpringSecurityAtmosphereInterceptor.class.getName());
-
-    }
-}

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/MgmtUiConfiguration.java

@@ -16,8 +16,11 @@ import org.springframework.context.MessageSource;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
 import org.springframework.context.annotation.PropertySource;
+import org.vaadin.spring.servlet.Vaadin4SpringServlet;
+
+import com.vaadin.server.SystemMessagesProvider;
+import com.vaadin.server.VaadinServlet;
 
 /**
  * Enables UI components for the Management UI.
@@ -25,21 +28,61 @@ import org.springframework.context.annotation.PropertySource;
  */
 @Configuration
 @ComponentScan
-@Import(AsyncVaadinServletConfiguration.class)
 @EnableConfigurationProperties(UiProperties.class)
 @PropertySource("classpath:/hawkbit-ui-defaults.properties")
 public class MgmtUiConfiguration {
 
+    /**
+     * Permission checker for UI.
+     * 
+     * @param permissionService
+     *            PermissionService
+     *
+     * @return Permission checker for UI
+     */
     @Bean
     @ConditionalOnMissingBean
     SpPermissionChecker spPermissionChecker(final PermissionService permissionService) {
         return new SpPermissionChecker(permissionService);
     }
 
+    /**
+     * Utility for Vaadin messages source.
+     * 
+     * @param source
+     *            Delegate MessageSource
+     *
+     * @return Vaadin messages source utility
+     */
     @Bean
     @ConditionalOnMissingBean
     VaadinMessageSource messageSourceVaadin(final MessageSource source) {
         return new VaadinMessageSource(source);
     }
 
+    /**
+     * Localized system message provider bean.
+     * 
+     * @param uiProperties
+     *            UiProperties
+     * @param i18n
+     *            VaadinMessageSource
+     *
+     * @return Localized system message provider
+     */
+    @Bean
+    @ConditionalOnMissingBean
+    SystemMessagesProvider systemMessagesProvider(final UiProperties uiProperties, final VaadinMessageSource i18n) {
+        return new LocalizedSystemMessagesProvider(uiProperties, i18n);
+    }
+
+    /**
+     * Vaadin4Spring servlet bean.
+     *
+     * @return Vaadin servlet for Spring
+     */
+    @Bean
+    public VaadinServlet vaadinServlet() {
+        return new Vaadin4SpringServlet();
+    }
 }

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/SpringSecurityAtmosphereInterceptor.java

@@ -1,42 +0,0 @@
-/**
- * Copyright (c) 2015 Bosch Software Innovations GmbH and others.
- *
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- */
-package org.eclipse.hawkbit.ui;
-
-import org.atmosphere.config.service.AtmosphereInterceptorService;
-import org.atmosphere.cpr.Action;
-import org.atmosphere.cpr.AtmosphereInterceptor;
-import org.atmosphere.cpr.AtmosphereInterceptorAdapter;
-import org.atmosphere.cpr.AtmosphereResource;
-import org.springframework.security.core.context.SecurityContext;
-import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
-
-/**
- * An {@link AtmosphereInterceptor} implementation which retrieves the
- * {@link SecurityContext} from the http-session and set in into the
- * {@link SecurityContextHolder}. This is necessary due that websocket requests
- * are not going through the spring security filter chain and the
- * {@link SecurityContext} will not be present in the current Thread.
- */
-@AtmosphereInterceptorService
-public class SpringSecurityAtmosphereInterceptor extends AtmosphereInterceptorAdapter {
-
-    @Override
-    public Action inspect(final AtmosphereResource r) {
-        final SecurityContext context = (SecurityContext) r.getRequest().getSession()
-                .getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY);
-        SecurityContextHolder.setContext(context);
-        return Action.CONTINUE;
-    }
-
-    @Override
-    public void postInspect(final AtmosphereResource r) {
-        SecurityContextHolder.clearContext();
-    }
-}

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/common/grid/header/AbstractEntityGridHeader.java

@@ -91,7 +91,7 @@ public abstract class AbstractEntityGridHeader extends AbstractGridHeader {
         this.resizeHeaderSupport = new ResizeHeaderSupport(i18n, getMaxMinIconId(), this::maximizeTable,
                 this::minimizeTable, this::onLoadIsTableMaximized);
 
-        addHeaderSupports(Arrays.asList(getSearchHeaderSupport(), filterButtonsHeaderSupport, resizeHeaderSupport));
+        addHeaderSupports(Arrays.asList(searchHeaderSupport, filterButtonsHeaderSupport, resizeHeaderSupport));
     }
 
     @Override

hawkbit-ui/src/main/java/org/eclipse/hawkbit/ui/menu/DashboardMenu.java

@@ -219,8 +219,10 @@ public final class DashboardMenu extends CustomComponent {
 
         final String logoutUrl = generateLogoutUrl();
 
-        settingsItem.addItem(i18n.getMessage("label.sign.out"),
-                selectedItem -> Page.getCurrent().setLocation(logoutUrl));
+        settingsItem.addItem(i18n.getMessage("label.sign.out"), selectedItem -> {
+            getUI().getSession().close();
+            Page.getCurrent().setLocation(logoutUrl);
+        });
         return settings;
     }
 

hawkbit-ui/src/test/java/org/eclipse/hawkbit/push/SpringSecurityAtmosphereInterceptorTest.java

@@ -1,76 +0,0 @@
-/**
- * Copyright (c) 2015 Bosch Software Innovations GmbH and others.
- *
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the Eclipse Public License v1.0
- * which accompanies this distribution, and is available at
- * http://www.eclipse.org/legal/epl-v10.html
- */
-package org.eclipse.hawkbit.push;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.when;
-
-import javax.servlet.http.HttpSession;
-
-import org.atmosphere.cpr.AtmosphereRequest;
-import org.atmosphere.cpr.AtmosphereResource;
-import org.eclipse.hawkbit.ui.SpringSecurityAtmosphereInterceptor;
-import org.junit.After;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.runners.MockitoJUnitRunner;
-import org.springframework.security.core.context.SecurityContext;
-import org.springframework.security.core.context.SecurityContextHolder;
-import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
-
-import io.qameta.allure.Description;
-import io.qameta.allure.Feature;
-import io.qameta.allure.Story;
-
-@Feature("Unit Tests - Management UI")
-@Story("Push Security")
-@RunWith(MockitoJUnitRunner.class)
-public class SpringSecurityAtmosphereInterceptorTest {
-
-    @Mock
-    private AtmosphereResource atmosphereResourceMock;
-    @Mock
-    private AtmosphereRequest atmosphereRequestMock;
-    @Mock
-    private SecurityContext sessionSecurityContextMock;
-    @Mock
-    private HttpSession httpSessionMock;
-
-    private final SpringSecurityAtmosphereInterceptor underTest = new SpringSecurityAtmosphereInterceptor();
-
-    @After
-    public void after() {
-        SecurityContextHolder.clearContext();
-    }
-
-    @Test
-    @Description("Verify that Security Context is set from Request to thread local when calling inspect")
-    public void inspectRetrievesSetsSecurityContextFromRequestToThreadLocal() {
-
-        when(atmosphereResourceMock.getRequest()).thenReturn(atmosphereRequestMock);
-        when(atmosphereRequestMock.getSession()).thenReturn(httpSessionMock);
-        when(httpSessionMock.getAttribute(Mockito.eq(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY)))
-                .thenReturn(sessionSecurityContextMock);
-        underTest.inspect(atmosphereResourceMock);
-        // verify
-        assertThat(SecurityContextHolder.getContext()).isEqualTo(sessionSecurityContextMock);
-    }
-
-    @Test
-    @Description("Verify that security Context gets cleared after atmosphere request")
-    public void afterAtmosphereRequestSecurityContextGetsCleared() {
-        SecurityContextHolder.setContext(sessionSecurityContextMock);
-
-        underTest.postInspect(atmosphereResourceMock);
-
-        assertThat(SecurityContextHolder.getContext()).isNotEqualTo(sessionSecurityContextMock);
-    }
-}