diff --git a/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java new file mode 100644 index 0000000000..16dca8e910 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/util/CRLFInjectionException.java @@ -0,0 +1,8 @@ +package sonia.scm.util; + +public class CRLFInjectionException extends IllegalArgumentException{ + + public CRLFInjectionException(String message) { + super(message); + } +} diff --git a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java index bc3f4e74cd..2744addc62 100644 --- a/scm-core/src/main/java/sonia/scm/util/HttpUtil.java +++ b/scm-core/src/main/java/sonia/scm/util/HttpUtil.java @@ -344,8 +344,7 @@ public final class HttpUtil "parameter \"{}\" contains a character which could be an indicator for a crlf injection", parameter); - throw new IllegalArgumentException( - "parameter contains an illegal character"); + throw new CRLFInjectionException("parameter contains an illegal character"); } } diff --git a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java index ecab36969c..ddcce9f4e8 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -12,6 +12,8 @@ public class VndMediaType { private static final String SUBTYPE_PREFIX = "vnd.scmm-"; public static final String PREFIX = TYPE + "/" + SUBTYPE_PREFIX; public static final String SUFFIX = "+json;v=" + VERSION; + public static final String PLAIN_TEXT_PREFIX = "text/" + SUBTYPE_PREFIX; + public static final String PLAIN_TEXT_SUFFIX = "+plain;v=" + VERSION; public static final String USER = PREFIX + "user" + SUFFIX; public static final String GROUP = PREFIX + "group" + SUFFIX; @@ -22,6 +24,7 @@ public class VndMediaType { public static final String TAG = PREFIX + "tag" + SUFFIX; public static final String TAG_COLLECTION = PREFIX + "tagCollection" + SUFFIX; public static final String BRANCH = PREFIX + "branch" + SUFFIX; + public static final String DIFF = PLAIN_TEXT_PREFIX + "diff" + PLAIN_TEXT_SUFFIX; public static final String USER_COLLECTION = PREFIX + "userCollection" + SUFFIX; public static final String GROUP_COLLECTION = PREFIX + "groupCollection" + SUFFIX; public static final String REPOSITORY_COLLECTION = PREFIX + "repositoryCollection" + SUFFIX; diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index 85764291a7..399dd26035 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -246,5 +246,40 @@ public class RepositoryAccessITCase { assertThat(changesets).size().isBetween(2, 3); // svn has an implicit root revision '0' that is extra to the two commits } + + @Test + public void shouldFindDiffs() throws IOException { + RepositoryClient repositoryClient = RepositoryUtil.createRepositoryClient(repositoryType, folder); + + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "a.txt", "a"); + RepositoryUtil.createAndCommitFile(repositoryClient, "scmadmin", "b.txt", "b"); + + String changesetsUrl = given() + .when() + .get(TestData.getDefaultRepositoryUrl(repositoryType)) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .path("_links.changesets.href"); + + String diffUrl = given() + .when() + .get(changesetsUrl) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .path("_embedded.changesets[0]._links.diff.href"); + + given() + .when() + .get(diffUrl) + .then() + .statusCode(HttpStatus.SC_OK) + .extract() + .body() + .asString() + .contains("diff"); + + } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/IllegalArgumentExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/rest/IllegalArgumentExceptionMapper.java index 4a8a5854f0..44d4d9194d 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/IllegalArgumentExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/IllegalArgumentExceptionMapper.java @@ -45,7 +45,8 @@ import javax.ws.rs.ext.Provider; * @author Sebastian Sdorra * @since 1.36 */ -@Provider @Slf4j +@Provider +@Slf4j public class IllegalArgumentExceptionMapper implements ExceptionMapper { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java new file mode 100644 index 0000000000..f4e8d3aa3c --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/CRLFInjectionExceptionMapper.java @@ -0,0 +1,13 @@ +package sonia.scm.api.v2.resources; + +import sonia.scm.api.rest.StatusExceptionMapper; +import sonia.scm.util.CRLFInjectionException; + +import javax.ws.rs.core.Response; + +public class CRLFInjectionExceptionMapper extends StatusExceptionMapper { + + public CRLFInjectionExceptionMapper() { + super(CRLFInjectionException.class, Response.Status.BAD_REQUEST); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java index 176a86dda7..99a996b02f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DiffRootResource.java @@ -1,27 +1,71 @@ package sonia.scm.api.v2.resources; -import javax.ws.rs.DefaultValue; +import com.webcohesion.enunciate.metadata.rs.ResponseCode; +import com.webcohesion.enunciate.metadata.rs.StatusCodes; +import sonia.scm.NotFoundException; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.RevisionNotFoundException; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.util.HttpUtil; +import sonia.scm.web.VndMediaType; + +import javax.inject.Inject; import javax.ws.rs.GET; import javax.ws.rs.Path; -import javax.ws.rs.QueryParam; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; +import javax.ws.rs.core.StreamingOutput; public class DiffRootResource { + public static final String HEADER_CONTENT_DISPOSITION = "Content-Disposition"; + private final RepositoryServiceFactory serviceFactory; - @GET - @Path("") - public Response getAll(@DefaultValue("0") @QueryParam("page") int page, - @DefaultValue("10") @QueryParam("pageSize") int pageSize, - @QueryParam("sortBy") String sortBy, - @DefaultValue("false") @QueryParam("desc") boolean desc) { - throw new UnsupportedOperationException(); + @Inject + public DiffRootResource(RepositoryServiceFactory serviceFactory) { + this.serviceFactory = serviceFactory; } + + /** + * Get the repository diff of a revision + * + * @param namespace repository namespace + * @param name repository name + * @param revision the revision + * @return the dif of the revision + * @throws NotFoundException if the repository is not found + */ @GET - @Path("{id}") - public Response get(String id) { - throw new UnsupportedOperationException(); + @Path("{revision}") + @Produces(VndMediaType.DIFF) + @StatusCodes({ + @ResponseCode(code = 200, condition = "success"), + @ResponseCode(code = 400, condition = "Bad Request"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user has no privileges to read the diff"), + @ResponseCode(code = 404, condition = "not found, no revision with the specified param for the repository available or repository not found"), + @ResponseCode(code = 500, condition = "internal server error") + }) + public Response get(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("revision") String revision) throws NotFoundException { + HttpUtil.checkForCRLFInjection(revision); + try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { + StreamingOutput responseEntry = output -> { + try { + repositoryService.getDiffCommand() + .setRevision(revision) + .retriveContent(output); + } catch (RevisionNotFoundException e) { + throw new WebApplicationException(Response.Status.NOT_FOUND); + } + }; + return Response.ok(responseEntry) + .header(HEADER_CONTENT_DISPOSITION, HttpUtil.createContentDispositionAttachmentHeader(String.format("%s-%s.diff", name, revision))) + .build(); + } } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java new file mode 100644 index 0000000000..0daeb07b7c --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DiffResourceTest.java @@ -0,0 +1,149 @@ +package sonia.scm.api.v2.resources; + + +import lombok.extern.slf4j.Slf4j; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; +import org.jboss.resteasy.core.Dispatcher; +import org.jboss.resteasy.mock.MockDispatcherFactory; +import org.jboss.resteasy.mock.MockHttpRequest; +import org.jboss.resteasy.mock.MockHttpResponse; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import sonia.scm.api.rest.AuthorizationExceptionMapper; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryNotFoundException; +import sonia.scm.repository.RevisionNotFoundException; +import sonia.scm.repository.api.DiffCommandBuilder; +import sonia.scm.repository.api.RepositoryService; +import sonia.scm.repository.api.RepositoryServiceFactory; +import sonia.scm.web.VndMediaType; + +import java.net.URISyntaxException; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.Assert.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.Silent.class) +@Slf4j +public class DiffResourceTest { + + + public static final String DIFF_PATH = "space/repo/diff/"; + public static final String DIFF_URL = "/" + RepositoryRootResource.REPOSITORIES_PATH_V2 + DIFF_PATH; + private final Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); + + @Mock + private RepositoryServiceFactory serviceFactory; + + @Mock + private RepositoryService service; + + @Mock + private DiffCommandBuilder diffCommandBuilder; + + private DiffRootResource diffRootResource; + + + private final Subject subject = mock(Subject.class); + private final ThreadState subjectThreadState = new SubjectThreadState(subject); + + + @Before + public void prepareEnvironment() throws Exception { + diffRootResource = new DiffRootResource(serviceFactory); + RepositoryRootResource repositoryRootResource = new RepositoryRootResource(MockProvider + .of(new RepositoryResource(null, null, null, null, null, + null, null, null, null, MockProvider.of(diffRootResource))), null); + dispatcher.getRegistry().addSingletonResource(repositoryRootResource); + when(serviceFactory.create(new NamespaceAndName("space", "repo"))).thenReturn(service); + when(serviceFactory.create(any(Repository.class))).thenReturn(service); + when(service.getRepository()).thenReturn(new Repository("repoId", "git", "space", "repo")); + dispatcher.getProviderFactory().registerProvider(NotFoundExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(AuthorizationExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(CRLFInjectionExceptionMapper.class); + when(service.getDiffCommand()).thenReturn(diffCommandBuilder); + subjectThreadState.bind(); + ThreadContext.bind(subject); + when(subject.isPermitted(any(String.class))).thenReturn(true); + } + + @After + public void cleanupContext() { + ThreadContext.unbindSubject(); + } + + @Test + public void shouldGetDiffs() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenReturn(diffCommandBuilder); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(200, response.getStatus()); + log.info("Response :{}", response.getContentAsString()); + assertThat(response.getStatus()) + .isEqualTo(200); + assertThat(response.getContentAsString()) + .isNotNull(); + String expectedHeader = "Content-Disposition"; + String expectedValue = "attachment; filename=\"repo-revision.diff\"; filename*=utf-8''repo-revision.diff"; + assertThat(response.getOutputHeaders().containsKey(expectedHeader)).isTrue(); + assertThat((String) response.getOutputHeaders().get("Content-Disposition").get(0)) + .contains(expectedValue); + } + + @Test + public void shouldGet404OnMissingRepository() throws URISyntaxException, RepositoryNotFoundException { + when(serviceFactory.create(any(NamespaceAndName.class))).thenThrow(RepositoryNotFoundException.class); + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(404, response.getStatus()); + } + + @Test + public void shouldGet404OnMissingRevision() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenThrow(RevisionNotFoundException.class); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "revision") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(404, response.getStatus()); + } + + @Test + public void shouldGet400OnCrlfInjection() throws Exception { + when(diffCommandBuilder.setRevision(anyString())).thenReturn(diffCommandBuilder); + when(diffCommandBuilder.retriveContent(any())).thenThrow(RevisionNotFoundException.class); + + MockHttpRequest request = MockHttpRequest + .get(DIFF_URL + "ny%0D%0ASet-cookie:%20Tamper=3079675143472450634") + .accept(VndMediaType.DIFF); + MockHttpResponse response = new MockHttpResponse(); + dispatcher.invoke(request, response); + assertEquals(400, response.getStatus()); + } + + + +} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java index a59ed0c103..2c4893b362 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/SourceRootResourceTest.java @@ -1,7 +1,6 @@ package sonia.scm.api.v2.resources; import org.jboss.resteasy.core.Dispatcher; -import org.jboss.resteasy.mock.MockDispatcherFactory; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before;