fix cookie path, if scm-manager runs with context path /

scm-webapp/src/main/java/sonia/scm/security/AccessTokenCookieIssuer.java

@@ -31,20 +31,19 @@
 package sonia.scm.security;
 
 import com.google.common.annotations.VisibleForTesting;
-import java.util.Date;
+import com.google.common.base.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sonia.scm.config.ScmConfiguration;
+import sonia.scm.util.HttpUtil;
+import sonia.scm.util.Util;
 
-import java.util.concurrent.TimeUnit;
 import javax.inject.Inject;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import sonia.scm.config.ScmConfiguration;
-import sonia.scm.util.HttpUtil;
-import sonia.scm.util.Util;
+import java.util.Date;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Generates cookies and invalidates access token cookies.
@@ -81,7 +80,7 @@ public final class AccessTokenCookieIssuer {
   public void authenticate(HttpServletRequest request, HttpServletResponse response, AccessToken accessToken) {
     LOG.trace("create and attach cookie for access token {}", accessToken.getId());
     Cookie c = new Cookie(HttpUtil.COOKIE_BEARER_AUTHENTICATION, accessToken.compact());
-    c.setPath(request.getContextPath());
+    c.setPath(contextPath(request));
     c.setMaxAge(getMaxAge(accessToken));
     c.setHttpOnly(isHttpOnly());
     c.setSecure(isSecure(request));
@@ -100,7 +99,7 @@ public final class AccessTokenCookieIssuer {
     LOG.trace("invalidates access token cookie");
     
     Cookie c = new Cookie(HttpUtil.COOKIE_BEARER_AUTHENTICATION, Util.EMPTY_STRING);
-    c.setPath(request.getContextPath());
+    c.setPath(contextPath(request));
     c.setMaxAge(0);
     c.setHttpOnly(isHttpOnly());
     c.setSecure(isSecure(request));
@@ -109,6 +108,15 @@ public final class AccessTokenCookieIssuer {
     response.addCookie(c);
   }
 
+  @VisibleForTesting
+  String contextPath(HttpServletRequest request) {
+    String contextPath = request.getContextPath();
+    if (Strings.isNullOrEmpty(contextPath)) {
+      return "/";
+    }
+    return contextPath;
+  }
+  
   private int getMaxAge(AccessToken accessToken){
     long maxAgeMs = accessToken.getExpiration().getTime() - new Date().getTime();
     return (int) TimeUnit.MILLISECONDS.toSeconds(maxAgeMs);

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

@@ -0,0 +1,113 @@
+package sonia.scm.security;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+import sonia.scm.config.ScmConfiguration;
+
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import java.util.Date;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AccessTokenCookieIssuerTest {
+
+  private ScmConfiguration configuration;
+
+  private AccessTokenCookieIssuer issuer;
+
+  @Mock
+  private HttpServletRequest request;
+
+  @Mock
+  private HttpServletResponse response;
+
+  @Mock
+  private AccessToken accessToken;
+
+  @Captor
+  private ArgumentCaptor<Cookie> cookieArgumentCaptor;
+
+  @Before
+  public void setUp() {
+    configuration = new ScmConfiguration();
+    issuer = new AccessTokenCookieIssuer(configuration);
+  }
+
+  @Test
+  public void testContextPath() {
+    assertContextPath("/scm", "/scm");
+    assertContextPath("/", "/");
+    assertContextPath("", "/");
+    assertContextPath(null, "/");
+  }
+
+  @Test
+  public void httpOnlyShouldBeEnabledIfXsrfProtectionIsDisabled() {
+    configuration.setEnabledXsrfProtection(false);
+
+    Cookie cookie = authenticate();
+
+    assertTrue(cookie.isHttpOnly());
+  }
+
+  @Test
+  public void httpOnlyShouldBeDisabled() {
+    Cookie cookie = authenticate();
+
+    assertFalse(cookie.isHttpOnly());
+  }
+
+  @Test
+  public void secureShouldBeSetIfTheRequestIsSecure() {
+    when(request.isSecure()).thenReturn(true);
+
+    Cookie cookie = authenticate();
+
+    assertTrue(cookie.getSecure());
+  }
+
+  @Test
+  public void secureShouldBeDisabledIfTheRequestIsNotSecure() {
+    when(request.isSecure()).thenReturn(false);
+
+    Cookie cookie = authenticate();
+
+    assertFalse(cookie.getSecure());
+  }
+
+  @Test
+  public void testInvalidate() {
+    issuer.invalidate(request, response);
+
+    verify(response).addCookie(cookieArgumentCaptor.capture());
+    Cookie cookie = cookieArgumentCaptor.getValue();
+
+    assertEquals(0, cookie.getMaxAge());
+  }
+
+  private Cookie authenticate() {
+    when(accessToken.getExpiration()).thenReturn(new Date());
+
+    issuer.authenticate(request, response, accessToken);
+
+    verify(response).addCookie(cookieArgumentCaptor.capture());
+    return cookieArgumentCaptor.getValue();
+  }
+
+
+  private void assertContextPath(String contextPath, String expected) {
+    when(request.getContextPath()).thenReturn(contextPath);
+    assertEquals(expected, issuer.contextPath(request));
+  }
+}