Merge pull request #87 from overleaf/em-strip-jwt

Strip token from blob URLs when using cache

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCache.java

@@ -43,7 +43,7 @@ public class UrlResourceCache implements ResourceCache {
             Map<String, byte[]> fetchedUrls,
             Optional<Long> maxFileSize
     ) throws IOException, SizeLimitExceededException {
-        String path = dbStore.getPathForURLInProject(projectName, url);
+        String path = dbStore.getPathForURLInProject(projectName, getCacheKeyFromUrl(url));
         byte[] contents;
         if (path == null) {
             path = newPath;
@@ -118,8 +118,25 @@ public class UrlResourceCache implements ResourceCache {
             throw new SizeLimitExceededException(
                     Optional.of(path), contents.length, maxFileSize.get());
         }
-        dbStore.addURLIndexForProject(projectName, url, path);
+        dbStore.addURLIndexForProject(projectName, getCacheKeyFromUrl(url), path);
         return contents;
     }
 
+    /**
+     * Construct a suitable cache key from the given file URL.
+     *
+     * The file URL returned by the web service may contain a token parameter
+     * used for authentication. This token changes for every request, so we
+     * need to strip it from the query string before using the URL as a cache
+     * key.
+     */
+    private String getCacheKeyFromUrl(String url) {
+        // We're not doing proper URL parsing here, but it should be enough to
+        // remove the token without touching the important parts of the URL.
+        //
+        // The URL looks like:
+        //
+        // https://history.overleaf.com/api/projects/:project_id/blobs/:hash?token=:token&_path=:path
+        return url.replaceAll("token=[^&]*", "token=REMOVED");
+    }
 }

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/bridge/resource/UrlResourceCacheTest.java

@@ -3,7 +3,7 @@ package uk.ac.ic.wlgitbridge.bridge.resource;
 import io.netty.handler.codec.http.HttpHeaders;
 import io.netty.handler.codec.http.DefaultHttpHeaders;
 import org.junit.Test;
-import uk.ac.ic.wlgitbridge.bridge.db.noop.NoopDbStore;
+import uk.ac.ic.wlgitbridge.bridge.db.DBStore;
 import uk.ac.ic.wlgitbridge.bridge.util.CastUtil;
 import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException;
 import uk.ac.ic.wlgitbridge.io.http.ning.NingHttpClientFacade;
@@ -17,6 +17,7 @@ import java.util.concurrent.ExecutionException;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.verify;
 
 public class UrlResourceCacheTest {
 
@@ -28,8 +29,9 @@ public class UrlResourceCacheTest {
 
     private final NingHttpClientFacade http = mock(NingHttpClientFacade.class);
 
-    private final UrlResourceCache cache
-            = new UrlResourceCache(new NoopDbStore(), http);
+    private final DBStore dbStore = mock(DBStore.class);
+
+    private final UrlResourceCache cache = new UrlResourceCache(dbStore, http);
 
     private static HttpHeaders withContentLength(long cl) {
         return new DefaultHttpHeaders().add("Content-Length", String.valueOf(cl));
@@ -57,6 +59,10 @@ public class UrlResourceCacheTest {
                 PROJ, URL, NEW_PATH, new HashMap<>(), new HashMap<>(), max);
     }
 
+    private void getUrl(String url) throws IOException, SizeLimitExceededException {
+        cache.get(PROJ, url, NEW_PATH, new HashMap<>(), new HashMap<>(), Optional.empty());
+    }
+
     private void getWithMaxLength(long max)
             throws IOException, SizeLimitExceededException {
         getWithMaxLength(Optional.of(max));
@@ -104,4 +110,14 @@ public class UrlResourceCacheTest {
         getWithMaxLength(5);
     }
 
+    @Test
+    public void tokenIsRemovedFromCacheKey() throws Exception {
+        String url = "http://history.overleaf.com/projects/1234/blobs/abdef?token=secretencryptedstuff&_path=test.tex";
+        String cacheKey = "http://history.overleaf.com/projects/1234/blobs/abdef?token=REMOVED&_path=test.tex";
+        respondWithContentLength(123);
+        getUrl(url);
+        verify(dbStore).getPathForURLInProject(PROJ, cacheKey);
+        verify(dbStore).addURLIndexForProject(PROJ, cacheKey, NEW_PATH);
+    }
+
 }