diff --git a/scm-core/src/main/java/sonia/scm/security/BearerToken.java b/scm-core/src/main/java/sonia/scm/security/BearerToken.java index 43a225b5b6..f42147fbe2 100644 --- a/scm-core/src/main/java/sonia/scm/security/BearerToken.java +++ b/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); } } diff --git a/scm-core/src/main/java/sonia/scm/security/SessionId.java b/scm-core/src/main/java/sonia/scm/security/SessionId.java index 3c4fb126ff..7d1b657bfb 100644 --- a/scm-core/src/main/java/sonia/scm/security/SessionId.java +++ b/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 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); diff --git a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java index 8edc0c8e14..820f492b8f 100644 --- a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java +++ b/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 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) - { - return SCM_CLIENT_WUI.equalsIgnoreCase( - request.getHeader(HEADER_SCM_CLIENT)); + public static boolean isWUIRequest(HttpServletRequest request) { + Optional client = getHeaderOrGetParameter(request, HEADER_SCM_CLIENT); + return client.isPresent() && SCM_CLIENT_WUI.equalsIgnoreCase(client.get()); } //~--- methods -------------------------------------------------------------- diff --git a/scm-core/src/test/java/sonia/scm/security/SessionIdTest.java b/scm-core/src/test/java/sonia/scm/security/SessionIdTest.java new file mode 100644 index 0000000000..89bcf5f594 --- /dev/null +++ b/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(); + } +} diff --git a/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java b/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java index c827764a92..8e2804a749 100644 --- a/scm-core/src/test/java/sonia/scm/util/HttpUtilTest.java +++ b/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(); + } } diff --git a/scm-webapp/src/main/java/sonia/scm/web/BearerWebTokenGenerator.java b/scm-webapp/src/main/java/sonia/scm/web/BearerWebTokenGenerator.java index 4bb79e7b9a..21330c6e17 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/BearerWebTokenGenerator.java +++ b/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)) - { - String sessionId = request.getHeader(HttpUtil.HEADER_SCM_SESSION); - token = BearerToken.create(sessionId, authorization); + if (HttpUtil.AUTHORIZATION_SCHEME_BEARER.equalsIgnoreCase(scheme)) { + token = BearerToken.create(SessionId.from(request).orElse(null), authorization); } return token; diff --git a/scm-webapp/src/main/java/sonia/scm/web/CookieBearerWebTokenGenerator.java b/scm-webapp/src/main/java/sonia/scm/web/CookieBearerWebTokenGenerator.java index 2a0a97c07e..a6b2a16296 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/CookieBearerWebTokenGenerator.java +++ b/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) - { - for (Cookie cookie : cookies) - { - if (HttpUtil.COOKIE_BEARER_AUTHENTICATION.equals(cookie.getName())) - { - String sessionId = HttpUtil.getHeader(request, HttpUtil.HEADER_SCM_SESSION, null); - token = BearerToken.create(sessionId, cookie.getValue()); + if (cookies != null) { + for (Cookie cookie : cookies) { + if (HttpUtil.COOKIE_BEARER_AUTHENTICATION.equals(cookie.getName())) { + token = BearerToken.create(SessionId.from(request).orElse(null), cookie.getValue()); break; } diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index 897b251cfa..6213c234bb 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/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"); diff --git a/scm-webapp/src/test/java/sonia/scm/web/BearerWebTokenGeneratorTest.java b/scm-webapp/src/test/java/sonia/scm/web/BearerWebTokenGeneratorTest.java index 1c054111b7..d77d2068b9 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/BearerWebTokenGeneratorTest.java +++ b/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) diff --git a/scm-webapp/src/test/java/sonia/scm/web/CookieBearerWebTokenGeneratorTest.java b/scm-webapp/src/test/java/sonia/scm/web/CookieBearerWebTokenGeneratorTest.java index 45d2c46b83..8cfe8c1b31 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/CookieBearerWebTokenGeneratorTest.java +++ b/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");