From 9a2ed5273d281e77a6ba37f463b031a297b7985e Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Wed, 12 Apr 2023 11:43:34 +0200 Subject: [PATCH 01/29] Bump to 1.0.2-SNAPSHOT --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1ee276c1..d7875c32 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ eu.openanalytics containerproxy - 1.0.1 + 1.0.2-SNAPSHOT ContainerProxy jar From aed75c94d8b5805d8ad11ea4f0d3dda11c4998ab Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 11 May 2023 11:22:38 +0200 Subject: [PATCH 02/29] Fix #30568: prevent going back to IDP page --- .../impl/KeycloakAuthenticationBackend.java | 4 ++- .../impl/OpenIDAuthenticationBackend.java | 3 +- .../auth/impl/SAMLAuthenticationBackend.java | 3 +- .../security/WebSecurityConfig.java | 3 +- .../containerproxy/ui/AuthController.java | 9 +++++ .../resources/templates/auth-success.html | 34 +++++++++++++++++++ 6 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 src/main/resources/templates/auth-success.html diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java index ba06c71e..4e779f29 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java @@ -77,6 +77,8 @@ import java.util.Map; import java.util.stream.Collectors; +import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; + @Component public class KeycloakAuthenticationBackend implements IAuthenticationBackend { @@ -142,7 +144,7 @@ protected KeycloakAuthenticationProcessingFilter keycloakAuthenticationProcessin KeycloakAuthenticationProcessingFilter filter = new KeycloakAuthenticationProcessingFilter(authenticationManager, requestMatcher); filter.setSessionAuthenticationStrategy(sessionAuthenticationStrategy()); filter.setAuthenticationFailureHandler(keycloakAuthenticationFailureHandler()); - SimpleUrlAuthenticationSuccessHandler handler = new SimpleUrlAuthenticationSuccessHandler("/"); + SimpleUrlAuthenticationSuccessHandler handler = new SimpleUrlAuthenticationSuccessHandler(AUTH_SUCCESS_URL); handler.setAlwaysUseDefaultTargetUrl(true); filter.setAuthenticationSuccessHandler(handler); // Fix: call afterPropertiesSet manually, because Spring doesn't invoke it for some reason. diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java index 72f347e6..d2d3bb8e 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java @@ -77,6 +77,7 @@ import java.util.stream.Collectors; import static eu.openanalytics.containerproxy.auth.impl.oidc.OpenIDConfiguration.REG_ID; +import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; public class OpenIDAuthenticationBackend implements IAuthenticationBackend { @@ -119,7 +120,7 @@ public void configureHttpSecurity(HttpSecurity http, AuthorizedUrl anyRequestCon http .oauth2Login() .loginPage("/login") - .defaultSuccessUrl("/", true) + .defaultSuccessUrl(AUTH_SUCCESS_URL, true) .clientRegistrationRepository(clientRegistrationRepo) .authorizedClientService(oAuth2AuthorizedClientService) .authorizationEndpoint() diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java index 054d6ebf..537746b1 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java @@ -55,6 +55,7 @@ import static eu.openanalytics.containerproxy.auth.impl.saml.SAMLConfiguration.SAML_LOGOUT_SERVICE_LOCATION_PATH; import static eu.openanalytics.containerproxy.auth.impl.saml.SAMLConfiguration.SAML_LOGOUT_SERVICE_RESPONSE_LOCATION_PATH; import static eu.openanalytics.containerproxy.auth.impl.saml.SAMLConfiguration.SAML_SERVICE_LOCATION_PATH; +import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; @Component @ConditionalOnProperty(name = "proxy.authentication", havingValue = "saml") @@ -98,7 +99,7 @@ public void configureHttpSecurity(HttpSecurity http, AuthorizedUrl anyRequestCon .loginProcessingUrl(SAML_SERVICE_LOCATION_PATH) .authenticationManager(new ProviderManager(samlAuthenticationProvider)) .failureHandler(failureHandler) - .defaultSuccessUrl("/", true)) + .defaultSuccessUrl(AUTH_SUCCESS_URL, true)) .saml2Logout(saml -> saml .logoutUrl(SAML_LOGOUT_SERVICE_LOCATION_PATH) .logoutResponse(r -> r.logoutUrl(SAML_LOGOUT_SERVICE_RESPONSE_LOCATION_PATH)) diff --git a/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java b/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java index df0e16ce..1418f532 100644 --- a/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java +++ b/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java @@ -75,6 +75,7 @@ import java.util.List; import java.util.Set; +import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; import static eu.openanalytics.containerproxy.ui.TemplateResolverConfig.PROP_CORS_ALLOWED_ORIGINS; @Configuration @@ -232,7 +233,7 @@ public void handle(HttpServletRequest request, HttpServletResponse response, Acc http .formLogin() .loginPage("/login") - .defaultSuccessUrl("/", true) + .defaultSuccessUrl(AUTH_SUCCESS_URL, true) // TODO .and() .logout() .logoutUrl(auth.getLogoutURL()) diff --git a/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java b/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java index a7dd36e3..27f1b6d2 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java +++ b/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java @@ -50,6 +50,8 @@ @Controller public class AuthController extends BaseController { + public static final String AUTH_SUCCESS_URL = "/auth-success"; + @Inject private Environment environment; @@ -76,6 +78,13 @@ public Object getLoginPage(@RequestParam Optional error, ModelMap map) { } } + @RequestMapping(value = AUTH_SUCCESS_URL, method = RequestMethod.GET) + public String authSuccess(ModelMap map) { + prepareMap(map); + map.put("mainPage", ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString()); + return "auth-success"; + } + @RequestMapping(value = "/auth-error", method = RequestMethod.GET) public String getAuthErrorPage(ModelMap map) { prepareMap(map); diff --git a/src/main/resources/templates/auth-success.html b/src/main/resources/templates/auth-success.html new file mode 100644 index 00000000..614752e8 --- /dev/null +++ b/src/main/resources/templates/auth-success.html @@ -0,0 +1,34 @@ + + + + + + + + + From 2bdc9544f3c1d4ef76f9a7ba4ebec08c7c6c5d25 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 11 May 2023 11:39:17 +0200 Subject: [PATCH 03/29] Fix #30571: fix NPE in OpenIdReAuthorizeFilter --- .../auth/impl/oidc/OpenIdReAuthorizeFilter.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java index 8a953140..420779d0 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java @@ -109,8 +109,10 @@ protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull Ht * See {@link RefreshTokenOAuth2AuthorizedClientProvider} */ private boolean accessTokenExpired(OAuth2AuthorizedClient authorizedClient) { - return authorizedClient.getAccessToken().getExpiresAt() == null || - clock.instant().isAfter(authorizedClient.getAccessToken().getExpiresAt().minus(this.clockSkew)); + if (authorizedClient == null || authorizedClient.getAccessToken() == null || authorizedClient.getAccessToken().getExpiresAt() == null) { + return true; + } + return clock.instant().isAfter(authorizedClient.getAccessToken().getExpiresAt().minus(this.clockSkew)); } } From b488041f0daaed284d6fbc8a9d74783cf6382a5a Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 11 May 2023 12:30:28 +0200 Subject: [PATCH 04/29] Fix #330575: fix NPE in reActivateSession --- .../service/session/redis/RedisSessionService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/session/redis/RedisSessionService.java b/src/main/java/eu/openanalytics/containerproxy/service/session/redis/RedisSessionService.java index 1cbff535..2f1f241d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/session/redis/RedisSessionService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/session/redis/RedisSessionService.java @@ -90,8 +90,10 @@ public Integer getActiveUsersCount() { @Override public void reActivateSession(String sessionId) { - Session session = redisIndexedSessionRepository.findById(sessionId);//l.setLastAccessedTime - session.setLastAccessedTime(Instant.now()); + Session session = redisIndexedSessionRepository.findById(sessionId); + if (session != null) { + session.setLastAccessedTime(Instant.now()); + } } @Override From 3026c376791e88e24a256bbc463da192c452c7a2 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 15 May 2023 11:17:08 +0200 Subject: [PATCH 05/29] Fix #30569: fix refreshing of access tokens --- .../impl/oidc/OpenIdReAuthorizeFilter.java | 73 +++++++++++++------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java index 420779d0..ca5b8de6 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java @@ -30,7 +30,6 @@ import org.springframework.security.oauth2.client.OAuth2AuthorizedClientService; import org.springframework.security.oauth2.client.RefreshTokenOAuth2AuthorizedClientProvider; import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken; -import org.springframework.security.oauth2.core.OAuth2ErrorCodes; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -42,6 +41,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import java.io.IOException; import java.time.Clock; import java.time.Duration; @@ -50,19 +50,28 @@ /** * Ensures that the access token of the user is refreshed when needed. - * If the refresh token is invalid (e.g. because the session in the IdP expired), we throw a - * {@link ClientAuthorizationRequiredException} exception such that the user is can re-login. - * + * If the refresh token is invalid (e.g. because the session in the IdP expired), we first invalidate the session and then throw a + * {@link ClientAuthorizationRequiredException} exception such that the user can re-login. * This filter only applies to a limited set of requests and not to requests that are proxied to apps. - * Otherwise, this filter would be called too much and cause too much overhead. + * Otherwise, this filter would be called too much and cause too much overhead. In addition, this filter should + * only be used on non-ajax requests. This is required for the redirect to the IDP to properly work. + * + * A special case is the /refresh-openid endpoint, which does not throw the exception (i.e. it does not cause a redirect to the IDP) + * but only invalidates the session. This endpoint is frequently called (at least every <40 seconds) when the user is using an app. + * It refreshes the OIDC session as long as possible and when it fails (e.g. because the session was revoked or reached it max life), + * the session is invalidated. The app page detects this and shows a message that the user was logged out. + * + * See #30569, #28976 */ public class OpenIdReAuthorizeFilter extends OncePerRequestFilter { + private static final RequestMatcher REFRESH_OPENID_MATCHER = new AntPathRequestMatcher("/refresh-openid"); + private static final RequestMatcher REQUEST_MATCHER = new OrRequestMatcher( new AntPathRequestMatcher("/app/**"), new AntPathRequestMatcher("/app_i/**"), new AntPathRequestMatcher("/"), - new AntPathRequestMatcher("/heartbeat")); + REFRESH_OPENID_MATCHER); @Inject private OAuth2AuthorizedClientManager oAuth2AuthorizedClientManager; @@ -72,8 +81,8 @@ public class OpenIdReAuthorizeFilter extends OncePerRequestFilter { private final Clock clock = Clock.systemUTC(); - // use clock skew of 20 seconds instead of 60 seconds. Otherwise, if the access token is valid for 1 minute, it would get refreshed at each request. - private final Duration clockSkew = Duration.ofSeconds(20); + // use clock skew of 40 seconds instead of 60 seconds. Otherwise, if the access token is valid for 1 minute, it would get refreshed at each request. + private final Duration clockSkew = Duration.ofSeconds(40); @Override protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull HttpServletResponse response, @Nonnull FilterChain chain) throws ServletException, IOException { @@ -83,25 +92,32 @@ protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull Ht if (auth instanceof OAuth2AuthenticationToken) { OAuth2AuthorizedClient authorizedClient = oAuth2AuthorizedClientService.loadAuthorizedClient(REG_ID, auth.getName()); - if (accessTokenExpired(authorizedClient)) { - OAuth2AuthorizeRequest authorizeRequest = OAuth2AuthorizeRequest - .withAuthorizedClient(authorizedClient) - .principal(auth) - .build(); - - // re-authorize - try { - oAuth2AuthorizedClientManager.authorize(authorizeRequest); - } catch (ClientAuthorizationException ex) { - if (ex.getError().getErrorCode().equals(OAuth2ErrorCodes.INVALID_GRANT)) { - // if refresh token has expired or is invalid -> re-start authorization process - throw new ClientAuthorizationRequiredException(ex.getClientRegistrationId()); + if (authorizedClient == null) { + invalidateSession(request, response); + return; + } else { + if (accessTokenExpired(authorizedClient)) { + OAuth2AuthorizeRequest authorizeRequest = OAuth2AuthorizeRequest + .withAuthorizedClient(authorizedClient) + .principal(auth) + .build(); + + try { + oAuth2AuthorizedClientManager.authorize(authorizeRequest); + logger.info("Refresh"); + } catch (ClientAuthorizationException ex) { + invalidateSession(request, response); + return; } - throw ex; } } } } + if (REFRESH_OPENID_MATCHER.matches(request)) { + response.getWriter().write("{\"status\":\"success\"}"); + response.setStatus(200); + return; + } chain.doFilter(request, response); } @@ -115,4 +131,17 @@ private boolean accessTokenExpired(OAuth2AuthorizedClient authorizedClient) { return clock.instant().isAfter(authorizedClient.getAccessToken().getExpiresAt().minus(this.clockSkew)); } + private void invalidateSession(@Nonnull HttpServletRequest request, @Nonnull HttpServletResponse response) throws IOException { + HttpSession session = request.getSession(false); + if (session != null) { + session.invalidate(); + } + if (REFRESH_OPENID_MATCHER.matches(request)) { + response.getWriter().write("{\"status\":\"success\"}"); + response.setStatus(200); + } else { + throw new ClientAuthorizationRequiredException(REG_ID); + } + } + } From 05fe6e742f6bdd4601d099c84db22e61789c1a61 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 15 May 2023 12:12:44 +0200 Subject: [PATCH 06/29] Fix 30646: add option to disable logout on OIDC session expire --- .../impl/oidc/OpenIdReAuthorizeFilter.java | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java index ca5b8de6..dc67c9e6 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java @@ -20,6 +20,7 @@ */ package eu.openanalytics.containerproxy.auth.impl.oidc; +import org.springframework.core.env.Environment; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.oauth2.client.ClientAuthorizationException; @@ -36,6 +37,7 @@ import org.springframework.web.filter.OncePerRequestFilter; import javax.annotation.Nonnull; +import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.servlet.FilterChain; import javax.servlet.ServletException; @@ -79,11 +81,21 @@ public class OpenIdReAuthorizeFilter extends OncePerRequestFilter { @Inject private OAuth2AuthorizedClientService oAuth2AuthorizedClientService; + @Inject + private Environment environment; + private final Clock clock = Clock.systemUTC(); // use clock skew of 40 seconds instead of 60 seconds. Otherwise, if the access token is valid for 1 minute, it would get refreshed at each request. private final Duration clockSkew = Duration.ofSeconds(40); + private boolean ignoreLogout; + + @PostConstruct + public void init() { + ignoreLogout = environment.getProperty("proxy.openid.ignore-session-expire", Boolean.class, false); + } + @Override protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull HttpServletResponse response, @Nonnull FilterChain chain) throws ServletException, IOException { if (REQUEST_MATCHER.matches(request)) { @@ -93,8 +105,10 @@ protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull Ht OAuth2AuthorizedClient authorizedClient = oAuth2AuthorizedClientService.loadAuthorizedClient(REG_ID, auth.getName()); if (authorizedClient == null) { - invalidateSession(request, response); - return; + if (!ignoreLogout) { + invalidateSession(request, response, auth); + return; + } } else { if (accessTokenExpired(authorizedClient)) { OAuth2AuthorizeRequest authorizeRequest = OAuth2AuthorizeRequest @@ -104,10 +118,14 @@ protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull Ht try { oAuth2AuthorizedClientManager.authorize(authorizeRequest); - logger.info("Refresh"); + logger.debug(String.format("OpenID access token refreshed [user: %s]", auth.getName())); } catch (ClientAuthorizationException ex) { - invalidateSession(request, response); - return; + if (!ignoreLogout) { + invalidateSession(request, response, auth); + return; + } else { + logger.debug(String.format("OpenID access token expired, internal session stays active [user: %s]", auth.getName())); + } } } } @@ -131,7 +149,8 @@ private boolean accessTokenExpired(OAuth2AuthorizedClient authorizedClient) { return clock.instant().isAfter(authorizedClient.getAccessToken().getExpiresAt().minus(this.clockSkew)); } - private void invalidateSession(@Nonnull HttpServletRequest request, @Nonnull HttpServletResponse response) throws IOException { + private void invalidateSession(@Nonnull HttpServletRequest request, @Nonnull HttpServletResponse response, Authentication auth) throws IOException { + logger.debug(String.format("OpenID access token expired, invalidating internal session [user: %s]", auth.getName())); HttpSession session = request.getSession(false); if (session != null) { session.invalidate(); From ecb40a3c625db39ca46bb2c0db0fc0ad7d4b95ab Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 15 May 2023 12:12:56 +0200 Subject: [PATCH 07/29] Stop apps on http session destroy --- .../eu/openanalytics/containerproxy/service/ProxyService.java | 4 ++-- .../eu/openanalytics/containerproxy/service/UserService.java | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/ProxyService.java b/src/main/java/eu/openanalytics/containerproxy/service/ProxyService.java index 6eca4545..4700ca7d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/ProxyService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/ProxyService.java @@ -301,7 +301,7 @@ public Command stopProxy(Authentication user, Proxy proxy, boolean ignoreAccessC throw new AccessDeniedException(String.format("Cannot stop proxy %s: access denied", proxy.getId())); } - if (user != null) { + if (user != null && !ignoreAccessControl) { log.info(proxy, "Proxy being stopped by user " + UserService.getUserId(user)); } @@ -341,7 +341,7 @@ public Command pauseProxy(Authentication user, Proxy proxy, boolean ignoreAccess throw new IllegalArgumentException("Trying to pause a proxy when the backend does not support pausing apps"); } - if (user != null) { + if (user != null && !ignoreAccessControl) { log.info(proxy, "Proxy being paused by user " + UserService.getUserId(user)); } diff --git a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java index b291548b..61c20593 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java @@ -221,6 +221,7 @@ public void onHttpSessionDestroyedEvent(HttpSessionDestroyedEvent event) { if (securityContext == null) return; String userId = getUserId(securityContext.getAuthentication()); + if (logoutStrategy != null) logoutStrategy.onLogout(userId); log.info(String.format("User logged out [user: %s]", userId)); applicationEventPublisher.publishEvent(new UserLogoutEvent( @@ -230,6 +231,8 @@ public void onHttpSessionDestroyedEvent(HttpSessionDestroyedEvent event) { )); } else if (authBackend.getName().equals("none")) { String userId = Sha256.hash(event.getSession().getId()); + if (logoutStrategy != null) logoutStrategy.onLogout(userId); + log.info(String.format("Anonymous user logged out [user: %s]", userId)); applicationEventPublisher.publishEvent(new UserLogoutEvent( this, From f0b0247a6fdc5dcc87efd4b0e55c3fbcc456bff7 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 15 May 2023 14:45:19 +0200 Subject: [PATCH 08/29] Fix #30648: redirect to app after login --- .../impl/KeycloakAuthenticationBackend.java | 12 +++---- .../impl/OpenIDAuthenticationBackend.java | 13 +++++--- .../auth/impl/SAMLAuthenticationBackend.java | 8 ++++- .../security/WebSecurityConfig.java | 15 ++++++++- .../containerproxy/ui/AuthController.java | 31 ++++++++++--------- .../resources/templates/auth-success.html | 3 +- 6 files changed, 54 insertions(+), 28 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java index 4e779f29..ab303ebf 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java @@ -54,7 +54,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.session.SessionRegistryImpl; import org.springframework.security.web.AuthenticationEntryPoint; -import org.springframework.security.web.authentication.SimpleUrlAuthenticationSuccessHandler; +import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; import org.springframework.security.web.authentication.logout.LogoutFilter; import org.springframework.security.web.authentication.session.ChangeSessionIdAuthenticationStrategy; import org.springframework.security.web.authentication.session.CompositeSessionAuthenticationStrategy; @@ -77,8 +77,6 @@ import java.util.Map; import java.util.stream.Collectors; -import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; - @Component public class KeycloakAuthenticationBackend implements IAuthenticationBackend { @@ -94,6 +92,10 @@ public class KeycloakAuthenticationBackend implements IAuthenticationBackend { @Lazy AuthenticationManager authenticationManager; + @Inject + @Lazy + private SavedRequestAwareAuthenticationSuccessHandler successHandler; + @Override public String getName() { return NAME; @@ -144,9 +146,7 @@ protected KeycloakAuthenticationProcessingFilter keycloakAuthenticationProcessin KeycloakAuthenticationProcessingFilter filter = new KeycloakAuthenticationProcessingFilter(authenticationManager, requestMatcher); filter.setSessionAuthenticationStrategy(sessionAuthenticationStrategy()); filter.setAuthenticationFailureHandler(keycloakAuthenticationFailureHandler()); - SimpleUrlAuthenticationSuccessHandler handler = new SimpleUrlAuthenticationSuccessHandler(AUTH_SUCCESS_URL); - handler.setAlwaysUseDefaultTargetUrl(true); - filter.setAuthenticationSuccessHandler(handler); + filter.setAuthenticationSuccessHandler(successHandler); // Fix: call afterPropertiesSet manually, because Spring doesn't invoke it for some reason. filter.setApplicationContext(ctx); filter.afterPropertiesSet(); diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java index d2d3bb8e..6617d2d4 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/OpenIDAuthenticationBackend.java @@ -31,6 +31,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; import org.springframework.core.env.Environment; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -58,6 +59,7 @@ import org.springframework.security.oauth2.core.oidc.user.OidcUser; import org.springframework.security.oauth2.core.oidc.user.OidcUserAuthority; import org.springframework.security.web.authentication.AuthenticationFailureHandler; +import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.logout.LogoutSuccessHandler; import org.springframework.security.web.authentication.logout.SimpleUrlLogoutSuccessHandler; @@ -77,7 +79,6 @@ import java.util.stream.Collectors; import static eu.openanalytics.containerproxy.auth.impl.oidc.OpenIDConfiguration.REG_ID; -import static eu.openanalytics.containerproxy.ui.AuthController.AUTH_SUCCESS_URL; public class OpenIDAuthenticationBackend implements IAuthenticationBackend { @@ -93,6 +94,10 @@ public class OpenIDAuthenticationBackend implements IAuthenticationBackend { @Inject private ClientRegistrationRepository clientRegistrationRepo; + @Inject + @Lazy + private SavedRequestAwareAuthenticationSuccessHandler successHandler; + private static OAuth2AuthorizedClientService oAuth2AuthorizedClientService; @Autowired @@ -116,11 +121,11 @@ public boolean hasAuthorization() { @Override public void configureHttpSecurity(HttpSecurity http, AuthorizedUrl anyRequestConfigurer) throws Exception { anyRequestConfigurer.authenticated(); - + http .oauth2Login() .loginPage("/login") - .defaultSuccessUrl(AUTH_SUCCESS_URL, true) + .successHandler(successHandler) .clientRegistrationRepository(clientRegistrationRepo) .authorizedClientService(oAuth2AuthorizedClientService) .authorizationEndpoint() @@ -141,7 +146,7 @@ public void onAuthenticationFailure(HttpServletRequest request, HttpServletRespo .oidcUserService(createOidcUserService()) .and() .and() - .addFilterAfter(openIdReAuthorizeFilter, UsernamePasswordAuthenticationFilter.class); + .addFilterAfter(openIdReAuthorizeFilter, UsernamePasswordAuthenticationFilter.class); } private OAuth2AuthorizationRequestResolver authorizationRequestResolver() { diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java index 537746b1..a1426607 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SAMLAuthenticationBackend.java @@ -26,6 +26,7 @@ import eu.openanalytics.containerproxy.util.ContextPathHelper; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Lazy; import org.springframework.core.env.Environment; import org.springframework.security.authentication.ProviderManager; import org.springframework.security.config.annotation.ObjectPostProcessor; @@ -40,6 +41,7 @@ import org.springframework.security.saml2.provider.service.registration.RelyingPartyRegistrationRepository; import org.springframework.security.saml2.provider.service.servlet.filter.Saml2WebSsoAuthenticationFilter; import org.springframework.security.saml2.provider.service.web.authentication.logout.Saml2LogoutRequestResolver; +import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; import org.springframework.security.web.authentication.logout.LogoutFilter; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; @@ -76,6 +78,10 @@ public class SAMLAuthenticationBackend implements IAuthenticationBackend { @Inject private Saml2LogoutRequestResolver saml2LogoutRequestResolver; + @Inject + @Lazy + private SavedRequestAwareAuthenticationSuccessHandler successHandler; + @Override public String getName() { return NAME; @@ -99,7 +105,7 @@ public void configureHttpSecurity(HttpSecurity http, AuthorizedUrl anyRequestCon .loginProcessingUrl(SAML_SERVICE_LOCATION_PATH) .authenticationManager(new ProviderManager(samlAuthenticationProvider)) .failureHandler(failureHandler) - .defaultSuccessUrl(AUTH_SUCCESS_URL, true)) + .successHandler(successHandler)) .saml2Logout(saml -> saml .logoutUrl(SAML_LOGOUT_SERVICE_LOCATION_PATH) .logoutResponse(r -> r.logoutUrl(SAML_LOGOUT_SERVICE_RESPONSE_LOCATION_PATH)) diff --git a/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java b/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java index 1418f532..4bb5ac06 100644 --- a/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java +++ b/src/main/java/eu/openanalytics/containerproxy/security/WebSecurityConfig.java @@ -34,6 +34,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Lazy; import org.springframework.core.convert.converter.Converter; import org.springframework.core.env.Environment; import org.springframework.security.access.AccessDeniedException; @@ -57,6 +58,7 @@ import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken; import org.springframework.security.web.access.AccessDeniedHandler; import org.springframework.security.web.access.AccessDeniedHandlerImpl; +import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.security.web.csrf.MissingCsrfTokenException; import org.springframework.security.web.header.Header; @@ -102,6 +104,10 @@ public class WebSecurityConfig extends WebSecurityConfigurerAdapter { @Autowired(required=false) private List customConfigs; + @Inject + @Lazy + private SavedRequestAwareAuthenticationSuccessHandler successHandler; + public static final String PROP_DISABLE_NO_SNIFF_HEADER = "proxy.api-security.disable-no-sniff-header"; public static final String PROP_DISABLE_HSTS_HEADER = "proxy.api-security.disable-hsts-header"; public static final String PROP_DISABLE_XSS_PROTECTION_HEADER = "proxy.api-security.disable-xss-protection-header"; @@ -233,7 +239,7 @@ public void handle(HttpServletRequest request, HttpServletResponse response, Acc http .formLogin() .loginPage("/login") - .defaultSuccessUrl(AUTH_SUCCESS_URL, true) // TODO + .successHandler(successHandler) .and() .logout() .logoutUrl(auth.getLogoutURL()) @@ -323,6 +329,13 @@ public AuthenticationManager authenticationManagerBean() throws Exception { return super.authenticationManagerBean(); } + @Bean + public SavedRequestAwareAuthenticationSuccessHandler SavedRequestAwareAuthenticationSuccessHandler() { + SavedRequestAwareAuthenticationSuccessHandler savedRequestAwareAuthenticationSuccessHandler = new SavedRequestAwareAuthenticationSuccessHandler(); + savedRequestAwareAuthenticationSuccessHandler.setDefaultTargetUrl(AUTH_SUCCESS_URL); + return savedRequestAwareAuthenticationSuccessHandler; + } + private List
getCustomHeaders() { List
headers = new ArrayList<>(); diff --git a/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java b/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java index 27f1b6d2..802b82f3 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java +++ b/src/main/java/eu/openanalytics/containerproxy/ui/AuthController.java @@ -20,37 +20,28 @@ */ package eu.openanalytics.containerproxy.ui; -import javax.inject.Inject; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - +import eu.openanalytics.containerproxy.api.BaseController; import eu.openanalytics.containerproxy.auth.IAuthenticationBackend; import eu.openanalytics.containerproxy.auth.impl.OpenIDAuthenticationBackend; import eu.openanalytics.containerproxy.auth.impl.SAMLAuthenticationBackend; -import eu.openanalytics.containerproxy.auth.impl.saml.SAMLConfiguration; import org.springframework.core.env.Environment; -import org.springframework.http.HttpHeaders; -import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; - -import eu.openanalytics.containerproxy.api.BaseController; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.servlet.support.ServletUriComponentsBuilder; import org.springframework.web.servlet.view.RedirectView; -import java.io.IOException; -import java.net.URLEncoder; -import java.nio.charset.StandardCharsets; +import javax.inject.Inject; +import javax.servlet.http.HttpServletRequest; import java.util.Optional; @Controller public class AuthController extends BaseController { public static final String AUTH_SUCCESS_URL = "/auth-success"; + public static final String AUTH_SUCCESS_URL_SESSION_ATTR = "AUTH_SUCCESS_URL_SESSION_ATTR"; @Inject private Environment environment; @@ -79,9 +70,19 @@ public Object getLoginPage(@RequestParam Optional error, ModelMap map) { } @RequestMapping(value = AUTH_SUCCESS_URL, method = RequestMethod.GET) - public String authSuccess(ModelMap map) { + public String authSuccess(ModelMap map, HttpServletRequest request) { prepareMap(map); - map.put("mainPage", ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString()); + map.put("url", ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString()); // default url + + Object redirectUrl = request.getSession().getAttribute(AUTH_SUCCESS_URL_SESSION_ATTR); + if (redirectUrl instanceof String) { + request.getSession().removeAttribute(AUTH_SUCCESS_URL_SESSION_ATTR); + String sRedirectUrl = (String) redirectUrl; + // sanity check: does the redirect url start with the url of this current request + if (sRedirectUrl.startsWith(ServletUriComponentsBuilder.fromCurrentContextPath().build().toUriString())) { + map.put("url", redirectUrl); + } + } return "auth-success"; } diff --git a/src/main/resources/templates/auth-success.html b/src/main/resources/templates/auth-success.html index 614752e8..a2cdf891 100644 --- a/src/main/resources/templates/auth-success.html +++ b/src/main/resources/templates/auth-success.html @@ -28,7 +28,8 @@ + Test From 4f330080875e74981f8e9ec6313c8e01364bd1ac Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 13 Jun 2023 10:01:20 +0200 Subject: [PATCH 09/29] Improve swagger docs Hides the duplicate methods for the proxy endpoints. --- .../ContainerProxyApplication.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java index febeee85..bb2c1767 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java +++ b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java @@ -37,6 +37,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.bouncycastle.jce.provider.BouncyCastleProvider; +import org.springdoc.core.GroupedOpenApi; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; import org.springframework.boot.actuate.health.Health; @@ -71,9 +72,12 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.security.Security; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Properties; +import java.util.Set; import java.util.concurrent.Executor; import static eu.openanalytics.containerproxy.api.ApiSecurityService.PROP_API_SECURITY_HIDE_SPEC_DETAILS; @@ -345,4 +349,24 @@ private static void setDefaultProperties(SpringApplication app) { System.setProperty("jdk.serialSetFilterAfterRead", "true"); } + @Bean + public GroupedOpenApi groupOpenApi() { + return GroupedOpenApi.builder() + .group("v1") + .addOpenApiCustomiser(openApi -> { + Set endpoints = new HashSet<>(Arrays.asList("/app_direct_i/**", "/app_direct/**", "/app_proxy/{proxyId}/**", "/error")); + openApi.getPaths().entrySet().stream().filter(p -> endpoints.contains(p.getKey())) + .forEach(p -> { + p.getValue().setHead(null); + p.getValue().setPost(null); + p.getValue().setDelete(null); + p.getValue().setParameters(null); + p.getValue().setOptions(null); + p.getValue().setPut(null); + p.getValue().setPatch(null); + }); + }) + .build(); + } + } From 75b1b1de78e54b936d1138d435e64f8424c42d4d Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 13 Jun 2023 17:03:28 +0200 Subject: [PATCH 10/29] Disable Github Social for now --- pom.xml | 30 +++++++++---------- .../security/SocialSecurityConfig.java | 7 +++-- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index d7875c32..00e6ff7f 100644 --- a/pom.xml +++ b/pom.xml @@ -58,16 +58,16 @@ clojars https://clojars.org/repo/ - - - shibboleth - https://build.shibboleth.net/nexus/content/repositories/releases/ - - - - spring - https://repo.spring.io/plugins-release/ - + + + + + + + + + + @@ -212,11 +212,11 @@ spring-social-linkedin 1.0.2.RELEASE - - org.springframework.social - spring-social-github - 1.0.0.M4 - + + + + + com.github.spring-social spring-social-google diff --git a/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java b/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java index 5cfa983d..cc3e563d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java +++ b/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java @@ -58,7 +58,8 @@ import org.springframework.social.connect.web.ProviderSignInController; import org.springframework.social.connect.web.SignInAdapter; import org.springframework.social.facebook.connect.FacebookConnectionFactory; -import org.springframework.social.github.connect.GitHubConnectionFactory; +//import org.springframework.social.github.connect.GitHubConnectionFactory; +//import org.springframework.social.github.connect.GitHubConnectionFactory; import org.springframework.social.google.connect.GoogleConnectionFactory; import org.springframework.social.linkedin.connect.LinkedInConnectionFactory; import org.springframework.social.twitter.connect.TwitterConnectionFactory; @@ -152,8 +153,8 @@ public ConnectionFactory createConnectionFactory(String appId, String appSecr return factory; case linkedin: return new LinkedInConnectionFactory(appId, appSecret); - case github: - return new GitHubConnectionFactory(appId, appSecret); +//// case github: +// return new GitHubConnectionFactory(appId, appSecret); default: return null; } From 21b6f9de67da6522c16b94adad60b1549a2f9b1c Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 3 Jul 2023 16:55:46 +0200 Subject: [PATCH 11/29] Ref #30568: prevent issues with back button and openid --- src/main/resources/templates/auth-error.html | 3 +++ src/main/resources/templates/auth-success.html | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/main/resources/templates/auth-error.html b/src/main/resources/templates/auth-error.html index 12e7fe0f..e4f9ff6e 100644 --- a/src/main/resources/templates/auth-error.html +++ b/src/main/resources/templates/auth-error.html @@ -28,6 +28,9 @@ + diff --git a/src/main/resources/templates/auth-success.html b/src/main/resources/templates/auth-success.html index a2cdf891..27e7132e 100644 --- a/src/main/resources/templates/auth-success.html +++ b/src/main/resources/templates/auth-success.html @@ -27,7 +27,9 @@ From 7bc9b2cd0386f456830a7a7ac48d1f780e3bf631 Mon Sep 17 00:00:00 2001 From: Lucianos Lionakis Date: Mon, 3 Jul 2023 17:09:03 +0200 Subject: [PATCH 12/29] Add mobile viewport meta tags to template files --- src/main/resources/templates/app-access-denied.html | 1 + src/main/resources/templates/auth-error.html | 3 ++- src/main/resources/templates/auth-success.html | 1 + src/main/resources/templates/error.html | 1 + src/main/resources/templates/login.html | 1 + src/main/resources/templates/logout-success.html | 1 + src/main/resources/templates/startup.html | 1 + 7 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/resources/templates/app-access-denied.html b/src/main/resources/templates/app-access-denied.html index 9a10d2e3..c9b42d42 100644 --- a/src/main/resources/templates/app-access-denied.html +++ b/src/main/resources/templates/app-access-denied.html @@ -28,6 +28,7 @@ + diff --git a/src/main/resources/templates/auth-error.html b/src/main/resources/templates/auth-error.html index e4f9ff6e..f7398a10 100644 --- a/src/main/resources/templates/auth-error.html +++ b/src/main/resources/templates/auth-error.html @@ -28,6 +28,7 @@ + @@ -51,7 +52,7 @@

An error occurred during the authentication procedure.

margin-top: 20px; } - + diff --git a/src/main/resources/templates/auth-success.html b/src/main/resources/templates/auth-success.html index 27e7132e..f4679341 100644 --- a/src/main/resources/templates/auth-success.html +++ b/src/main/resources/templates/auth-success.html @@ -26,6 +26,7 @@ + diff --git a/src/main/resources/templates/login.html b/src/main/resources/templates/login.html index 9717aca6..697f25f7 100644 --- a/src/main/resources/templates/login.html +++ b/src/main/resources/templates/login.html @@ -28,6 +28,7 @@ + diff --git a/src/main/resources/templates/logout-success.html b/src/main/resources/templates/logout-success.html index 7b448f5d..dde4bf30 100644 --- a/src/main/resources/templates/logout-success.html +++ b/src/main/resources/templates/logout-success.html @@ -28,6 +28,7 @@ + diff --git a/src/main/resources/templates/startup.html b/src/main/resources/templates/startup.html index e7d7f1e4..1af5e1e2 100644 --- a/src/main/resources/templates/startup.html +++ b/src/main/resources/templates/startup.html @@ -25,6 +25,7 @@ ${application_name} +
From 9e394d0fee83721a611ec7eed55061df2f0a952a Mon Sep 17 00:00:00 2001 From: Lucianos Lionakis Date: Tue, 4 Jul 2023 14:12:42 +0200 Subject: [PATCH 13/29] add support for making users admin --- .../containerproxy/service/UserService.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java index 61c20593..c8b4fe7f 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java @@ -103,6 +103,22 @@ public String[] getAdminGroups() { return adminGroups.toArray(new String[adminGroups.size()]); } + + public String[] getAdminUsers() { + Set adminUsers = new HashSet<>(); + + // Support for old, non-array notation + String singleUser = environment.getProperty("proxy.admin-users"); + if (singleUser != null && !singleUser.isEmpty()) adminUsers.add(singleUser); + + for (int i=0 ;; i++) { + String userName = environment.getProperty(String.format("proxy.admin-users[%s]", i)); + if (userName == null || userName.isEmpty()) break; + adminUsers.add(userName); + } + + return adminUsers.toArray(new String[adminUsers.size()]); + } public String[] getGroups() { return getGroups(getCurrentAuth()); @@ -125,9 +141,18 @@ public boolean isAdmin() { } public boolean isAdmin(Authentication auth) { + if (!authBackend.hasAuthorization()) { + return false; + } + for (String adminGroup: getAdminGroups()) { if (isMember(auth, adminGroup)) return true; } + + String userName = getUserId(auth); + for (String adminUser: getAdminUsers()) { + if (userName != null && userName.equals(adminUser)) return true; + } return false; } From 948cc92f27abe8de0a4038d1ec5110da544a704e Mon Sep 17 00:00:00 2001 From: Lucianos Lionakis Date: Tue, 4 Jul 2023 14:36:12 +0200 Subject: [PATCH 14/29] Add accolades to relevant if statemets --- .../containerproxy/service/UserService.java | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java index c8b4fe7f..f669e69d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java @@ -93,11 +93,15 @@ public String[] getAdminGroups() { // Support for old, non-array notation String singleGroup = environment.getProperty("proxy.admin-groups"); - if (singleGroup != null && !singleGroup.isEmpty()) adminGroups.add(singleGroup.toUpperCase()); + if (singleGroup != null && !singleGroup.isEmpty()) { + adminGroups.add(singleGroup.toUpperCase()); + } for (int i=0 ;; i++) { String groupName = environment.getProperty(String.format("proxy.admin-groups[%s]", i)); - if (groupName == null || groupName.isEmpty()) break; + if (groupName == null || groupName.isEmpty()) { + break; + } adminGroups.add(groupName.toUpperCase()); } @@ -109,11 +113,15 @@ public String[] getAdminUsers() { // Support for old, non-array notation String singleUser = environment.getProperty("proxy.admin-users"); - if (singleUser != null && !singleUser.isEmpty()) adminUsers.add(singleUser); + if (singleUser != null && !singleUser.isEmpty()) { + adminUsers.add(singleUser); + } for (int i=0 ;; i++) { String userName = environment.getProperty(String.format("proxy.admin-users[%s]", i)); - if (userName == null || userName.isEmpty()) break; + if (userName == null || userName.isEmpty()) { + break; + } adminUsers.add(userName); } @@ -146,12 +154,16 @@ public boolean isAdmin(Authentication auth) { } for (String adminGroup: getAdminGroups()) { - if (isMember(auth, adminGroup)) return true; + if (isMember(auth, adminGroup)) { + return true; + } } String userName = getUserId(auth); for (String adminUser: getAdminUsers()) { - if (userName != null && userName.equals(adminUser)) return true; + if (userName != null && userName.equals(adminUser)) { + return true; + } } return false; } From 2051f1498e934c572ac3d8c8424f16d4f839519e Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 4 Jul 2023 15:32:02 +0200 Subject: [PATCH 15/29] Refactor admin user and group handling --- .../service/RuntimeValueService.java | 3 ++- .../containerproxy/service/UserService.java | 18 +++++++++--------- .../spec/expression/SpecExpressionContext.java | 2 +- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/RuntimeValueService.java b/src/main/java/eu/openanalytics/containerproxy/service/RuntimeValueService.java index 6e0a62f3..431c32b6 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/RuntimeValueService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/RuntimeValueService.java @@ -53,6 +53,7 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; +import java.util.List; import java.util.Map; import java.util.Optional; @@ -103,7 +104,7 @@ public Proxy addRuntimeValuesBeforeSpel(Authentication user, ProxySpec spec, Pro proxyBuilder.addRuntimeValue(new RuntimeValue(RealmIdKey.inst, identifierService.realmId), false); } proxyBuilder.addRuntimeValue(new RuntimeValue(UserIdKey.inst, proxy.getUserId()), false); - String[] groups = UserService.getGroups(user); + List groups = UserService.getGroups(user); proxyBuilder.addRuntimeValue(new RuntimeValue(UserGroupsKey.inst, String.join(",", groups)), true); proxyBuilder.addRuntimeValue(new RuntimeValue(CreatedTimestampKey.inst, Long.toString(proxy.getCreatedTimestamp())), false); diff --git a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java index f669e69d..8ebbf2e9 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java @@ -88,7 +88,7 @@ public String getCurrentUserId() { return getUserId(getCurrentAuth()); } - public String[] getAdminGroups() { + public Set getAdminGroups() { Set adminGroups = new HashSet<>(); // Support for old, non-array notation @@ -105,10 +105,10 @@ public String[] getAdminGroups() { adminGroups.add(groupName.toUpperCase()); } - return adminGroups.toArray(new String[adminGroups.size()]); + return adminGroups; } - public String[] getAdminUsers() { + public Set getAdminUsers() { Set adminUsers = new HashSet<>(); // Support for old, non-array notation @@ -125,14 +125,14 @@ public String[] getAdminUsers() { adminUsers.add(userName); } - return adminUsers.toArray(new String[adminUsers.size()]); + return adminUsers; } - public String[] getGroups() { + public List getGroups() { return getGroups(getCurrentAuth()); } - public static String[] getGroups(Authentication auth) { + public static List getGroups(Authentication auth) { List groups = new ArrayList<>(); if (auth != null) { for (GrantedAuthority grantedAuth: auth.getAuthorities()) { @@ -141,7 +141,7 @@ public static String[] getGroups(Authentication auth) { groups.add(authName); } } - return groups.toArray(new String[groups.size()]); + return groups; } public boolean isAdmin() { @@ -149,7 +149,7 @@ public boolean isAdmin() { } public boolean isAdmin(Authentication auth) { - if (!authBackend.hasAuthorization()) { + if (!authBackend.hasAuthorization() || auth == null) { return false; } @@ -161,7 +161,7 @@ public boolean isAdmin(Authentication auth) { String userName = getUserId(auth); for (String adminUser: getAdminUsers()) { - if (userName != null && userName.equals(adminUser)) { + if (userName != null && userName.equalsIgnoreCase(adminUser)) { return true; } } diff --git a/src/main/java/eu/openanalytics/containerproxy/spec/expression/SpecExpressionContext.java b/src/main/java/eu/openanalytics/containerproxy/spec/expression/SpecExpressionContext.java index 141eb9f4..c755c89b 100644 --- a/src/main/java/eu/openanalytics/containerproxy/spec/expression/SpecExpressionContext.java +++ b/src/main/java/eu/openanalytics/containerproxy/spec/expression/SpecExpressionContext.java @@ -135,7 +135,7 @@ public static SpecExpressionContext create(SpecExpressionContextBuilder builder, builder.ldapUser = (LdapUserDetails) o; } if (o instanceof Authentication) { - builder.groups = Arrays.asList(UserService.getGroups((Authentication) o)); + builder.groups = UserService.getGroups((Authentication) o); builder.userId = UserService.getUserId(((Authentication) o)); } } From fa2ba1c9094d45b789d1fac4a0d17a5ceda0a4b8 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 4 Jul 2023 15:35:20 +0200 Subject: [PATCH 16/29] Read admin-users and groups property only once --- .../containerproxy/service/UserService.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java index 8ebbf2e9..c56f27ca 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/UserService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/UserService.java @@ -48,6 +48,7 @@ import org.springframework.web.context.request.RequestContextHolder; import org.springframework.web.context.request.ServletRequestAttributes; +import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.servlet.http.HttpSession; import java.util.ArrayList; @@ -80,23 +81,18 @@ public class UserService { @Lazy private ProxyAccessControlService accessControlService; - public Authentication getCurrentAuth() { - return SecurityContextHolder.getContext().getAuthentication(); - } - - public String getCurrentUserId() { - return getUserId(getCurrentAuth()); - } - - public Set getAdminGroups() { - Set adminGroups = new HashSet<>(); - + private final Set adminGroups = new HashSet<>(); + private final Set adminUsers = new HashSet<>(); + + @PostConstruct + public void init() { + // load admin groups // Support for old, non-array notation String singleGroup = environment.getProperty("proxy.admin-groups"); if (singleGroup != null && !singleGroup.isEmpty()) { adminGroups.add(singleGroup.toUpperCase()); } - + for (int i=0 ;; i++) { String groupName = environment.getProperty(String.format("proxy.admin-groups[%s]", i)); if (groupName == null || groupName.isEmpty()) { @@ -105,12 +101,7 @@ public Set getAdminGroups() { adminGroups.add(groupName.toUpperCase()); } - return adminGroups; - } - - public Set getAdminUsers() { - Set adminUsers = new HashSet<>(); - + // load admin users // Support for old, non-array notation String singleUser = environment.getProperty("proxy.admin-users"); if (singleUser != null && !singleUser.isEmpty()) { @@ -124,7 +115,21 @@ public Set getAdminUsers() { } adminUsers.add(userName); } + } + + public Set getAdminGroups() { + return adminGroups; + } + public Authentication getCurrentAuth() { + return SecurityContextHolder.getContext().getAuthentication(); + } + + public String getCurrentUserId() { + return getUserId(getCurrentAuth()); + } + + public Set getAdminUsers() { return adminUsers; } From 565e62538e107317772288b450aedfb754ee9769 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Fri, 7 Jul 2023 15:34:33 +0200 Subject: [PATCH 17/29] Fix #31010: include port in X-Forwarded-Host header --- .../containerproxy/util/ProxyMappingManager.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java b/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java index bf81a99b..fd9576ea 100644 --- a/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java +++ b/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java @@ -44,6 +44,7 @@ import io.undertow.util.StatusCodes; import org.springframework.context.event.ContextClosedEvent; import org.springframework.context.event.EventListener; +import org.springframework.security.web.header.Header; import org.springframework.stereotype.Component; import javax.inject.Inject; @@ -171,6 +172,13 @@ public void dispatchAsync(String proxyId, String mapping, HttpServletRequest req if (exchangeCustomizer != null) { exchangeCustomizer.accept(exchange); } + // see #31010 + // by default Undertow adds a Headers.X_FORWARDED_HOST header using exchange.getHost(), this never includes the server port + // however, the original Host header includes the port if using a non-standard port (i.e. not 80 and 443) + // this causes problems for applications comparing the Host and/or Origin header + // therefore we set the header here using exchange.getHostAndPort(), which only includes the port if non-standard port such that it matches the Host header + // note: if we set the header here, undertow does not override it + exchange.getRequestHeaders().put(Headers.X_FORWARDED_HOST, exchange.getHostAndPort()); exchange.addDefaultResponseListener(defaultResponseListener); request.startAsync(); request.getRequestDispatcher(targetPath).forward(request, response); From ad634b65a5b6561cdd7c98d1408109ccb3b9e467 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 10 Jul 2023 11:40:33 +0200 Subject: [PATCH 18/29] Update API documentation --- .../containerproxy/api/ProxyStatusController.java | 10 +++++----- .../containerproxy/util/ProxyMappingManager.java | 1 + 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java b/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java index 7b7b765b..8f90ee7a 100644 --- a/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java +++ b/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java @@ -80,10 +80,10 @@ public class ProxyStatusController { mediaType = "application/json", schema = @Schema(implementation = ChangeProxyStatusDto.class), examples = { - @ExampleObject(name = "Stopping", description = "Stop a proxy.", value = "{\"desiredStatus\": \"Stopping\"}"), - @ExampleObject(name = "Pausing", description = "Pause a proxy.", value = "{\"desiredStatus\": \"Pausing\"}"), - @ExampleObject(name = "Resuming", description = "Resume a proxy.", value = "{\"desiredStatus\": \"Resuming\"}"), - @ExampleObject(name = "Resuming with parameters", description = "Resume a proxy.", value = "{\"desiredStatus\": \"Resuming\", \"parameters\":{\"resources\":\"2 CPU cores - 8G RAM\",\"other_parameter\":\"example\"}}") + @ExampleObject(name = "Stopping", description = "Stop a proxy.", value = "{\"desiredState\": \"Stopping\"}"), + @ExampleObject(name = "Pausing", description = "Pause a proxy.", value = "{\"desiredState\": \"Pausing\"}"), + @ExampleObject(name = "Resuming", description = "Resume a proxy.", value = "{\"desiredState\": \"Resuming\"}"), + @ExampleObject(name = "Resuming with parameters", description = "Resume a proxy.", value = "{\"desiredState\": \"Resuming\", \"parameters\":{\"resources\":\"2 CPU cores - 8G RAM\",\"other_parameter\":\"example\"}}") } ) ) @@ -148,7 +148,7 @@ public ResponseEntity> changeProxyStatus(@PathVariable String } asyncProxyService.stopProxy(proxy, false); } else { - return ApiResponse.fail("Invalid desiredStatus"); + return ApiResponse.fail("Invalid desiredState"); } return ApiResponse.success(); diff --git a/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java b/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java index fd9576ea..a61d589d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java +++ b/src/main/java/eu/openanalytics/containerproxy/util/ProxyMappingManager.java @@ -190,6 +190,7 @@ public void onApplicationEvent(ContextClosedEvent event) { } private final DefaultResponseListener defaultResponseListener = responseExchange -> { + // note: if ShinyProxy was restarted it can take up to one minute for the request to timeout/fail if (!responseExchange.isResponseChannelAvailable()) { return false; } From 84726f8aae544c88d6f2f504adfce2904ad14c02 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 10 Jul 2023 14:34:39 +0200 Subject: [PATCH 19/29] Fix #31016: allow to stop stopping app --- .../containerproxy/api/ProxyStatusController.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java b/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java index 8f90ee7a..e91458b9 100644 --- a/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java +++ b/src/main/java/eu/openanalytics/containerproxy/api/ProxyStatusController.java @@ -140,11 +140,8 @@ public ResponseEntity> changeProxyStatus(@PathVariable String return ApiResponse.fail(ex.getMessage()); } } else if (changeProxyStateDto.getDesiredState().equals("Stopping")) { - if (!proxy.getStatus().equals(ProxyStatus.New) - && !proxy.getStatus().equals(ProxyStatus.Up) - && !proxy.getStatus().equals(ProxyStatus.Resuming) - && !proxy.getStatus().equals(ProxyStatus.Paused)) { - return ApiResponse.fail(String.format("Cannot stop proxy because it is not in New, Up or Paused status (status is %s)", proxy.getStatus())); + if (proxy.getStatus().equals(ProxyStatus.Stopped)) { + return ApiResponse.fail("Cannot stop proxy because it is already stopped"); } asyncProxyService.stopProxy(proxy, false); } else { From b70499d011e759498614add0e13b196e22a65f6f Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Mon, 10 Jul 2023 15:54:09 +0200 Subject: [PATCH 20/29] Fix JSON responses --- .../api/ProxyRouteController.java | 1 + .../impl/oidc/OpenIdReAuthorizeFilter.java | 7 ++- .../util/ImmediateJsonResponse.java | 43 +++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 src/main/java/eu/openanalytics/containerproxy/util/ImmediateJsonResponse.java diff --git a/src/main/java/eu/openanalytics/containerproxy/api/ProxyRouteController.java b/src/main/java/eu/openanalytics/containerproxy/api/ProxyRouteController.java index fb2fa92d..092d0670 100644 --- a/src/main/java/eu/openanalytics/containerproxy/api/ProxyRouteController.java +++ b/src/main/java/eu/openanalytics/containerproxy/api/ProxyRouteController.java @@ -23,6 +23,7 @@ import eu.openanalytics.containerproxy.model.runtime.Proxy; import eu.openanalytics.containerproxy.service.ProxyService; import eu.openanalytics.containerproxy.service.UserService; +import eu.openanalytics.containerproxy.util.ImmediateJsonResponse; import eu.openanalytics.containerproxy.util.ProxyMappingManager; import eu.openanalytics.containerproxy.util.ContextPathHelper; import org.springframework.stereotype.Controller; diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java index dc67c9e6..7c613e59 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/oidc/OpenIdReAuthorizeFilter.java @@ -20,6 +20,7 @@ */ package eu.openanalytics.containerproxy.auth.impl.oidc; +import eu.openanalytics.containerproxy.util.ImmediateJsonResponse; import org.springframework.core.env.Environment; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; @@ -132,8 +133,7 @@ protected void doFilterInternal(@Nonnull HttpServletRequest request, @Nonnull Ht } } if (REFRESH_OPENID_MATCHER.matches(request)) { - response.getWriter().write("{\"status\":\"success\"}"); - response.setStatus(200); + ImmediateJsonResponse.write(response, 200, "{\"status\":\"success\"}"); return; } chain.doFilter(request, response); @@ -156,8 +156,7 @@ private void invalidateSession(@Nonnull HttpServletRequest request, @Nonnull Htt session.invalidate(); } if (REFRESH_OPENID_MATCHER.matches(request)) { - response.getWriter().write("{\"status\":\"success\"}"); - response.setStatus(200); + ImmediateJsonResponse.write(response, 200, "{\"status\":\"success\"}"); } else { throw new ClientAuthorizationRequiredException(REG_ID); } diff --git a/src/main/java/eu/openanalytics/containerproxy/util/ImmediateJsonResponse.java b/src/main/java/eu/openanalytics/containerproxy/util/ImmediateJsonResponse.java new file mode 100644 index 00000000..3ab4a45a --- /dev/null +++ b/src/main/java/eu/openanalytics/containerproxy/util/ImmediateJsonResponse.java @@ -0,0 +1,43 @@ +/** + * ContainerProxy + * + * Copyright (C) 2016-2023 Open Analytics + * + * =========================================================================== + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the Apache License as published by + * The Apache Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * Apache License for more details. + * + * You should have received a copy of the Apache License + * along with this program. If not, see + */ +package eu.openanalytics.containerproxy.util; + +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; + +public class ImmediateJsonResponse { + + /** + * Helper that writes a JSON response to a HttpServletResponse without using Spring. + * This can be used in places where no Spring logic can be used (e.g. Filters) + * @param response the response to write into + * @param status the status code of the response + * @param json the jsons string to write + * @throws IOException + */ + public static void write(HttpServletResponse response, int status, String json) throws IOException { + response.setCharacterEncoding("UTF-8"); + response.setContentType("application/json"); + response.getWriter().write(json); + response.setStatus(status); + } + +} From 2e9a9cacf1f313e2fc878ae3d20d161ebf46b1db Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 11 Jul 2023 11:37:44 +0200 Subject: [PATCH 21/29] Fix #30808: log warning when using fallback config --- .../containerproxy/ContainerProxyApplication.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java index bb2c1767..0ea5edb0 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java +++ b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java @@ -123,7 +123,11 @@ public static void main(String[] args) { app.addListeners(new LoggingConfigurer()); boolean hasExternalConfig = Files.exists(Paths.get(CONFIG_FILENAME)); - if (!hasExternalConfig) app.setAdditionalProfiles(CONFIG_DEMO_PROFILE); + if (!hasExternalConfig) { + app.setAdditionalProfiles(CONFIG_DEMO_PROFILE); + Logger log = LogManager.getLogger(ContainerProxyApplication.class); + log.warn("WARNING: Did not found configuration, using fallback configuration!"); + } setDefaultProperties(app); From a518fa851e5c80f2af6ecced102e5c0ab1760bd9 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 11 Jul 2023 11:46:37 +0200 Subject: [PATCH 22/29] Log warning when app recovery and HA are used together --- .../containerproxy/ContainerProxyApplication.java | 6 ++++++ .../containerproxy/service/AppRecoveryService.java | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java index 0ea5edb0..40b2248e 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java +++ b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java @@ -81,6 +81,8 @@ import java.util.concurrent.Executor; import static eu.openanalytics.containerproxy.api.ApiSecurityService.PROP_API_SECURITY_HIDE_SPEC_DETAILS; +import static eu.openanalytics.containerproxy.service.AppRecoveryService.PROPERTY_RECOVER_RUNNING_PROXIES; +import static eu.openanalytics.containerproxy.service.AppRecoveryService.PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG; import static eu.openanalytics.containerproxy.service.ProxyService.PROPERTY_STOP_PROXIES_ON_SHUTDOWN; @EnableScheduling @@ -170,6 +172,10 @@ public void init() { // running in HA mode, but proxies are removed when shutting down log.warn("WARNING: Invalid configuration detected: store-mode is set to Redis (i.e. High-Availability mode), but proxies are stopped at shutdown of server!"); } + if (environment.getProperty( PROPERTY_RECOVER_RUNNING_PROXIES, Boolean.class, false) || + environment.getProperty( PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG, Boolean.class, false) ) { + log.warn("WARNING: Invalid configuration detected: cannot use store-mode with Redis (i.e. High-Availability mode) and app recovery at the same time. Disable app recovery!"); + } } boolean hideSpecDetails = environment.getProperty(PROP_API_SECURITY_HIDE_SPEC_DETAILS, Boolean.class, true); diff --git a/src/main/java/eu/openanalytics/containerproxy/service/AppRecoveryService.java b/src/main/java/eu/openanalytics/containerproxy/service/AppRecoveryService.java index c244ca0b..044dbb60 100644 --- a/src/main/java/eu/openanalytics/containerproxy/service/AppRecoveryService.java +++ b/src/main/java/eu/openanalytics/containerproxy/service/AppRecoveryService.java @@ -52,8 +52,8 @@ @Service public class AppRecoveryService { - protected static final String PROPERTY_RECOVER_RUNNING_PROXIES = "proxy.recover-running-proxies"; - protected static final String PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG = "proxy.recover-running-proxies-from-different-config"; + public static final String PROPERTY_RECOVER_RUNNING_PROXIES = "proxy.recover-running-proxies"; + public static final String PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG = "proxy.recover-running-proxies-from-different-config"; private final Logger log = LogManager.getLogger(AppRecoveryService.class); From 05d1fdb796fd245d8284e2e4633978089c9b8da0 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 13 Jul 2023 10:53:03 +0200 Subject: [PATCH 23/29] Fix #30915: re-enable GitHub social --- pom.xml | 24 +++++++------------ .../security/SocialSecurityConfig.java | 7 +++--- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 00e6ff7f..ccddab9e 100644 --- a/pom.xml +++ b/pom.xml @@ -58,16 +58,10 @@ clojars https://clojars.org/repo/ - - - - - - - - - - + + oa-nexus-releases + https://nexus.openanalytics.eu/repository/releases + @@ -212,11 +206,11 @@ spring-social-linkedin 1.0.2.RELEASE - - - - - + + org.springframework.social + spring-social-github + 1.0.0.M4 + com.github.spring-social spring-social-google diff --git a/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java b/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java index cc3e563d..5cfa983d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java +++ b/src/main/java/eu/openanalytics/containerproxy/security/SocialSecurityConfig.java @@ -58,8 +58,7 @@ import org.springframework.social.connect.web.ProviderSignInController; import org.springframework.social.connect.web.SignInAdapter; import org.springframework.social.facebook.connect.FacebookConnectionFactory; -//import org.springframework.social.github.connect.GitHubConnectionFactory; -//import org.springframework.social.github.connect.GitHubConnectionFactory; +import org.springframework.social.github.connect.GitHubConnectionFactory; import org.springframework.social.google.connect.GoogleConnectionFactory; import org.springframework.social.linkedin.connect.LinkedInConnectionFactory; import org.springframework.social.twitter.connect.TwitterConnectionFactory; @@ -153,8 +152,8 @@ public ConnectionFactory createConnectionFactory(String appId, String appSecr return factory; case linkedin: return new LinkedInConnectionFactory(appId, appSecret); -//// case github: -// return new GitHubConnectionFactory(appId, appSecret); + case github: + return new GitHubConnectionFactory(appId, appSecret); default: return null; } From 06c9b7791d4a6ccf7d39171a3df8a1fab70af9d5 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 13 Jul 2023 11:12:01 +0200 Subject: [PATCH 24/29] Fix #30915: add warning for deprecated social auth --- .../auth/impl/SocialAuthenticationBackend.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SocialAuthenticationBackend.java b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SocialAuthenticationBackend.java index 98dd8c4b..527a4271 100644 --- a/src/main/java/eu/openanalytics/containerproxy/auth/impl/SocialAuthenticationBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/auth/impl/SocialAuthenticationBackend.java @@ -20,6 +20,8 @@ */ package eu.openanalytics.containerproxy.auth.impl; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configurers.ExpressionUrlAuthorizationConfigurer.AuthorizedUrl; @@ -33,6 +35,15 @@ public class SocialAuthenticationBackend implements IAuthenticationBackend { public static final String NAME = "social"; + + public SocialAuthenticationBackend() { + Logger logger = LoggerFactory.getLogger(getClass()); + logger.warn("WARNING: ###"); + logger.warn("WARNING: ###"); + logger.warn("WARNING: Social authentication is deprecated and will be removed in the next version (3.1.0)!"); + logger.warn("WARNING: ###"); + logger.warn("WARNING: ###"); + } @Override public String getName() { From 558720ef9703bfe67f26a49bc4035fc3fa722835 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 13 Jul 2023 14:12:33 +0200 Subject: [PATCH 25/29] Fix #30977: add warnings for invalid session configuration --- .../containerproxy/ContainerProxyApplication.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java index 40b2248e..948fb786 100644 --- a/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java +++ b/src/main/java/eu/openanalytics/containerproxy/ContainerProxyApplication.java @@ -162,7 +162,6 @@ public void init() { log.warn("WARNING: Invalid configuration detected: same-site-cookie policy is set to None, but secure-cookies are not enabled. Secure cookies must be enabled when using None as same-site-cookie policy "); } - if (environment.getProperty("proxy.store-mode", "").equalsIgnoreCase("Redis")) { if (!environment.getProperty("spring.session.store-type", "").equalsIgnoreCase("redis")) { // running in HA mode, but not using Redis sessions @@ -178,6 +177,20 @@ public void init() { } } + if (environment.getProperty("spring.session.store-type", "").equalsIgnoreCase("redis")) { + if (!environment.getProperty("proxy.store-mode", "").equalsIgnoreCase("Redis")) { + // using Redis sessions, but not running in HA mode -> this does not make sense + // even with one replica, the HA mode should be used in order for the server to survive restarts (which is the reason Redis sessions are used) + log.warn("WARNING: Invalid configuration detected: user sessions are stored in Redis, but store-more is not set to Redis. Change store-mode so that app sessions are stored in Redis!"); + } + if (environment.getProperty( PROPERTY_RECOVER_RUNNING_PROXIES, Boolean.class, false) || + environment.getProperty( PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG, Boolean.class, false) ) { + // using Redis sessions together with app recovery -> this does not make sense + // if already using Redis for sessions there is no reason to not store app sessions + log.warn("WARNING: Invalid configuration detected: user sessions are stored in Redis and App Recovery is enabled. Instead of using App Recovery, change store-mode so that app sessions are stored in Redis!"); + } + } + boolean hideSpecDetails = environment.getProperty(PROP_API_SECURITY_HIDE_SPEC_DETAILS, Boolean.class, true); if (!hideSpecDetails) { log.warn("WARNING: Insecure configuration detected: The API is configured to return the full spec of proxies, " + From a8e9879d76139c33e16d8fc3c2661ef62c176868 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 18 Jul 2023 10:55:13 +0200 Subject: [PATCH 26/29] Improve tests for new k8s versions --- .../test/helpers/KubernetesTestBase.java | 14 + .../test/proxy/TestIntegrationOnKube.java | 351 ++++++------------ 2 files changed, 133 insertions(+), 232 deletions(-) diff --git a/src/test/java/eu/openanalytics/containerproxy/test/helpers/KubernetesTestBase.java b/src/test/java/eu/openanalytics/containerproxy/test/helpers/KubernetesTestBase.java index a43cafd7..635307aa 100644 --- a/src/test/java/eu/openanalytics/containerproxy/test/helpers/KubernetesTestBase.java +++ b/src/test/java/eu/openanalytics/containerproxy/test/helpers/KubernetesTestBase.java @@ -22,11 +22,14 @@ import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.NamespacedKubernetesClient; +import org.junit.jupiter.api.Assertions; import java.util.Arrays; import java.util.List; +import java.util.stream.Collectors; public abstract class KubernetesTestBase { @@ -88,4 +91,15 @@ private void createNamespaces() { } } + + protected List getSecrets(String namespace) { + return client.secrets().inNamespace(namespace).list().getItems().stream().filter(it -> !it.getMetadata().getName().startsWith("default-token")).collect(Collectors.toList()); + } + + protected Secret getSingleSecret(String namespace) { + List secrets = getSecrets(namespace); + Assertions.assertEquals(1, secrets.size()); + return secrets.get(0); + } + } diff --git a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java index 1c8ba254..1786475a 100644 --- a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java +++ b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnKube.java @@ -44,7 +44,6 @@ import io.fabric8.kubernetes.api.model.ResourceRequirements; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.kubernetes.api.model.SecretBuilder; -import io.fabric8.kubernetes.api.model.SecretList; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceAccountBuilder; import io.fabric8.kubernetes.api.model.ServiceList; @@ -125,7 +124,7 @@ public void launchProxy() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); Assertions.assertFalse(pod.getSpec().getContainers().get(0).getSecurityContext().getPrivileged()); ServiceList serviceList = client.services().inNamespace(namespace).list(); @@ -174,7 +173,7 @@ public void launchProxyWithVolumes() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // Check Volumes Assertions.assertTrue(pod.getSpec().getVolumes().size() >= 2); // at least two volumes should be created @@ -227,7 +226,7 @@ public void launchProxyWithEnv() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // Check Env Variables List envList = pod.getSpec().getContainers().get(0).getEnv(); @@ -283,7 +282,7 @@ public void launchProxyWithSecretRef() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // Check Env Variables List envList = pod.getSpec().getContainers().get(0).getEnv(); @@ -323,7 +322,7 @@ public void launchProxyWithResources() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // Check Resource limits/requests ResourceRequirements req = pod.getSpec().getContainers().get(0).getResources(); @@ -363,7 +362,7 @@ public void launchProxyWithPrivileged() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); Assertions.assertTrue(pod.getSpec().getContainers().get(0).getSecurityContext().getPrivileged()); @@ -421,7 +420,7 @@ public void launchProxyWithPodPatches() { Assertions.assertEquals(serviceAccountName, pod.getSpec().getServiceAccount()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); serviceList = client.services().inNamespace(overriddenNamespace).list(); Assertions.assertEquals(1, serviceList.getItems().size()); @@ -474,7 +473,7 @@ public void launchProxyWithPatchesWithMerging() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // Check volumes: one HostPath is added using SP and one using patches Assertions.assertTrue(pod.getSpec().getVolumes().size() >= 2); // at least two volumes should be created @@ -526,7 +525,7 @@ public void launchProxyWithAdditionalManifests() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); PersistentVolumeClaimList claimList = client.persistentVolumeClaims().inNamespace(overriddenNamespace).list(); Assertions.assertEquals(1, claimList.getItems().size()); @@ -535,15 +534,9 @@ public void launchProxyWithAdditionalManifests() { Assertions.assertEquals("manifests-pvc", claim.getMetadata().getName()); // secret has no namespace defined -> should be created in the namespace defined by the pod patches - SecretList secretList = client.secrets().inNamespace(overriddenNamespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - } + Secret secret = getSingleSecret(overriddenNamespace); + Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); proxyService.stopProxy(null, proxy, true).run(); @@ -554,9 +547,7 @@ public void launchProxyWithAdditionalManifests() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // all additional manifests should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); Assertions.assertEquals(0, client.persistentVolumeClaims().inNamespace(overriddenNamespace).list().getItems().size()); Assertions.assertEquals(0, proxyService.getProxies(null, true).size()); @@ -605,7 +596,7 @@ public void launchProxyWithAdditionalManifestsOfWhichOneAlreadyExists() throws E Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); PersistentVolumeClaimList claimList = client.persistentVolumeClaims().inNamespace(overriddenNamespace).list(); Assertions.assertEquals(1, claimList.getItems().size()); @@ -614,15 +605,9 @@ public void launchProxyWithAdditionalManifestsOfWhichOneAlreadyExists() throws E Assertions.assertEquals("manifests-pvc", claim.getMetadata().getName()); // secret has no namespace defined -> should be created in the namespace defined by the pod patches - SecretList secretList = client.secrets().inNamespace(overriddenNamespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - } + Secret secret = getSingleSecret(overriddenNamespace); + Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); proxyService.stopProxy(null, proxy, true).run(); @@ -633,9 +618,7 @@ public void launchProxyWithAdditionalManifestsOfWhichOneAlreadyExists() throws E podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // all additional manifests should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); Assertions.assertEquals(0, client.persistentVolumeClaims().inNamespace(overriddenNamespace).list().getItems().size()); Assertions.assertEquals(0, proxyService.getProxies(null, true).size()); @@ -664,7 +647,7 @@ public void launchProxyWithExpressionInPatchAndManifests() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // check env variables @@ -725,7 +708,7 @@ public void launchProxyWithAdditionalPersistentManifests() throws Exception { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); PersistentVolumeClaimList claimList = client.persistentVolumeClaims().inNamespace(overriddenNamespace).list(); Assertions.assertEquals(1, claimList.getItems().size()); @@ -734,15 +717,9 @@ public void launchProxyWithAdditionalPersistentManifests() throws Exception { Assertions.assertEquals("manifests-pvc", claim.getMetadata().getName()); // secret has no namespace defined -> should be created in the namespace defined by the pod patches - SecretList secretList = client.secrets().inNamespace(overriddenNamespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - } + Secret secret = getSingleSecret(overriddenNamespace); + Assertions.assertEquals(overriddenNamespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); proxyService.stopProxy(null, proxy, true).run(); @@ -753,9 +730,7 @@ public void launchProxyWithAdditionalPersistentManifests() throws Exception { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); // the PVC should not be deleted Assertions.assertEquals(1, client.persistentVolumeClaims().inNamespace(overriddenNamespace).list().getItems().size()); @@ -782,20 +757,14 @@ public void launchProxyWithManifestPolicyCreateOnce() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the application.yml - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the application.yml + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -806,9 +775,7 @@ public void launchProxyWithManifestPolicyCreateOnce() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); }); // case 2: secret does already exist, check that it does not get overridden setup((client, namespace, overriddenNamespace) -> { @@ -838,20 +805,14 @@ public void launchProxyWithManifestPolicyCreateOnce() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the secret we created above not from the application.yml - Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the secret we created above not from the application.yml + Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -862,9 +823,7 @@ public void launchProxyWithManifestPolicyCreateOnce() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(namespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(namespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(namespace).size()); }); } @@ -887,20 +846,14 @@ public void launchProxyWithManifestPolicyPatch() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the application.yml - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the application.yml + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -911,9 +864,7 @@ public void launchProxyWithManifestPolicyPatch() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(namespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(namespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(namespace).size()); }); // case 2: secret does already exist, check that it gets overridden @@ -946,22 +897,16 @@ public void launchProxyWithManifestPolicyPatch() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the application.yml not from the secret we created above - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - // since the secret should not have been replaced, the creation timestamp must be equal - Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the application.yml not from the secret we created above + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); + // since the secret should not have been replaced, the creation timestamp must be equal + Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); proxyService.stopProxy(null, proxy, true).run(); @@ -972,9 +917,7 @@ public void launchProxyWithManifestPolicyPatch() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(namespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(namespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); }); } @@ -1005,12 +948,10 @@ public void launchProxyWithManifestPolicyDelete() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); proxyService.stopProxy(null, proxy, true).run(); @@ -1042,20 +983,14 @@ public void launchProxyWithManifestPolicyReplace() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the application.yml - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the application.yml + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -1066,9 +1001,7 @@ public void launchProxyWithManifestPolicyReplace() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); }); // case 2: secret does already exist, check that it gets overridden @@ -1101,22 +1034,16 @@ public void launchProxyWithManifestPolicyReplace() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the application.yml not from the secret we created above - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - // since the secret was replaced, the creation timestamp must be different - Assertions.assertNotEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the application.yml not from the secret we created above + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); + // since the secret was replaced, the creation timestamp must be different + Assertions.assertNotEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); proxyService.stopProxy(null, proxy, true).run(); @@ -1127,9 +1054,7 @@ public void launchProxyWithManifestPolicyReplace() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); }); } @@ -1150,19 +1075,13 @@ public void launchProxyWithPersistentManifestPolicyCreateOnce() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -1173,7 +1092,7 @@ public void launchProxyWithPersistentManifestPolicyCreateOnce() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); String originalCreationTimestamp = client.secrets().inNamespace(namespace) .withName("manifests-secret").get().getMetadata().getCreationTimestamp(); @@ -1182,18 +1101,12 @@ public void launchProxyWithPersistentManifestPolicyCreateOnce() { proxy = proxyService.startProxy(spec); // secret has no namespace defined -> should be created in the default namespace - secretList= client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - // secret should have not been modified -> check timestamp - Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); - } + secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); + // secret should have not been modified -> check timestamp + Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); proxyService.stopProxy(null, proxy, true).run(); @@ -1204,7 +1117,7 @@ public void launchProxyWithPersistentManifestPolicyCreateOnce() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); }); } @@ -1227,19 +1140,13 @@ public void launchProxyWithPersistentManifestPolicyPatch() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -1250,7 +1157,7 @@ public void launchProxyWithPersistentManifestPolicyPatch() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); String originalCreationTimestamp = client.secrets().inNamespace(namespace) .withName("manifests-secret").get().getMetadata().getCreationTimestamp(); @@ -1261,19 +1168,13 @@ public void launchProxyWithPersistentManifestPolicyPatch() { proxy = proxyService.startProxy(spec); // secret has no namespace defined -> should be created in the default namespace - secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - // secret should have the value from the second spec - Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); - // since the secret should not have been replaced, the creation timestamp must be equal - Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); - } + secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + // secret should have the value from the second spec + Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); + // since the secret should not have been replaced, the creation timestamp must be equal + Assertions.assertEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); proxyService.stopProxy(null, proxy, true).run(); @@ -1284,7 +1185,7 @@ public void launchProxyWithPersistentManifestPolicyPatch() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); }); } @@ -1315,12 +1216,10 @@ public void launchProxyWithPersistentManifestPolicyDelete() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // the secret should be deleted - Assertions.assertEquals(1, client.secrets().inNamespace(overriddenNamespace).list().getItems().size()); - Assertions.assertTrue(client.secrets().inNamespace(overriddenNamespace).list() - .getItems().get(0).getMetadata().getName().startsWith("default-token")); + Assertions.assertEquals(0, getSecrets(overriddenNamespace).size()); proxyService.stopProxy(null, proxy, true).run(); @@ -1350,19 +1249,13 @@ public void launchProxyWithPersistentManifestPolicyReplace() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); // secret has no namespace defined -> should be created in the default namespace - SecretList secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); - } + Secret secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + Assertions.assertEquals("cGFzc3dvcmQ=", secret.getData().get("password")); proxyService.stopProxy(null, proxy, true).run(); @@ -1373,7 +1266,7 @@ public void launchProxyWithPersistentManifestPolicyReplace() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); String originalCreationTimestamp = client.secrets().inNamespace(namespace) .withName("manifests-secret").get().getMetadata().getCreationTimestamp(); @@ -1383,18 +1276,12 @@ public void launchProxyWithPersistentManifestPolicyReplace() { proxy = proxyService.startProxy(spec); // secret has no namespace defined -> should be created in the default namespace - secretList = client.secrets().inNamespace(namespace).list(); - Assertions.assertEquals(2, secretList.getItems().size()); - for (Secret secret : secretList.getItems()) { - if (secret.getMetadata().getName().startsWith("default-token")) { - continue; - } - Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); - Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); - Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); - // since the secret should have been replaced, the creation timestamp must be different - Assertions.assertNotEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); - } + secret = getSingleSecret(namespace); + Assertions.assertEquals(namespace, secret.getMetadata().getNamespace()); + Assertions.assertEquals("manifests-secret", secret.getMetadata().getName()); + Assertions.assertEquals("b2xkX3Bhc3N3b3Jk", secret.getData().get("password")); + // since the secret should have been replaced, the creation timestamp must be different + Assertions.assertNotEquals(originalCreationTimestamp, secret.getMetadata().getCreationTimestamp()); proxyService.stopProxy(null, proxy, true).run(); @@ -1405,7 +1292,7 @@ public void launchProxyWithPersistentManifestPolicyReplace() { podList = client.pods().inNamespace(namespace).list(); Assertions.assertEquals(0, podList.getItems().size()); // the secret should not be deleted - Assertions.assertEquals(2, client.secrets().inNamespace(namespace).list().getItems().size()); + Assertions.assertEquals(1, getSecrets(namespace).size()); }); } @@ -1429,7 +1316,7 @@ public void advancedRuntimeLabels() { Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); Assertions.assertFalse(pod.getSpec().getContainers().get(0).getSecurityContext().getPrivileged()); ServiceList serviceList = client.services().inNamespace(namespace).list(); @@ -1508,7 +1395,7 @@ public void launchProxyWithParameters() { ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("ledfan/rstudio_base_r:4_0_5", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("ledfan/rstudio_base_r:4_0_5")); Assertions.assertEquals("2", pod.getSpec().getContainers().get(0).getResources().getLimits().get("memory").getAmount()); List envList = pod.getSpec().getContainers().get(0).getEnv(); Map env = envList.stream().collect(Collectors.toMap(EnvVar::getName, e -> e)); @@ -1559,7 +1446,7 @@ public void launchProxyWithParametersWithNullValueSetName() { ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("ledfan/rstudio_base_r:4_0_5", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("ledfan/rstudio_base_r:4_0_5")); Assertions.assertEquals("2", pod.getSpec().getContainers().get(0).getResources().getLimits().get("memory").getAmount()); List envList = pod.getSpec().getContainers().get(0).getEnv(); Map env = envList.stream().collect(Collectors.toMap(EnvVar::getName, e -> e)); @@ -1639,7 +1526,7 @@ public void launchProxyWithParametersFinalResolve() { ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("ledfan/rstudio_base_r:4_0_5", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("ledfan/rstudio_base_r:4_0_5")); Assertions.assertEquals("2", pod.getSpec().getContainers().get(0).getResources().getLimits().get("memory").getAmount()); List envList = pod.getSpec().getContainers().get(0).getEnv(); Map env = envList.stream().collect(Collectors.toMap(EnvVar::getName, e -> e)); @@ -1701,7 +1588,7 @@ public void launchProxyWithAdditionalPersistentManifestsUsingAuthObjects() throw Assertions.assertEquals(1, pod.getStatus().getContainerStatuses().size()); ContainerStatus container = pod.getStatus().getContainerStatuses().get(0); Assertions.assertEquals(true, container.getReady()); - Assertions.assertEquals("openanalytics/shinyproxy-demo:latest", container.getImage()); + Assertions.assertTrue(container.getImage().endsWith("openanalytics/shinyproxy-demo:latest")); ConfigMap configMap1 = client.configMaps().inNamespace(overriddenNamespace).withName("configmap1").get(); Assertions.assertTrue(configMap1.getData().containsKey("test.txt")); From 3990072580b765e2fca23ccdcb1efd0756e2582b Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 18 Jul 2023 10:56:45 +0200 Subject: [PATCH 27/29] GHA: test latests k8s version --- .github/workflows/workflows.yaml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/workflows.yaml b/.github/workflows/workflows.yaml index 62307104..aed87abf 100644 --- a/.github/workflows/workflows.yaml +++ b/.github/workflows/workflows.yaml @@ -13,9 +13,11 @@ jobs: kubernetes: - 'v1.21.6' - 'v1.22.17' - - 'v1.23.15' - - 'v1.24.9' - - 'v1.25.5' + - 'v1.23.17' + - 'v1.24.15' + - 'v1.25.11' + - 'v1.26.6' + - 'v1.27.3' steps: - uses: actions/checkout@v2 From 9666758a0c012afc909e9cd96ab975e0b8065ae0 Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Thu, 20 Jul 2023 16:00:44 +0200 Subject: [PATCH 28/29] Improve GitHub actions workflow --- .github/workflows/workflows.yaml | 6 ++++-- .../containerproxy/backend/docker/DockerEngineBackend.java | 1 + .../containerproxy/test/proxy/TestIntegrationOnSwarm.java | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/workflows.yaml b/.github/workflows/workflows.yaml index aed87abf..0a5cfd13 100644 --- a/.github/workflows/workflows.yaml +++ b/.github/workflows/workflows.yaml @@ -34,9 +34,11 @@ jobs: - name: Setup Minikube uses: manusa/actions-setup-minikube@v2.7.2 with: - minikube version: 'v1.28.0' + minikube version: 'v1.30.1' kubernetes version: ${{ matrix.kubernetes }} github token: ${{ secrets.GITHUB_TOKEN }} + container runtime: containerd + driver: docker - name: Setup Docker run: sudo apt-get -qq -y install conntrack socat ; nohup socat TCP-LISTEN:2375,reuseaddr,fork UNIX-CONNECT:/var/run/docker.sock & - name: Setup Docker Swarm @@ -54,7 +56,7 @@ jobs: docker ps -a kubectl get pod -A - name: Run Tests - run: mvn -B test + run: mvn -B test dependency: runs-on: ubuntu-latest diff --git a/src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerEngineBackend.java b/src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerEngineBackend.java index b4c6b599..8eeec70d 100644 --- a/src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerEngineBackend.java +++ b/src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerEngineBackend.java @@ -202,6 +202,7 @@ public List scanExistingContainers() throws Exception { for (com.spotify.docker.client.messages.Container container: dockerClient.listContainers(ListContainersParam.allContainers())) { if (!container.state().equalsIgnoreCase("running")) { + log.warn("Ignoring container {} because it is not running, {}", container.id(), container.state()); continue; // not recovering stopped/broken apps } diff --git a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnSwarm.java b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnSwarm.java index c4c9223a..3b46e4c2 100644 --- a/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnSwarm.java +++ b/src/test/java/eu/openanalytics/containerproxy/test/proxy/TestIntegrationOnSwarm.java @@ -72,6 +72,7 @@ private boolean checkEverythingCleanedUp() throws DockerCertificateException, Do @AfterEach public void waitForCleanup() throws InterruptedException, DockerException, DockerCertificateException { + Thread.sleep(3_000); // wait before checking for (int i = 0; i < 120; i++) { if (checkEverythingCleanedUp()) { break; From a1289a6640cccfbcd751dc1da9427ad109a957bd Mon Sep 17 00:00:00 2001 From: Tobia De Koninck Date: Tue, 25 Jul 2023 15:59:00 +0200 Subject: [PATCH 29/29] Release 1.0.2 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index ccddab9e..0a36fd60 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ eu.openanalytics containerproxy - 1.0.2-SNAPSHOT + 1.0.2 ContainerProxy jar