From d86b2f70c33506fdb7d3baa7afd68dcedb1785d2 Mon Sep 17 00:00:00 2001
From: Sebastian Sdorra <sebastian.sdorra@cloudogu.com>
Date: Sun, 8 Nov 2020 12:23:15 +0100
Subject: [PATCH] Disable xsrf for mercurial hook tokens

---
 .../scm/repository/DefaultHgEnvironmentBuilder.java  | 12 ++++++++++--
 .../repository/DefaultHgEnvironmentBuilderTest.java  |  3 ++-
 .../sonia/scm/security/JwtAccessTokenBuilder.java    | 10 +++++-----
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java
index c4f37bda3b..215025da43 100644
--- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java
+++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/DefaultHgEnvironmentBuilder.java
@@ -32,6 +32,7 @@ import sonia.scm.repository.hooks.HookServer;
 import sonia.scm.security.AccessToken;
 import sonia.scm.security.AccessTokenBuilderFactory;
 import sonia.scm.security.CipherUtil;
+import sonia.scm.security.Xsrf;
 import sonia.scm.web.HgUtil;
 
 import javax.inject.Inject;
@@ -109,11 +110,18 @@ public class DefaultHgEnvironmentBuilder implements HgEnvironmentBuilder {
 
   private void write(ImmutableMap.Builder<String, String> env) {
     env.put(ENV_HOOK_PORT, String.valueOf(getHookPort()));
-    AccessToken accessToken = accessTokenBuilderFactory.create().build();
-    env.put(ENV_BEARER_TOKEN, CipherUtil.getInstance().encode(accessToken.compact()));
+    env.put(ENV_BEARER_TOKEN, accessToken());
     env.put(ENV_CHALLENGE, hookEnvironment.getChallenge());
   }
 
+  private String accessToken() {
+    AccessToken accessToken = accessTokenBuilderFactory.create()
+      // disable xsrf protection, because we can not access the http servlet request for verification
+      .custom(Xsrf.TOKEN_KEY, null)
+      .build();
+    return CipherUtil.getInstance().encode(accessToken.compact());
+  }
+
   private synchronized int getHookPort() {
     if (hookPort > 0) {
       return hookPort;
diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java
index 53ae99e631..c28bb21e78 100644
--- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java
+++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/DefaultHgEnvironmentBuilderTest.java
@@ -38,6 +38,7 @@ import sonia.scm.repository.hooks.HookServer;
 import sonia.scm.security.AccessToken;
 import sonia.scm.security.AccessTokenBuilderFactory;
 import sonia.scm.security.CipherUtil;
+import sonia.scm.security.Xsrf;
 
 import javax.annotation.Nonnull;
 import java.io.File;
@@ -118,7 +119,7 @@ class DefaultHgEnvironmentBuilderTest {
 
   private void applyAccessToken(String compact) {
     AccessToken accessToken = mock(AccessToken.class);
-    when(accessTokenBuilderFactory.create().build()).thenReturn(accessToken);
+    when(accessTokenBuilderFactory.create().custom(Xsrf.TOKEN_KEY, null).build()).thenReturn(accessToken);
     when(accessToken.compact()).thenReturn(compact);
   }
 
diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
index 2455a01eba..c0d8521220 100644
--- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
+++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java
@@ -30,6 +30,7 @@ import com.google.common.collect.Maps;
 import io.jsonwebtoken.Claims;
 import io.jsonwebtoken.Jwts;
 import io.jsonwebtoken.SignatureAlgorithm;
+import io.jsonwebtoken.security.Keys;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.authz.AuthorizationException;
 import org.apache.shiro.subject.Subject;
@@ -87,8 +88,6 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
 
   @Override
   public JwtAccessTokenBuilder custom(String key, Object value) {
-    Preconditions.checkArgument(!Strings.isNullOrEmpty(key), "null or empty value not allowed");
-    Preconditions.checkArgument(value != null, "null or empty value not allowed");
     this.custom.put(key, value);
     return this;
   }
@@ -183,8 +182,8 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
 
 
     if (refreshableFor > 0) {
-      long refreshExpiration = refreshableForUnit.toMillis(refreshableFor);
-      claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + refreshExpiration).getTime());
+      long re = refreshableForUnit.toMillis(refreshableFor);
+      claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, new Date(now.toEpochMilli() + re).getTime());
     } else if (refreshExpiration != null) {
       claims.put(JwtAccessToken.REFRESHABLE_UNTIL_CLAIM_KEY, Date.from(refreshExpiration));
     }
@@ -198,10 +197,11 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder {
       claims.setIssuer(issuer);
     }
 
+
     // sign token and create compact version
     String compact = Jwts.builder()
         .setClaims(claims)
-        .signWith(SignatureAlgorithm.HS256, key.getBytes())
+        .signWith(Keys.hmacShaKeyFor(key.getBytes()), SignatureAlgorithm.HS256)
         .compact();
 
     return new JwtAccessToken(claims, compact);