X-SCM-Session-ID and X-SCM-Client could now be send via query parameter

The use of query parameters is required for SSE, because the standard does not support header.
This works currently only for GET request to avoid parsing of request body.

scm-core/src/main/java/sonia/scm/security/BearerToken.java

@@ -96,17 +96,13 @@ public final class BearerToken implements AuthenticationToken {
   /**
    * Creates a new {@link BearerToken} from raw string representation for the given ui session id.
    *
-   * @param sessionId session id of the client
+   * @param session session id of the client
    * @param rawToken bearer token string representation
    *
    * @return new bearer token
    */
-  public static BearerToken create(@Nullable String sessionId, String rawToken) {
+  public static BearerToken create(@Nullable SessionId session, String rawToken) {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(rawToken), "raw token is required");
-    SessionId session = null;
-    if (!Strings.isNullOrEmpty(sessionId)) {
-      session = SessionId.valueOf(sessionId);
-    }
     return new BearerToken(session, rawToken);
   }
 }

scm-core/src/main/java/sonia/scm/security/SessionId.java

@@ -1,14 +1,23 @@
 package sonia.scm.security;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
+import lombok.EqualsAndHashCode;
+import sonia.scm.util.HttpUtil;
 
-import java.util.Objects;
+import javax.servlet.http.HttpServletRequest;
+import java.io.Serializable;
+import java.util.Optional;
 
 /**
  * Client side session id.
  */
-public final class SessionId {
+@EqualsAndHashCode
+public final class SessionId implements Serializable {
+
+  @VisibleForTesting
+  public static final String PARAMETER = "X-SCM-Session-ID";
 
   private final String value;
 
@@ -16,24 +25,15 @@ public final class SessionId {
     this.value = value;
   }
 
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (o == null || getClass() != o.getClass()) return false;
-    SessionId sessionID = (SessionId) o;
-    return Objects.equals(value, sessionID.value);
-  }
-
-  @Override
-  public int hashCode() {
-    return Objects.hash(value);
-  }
-
   @Override
   public String toString() {
     return value;
   }
 
+  public static Optional<SessionId> from(HttpServletRequest request) {
+    return HttpUtil.getHeaderOrGetParameter(request, PARAMETER).map(SessionId::valueOf);
+  }
+
   public static SessionId valueOf(String value) {
     Preconditions.checkArgument(!Strings.isNullOrEmpty(value), "session id could not be empty or null");
     return new SessionId(value);

scm-core/src/main/java/sonia/scm/util/HttpUtil.java

@@ -37,7 +37,6 @@ package sonia.scm.util;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
-import com.google.common.base.MoreObjects;
 import com.google.common.base.Strings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -52,6 +51,7 @@ import java.net.URLDecoder;
 import java.net.URLEncoder;
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -116,12 +116,6 @@ public final class HttpUtil
    */
   public static final String HEADER_SCM_CLIENT = "X-SCM-Client";
 
-  /**
-   * header for identifying the scm-manager client session
-   * @since 2.0.0
-   */
-  public static final String HEADER_SCM_SESSION = "X-SCM-Session-ID";
-
   /** Field description */
   public static final String HEADER_USERAGENT = "User-Agent";
 
@@ -830,6 +824,24 @@ public final class HttpUtil
     return uri;
   }
 
+  /**
+   * Returns header value or query parameter if the request is a get request.
+   *
+   * @param request http request
+   * @param parameter name of header/parameter
+   *
+   * @return header value or query parameter
+   *
+   * @since 2.0.0
+   */
+  public static Optional<String> getHeaderOrGetParameter(HttpServletRequest request, String parameter) {
+    String value = request.getHeader(parameter);
+    if (Strings.isNullOrEmpty(value) && "GET".equalsIgnoreCase(request.getMethod())) {
+      value = request.getParameter(parameter);
+    }
+    return Optional.ofNullable(value);
+  }
+
   /**
    * Returns the given uri without leading separator.
    *
@@ -882,16 +894,14 @@ public final class HttpUtil
   /**
    * Returns true if the http request is send by the scm-manager web interface.
    *
-   *
    * @param request http request
    *
    * @return true if the request comes from the web interface.
    * @since 1.19
    */
-  public static boolean isWUIRequest(HttpServletRequest request)
+  public static boolean isWUIRequest(HttpServletRequest request) {
-  {
+    Optional<String> client = getHeaderOrGetParameter(request, HEADER_SCM_CLIENT);
-    return SCM_CLIENT_WUI.equalsIgnoreCase(
+    return client.isPresent() && SCM_CLIENT_WUI.equalsIgnoreCase(client.get());
-      request.getHeader(HEADER_SCM_CLIENT));
   }
 
   //~--- methods --------------------------------------------------------------

scm-core/src/test/java/sonia/scm/security/SessionIdTest.java

@@ -0,0 +1,42 @@
+package sonia.scm.security;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import javax.servlet.http.HttpServletRequest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.lenient;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class SessionIdTest {
+
+  @Mock
+  private HttpServletRequest request;
+
+  @Test
+  void shouldReturnSessionIdFromHeader() {
+    when(request.getHeader(SessionId.PARAMETER)).thenReturn("abc42");
+
+    assertThat(SessionId.from(request)).contains(SessionId.valueOf("abc42"));
+  }
+
+  @Test
+  void shouldReturnSessionIdFromQueryParameter() {
+    when(request.getMethod()).thenReturn("GET");
+    when(request.getParameter(SessionId.PARAMETER)).thenReturn("abc42");
+
+    assertThat(SessionId.from(request)).contains(SessionId.valueOf("abc42"));
+  }
+
+  @Test
+  void shouldReturnSessionIdFromQueryParameterOnlyForGetRequest() {
+    when(request.getMethod()).thenReturn("POST");
+    lenient().when(request.getParameter(SessionId.PARAMETER)).thenReturn("abc42");
+
+    assertThat(SessionId.from(request)).isEmpty();
+  }
+}

scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java

@@ -38,7 +38,9 @@ package sonia.scm.util;
 import org.junit.Test;
 
 import sonia.scm.config.ScmConfiguration;
+import sonia.scm.security.SessionId;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.*;
 
 import static org.mockito.Mockito.*;
@@ -466,4 +468,47 @@ public class HttpUtilTest
     assertEquals("test/two/three",
       HttpUtil.getUriWithoutStartSeperator("test/two/three"));
   }
+
+  @Test
+  public void testGetHeaderOrGetParameterWithHeader() {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getHeader("Domain")).thenReturn("hitchhiker");
+
+    assertThat(HttpUtil.getHeaderOrGetParameter(request, "Domain")).contains("hitchhiker");
+  }
+
+  @Test
+  public void testGetHeaderOrGetParameterWithParameter() {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getMethod()).thenReturn("GET");
+    when(request.getParameter("Domain")).thenReturn("hitchhiker");
+
+    assertThat(HttpUtil.getHeaderOrGetParameter(request, "Domain")).contains("hitchhiker");
+  }
+
+  @Test
+  public void testGetHeaderOrGetParameterOnPost() {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getMethod()).thenReturn("POST");
+    lenient().when(request.getParameter("Domain")).thenReturn("hitchhiker");
+
+    assertThat(HttpUtil.getHeaderOrGetParameter(request, "Domain")).isEmpty();
+  }
+
+  @Test
+  public void testIsWUIRequestWithHeader() {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI);
+
+    assertThat(HttpUtil.isWUIRequest(request)).isTrue();
+  }
+
+  @Test
+  public void testIsWUIRequestWithParameter() {
+    HttpServletRequest request = mock(HttpServletRequest.class);
+    when(request.getMethod()).thenReturn("GET");
+    when(request.getParameter(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI);
+
+    assertThat(HttpUtil.isWUIRequest(request)).isTrue();
+  }
 }

scm-webapp/src/main/java/sonia/scm/web/BearerWebTokenGenerator.java

@@ -35,6 +35,7 @@ package sonia.scm.web;
 
 import sonia.scm.plugin.Extension;
 import sonia.scm.security.BearerToken;
+import sonia.scm.security.SessionId;
 import sonia.scm.util.HttpUtil;
 
 //~--- JDK imports ------------------------------------------------------------
@@ -68,10 +69,8 @@ public class BearerWebTokenGenerator extends SchemeBasedWebTokenGenerator
   {
     BearerToken token = null;
 
-    if (HttpUtil.AUTHORIZATION_SCHEME_BEARER.equalsIgnoreCase(scheme))
+    if (HttpUtil.AUTHORIZATION_SCHEME_BEARER.equalsIgnoreCase(scheme)) {
-    {
+      token = BearerToken.create(SessionId.from(request).orElse(null), authorization);
-      String sessionId = request.getHeader(HttpUtil.HEADER_SCM_SESSION);
-      token = BearerToken.create(sessionId, authorization);
     }
 
     return token;

scm-webapp/src/main/java/sonia/scm/web/CookieBearerWebTokenGenerator.java

@@ -40,6 +40,8 @@ import sonia.scm.security.BearerToken;
 
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
+
+import sonia.scm.security.SessionId;
 import sonia.scm.util.HttpUtil;
 
 /**
@@ -62,19 +64,14 @@ public class CookieBearerWebTokenGenerator implements WebTokenGenerator
    * @return {@link BearerToken} or {@code null}
    */
   @Override
-  public BearerToken createToken(HttpServletRequest request)
+  public BearerToken createToken(HttpServletRequest request) {
-  {
     BearerToken token = null;
     Cookie[] cookies = request.getCookies();
 
-    if (cookies != null)
+    if (cookies != null) {
-    {
+      for (Cookie cookie : cookies) {
-      for (Cookie cookie : cookies)
+        if (HttpUtil.COOKIE_BEARER_AUTHENTICATION.equals(cookie.getName())) {
-      {
+          token = BearerToken.create(SessionId.from(request).orElse(null), cookie.getValue());
-        if (HttpUtil.COOKIE_BEARER_AUTHENTICATION.equals(cookie.getName()))
-        {
-          String sessionId = HttpUtil.getHeader(request, HttpUtil.HEADER_SCM_SESSION, null);
-          token = BearerToken.create(sessionId, cookie.getValue());
 
           break;
         }

scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java

@@ -31,7 +31,6 @@
 
 package sonia.scm.security;
 
-import com.google.common.collect.ImmutableSet;
 import org.apache.shiro.authc.AuthenticationInfo;
 import org.apache.shiro.authc.UsernamePasswordToken;
 import org.junit.jupiter.api.BeforeEach;
@@ -42,7 +41,6 @@ import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.HashMap;
-import java.util.Set;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -84,7 +82,7 @@ class BearerRealmTest {
 
   @Test
   void shouldDoGetAuthentication() {
-    BearerToken bearerToken = BearerToken.create("__session__", "__bearer__");
+    BearerToken bearerToken = BearerToken.create(SessionId.valueOf("__session__"), "__bearer__");
     AccessToken accessToken = mock(AccessToken.class);
 
     when(accessToken.getSubject()).thenReturn("trillian");

scm-webapp/src/test/java/sonia/scm/web/BearerWebTokenGeneratorTest.java

@@ -31,20 +31,19 @@
 
 package sonia.scm.web;
 
-import javax.servlet.http.HttpServletRequest;
 import org.apache.shiro.authc.AuthenticationToken;
-
-
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.mockito.Mock;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.*;
 import org.mockito.junit.jupiter.MockitoExtension;
 import sonia.scm.security.BearerToken;
 import sonia.scm.security.SessionId;
-import sonia.scm.util.HttpUtil;
+
+import javax.servlet.http.HttpServletRequest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.when;
 
 /**
  *
@@ -90,7 +89,7 @@ class BearerWebTokenGeneratorTest {
   @Test
   void shouldCreateTokenWithSessionId(){
     doReturn("Bearer asd").when(request).getHeader("Authorization");
-    doReturn("bcd123").when(request).getHeader(HttpUtil.HEADER_SCM_SESSION);
+    doReturn("bcd123").when(request).getHeader(SessionId.PARAMETER);
 
     AuthenticationToken token = tokenGenerator.createToken(request);
     assertThat(token)

scm-webapp/src/test/java/sonia/scm/web/CookieBearerWebTokenGeneratorTest.java

@@ -76,7 +76,7 @@ class CookieBearerWebTokenGeneratorTest {
 
   @Test
   void shouldCreateTokenWithSessionId() {
-    when(request.getHeader(HttpUtil.HEADER_SCM_SESSION)).thenReturn("abc123");
+    when(request.getHeader(SessionId.PARAMETER)).thenReturn("abc123");
 
     assignBearerCookie("authc");