Merge pull request #24721 from overleaf/msm-cleanup-git-oauth-secret

[git-bridge] Cleanup `oauth` clientID/secret

GitOrigin-RevId: 48144d928119782d1c7b048b0cb6a4afb6072f28

services/git-bridge/README.md

@@ -76,12 +76,10 @@ The configuration file is in `.json` format.
         "postbackBaseUrl" (string): the postback url,
         "serviceName" (string): current name of writeLaTeX
                                 in case it ever changes,
-        "oauth2" (object): { null or missing if oauth2 shouldn't be used
-            "oauth2ClientID" (string): oauth2 client ID,
-            "oauth2ClientSecret" (string): oauth2 client secret,
-            "oauth2Server" (string): oauth2 server,
-                                     with protocol and
-                                     without trailing slash
+        "oauth2Server" (string): oauth2 server,
+                                 with protocol and
+                                 without trailing slash,
+                                 null or missing if oauth2 shouldn't be used
         },
         "repoStore" (object, optional): { configure the repo store
             "maxFileSize" (long, optional): maximum size of a file, inclusive

services/git-bridge/conf/envsubst_template.json

@@ -7,11 +7,7 @@
   "apiBaseUrl": "${GIT_BRIDGE_API_BASE_URL:-https://localhost/api/v0}",
   "postbackBaseUrl": "${GIT_BRIDGE_POSTBACK_BASE_URL:-https://localhost}",
   "serviceName": "${GIT_BRIDGE_SERVICE_NAME:-Overleaf}",
-  "oauth2": {
-    "oauth2ClientID": "${GIT_BRIDGE_OAUTH2_CLIENT_ID}",
-    "oauth2ClientSecret": "${GIT_BRIDGE_OAUTH2_CLIENT_SECRET}",
-    "oauth2Server": "${GIT_BRIDGE_OAUTH2_SERVER:-https://localhost}"
-  },
+  "oauth2Server": "${GIT_BRIDGE_OAUTH2_SERVER:-https://localhost}",
   "userPasswordEnabled": ${GIT_BRIDGE_USER_PASSWORD_ENABLED:-false},
   "repoStore": {
     "maxFileNum": ${GIT_BRIDGE_REPOSTORE_MAX_FILE_NUM:-2000},

services/git-bridge/conf/example_config.json

@@ -7,11 +7,7 @@
     "apiBaseUrl": "https://localhost/api/v0",
     "postbackBaseUrl": "https://localhost",
     "serviceName": "Overleaf",
-    "oauth2": {
-        "oauth2ClientID": "asdf",
-        "oauth2ClientSecret": "asdf",
-        "oauth2Server": "https://localhost"
-    },
+    "oauth2Server": "https://localhost",
     "repoStore": {
         "maxFileNum": 2000,
         "maxFileSize": 52428800

services/git-bridge/conf/local.json

@@ -7,11 +7,7 @@
   "apiBaseUrl": "http://v2.overleaf.test:3000/api/v0",
   "postbackBaseUrl": "http://git-bridge:8000",
   "serviceName": "Overleaf",
-  "oauth2": {
-    "oauth2ClientID": "264c723c925c13590880751f861f13084934030c13b4452901e73bdfab226edc",
-    "oauth2ClientSecret": "e6b2e9eee7ae2bb653823250bb69594a91db0547fe3790a7135acb497108e62d",
-    "oauth2Server": "http://v2.overleaf.test:3000"
-  },
+  "oauth2Server": "http://v2.overleaf.test:3000",
   "repoStore": {
     "maxFileNum": 2000,
     "maxFileSize": 52428800

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/application/config/Config.java

@@ -30,7 +30,7 @@ public class Config implements JSONSource {
         config.apiBaseURL,
         config.postbackURL,
         config.serviceName,
-        Oauth2.asSanitised(config.oauth2),
+        config.oauth2Server,
         config.userPasswordEnabled,
         config.repoStore,
         SwapStoreConfig.sanitisedCopy(config.swapStore),
@@ -46,7 +46,7 @@ public class Config implements JSONSource {
   private String apiBaseURL;
   private String postbackURL;
   private String serviceName;
-  @Nullable private Oauth2 oauth2;
+  @Nullable private String oauth2Server;
   private boolean userPasswordEnabled;
   @Nullable private RepoStoreConfig repoStore;
   @Nullable private SwapStoreConfig swapStore;
@@ -70,7 +70,7 @@ public class Config implements JSONSource {
       String apiBaseURL,
       String postbackURL,
       String serviceName,
-      Oauth2 oauth2,
+      String oauth2Server,
       boolean userPasswordEnabled,
       RepoStoreConfig repoStore,
       SwapStoreConfig swapStore,
@@ -84,7 +84,7 @@ public class Config implements JSONSource {
     this.apiBaseURL = apiBaseURL;
     this.postbackURL = postbackURL;
     this.serviceName = serviceName;
-    this.oauth2 = oauth2;
+    this.oauth2Server = oauth2Server;
     this.userPasswordEnabled = userPasswordEnabled;
     this.repoStore = repoStore;
     this.swapStore = swapStore;
@@ -116,7 +116,7 @@ public class Config implements JSONSource {
     if (!postbackURL.endsWith("/")) {
       postbackURL += "/";
     }
-    oauth2 = new Gson().fromJson(configObject.get("oauth2"), Oauth2.class);
+    oauth2Server = getOptionalString(configObject, "oauth2Server");
     userPasswordEnabled = getOptionalString(configObject, "userPasswordEnabled").equals("true");
     repoStore = new Gson().fromJson(configObject.get("repoStore"), RepoStoreConfig.class);
     swapStore = new Gson().fromJson(configObject.get("swapStore"), SwapStoreConfig.class);
@@ -166,19 +166,12 @@ public class Config implements JSONSource {
     return postbackURL;
   }
 
-  public boolean isUsingOauth2() {
-    return oauth2 != null;
-  }
-
   public boolean isUserPasswordEnabled() {
     return userPasswordEnabled;
   }
 
-  public Oauth2 getOauth2() {
-    if (!isUsingOauth2()) {
-      throw new AssertionError("Getting oauth2 when not using it");
-    }
-    return oauth2;
+  public String getOauth2Server() {
+    return oauth2Server;
   }
 
   public Optional<RepoStoreConfig> getRepoStore() {

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/application/config/Oauth2.java

@@ -1,33 +0,0 @@
-package uk.ac.ic.wlgitbridge.application.config;
-
-/*
- * Created by winston on 25/10/15.
- */
-public class Oauth2 {
-
-  private final String oauth2ClientID;
-  private final String oauth2ClientSecret;
-  private final String oauth2Server;
-
-  public Oauth2(String oauth2ClientID, String oauth2ClientSecret, String oauth2Server) {
-    this.oauth2ClientID = oauth2ClientID;
-    this.oauth2ClientSecret = oauth2ClientSecret;
-    this.oauth2Server = oauth2Server;
-  }
-
-  public String getOauth2ClientID() {
-    return oauth2ClientID;
-  }
-
-  public String getOauth2ClientSecret() {
-    return oauth2ClientSecret;
-  }
-
-  public String getOauth2Server() {
-    return oauth2Server;
-  }
-
-  public static Oauth2 asSanitised(Oauth2 oauth2) {
-    return new Oauth2("<oauth2ClientID>", "<oauth2ClientSecret>", oauth2.oauth2Server);
-  }
-}

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/GitBridgeServer.java

@@ -151,9 +151,9 @@ public class GitBridgeServer {
       throws ServletException {
     final ServletContextHandler servletContextHandler =
         new ServletContextHandler(ServletContextHandler.SESSIONS);
-    if (config.isUsingOauth2()) {
+    if (config.getOauth2Server() != null) {
       Filter filter =
-          new Oauth2Filter(snapshotApi, config.getOauth2(), config.isUserPasswordEnabled());
+          new Oauth2Filter(snapshotApi, config.getOauth2Server(), config.isUserPasswordEnabled());
       servletContextHandler.addFilter(
           new FilterHolder(filter), "/*", EnumSet.of(DispatcherType.REQUEST));
     }

services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java

@@ -13,7 +13,6 @@ import javax.servlet.*;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import org.apache.commons.codec.binary.Base64;
-import uk.ac.ic.wlgitbridge.application.config.Oauth2;
 import uk.ac.ic.wlgitbridge.bridge.snapshot.SnapshotApi;
 import uk.ac.ic.wlgitbridge.util.Instance;
 import uk.ac.ic.wlgitbridge.util.Log;
@@ -28,13 +27,13 @@ public class Oauth2Filter implements Filter {
 
   private final SnapshotApi snapshotApi;
 
-  private final Oauth2 oauth2;
+  private final String oauth2Server;
 
   private final boolean isUserPasswordEnabled;
 
-  public Oauth2Filter(SnapshotApi snapshotApi, Oauth2 oauth2, boolean isUserPasswordEnabled) {
+  public Oauth2Filter(SnapshotApi snapshotApi, String oauth2Server, boolean isUserPasswordEnabled) {
     this.snapshotApi = snapshotApi;
-    this.oauth2 = oauth2;
+    this.oauth2Server = oauth2Server;
     this.isUserPasswordEnabled = isUserPasswordEnabled;
   }
 
@@ -108,7 +107,7 @@ public class Oauth2Filter implements Filter {
       // fail later (for example, in the unlikely event that the token
       // expired between the two requests). In that case, JGit will
       // return a 401 without a custom error message.
-      int statusCode = checkAccessToken(oauth2, password, getClientIp(request));
+      int statusCode = checkAccessToken(this.oauth2Server, password, getClientIp(request));
       if (statusCode == 429) {
         handleRateLimit(projectId, username, request, response);
         return;
@@ -238,10 +237,9 @@ public class Oauth2Filter implements Filter {
             "your Overleaf Account Settings."));
   }
 
-  private int checkAccessToken(Oauth2 oauth2, String accessToken, String clientIp)
+  private int checkAccessToken(String oauth2Server, String accessToken, String clientIp)
       throws IOException {
-    GenericUrl url =
-        new GenericUrl(oauth2.getOauth2Server() + "/oauth/token/info?client_ip=" + clientIp);
+    GenericUrl url = new GenericUrl(oauth2Server + "/oauth/token/info?client_ip=" + clientIp);
     HttpRequest request = Instance.httpRequestFactory.buildGetRequest(url);
     HttpHeaders headers = new HttpHeaders();
     headers.setAuthorization("Bearer " + accessToken);

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java

@@ -1495,13 +1495,9 @@ public class WLGitBridgeIntegrationTest {
             + port
             + "\",\n"
             + "    \"serviceName\": \"Overleaf\",\n"
-            + "    \"oauth2\": {\n"
-            + "        \"oauth2ClientID\": \"clientID\",\n"
-            + "        \"oauth2ClientSecret\": \"oauth2 client secret\",\n"
-            + "        \"oauth2Server\": \"http://127.0.0.1:"
+            + "    \"oauth2Server\": \"http://127.0.0.1:"
             + apiPort
-            + "\"\n"
-            + "    }";
+            + "\"";
     if (swapCfg != null) {
       cfgStr +=
           ",\n"
@@ -1524,7 +1520,6 @@ public class WLGitBridgeIntegrationTest {
               + ",\n"
               + "        \"intervalMillis\": "
               + swapCfg.getIntervalMillis()
-              + "\n"
               + "    }\n";
     }
     cfgStr += "}\n";

services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/config/ConfigTest.java

@@ -23,11 +23,7 @@ public class ConfigTest {
                 + "    \"apiBaseUrl\": \"http://127.0.0.1:60000/api/v0\",\n"
                 + "    \"postbackBaseUrl\": \"http://127.0.0.1\",\n"
                 + "    \"serviceName\": \"Overleaf\",\n"
-                + "    \"oauth2\": {\n"
-                + "        \"oauth2ClientID\": \"clientID\",\n"
-                + "        \"oauth2ClientSecret\": \"oauth2 client secret\",\n"
-                + "        \"oauth2Server\": \"https://www.overleaf.com\"\n"
-                + "    }\n"
+                + "    \"oauth2Server\": \"https://www.overleaf.com\"\n"
                 + "}\n");
     Config config = new Config(reader);
     assertEquals(80, config.getPort());
@@ -35,10 +31,7 @@ public class ConfigTest {
     assertEquals("http://127.0.0.1:60000/api/v0/", config.getAPIBaseURL());
     assertEquals("http://127.0.0.1/", config.getPostbackURL());
     assertEquals("Overleaf", config.getServiceName());
-    assertTrue(config.isUsingOauth2());
-    assertEquals("clientID", config.getOauth2().getOauth2ClientID());
-    assertEquals("oauth2 client secret", config.getOauth2().getOauth2ClientSecret());
-    assertEquals("https://www.overleaf.com", config.getOauth2().getOauth2Server());
+    assertEquals("https://www.overleaf.com", config.getOauth2Server());
   }
 
   @Test(expected = AssertionError.class)
@@ -53,7 +46,7 @@ public class ConfigTest {
                 + "    \"apiBaseUrl\": \"http://127.0.0.1:60000/api/v0\",\n"
                 + "    \"postbackBaseUrl\": \"http://127.0.0.1\",\n"
                 + "    \"serviceName\": \"Overleaf\",\n"
-                + "    \"oauth2\": null\n"
+                + "    \"oauth2Server\": null\n"
                 + "}\n");
     Config config = new Config(reader);
     assertEquals(80, config.getPort());
@@ -61,8 +54,7 @@ public class ConfigTest {
     assertEquals("http://127.0.0.1:60000/api/v0/", config.getAPIBaseURL());
     assertEquals("http://127.0.0.1/", config.getPostbackURL());
     assertEquals("Overleaf", config.getServiceName());
-    assertFalse(config.isUsingOauth2());
-    config.getOauth2();
+    assertNull(config.getOauth2Server());
   }
 
   @Test
@@ -77,11 +69,7 @@ public class ConfigTest {
                 + "    \"apiBaseUrl\": \"http://127.0.0.1:60000/api/v0\",\n"
                 + "    \"postbackBaseUrl\": \"http://127.0.0.1\",\n"
                 + "    \"serviceName\": \"Overleaf\",\n"
-                + "    \"oauth2\": {\n"
-                + "        \"oauth2ClientID\": \"my oauth2 client id\",\n"
-                + "        \"oauth2ClientSecret\": \"my oauth2 client secret\",\n"
-                + "        \"oauth2Server\": \"https://www.overleaf.com\"\n"
-                + "    }\n"
+                + "    \"oauth2Server\": \"https://www.overleaf.com\"\n"
                 + "}\n");
     Config config = new Config(reader);
     String expected =
@@ -94,11 +82,7 @@ public class ConfigTest {
             + "  \"apiBaseURL\": \"http://127.0.0.1:60000/api/v0/\",\n"
             + "  \"postbackURL\": \"http://127.0.0.1/\",\n"
             + "  \"serviceName\": \"Overleaf\",\n"
-            + "  \"oauth2\": {\n"
-            + "    \"oauth2ClientID\": \"<oauth2ClientID>\",\n"
-            + "    \"oauth2ClientSecret\": \"<oauth2ClientSecret>\",\n"
-            + "    \"oauth2Server\": \"https://www.overleaf.com\"\n"
-            + "  },\n"
+            + "  \"oauth2Server\": \"https://www.overleaf.com\",\n"
             + "  \"userPasswordEnabled\": false,\n"
             + "  \"repoStore\": null,\n"
             + "  \"swapStore\": null,\n"