From 37e33644b2aa0a5f708fcc0c5ebc736aa2faf57a Mon Sep 17 00:00:00 2001 From: Sergey Grigoriev Date: Mon, 2 Sep 2024 12:53:39 +0200 Subject: [PATCH] fix: use separate attribute for XSRF token in LogoutFilter (#143) --- .../generic/auth/XsrfTokenValidator.java | 2 +- .../generic/rest/filter/LogoutFilter.java | 23 +++++++++++++------ .../generic/rest/filter/LogoutFilterTest.java | 16 +++++++++++-- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/auth/XsrfTokenValidator.java b/app/src/main/java/ch/sbb/polarion/extension/generic/auth/XsrfTokenValidator.java index 5cea4a2..2552783 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/auth/XsrfTokenValidator.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/auth/XsrfTokenValidator.java @@ -31,7 +31,7 @@ public class XsrfTokenValidator extends AbstractAuthValidator { @Override public void updateRequestContext(@NotNull ContainerRequestContext requestContext, @NotNull Subject subject) { super.updateRequestContext(requestContext, subject); - requestContext.setProperty(LogoutFilter.ASYNC_SKIP_LOGOUT, Boolean.TRUE); + requestContext.setProperty(LogoutFilter.XSRF_SKIP_LOGOUT, Boolean.TRUE); } private boolean isXsrfTokenValid(@NotNull String userId, @NotNull String encryptedXsrfToken) { diff --git a/app/src/main/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilter.java b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilter.java index 2e34a69..742111c 100644 --- a/app/src/main/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilter.java +++ b/app/src/main/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilter.java @@ -1,15 +1,14 @@ package ch.sbb.polarion.extension.generic.rest.filter; -import java.io.IOException; +import com.polarion.platform.core.PlatformContext; +import com.polarion.platform.security.ISecurityService; import javax.security.auth.Subject; import javax.ws.rs.container.ContainerRequestContext; import javax.ws.rs.container.ContainerResponseContext; import javax.ws.rs.container.ContainerResponseFilter; import javax.ws.rs.ext.Provider; - -import com.polarion.platform.core.PlatformContext; -import com.polarion.platform.security.ISecurityService; +import java.io.IOException; @Secured @Provider @@ -18,6 +17,10 @@ public class LogoutFilter implements ContainerResponseFilter { // In this case async process itself is responsible for logout after the completion public static final String ASYNC_SKIP_LOGOUT = "async.skip.logout"; + // This request property should be set by XSRF token validation to prevent logout in response filter + // In this case async process should also skip logout after the completion + public static final String XSRF_SKIP_LOGOUT = "xsrf.skip.logout"; + private final ISecurityService securityService; public LogoutFilter(ISecurityService securityService) { @@ -30,10 +33,16 @@ public LogoutFilter() { } @Override - public void filter(ContainerRequestContext requestContext, - ContainerResponseContext responseContext) throws IOException { + public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException { + if (requestContext.getProperty(ASYNC_SKIP_LOGOUT) == Boolean.TRUE) { + return; + } + if (requestContext.getProperty(XSRF_SKIP_LOGOUT) == Boolean.TRUE) { + return; + } + Subject subject = (Subject) requestContext.getProperty(AuthenticationFilter.USER_SUBJECT); - if ((requestContext.getProperty(ASYNC_SKIP_LOGOUT) != Boolean.TRUE) && (subject != null)) { + if (subject != null) { securityService.logout(subject); } } diff --git a/app/src/test/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilterTest.java b/app/src/test/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilterTest.java index 9574e72..cd6b082 100644 --- a/app/src/test/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilterTest.java +++ b/app/src/test/java/ch/sbb/polarion/extension/generic/rest/filter/LogoutFilterTest.java @@ -13,6 +13,7 @@ import com.polarion.platform.security.ISecurityService; import static ch.sbb.polarion.extension.generic.rest.filter.LogoutFilter.ASYNC_SKIP_LOGOUT; +import static ch.sbb.polarion.extension.generic.rest.filter.LogoutFilter.XSRF_SKIP_LOGOUT; import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) @@ -27,6 +28,8 @@ class LogoutFilterTest { void successfulLogoutCallIfSubjectExists() throws IOException { Subject subject = new Subject(); when(requestContext.getProperty(AuthenticationFilter.USER_SUBJECT)).thenReturn(subject); + when(requestContext.getProperty(ASYNC_SKIP_LOGOUT)).thenReturn(Boolean.FALSE); + when(requestContext.getProperty(XSRF_SKIP_LOGOUT)).thenReturn(Boolean.FALSE); LogoutFilter filter = new LogoutFilter(securityService); filter.filter(requestContext, null); verify(securityService, times(1)).logout(subject); @@ -35,6 +38,8 @@ void successfulLogoutCallIfSubjectExists() throws IOException { @Test void doNotCallLogoutIfThereIsNoSubject() throws IOException { when(requestContext.getProperty(AuthenticationFilter.USER_SUBJECT)).thenReturn(null); + when(requestContext.getProperty(ASYNC_SKIP_LOGOUT)).thenReturn(Boolean.FALSE); + when(requestContext.getProperty(XSRF_SKIP_LOGOUT)).thenReturn(Boolean.FALSE); LogoutFilter filter = new LogoutFilter(securityService); filter.filter(requestContext, null); verify(securityService, times(0)).logout(any()); @@ -42,11 +47,18 @@ void doNotCallLogoutIfThereIsNoSubject() throws IOException { @Test void doNotCallLogoutIfAsyncSkipPropertyIsSet() throws IOException { - Subject subject = new Subject(); - when(requestContext.getProperty(AuthenticationFilter.USER_SUBJECT)).thenReturn(subject); when(requestContext.getProperty(ASYNC_SKIP_LOGOUT)).thenReturn(Boolean.TRUE); LogoutFilter filter = new LogoutFilter(securityService); filter.filter(requestContext, null); verify(securityService, times(0)).logout(any()); } + + @Test + void doNotCallLogoutIfXsrfSkipPropertyIsSet() throws IOException { + when(requestContext.getProperty(ASYNC_SKIP_LOGOUT)).thenReturn(Boolean.FALSE); + when(requestContext.getProperty(XSRF_SKIP_LOGOUT)).thenReturn(Boolean.TRUE); + LogoutFilter filter = new LogoutFilter(securityService); + filter.filter(requestContext, null); + verify(securityService, times(0)).logout(any()); + } }