From e18480ad2b24b1827e6d5914cdf42bfb8783a16b Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Mon, 30 Nov 2020 15:25:29 +0100 Subject: [PATCH] add permission check to hal links for tags --- .../scm/repository/spi/HgTagsCommand.java | 2 +- .../scm/repository/spi/HgTagCommandTest.java | 5 +-- .../scm/repository/spi/HgTagsCommandTest.java | 30 +++++++++++++ .../api/v2/resources/BranchRootResource.java | 2 +- .../DefaultChangesetToChangesetDtoMapper.java | 7 ++- .../resources/TagCollectionToDtoMapper.java | 11 +++-- .../scm/api/v2/resources/TagRootResource.java | 6 +-- .../api/v2/resources/TagToTagDtoMapper.java | 6 +-- .../v2/resources/TagToTagDtoMapperTest.java | 45 ++++++++++++++++--- 9 files changed, 89 insertions(+), 25 deletions(-) diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgTagsCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgTagsCommand.java index d5d48b5018..c48fb04445 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgTagsCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgTagsCommand.java @@ -39,7 +39,7 @@ import java.util.List; */ public class HgTagsCommand extends AbstractCommand implements TagsCommand { - public static final String DEFAULT_TAG_NAME = "default"; + public static final String DEFAULT_TAG_NAME = "tip"; /** * Constructs ... diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagCommandTest.java index 30cb0d6765..e15d54dcb6 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagCommandTest.java @@ -34,7 +34,6 @@ import sonia.scm.repository.api.TagCreateRequest; import sonia.scm.repository.api.TagDeleteRequest; import sonia.scm.repository.work.NoneCachingWorkingCopyPool; import sonia.scm.repository.work.WorkdirProvider; -import sonia.scm.web.HgRepositoryEnvironmentBuilder; import java.util.List; @@ -46,10 +45,8 @@ public class HgTagCommandTest extends AbstractHgCommandTestBase { @Before public void initWorkingCopyFactory() { - HgRepositoryEnvironmentBuilder hgRepositoryEnvironmentBuilder = - new HgRepositoryEnvironmentBuilder(handler, HgTestUtil.createHookManager()); - workingCopyFactory = new SimpleHgWorkingCopyFactory(Providers.of(hgRepositoryEnvironmentBuilder), new NoneCachingWorkingCopyPool(new WorkdirProvider())) { + workingCopyFactory = new SimpleHgWorkingCopyFactory(new NoneCachingWorkingCopyPool(new WorkdirProvider())) { @Override public void configure(PullCommand pullCommand) { // we do not want to configure http hooks in this unit test diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagsCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagsCommandTest.java index e9bb65bf89..2d5f171b16 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagsCommandTest.java +++ b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgTagsCommandTest.java @@ -24,15 +24,34 @@ package sonia.scm.repository.spi; +import com.aragost.javahg.commands.PullCommand; +import org.junit.Before; import org.junit.Test; import sonia.scm.repository.Tag; +import sonia.scm.repository.api.TagCommandBuilder; +import sonia.scm.repository.work.NoneCachingWorkingCopyPool; +import sonia.scm.repository.work.WorkdirProvider; +import java.io.IOException; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; public class HgTagsCommandTest extends AbstractHgCommandTestBase { + private SimpleHgWorkingCopyFactory workingCopyFactory; + + @Before + public void initWorkingCopyFactory() { + + workingCopyFactory = new SimpleHgWorkingCopyFactory(new NoneCachingWorkingCopyPool(new WorkdirProvider())) { + @Override + public void configure(PullCommand pullCommand) { + // we do not want to configure http hooks in this unit test + } + }; + } + @Test public void shouldGetTagDatesCorrectly() { HgTagsCommand hgTagsCommand = new HgTagsCommand(cmdContext); @@ -42,4 +61,15 @@ public class HgTagsCommandTest extends AbstractHgCommandTestBase { assertThat(tags.get(0).getDate()).contains(1339586381000L); } + @Test + public void shouldNotDie() throws IOException { + HgTagCommand hgTagCommand = new HgTagCommand(cmdContext, workingCopyFactory); + new TagCommandBuilder(cmdContext.get(), hgTagCommand).create().setRevision("79b6baf49711").setName("newtag").execute(); + + HgTagsCommand hgTagsCommand = new HgTagsCommand(cmdContext); + final List tags = hgTagsCommand.getTags(); + + assertThat(tags).isNotEmpty(); + } + } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java index 258dd1e3e5..8ef295c4be 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/BranchRootResource.java @@ -354,7 +354,7 @@ public class BranchRootResource { @PathParam("name") String name, @PathParam("branch") String branch) { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { - RepositoryPermissions.modify(repositoryService.getRepository()).check(); + RepositoryPermissions.push(repositoryService.getRepository()).check(); Optional branchToBeDeleted = repositoryService.getBranchesCommand().getBranches().getBranches().stream() .filter(b -> b.getName().equalsIgnoreCase(branch)) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java index 69977065e7..f3ae07fac9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/DefaultChangesetToChangesetDtoMapper.java @@ -36,6 +36,7 @@ import sonia.scm.repository.Changeset; import sonia.scm.repository.Contributor; import sonia.scm.repository.Person; import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.Signature; import sonia.scm.repository.Tags; import sonia.scm.repository.api.Command; @@ -128,9 +129,11 @@ public abstract class DefaultChangesetToChangesetDtoMapper extends HalAppenderMa } if (tags != null) { embeddedBuilder.with("tags", tagCollectionToDtoMapper.getTagDtoList(namespace, name, - getListOfObjects(source.getTags(), tags::getTagByName))); + getListOfObjects(source.getTags(), tags::getTagByName), repository)); + } + if (RepositoryPermissions.push(repository).isPermitted()) { + linksBuilder.single(link("tag", resourceLinks.tag().create(namespace, name))); } - linksBuilder.single(link("tag", resourceLinks.tag().create(namespace, name))); } if (repositoryService.isSupported(Command.BRANCHES)) { embeddedBuilder.with("branches", branchCollectionToDtoMapper.getBranchDtoList(repository, diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagCollectionToDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagCollectionToDtoMapper.java index 625a4eda0f..b7ab918128 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagCollectionToDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagCollectionToDtoMapper.java @@ -29,6 +29,8 @@ import de.otto.edison.hal.Embedded; import de.otto.edison.hal.HalRepresentation; import de.otto.edison.hal.Links; import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.Tag; import java.util.Collection; @@ -50,12 +52,13 @@ public class TagCollectionToDtoMapper { this.tagToTagDtoMapper = tagToTagDtoMapper; } - public HalRepresentation map(String namespace, String name, Collection tags) { - return new HalRepresentation(createLinks(namespace, name), embedDtos(getTagDtoList(namespace, name, tags))); + public HalRepresentation map(String namespace, String name, Collection tags, Repository repository) { + return new HalRepresentation(createLinks(namespace, name), embedDtos(getTagDtoList(namespace, name, tags, repository))); } - public List getTagDtoList(String namespace, String name, Collection tags) { - return tags.stream().map(tag -> tagToTagDtoMapper.map(tag, new NamespaceAndName(namespace, name))).collect(toList()); + public List getTagDtoList(String namespace, String name, Collection tags, Repository repository) { + final NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name); + return tags.stream().map(tag -> tagToTagDtoMapper.map(tag, namespaceAndName, repository)).collect(toList()); } private Links createLinks(String namespace, String name) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagRootResource.java index 0f5cb2c450..37760e070a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagRootResource.java @@ -100,7 +100,7 @@ public class TagRootResource { try (RepositoryService repositoryService = serviceFactory.create(new NamespaceAndName(namespace, name))) { Tags tags = getTags(repositoryService); if (tags != null && tags.getTags() != null) { - return Response.ok(tagCollectionToDtoMapper.map(namespace, name, tags.getTags())).build(); + return Response.ok(tagCollectionToDtoMapper.map(namespace, name, tags.getTags(), repositoryService.getRepository())).build(); } else { return Response.status(Response.Status.INTERNAL_SERVER_ERROR) .entity("Error on getting tag from repository.") @@ -194,7 +194,7 @@ public class TagRootResource { .filter(t -> tagName.equals(t.getName())) .findFirst() .orElseThrow(() -> createNotFoundException(namespace, name, tagName)); - return Response.ok(tagToTagDtoMapper.map(tag, namespaceAndName)).build(); + return Response.ok(tagToTagDtoMapper.map(tag, namespaceAndName, repositoryService.getRepository())).build(); } else { return Response.status(Response.Status.INTERNAL_SERVER_ERROR) .entity("Error on getting tag from repository.") @@ -230,7 +230,7 @@ public class TagRootResource { public Response delete(@PathParam("namespace") String namespace, @PathParam("name") String name, @PathParam("tagName") String tagName) { NamespaceAndName namespaceAndName = new NamespaceAndName(namespace, name); try (RepositoryService repositoryService = serviceFactory.create(namespaceAndName)) { - RepositoryPermissions.modify(repositoryService.getRepository()).check(); + RepositoryPermissions.push(repositoryService.getRepository()).check(); if (tagExists(tagName, repositoryService)) { repositoryService.getTagCommand().delete() diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagToTagDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagToTagDtoMapper.java index 43c3777eac..19747d21c7 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagToTagDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/TagToTagDtoMapper.java @@ -55,16 +55,16 @@ public abstract class TagToTagDtoMapper extends HalAppenderMapper { @Mapping(target = "date", source = "date", qualifiedByName = "mapDate") @Mapping(target = "attributes", ignore = true) // We do not map HAL attributes @Mapping(target = "signatures") - public abstract TagDto map(Tag tag, @Context NamespaceAndName namespaceAndName); + public abstract TagDto map(Tag tag, @Context NamespaceAndName namespaceAndName, @Context Repository repository); @ObjectFactory - TagDto createDto(@Context NamespaceAndName namespaceAndName, Tag tag) { + TagDto createDto(@Context NamespaceAndName namespaceAndName, @Context Repository repository, Tag tag) { Links.Builder linksBuilder = linkingTo() .self(resourceLinks.tag().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getName())) .single(link("sources", resourceLinks.source().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getRevision()))) .single(link("changeset", resourceLinks.changeset().self(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getRevision()))); - if (tag.getDeletable()) { + if (tag.getDeletable() && RepositoryPermissions.push(repository).isPermitted()) { linksBuilder .single(link("delete", resourceLinks.tag().delete(namespaceAndName.getNamespace(), namespaceAndName.getName(), tag.getName()))); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagToTagDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagToTagDtoMapperTest.java index 0e37db28c8..c0d1bb8289 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagToTagDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/TagToTagDtoMapperTest.java @@ -24,11 +24,19 @@ package sonia.scm.api.v2.resources; +import org.apache.shiro.subject.Subject; +import org.apache.shiro.util.ThreadContext; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryTestData; import sonia.scm.repository.Signature; import sonia.scm.repository.SignatureStatus; import sonia.scm.repository.Tag; @@ -38,6 +46,7 @@ import java.time.Instant; import java.util.Collections; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class TagToTagDtoMapperTest { @@ -48,6 +57,19 @@ class TagToTagDtoMapperTest { @InjectMocks private TagToTagDtoMapperImpl mapper; + @Mock + private Subject subject; + + @BeforeEach + void setupSubject() { + ThreadContext.bind(subject); + } + + @AfterEach + void tearDown() { + ThreadContext.unbindSubject(); + } + @Test void shouldAppendLinks() { HalEnricherRegistry registry = new HalEnricherRegistry(); @@ -58,20 +80,20 @@ class TagToTagDtoMapperTest { }); mapper.setRegistry(registry); - TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); assertThat(dto.getLinks().getLinkBy("yo").get().getHref()).isEqualTo("http://hitchhiker/hog/1.0.0"); } @Test void shouldMapDate() { final long now = Instant.now().getEpochSecond() * 1000; - TagDto dto = mapper.map(new Tag("1.0.0", "42", now), new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(new Tag("1.0.0", "42", now), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); assertThat(dto.getDate()).isEqualTo(Instant.ofEpochMilli(now)); } @Test void shouldContainSignatureArray() { - TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(new Tag("1.0.0", "42"), new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); assertThat(dto.getSignatures()).isNotNull(); } @@ -79,21 +101,30 @@ class TagToTagDtoMapperTest { void shouldMapSignatures() { final Tag tag = new Tag("1.0.0", "42"); tag.addSignature(new Signature("29v391239v", "gpg", SignatureStatus.VERIFIED, "me", Collections.emptySet())); - TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); assertThat(dto.getSignatures()).isNotEmpty(); } @Test - void shouldAddDeleteLinksByDefault() { + void shouldAddDeleteLink() { + Repository repository = RepositoryTestData.createHeartOfGold(); + when(subject.isPermitted("repository:push:" + repository.getId())).thenReturn(true); final Tag tag = new Tag("1.0.0", "42"); - TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(tag, new NamespaceAndName(repository.getNamespace(), repository.getName()), repository); assertThat(dto.getLinks().getLinkBy("delete")).isNotEmpty(); } + @Test + void shouldNotAddDeleteLinkIfPermissionsAreMissing() { + final Tag tag = new Tag("1.0.0", "42"); + TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); + assertThat(dto.getLinks().getLinkBy("delete")).isEmpty(); + } + @Test void shouldNotAddDeleteLinksForUndeletableTags() { final Tag tag = new Tag("1.0.0", "42", null, false); - TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog")); + TagDto dto = mapper.map(tag, new NamespaceAndName("hitchhiker", "hog"), RepositoryTestData.createHeartOfGold()); assertThat(dto.getLinks().getLinkBy("delete")).isEmpty(); }