SONAR-17645 Support user commissioning and decomissioning through SCIM for Okta

2 files changed, 54 insertions, 41 deletions

diff --git a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
index 715727a4a05..92b4cb26572 100644
--- a/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
+++ b/server/sonar-webserver/src/main/java/org/sonar/server/platform/web/MasterServletFilter.java
@@ -21,8 +21,8 @@ package org.sonar.server.platform.web;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import com.google.common.collect.Lists;
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -44,6 +44,7 @@ import org.sonar.server.platform.PlatformImpl;
  */
 public class MasterServletFilter implements Filter {
 
+  private static final String SCIM_FILTER_PATH = "/api/scim/v2/";
   private static volatile MasterServletFilter instance;
   private ServletFilter[] filters;
   private FilterConfig config;
@@ -78,17 +79,29 @@ public class MasterServletFilter implements Filter {
   }
 
   public void initFilters(List<ServletFilter> filterExtensions) {
-    List<ServletFilter> filterList = Lists.newArrayList();
+    LinkedList<ServletFilter> filterList = new LinkedList<>();
     for (ServletFilter extension : filterExtensions) {
       try {
         Loggers.get(MasterServletFilter.class).info(String.format("Initializing servlet filter %s [pattern=%s]", extension, extension.doGetPattern().label()));
         extension.init(config);
-        filterList.add(extension);
+        // As for scim we need to intercept traffic to URLs with path parameters
+        // and that use is not properly handled when dealing with inclusions/exclusions of the WebServiceFilter,
+        // we need to make sure the Scim filters are invoked before the WebserviceFilter
+        if (isScimFilter(extension)) {
+          filterList.addFirst(extension);
+        } else {
+          filterList.addLast(extension);
+        }
       } catch (Exception e) {
         throw new IllegalStateException("Fail to initialize servlet filter: " + extension + ". Message: " + e.getMessage(), e);
       }
     }
-    filters = filterList.toArray(new ServletFilter[filterList.size()]);
+    filters = filterList.toArray(new ServletFilter[0]);
+  }
+
+  private static boolean isScimFilter(ServletFilter extension) {
+    return extension.doGetPattern().getInclusions().stream()
+      .anyMatch(s -> s.startsWith(SCIM_FILTER_PATH));
   }
 
   @Override
@@ -99,16 +112,17 @@ public class MasterServletFilter implements Filter {
     } else {
       String path = hsr.getRequestURI().replaceFirst(hsr.getContextPath(), "");
       GodFilterChain godChain = new GodFilterChain(chain);
-
-      for (ServletFilter filter : filters) {
-        if (filter.doGetPattern().matches(path)) {
-          godChain.addFilter(filter);
-        }
-      }
+      buildGodchain(path, godChain);
       godChain.doFilter(hsr, response);
     }
   }
 
+  private void buildGodchain(String path, GodFilterChain godChain) {
+    Arrays.stream(filters)
+      .filter(filter -> filter.doGetPattern().matches(path))
+      .forEachOrdered(godChain::addFilter);
+  }
+
   @Override
   public void destroy() {
     for (ServletFilter filter : filters) {
diff --git a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
index 046d8117a19..1d3c54d63e3 100644
--- a/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
+++ b/server/sonar-webserver/src/test/java/org/sonar/server/platform/web/MasterServletFilterTest.java
@@ -31,6 +31,8 @@ import javax.servlet.http.HttpServletResponse;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.mockito.InOrder;
+import org.mockito.Mockito;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
 import org.sonar.api.web.ServletFilter;
@@ -41,6 +43,7 @@ import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -48,7 +51,6 @@ import static org.mockito.Mockito.when;
 
 public class MasterServletFilterTest {
 
-
   @Rule
   public LogTester logTester = new LogTester();
 
@@ -108,22 +110,39 @@ public class MasterServletFilterTest {
   }
 
   @Test
-  public void should_keep_filter_ordering() throws Exception {
-    TrueFilter filter1 = new TrueFilter();
-    TrueFilter filter2 = new TrueFilter();
-
-    MasterServletFilter filters = new MasterServletFilter();
-    filters.init(mock(FilterConfig.class), asList(filter1, filter2));
-
+  public void should_add_scim_filter_first_for_scim_request() throws Exception {
+    String scimPath = "/api/scim/v2/Groups";
     HttpServletRequest request = mock(HttpServletRequest.class);
-    when(request.getRequestURI()).thenReturn("/foo/bar");
+    when(request.getRequestURI()).thenReturn(scimPath);
     when(request.getContextPath()).thenReturn("");
     ServletResponse response = mock(HttpServletResponse.class);
     FilterChain chain = mock(FilterChain.class);
+
+    ServletFilter filter1 = mockFilter(ServletFilter.class, request, response);
+    ServletFilter filter2 = mockFilter(ServletFilter.class, request, response);
+    ServletFilter filter3 = mockFilter(WebServiceFilter.class, request, response);
+    when(filter3.doGetPattern()).thenReturn(UrlPattern.builder().includes(scimPath).build());
+
+    MasterServletFilter filters = new MasterServletFilter();
+    filters.init(mock(FilterConfig.class), asList(filter3, filter1, filter2));
+
     filters.doFilter(request, response, chain);
 
-    assertThat(filter1.count).isOne();
-    assertThat(filter2.count).isEqualTo(2);
+    InOrder inOrder = Mockito.inOrder(filter1, filter2, filter3);
+    inOrder.verify(filter3).doFilter(any(), any(), any());
+    inOrder.verify(filter1).doFilter(any(), any(), any());
+    inOrder.verify(filter2).doFilter(any(), any(), any());
+  }
+
+  private ServletFilter mockFilter(Class<? extends ServletFilter> filterClazz, HttpServletRequest request, ServletResponse response) throws IOException, ServletException {
+    ServletFilter filter = mock(filterClazz);
+    when(filter.doGetPattern()).thenReturn(UrlPattern.builder().build());
+    doAnswer(invocation -> {
+      FilterChain argument = invocation.getArgument(2, FilterChain.class);
+      argument.doFilter(request, response);
+      return null;
+    }).when(filter).doFilter(any(), any(), any());
+    return filter;
   }
 
   @Test
@@ -143,26 +162,6 @@ public class MasterServletFilterTest {
     return filter;
   }
 
-  private static final class TrueFilter extends ServletFilter {
-    private static int globalCount = 0;
-    private int count = 0;
-
-    @Override
-    public void init(FilterConfig filterConfig) {
-    }
-
-    @Override
-    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
-      globalCount++;
-      count = globalCount;
-      filterChain.doFilter(servletRequest, servletResponse);
-    }
-
-    @Override
-    public void destroy() {
-    }
-  }
-
   private static class PatternFilter extends ServletFilter {
 
     private final UrlPattern urlPattern;