diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java index 2a5e550021..38ed53f504 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/Bridge.java @@ -678,7 +678,7 @@ public class Bridge { private void makeCommitsFromSnapshots( ProjectRepo repo, Collection snapshots - ) throws IOException, SizeLimitExceededException { + ) throws IOException, GitUserException { String name = repo.getProjectName(); for (Snapshot snapshot : snapshots) { Map fileTable = repo.getFiles(); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java index 618af58f23..741030be1a 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/GitProjectRepo.java @@ -9,6 +9,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import uk.ac.ic.wlgitbridge.data.filestore.GitDirectoryContents; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; +import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import uk.ac.ic.wlgitbridge.git.util.RepositoryObjectTreeWalker; import uk.ac.ic.wlgitbridge.util.Log; @@ -62,7 +63,7 @@ public class GitProjectRepo implements ProjectRepo { @Override public Map getFiles() - throws IOException, SizeLimitExceededException { + throws IOException, GitUserException { Preconditions.checkState(repository.isPresent()); return new RepositoryObjectTreeWalker( repository.get() diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/ProjectRepo.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/ProjectRepo.java index c327ac1315..c3ac9c2f3a 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/ProjectRepo.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/bridge/repo/ProjectRepo.java @@ -2,6 +2,7 @@ package uk.ac.ic.wlgitbridge.bridge.repo; import uk.ac.ic.wlgitbridge.data.filestore.GitDirectoryContents; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; +import uk.ac.ic.wlgitbridge.git.exception.GitUserException; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import java.io.IOException; @@ -24,7 +25,7 @@ public interface ProjectRepo { ) throws IOException; Map getFiles( - ) throws IOException, SizeLimitExceededException; + ) throws IOException, GitUserException; Collection commitAndGetMissing( GitDirectoryContents gitDirectoryContents diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/InvalidGitRepository.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/InvalidGitRepository.java new file mode 100644 index 0000000000..465a3fc726 --- /dev/null +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/exception/InvalidGitRepository.java @@ -0,0 +1,22 @@ +package uk.ac.ic.wlgitbridge.git.exception; + +import java.util.Arrays; +import java.util.List; + +public class InvalidGitRepository extends GitUserException { + + @Override + public String getMessage() { + return "invalid git repo"; + } + + @Override + public List getDescriptionLines() { + return Arrays.asList( + "Your Git repository is invalid.", + "If your project contains a Git submodule,", + "please remove it and try again." + ); + } + +} diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java index f35fd0fc54..50fe9ae56d 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/handler/hook/WriteLatexPutHook.java @@ -74,7 +74,7 @@ public class WriteLatexPutHook implements PreReceiveHook { ); } catch (OutOfDateException e) { receiveCommand.setResult(Result.REJECTED_NONFASTFORWARD); - } catch (SnapshotPostException e) { + } catch (GitUserException e) { handleSnapshotPostException(receivePack, receiveCommand, e); } catch (Throwable t) { Log.warn("Throwable on pre receive: ", t); @@ -90,7 +90,7 @@ public class WriteLatexPutHook implements PreReceiveHook { private void handleSnapshotPostException( ReceivePack receivePack, ReceiveCommand receiveCommand, - SnapshotPostException e + GitUserException e ) { String message = e.getMessage(); receivePack.sendError(message); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/util/RepositoryObjectTreeWalker.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/util/RepositoryObjectTreeWalker.java index b31a168106..262cdd5d17 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/util/RepositoryObjectTreeWalker.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/git/util/RepositoryObjectTreeWalker.java @@ -8,6 +8,7 @@ import org.eclipse.jgit.treewalk.TreeWalk; import uk.ac.ic.wlgitbridge.data.filestore.RawDirectory; import uk.ac.ic.wlgitbridge.data.filestore.RawFile; import uk.ac.ic.wlgitbridge.data.filestore.RepositoryFile; +import uk.ac.ic.wlgitbridge.git.exception.InvalidGitRepository; import uk.ac.ic.wlgitbridge.git.exception.SizeLimitExceededException; import java.io.IOException; @@ -44,7 +45,7 @@ public class RepositoryObjectTreeWalker { } public RawDirectory getDirectoryContents( - ) throws IOException, SizeLimitExceededException { + ) throws IOException, SizeLimitExceededException, InvalidGitRepository { return new RawDirectory(walkGitObjectTree()); } @@ -63,7 +64,7 @@ public class RepositoryObjectTreeWalker { } private Map walkGitObjectTree( - ) throws IOException, SizeLimitExceededException { + ) throws IOException, SizeLimitExceededException, InvalidGitRepository { Map fileContentsTable = new HashMap<>(); if (treeWalk == null) { return fileContentsTable; @@ -71,9 +72,13 @@ public class RepositoryObjectTreeWalker { while (treeWalk.next()) { String path = treeWalk.getPathString(); + ObjectId objectId = treeWalk.getObjectId(0); + if (!repository.hasObject(objectId)) { + throw new InvalidGitRepository(); + } try { byte[] content = repository.open( - treeWalk.getObjectId(0) + objectId ).getBytes(); fileContentsTable.put(path, new RepositoryFile(path, content)); } catch (LargeObjectException e) { diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java index d2b650d717..32d8e52217 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/server/Oauth2Filter.java @@ -8,6 +8,7 @@ import uk.ac.ic.wlgitbridge.application.config.Oauth2; import uk.ac.ic.wlgitbridge.snapshot.base.ForbiddenException; import uk.ac.ic.wlgitbridge.snapshot.getdoc.GetDocRequest; import uk.ac.ic.wlgitbridge.util.Instance; +import uk.ac.ic.wlgitbridge.util.Log; import uk.ac.ic.wlgitbridge.util.Util; import javax.servlet.*; @@ -34,6 +35,17 @@ public class Oauth2Filter implements Filter { @Override public void init(FilterConfig filterConfig) {} + /** + * The original request from git will not contain the Authorization header. + * + * So, for projects that need auth, we return 401. Git will swallow this + * and prompt the user for user/pass, and then make a brand new request. + * @param servletRequest + * @param servletResponse + * @param filterChain + * @throws IOException + * @throws ServletException + */ @Override public void doFilter( ServletRequest servletRequest, @@ -44,24 +56,29 @@ public class Oauth2Filter implements Filter { ((Request) servletRequest).getRequestURI().split("/")[1], ".git" ); + Log.info("[{}] Checking if auth needed", project); GetDocRequest doc = new GetDocRequest(project); doc.request(); try { doc.getResult(); } catch (ForbiddenException e) { + Log.info("[{}] Auth needed", project); getAndInjectCredentials( + project, servletRequest, servletResponse, filterChain ); return; } + Log.info("[{}] Auth not needed"); filterChain.doFilter(servletRequest, servletResponse); } // TODO: this is ridiculous. Check for error cases first, then return/throw // TODO: also, use an Optional credential, since we treat it as optional private void getAndInjectCredentials( + String projectName, ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain @@ -71,6 +88,7 @@ public class Oauth2Filter implements Filter { String authHeader = request.getHeader("Authorization"); if (authHeader != null) { + Log.info("[{}] Authorization header present"); StringTokenizer st = new StringTokenizer(authHeader); if (st.hasMoreTokens()) { String basic = st.nextToken(); @@ -100,10 +118,9 @@ public class Oauth2Filter implements Filter { oauth2.getOauth2ClientID(), oauth2.getOauth2ClientSecret() ) - ) - .execute().getAccessToken(); + ).execute().getAccessToken(); } catch (TokenResponseException e) { - unauthorized(response); + unauthorized(projectName, response); return; } final Credential cred = new Credential.Builder( @@ -118,7 +135,7 @@ public class Oauth2Filter implements Filter { servletResponse ); } else { - unauthorized(response); + unauthorized(projectName, response); } } catch (UnsupportedEncodingException e) { throw new Error("Couldn't retrieve authentication", e); @@ -126,7 +143,7 @@ public class Oauth2Filter implements Filter { } } } else { - unauthorized(response); + unauthorized(projectName, response); } } @@ -134,8 +151,10 @@ public class Oauth2Filter implements Filter { public void destroy() {} private void unauthorized( + String projectName, ServletResponse servletResponse ) throws IOException { + Log.info("[{}] Unauthorized", projectName); HttpServletResponse response = (HttpServletResponse) servletResponse; response.setContentType("text/plain"); response.setHeader("WWW-Authenticate", "Basic realm=\"Git Bridge\""); diff --git a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/SnapshotAPIRequest.java b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/SnapshotAPIRequest.java index b659653760..d9a77e1df4 100644 --- a/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/SnapshotAPIRequest.java +++ b/services/git-bridge/src/main/java/uk/ac/ic/wlgitbridge/snapshot/base/SnapshotAPIRequest.java @@ -39,6 +39,13 @@ public abstract class SnapshotAPIRequest extends Request { ).intercept(request1); oauth2.intercept(request1); }); + } else { + request.setInterceptor(request1 -> { + new BasicAuthentication( + USERNAME, + PASSWORD + ).intercept(request1); + }); } } diff --git a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java index 67d3e985ed..6d8a2b45ac 100644 --- a/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java +++ b/services/git-bridge/src/test/java/uk/ac/ic/wlgitbridge/application/WLGitBridgeIntegrationTest.java @@ -107,6 +107,9 @@ public class WLGitBridgeIntegrationTest { put("wlgbCanSwapProjects", new HashMap() {{ put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/wlgbCanSwapProjects/state/state.json")).build()); }}); + put("pushSubmoduleFailsWithInvalidGitRepo", new HashMap() {{ + put("state", new SnapshotAPIStateBuilder(getResourceAsStream("/pushSubmoduleFailsWithInvalidGitRepo/state/state.json")).build()); + }}); }}; @Rule @@ -624,6 +627,43 @@ public class WLGitBridgeIntegrationTest { wlgb.stop(); } + private static final List EXPECTED_OUT_PUSH_SUBMODULE = Arrays.asList( + "remote: hint: Your Git repository is invalid.", + "remote: hint: If your project contains a Git submodule,", + "remote: hint: please remove it and try again.", + "To http://127.0.0.1:33875/testproj.git", + "! [remote rejected] master -> master (invalid git repo)", + "error: failed to push some refs to 'http://127.0.0.1:33875/testproj.git'" + ); + + @Test + public void pushSubmoduleFailsWithInvalidGitRepo() throws IOException, GitAPIException, InterruptedException { + MockSnapshotServer server = new MockSnapshotServer(3875, getResource("/pushSubmoduleFailsWithInvalidGitRepo").toFile()); + server.start(); + server.setState(states.get("pushSubmoduleFailsWithInvalidGitRepo").get("state")); + GitBridgeApp wlgb = new GitBridgeApp(new String[] { + makeConfigFile(33875, 3875) + }); + wlgb.run(); + File dir = folder.newFolder(); + File testprojDir = cloneRepository("testproj", 33875, dir); + runtime.exec("mkdir sub", null, testprojDir).waitFor(); + File sub = new File(testprojDir, "sub"); + runtime.exec("touch sub.txt", null, sub).waitFor(); + runtime.exec("git init", null, sub).waitFor(); + runtime.exec("git add -A", null, sub).waitFor(); + runtime.exec("git commit -m \"sub\"", null, sub).waitFor(); + runtime.exec("git add -A", null, testprojDir).waitFor(); + runtime.exec("git commit -m \"push\"", null, testprojDir).waitFor(); + Process gitPush = runtime.exec("git push", null, testprojDir); + int pushExitCode = gitPush.waitFor(); + wlgb.stop(); + assertEquals(1, pushExitCode); + List actual = Util.linesFromStream(gitPush.getErrorStream(), 2, "[K"); + assertEquals(EXPECTED_OUT_PUSH_SUBMODULE, actual); + wlgb.stop(); + } + private File cloneRepository(String repositoryName, int port, File dir) throws IOException, InterruptedException { String repo = "git clone http://127.0.0.1:" + port + "/" + repositoryName + ".git"; Process gitProcess = runtime.exec(repo, null, dir); diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/state.json b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/state.json new file mode 100644 index 0000000000..d876a5492c --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/state.json @@ -0,0 +1,46 @@ +[ + { + "project": "testproj", + "getDoc": { + "versionID": 1, + "createdAt": "2014-11-30T18:40:58.123Z", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1" + }, + "getSavedVers": [ + { + "versionID": 1, + "comment": "added more info on doc GET and error details", + "email": "jdleesmiller+1@gmail.com", + "name": "John+1", + "createdAt": "2014-11-30T18:47:01.333Z" + } + ], + "getForVers": [ + { + "versionID": 1, + "srcs": [ + { + "content": "content\n", + "path": "main.tex" + }, + { + "content": "This text is from another file.", + "path": "foo/bar/test.tex" + } + ], + "atts": [ + { + "url": "http://127.0.0.1:3875/state/testproj/min_mean_wait_evm_7_eps_150dpi.png", + "path": "min_mean_wait_evm_7_eps_150dpi.png" + } + ] + } + ], + "push": "success", + "postback": { + "type": "success", + "versionID": 2 + } + } +] \ No newline at end of file diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/foo/bar/test.tex b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/foo/bar/test.tex new file mode 100644 index 0000000000..046794f19a --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/foo/bar/test.tex @@ -0,0 +1 @@ +This text is from another file. \ No newline at end of file diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/main.tex b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/main.tex new file mode 100644 index 0000000000..d95f3ad14d --- /dev/null +++ b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/main.tex @@ -0,0 +1 @@ +content diff --git a/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/min_mean_wait_evm_7_eps_150dpi.png b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/min_mean_wait_evm_7_eps_150dpi.png new file mode 100644 index 0000000000..74e1fcd990 Binary files /dev/null and b/services/git-bridge/src/test/resources/uk/ac/ic/wlgitbridge/WLGitBridgeIntegrationTest/pushSubmoduleFailsWithInvalidGitRepo/state/testproj/min_mean_wait_evm_7_eps_150dpi.png differ