diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java new file mode 100644 index 0000000000..44a1cce95a --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandContext.java @@ -0,0 +1,20 @@ +package sonia.scm.protocolcommand; + +import lombok.AllArgsConstructor; +import lombok.Getter; + +import java.io.InputStream; +import java.io.OutputStream; + +@Getter +@AllArgsConstructor +public class CommandContext { + + private String command; + private String[] args; + + private InputStream inputStream; + private OutputStream outputStream; + private OutputStream errorStream; + +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java new file mode 100644 index 0000000000..24bf7d30fa --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreter.java @@ -0,0 +1,10 @@ +package sonia.scm.protocolcommand; + +public interface CommandInterpreter { + + String[] getParsedArgs(); + + ScmCommandProtocol getProtocolHandler(); + + RepositoryContextResolver getRepositoryContextResolver(); +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreterFactory.java b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreterFactory.java new file mode 100644 index 0000000000..9d6bfa1d7f --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/CommandInterpreterFactory.java @@ -0,0 +1,10 @@ +package sonia.scm.protocolcommand; + +import sonia.scm.plugin.ExtensionPoint; + +import java.util.Optional; + +@ExtensionPoint +public interface CommandInterpreterFactory { + Optional canHandle(String command); +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java new file mode 100644 index 0000000000..a64d5a6047 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContext.java @@ -0,0 +1,23 @@ +package sonia.scm.protocolcommand; + +import sonia.scm.repository.Repository; + +import java.nio.file.Path; + +public class RepositoryContext { + private Repository repository; + private Path directory; + + public RepositoryContext(Repository repository, Path directory) { + this.repository = repository; + this.directory = directory; + } + + public Repository getRepository() { + return repository; + } + + public Path getDirectory() { + return directory; + } +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java new file mode 100644 index 0000000000..2c48ece185 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/RepositoryContextResolver.java @@ -0,0 +1,8 @@ +package sonia.scm.protocolcommand; + +@FunctionalInterface +public interface RepositoryContextResolver { + + RepositoryContext resolve(String[] args); + +} diff --git a/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java new file mode 100644 index 0000000000..e7dbf65cad --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/protocolcommand/ScmCommandProtocol.java @@ -0,0 +1,9 @@ +package sonia.scm.protocolcommand; + +import java.io.IOException; + +public interface ScmCommandProtocol { + + void handle(CommandContext context, RepositoryContext repositoryContext) throws IOException; + +} diff --git a/scm-core/src/main/java/sonia/scm/repository/Repository.java b/scm-core/src/main/java/sonia/scm/repository/Repository.java index 18613c1a12..665f63487d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/Repository.java +++ b/scm-core/src/main/java/sonia/scm/repository/Repository.java @@ -307,8 +307,8 @@ public class Repository extends BasicPropertiesAware implements ModelObject, Per this.permissions.add(newPermission); } - public void removePermission(RepositoryPermission permission) { - this.permissions.remove(permission); + public boolean removePermission(RepositoryPermission permission) { + return this.permissions.remove(permission); } public void setPublicReadable(boolean publicReadable) { diff --git a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java index 54163e0393..4a458a6e6a 100644 --- a/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java +++ b/scm-core/src/main/java/sonia/scm/repository/RepositoryPermission.java @@ -45,6 +45,7 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlRootElement; import java.io.Serializable; import java.util.Collection; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.Set; @@ -55,6 +56,8 @@ import static java.util.Collections.unmodifiableSet; /** * Permissions controls the access to {@link Repository}. + * This object should be immutable, but could not be due to mapstruct. Do not modify instances of this because this + * would change the hash code and therefor make it undeletable in a repository. * * @author Sebastian Sdorra */ @@ -64,22 +67,26 @@ public class RepositoryPermission implements PermissionObject, Serializable { private static final long serialVersionUID = -2915175031430884040L; + public static final String REPOSITORY_MODIFIED_EXCEPTION_TEXT = "repository permission must not be modified"; - private boolean groupPermission = false; + private Boolean groupPermission; private String name; @XmlElement(name = "verb") private Set verbs; /** - * Constructs a new {@link RepositoryPermission}. - * This constructor is used by JAXB and mapstruct. + * This constructor exists for mapstruct and JAXB, only -- do not use this in "normal" code. + * + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public RepositoryPermission() {} public RepositoryPermission(String name, Collection verbs, boolean groupPermission) { this.name = name; - this.verbs = unmodifiableSet(new LinkedHashSet<>(verbs)); + this.verbs = new LinkedHashSet<>(verbs); this.groupPermission = groupPermission; } @@ -163,7 +170,7 @@ public class RepositoryPermission implements PermissionObject, Serializable */ public Collection getVerbs() { - return verbs == null? emptyList(): verbs; + return verbs == null ? emptyList() : Collections.unmodifiableSet(verbs); } /** @@ -181,35 +188,50 @@ public class RepositoryPermission implements PermissionObject, Serializable //~--- set methods ---------------------------------------------------------- /** - * Sets true if the permission is a group permission. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param groupPermission true if the permission is a group permission + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setGroupPermission(boolean groupPermission) { + if (this.groupPermission != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.groupPermission = groupPermission; } /** - * The name of the user or group. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param name name of the user or group + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setName(String name) { + if (this.name != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.name = name; } /** - * Sets the verb of the permission. + * Use this for creation only. This will throw an {@link IllegalStateException} when modified. + * @throws IllegalStateException when modified after the value has been set once. * - * - * @param verbs verbs of the permission + * @deprecated Do not use this for "normal" code. + * Use {@link RepositoryPermission#RepositoryPermission(String, Collection, boolean)} instead. */ + @Deprecated public void setVerbs(Collection verbs) { + if (this.verbs != null) { + throw new IllegalStateException(REPOSITORY_MODIFIED_EXCEPTION_TEXT); + } this.verbs = unmodifiableSet(new LinkedHashSet<>(verbs)); } } diff --git a/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java b/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java index d65358a66e..c7f05c5f3c 100644 --- a/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/RepositoryPermissionTest.java @@ -50,8 +50,7 @@ class RepositoryPermissionTest { @Test void shouldBeEqualWithRedundantVerbs() { RepositoryPermission permission1 = new RepositoryPermission("name1", asList("one", "two"), false); - RepositoryPermission permission2 = new RepositoryPermission("name1", asList("one", "two"), false); - permission2.setVerbs(asList("one", "two", "two")); + RepositoryPermission permission2 = new RepositoryPermission("name1", asList("one", "two", "two"), false); assertThat(permission1).isEqualTo(permission2); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java new file mode 100644 index 0000000000..4fc3a5415c --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java @@ -0,0 +1,42 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.resolver.ReceivePackFactory; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.repository.spi.HookEventFacade; +import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitReceiveHook; + +public abstract class BaseReceivePackFactory implements ReceivePackFactory { + + private final GitRepositoryHandler handler; + private final GitReceiveHook hook; + + protected BaseReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + this.handler = handler; + this.hook = new GitReceiveHook(hookEventFacade, handler); + } + + @Override + public final ReceivePack create(T connection, Repository repository) throws ServiceNotAuthorizedException, ServiceNotEnabledException { + ReceivePack receivePack = createBasicReceivePack(connection, repository); + receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); + + receivePack.setPreReceiveHook(hook); + receivePack.setPostReceiveHook(hook); + // apply collecting listener, to be able to check which commits are new + CollectingPackParserListener.set(receivePack); + + return receivePack; + } + + protected abstract ReceivePack createBasicReceivePack(T request, Repository repository) + throws ServiceNotEnabledException, ServiceNotAuthorizedException; + + private boolean isNonFastForwardAllowed() { + return ! handler.getConfig().isNonFastForwardDisallowed(); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreter.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreter.java new file mode 100644 index 0000000000..d4e6f8edbb --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreter.java @@ -0,0 +1,32 @@ +package sonia.scm.protocolcommand.git; + +import sonia.scm.protocolcommand.CommandInterpreter; +import sonia.scm.protocolcommand.RepositoryContextResolver; +import sonia.scm.protocolcommand.ScmCommandProtocol; + +class GitCommandInterpreter implements CommandInterpreter { + private final GitRepositoryContextResolver gitRepositoryContextResolver; + private final GitCommandProtocol gitCommandProtocol; + private final String[] args; + + GitCommandInterpreter(GitRepositoryContextResolver gitRepositoryContextResolver, GitCommandProtocol gitCommandProtocol, String[] args) { + this.gitRepositoryContextResolver = gitRepositoryContextResolver; + this.gitCommandProtocol = gitCommandProtocol; + this.args = args; + } + + @Override + public String[] getParsedArgs() { + return args; + } + + @Override + public ScmCommandProtocol getProtocolHandler() { + return gitCommandProtocol; + } + + @Override + public RepositoryContextResolver getRepositoryContextResolver() { + return gitRepositoryContextResolver; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreterFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreterFactory.java new file mode 100644 index 0000000000..bd38dc01db --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandInterpreterFactory.java @@ -0,0 +1,37 @@ +package sonia.scm.protocolcommand.git; + +import sonia.scm.plugin.Extension; +import sonia.scm.protocolcommand.CommandInterpreter; +import sonia.scm.protocolcommand.CommandInterpreterFactory; + +import javax.inject.Inject; +import java.util.Optional; + +import static java.util.Optional.empty; +import static java.util.Optional.of; + +@Extension +public class GitCommandInterpreterFactory implements CommandInterpreterFactory { + private final GitCommandProtocol gitCommandProtocol; + private final GitRepositoryContextResolver gitRepositoryContextResolver; + + @Inject + public GitCommandInterpreterFactory(GitCommandProtocol gitCommandProtocol, GitRepositoryContextResolver gitRepositoryContextResolver) { + this.gitCommandProtocol = gitCommandProtocol; + this.gitRepositoryContextResolver = gitRepositoryContextResolver; + } + + @Override + public Optional canHandle(String command) { + try { + String[] args = GitCommandParser.parse(command); + if (args[0].startsWith("git")) { + return of(new GitCommandInterpreter(gitRepositoryContextResolver, gitCommandProtocol, args)); + } else { + return empty(); + } + } catch (IllegalArgumentException e) { + return empty(); + } + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandParser.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandParser.java new file mode 100644 index 0000000000..624b951d42 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandParser.java @@ -0,0 +1,88 @@ +package sonia.scm.protocolcommand.git; + +import java.util.ArrayList; +import java.util.List; + +class GitCommandParser { + + private GitCommandParser() { + } + + static String[] parse(String command) { + List strs = parseDelimitedString(command, " ", true); + String[] args = strs.toArray(new String[strs.size()]); + for (int i = 0; i < args.length; i++) { + String argVal = args[i]; + if (argVal.startsWith("'") && argVal.endsWith("'")) { + args[i] = argVal.substring(1, argVal.length() - 1); + argVal = args[i]; + } + if (argVal.startsWith("\"") && argVal.endsWith("\"")) { + args[i] = argVal.substring(1, argVal.length() - 1); + } + } + + if (args.length != 2) { + throw new IllegalArgumentException("Invalid git command line (no arguments): " + command); + } + return args; + } + + private static List parseDelimitedString(String value, String delim, boolean trim) { + if (value == null) { + value = ""; + } + + List list = new ArrayList<>(); + StringBuilder sb = new StringBuilder(); + int expecting = 7; + boolean isEscaped = false; + + for(int i = 0; i < value.length(); ++i) { + char c = value.charAt(i); + boolean isDelimiter = delim.indexOf(c) >= 0; + if (!isEscaped && c == '\\') { + isEscaped = true; + } else { + if (isEscaped) { + sb.append(c); + } else if (isDelimiter && (expecting & 2) != 0) { + if (trim) { + String str = sb.toString(); + list.add(str.trim()); + } else { + list.add(sb.toString()); + } + + sb.delete(0, sb.length()); + expecting = 7; + } else if (c == '"' && (expecting & 4) != 0) { + sb.append(c); + expecting = 9; + } else if (c == '"' && (expecting & 8) != 0) { + sb.append(c); + expecting = 7; + } else { + if ((expecting & 1) == 0) { + throw new IllegalArgumentException("Invalid delimited string: " + value); + } + + sb.append(c); + } + + isEscaped = false; + } + } + + if (sb.length() > 0) { + if (trim) { + String str = sb.toString(); + list.add(str.trim()); + } else { + list.add(sb.toString()); + } + } + + return list; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java new file mode 100644 index 0000000000..b11ea80cfe --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java @@ -0,0 +1,74 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.RepositoryCache; +import org.eclipse.jgit.transport.ReceivePack; +import org.eclipse.jgit.transport.RemoteConfig; +import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import org.eclipse.jgit.util.FS; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.plugin.Extension; +import sonia.scm.protocolcommand.CommandContext; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.ScmCommandProtocol; +import sonia.scm.repository.RepositoryPermissions; + +import javax.inject.Inject; +import java.io.IOException; + +@Extension +public class GitCommandProtocol implements ScmCommandProtocol { + + private static final Logger LOG = LoggerFactory.getLogger(GitCommandProtocol.class); + + private ScmUploadPackFactory uploadPackFactory; + private ScmReceivePackFactory receivePackFactory; + + @Inject + public GitCommandProtocol(ScmUploadPackFactory uploadPackFactory, ScmReceivePackFactory receivePackFactory) { + this.uploadPackFactory = uploadPackFactory; + this.receivePackFactory = receivePackFactory; + } + + @Override + public void handle(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + String subCommand = commandContext.getArgs()[0]; + + if (RemoteConfig.DEFAULT_UPLOAD_PACK.equals(subCommand)) { + LOG.trace("got upload pack"); + upload(commandContext, repositoryContext); + } else if (RemoteConfig.DEFAULT_RECEIVE_PACK.equals(subCommand)) { + LOG.trace("got receive pack"); + receive(commandContext, repositoryContext); + } else { + throw new IllegalArgumentException("Unknown git command: " + commandContext.getCommand()); + } + } + + private void receive(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + RepositoryPermissions.push(repositoryContext.getRepository()).check(); + try (Repository repository = open(repositoryContext)) { + ReceivePack receivePack = receivePackFactory.create(repositoryContext, repository); + receivePack.receive(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); + } catch (ServiceNotEnabledException | ServiceNotAuthorizedException e) { + throw new IOException("error creating receive pack for ssh", e); + } + } + + private void upload(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { + RepositoryPermissions.pull(repositoryContext.getRepository()).check(); + try (Repository repository = open(repositoryContext)) { + UploadPack uploadPack = uploadPackFactory.create(repositoryContext, repository); + uploadPack.upload(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); + } + } + + private Repository open(RepositoryContext repositoryContext) throws IOException { + RepositoryCache.FileKey key = RepositoryCache.FileKey.lenient(repositoryContext.getDirectory().toFile(), FS.DETECTED); + return key.open(true); + } + +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java new file mode 100644 index 0000000000..8acfc68dce --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolver.java @@ -0,0 +1,44 @@ +package sonia.scm.protocolcommand.git; + +import com.google.common.base.Splitter; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.RepositoryContextResolver; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; +import sonia.scm.repository.RepositoryManager; + +import javax.inject.Inject; +import java.nio.file.Path; +import java.util.Iterator; + +public class GitRepositoryContextResolver implements RepositoryContextResolver { + + private RepositoryManager repositoryManager; + private RepositoryLocationResolver locationResolver; + + @Inject + public GitRepositoryContextResolver(RepositoryManager repositoryManager, RepositoryLocationResolver locationResolver) { + this.repositoryManager = repositoryManager; + this.locationResolver = locationResolver; + } + + public RepositoryContext resolve(String[] args) { + NamespaceAndName namespaceAndName = extractNamespaceAndName(args); + Repository repository = repositoryManager.get(namespaceAndName); + Path path = locationResolver.getPath(repository.getId()).resolve("data"); + return new RepositoryContext(repository, path); + } + + private NamespaceAndName extractNamespaceAndName(String[] args) { + String path = args[args.length - 1]; + Iterator it = Splitter.on('/').omitEmptyStrings().split(path).iterator(); + String type = it.next(); + if ("repo".equals(type)) { + String ns = it.next(); + String name = it.next(); + return new NamespaceAndName(ns, name); + } + return null; + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java new file mode 100644 index 0000000000..105a04ccdb --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java @@ -0,0 +1,21 @@ +package sonia.scm.protocolcommand.git; + +import com.google.inject.Inject; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.ReceivePack; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.repository.spi.HookEventFacade; + +public class ScmReceivePackFactory extends BaseReceivePackFactory { + + @Inject + public ScmReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + super(handler, hookEventFacade); + } + + @Override + protected ReceivePack createBasicReceivePack(RepositoryContext repositoryContext, Repository repository) { + return new ReceivePack(repository); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmUploadPackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmUploadPackFactory.java new file mode 100644 index 0000000000..962437a59b --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmUploadPackFactory.java @@ -0,0 +1,13 @@ +package sonia.scm.protocolcommand.git; + +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.UploadPack; +import org.eclipse.jgit.transport.resolver.UploadPackFactory; +import sonia.scm.protocolcommand.RepositoryContext; + +public class ScmUploadPackFactory implements UploadPackFactory { + @Override + public UploadPack create(RepositoryContext repositoryContext, Repository repository) { + return new UploadPack(repository); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitPushOrPullCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitPushOrPullCommand.java index a9b9e25aca..95215607fe 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitPushOrPullCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitPushOrPullCommand.java @@ -43,6 +43,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.transport.PushResult; import org.eclipse.jgit.transport.RemoteRefUpdate; +import org.eclipse.jgit.transport.ScmTransportProtocol; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.GitRepositoryHandler; @@ -62,7 +63,7 @@ public abstract class AbstractGitPushOrPullCommand extends AbstractGitCommand { /** Field description */ - private static final String SCHEME = "scm://"; + private static final String SCHEME = ScmTransportProtocol.NAME + "://"; /** * the logger for AbstractGitPushOrPullCommand @@ -167,7 +168,7 @@ public abstract class AbstractGitPushOrPullCommand extends AbstractGitCommand } else { - throw new IllegalArgumentException("repository or url is requiered"); + throw new IllegalArgumentException("repository or url is required"); } return url; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java index 5cb8007986..b59fa7526b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java @@ -35,7 +35,6 @@ package sonia.scm.web; //~--- non-JDK imports -------------------------------------------------------- -import com.google.common.annotations.VisibleForTesting; import com.google.inject.Inject; import org.eclipse.jgit.http.server.resolver.DefaultReceivePackFactory; import org.eclipse.jgit.lib.Repository; @@ -43,6 +42,7 @@ import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.resolver.ReceivePackFactory; import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; +import sonia.scm.protocolcommand.git.BaseReceivePackFactory; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.spi.HookEventFacade; @@ -56,42 +56,20 @@ import javax.servlet.http.HttpServletRequest; * * @author Sebastian Sdorra */ -public class GitReceivePackFactory implements ReceivePackFactory +public class GitReceivePackFactory extends BaseReceivePackFactory { - private final GitRepositoryHandler handler; - - private ReceivePackFactory wrapped; - - private final GitReceiveHook hook; + private ReceivePackFactory wrapped; @Inject public GitReceivePackFactory(GitRepositoryHandler handler, HookEventFacade hookEventFacade) { - this.handler = handler; - this.hook = new GitReceiveHook(hookEventFacade, handler); - this.wrapped = new DefaultReceivePackFactory(); + super(handler, hookEventFacade); + this.wrapped = new DefaultReceivePackFactory(); } @Override - public ReceivePack create(HttpServletRequest request, Repository repository) + protected ReceivePack createBasicReceivePack(HttpServletRequest request, Repository repository) throws ServiceNotEnabledException, ServiceNotAuthorizedException { - ReceivePack receivePack = wrapped.create(request, repository); - receivePack.setAllowNonFastForwards(isNonFastForwardAllowed()); - - receivePack.setPreReceiveHook(hook); - receivePack.setPostReceiveHook(hook); - // apply collecting listener, to be able to check which commits are new - CollectingPackParserListener.set(receivePack); - - return receivePack; - } - - private boolean isNonFastForwardAllowed() { - return ! handler.getConfig().isNonFastForwardDisallowed(); - } - - @VisibleForTesting - void setWrapped(ReceivePackFactory wrapped) { - this.wrapped = wrapped; + return wrapped.create(request, repository); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitReceivePackFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java similarity index 80% rename from scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitReceivePackFactoryTest.java rename to scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java index 4ed9d5a46a..fa1a97ec3b 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/GitReceivePackFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/BaseReceivePackFactoryTest.java @@ -29,13 +29,15 @@ * */ -package sonia.scm.web; +package sonia.scm.protocolcommand.git; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ReceivePack; import org.eclipse.jgit.transport.resolver.ReceivePackFactory; +import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; +import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -45,21 +47,20 @@ import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; import sonia.scm.repository.GitConfig; import sonia.scm.repository.GitRepositoryHandler; +import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitReceiveHook; -import javax.servlet.http.HttpServletRequest; import java.io.File; import java.io.IOException; import static org.hamcrest.Matchers.instanceOf; import static org.junit.Assert.*; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -/** - * Unit tests for {@link GitReceivePackFactory}. - */ @RunWith(MockitoJUnitRunner.class) -public class GitReceivePackFactoryTest { +public class BaseReceivePackFactoryTest { @Mock private GitRepositoryHandler handler; @@ -67,12 +68,11 @@ public class GitReceivePackFactoryTest { private GitConfig config; @Mock - private ReceivePackFactory wrappedReceivePackFactory; + private ReceivePackFactory wrappedReceivePackFactory; - private GitReceivePackFactory factory; + private BaseReceivePackFactory factory; - @Mock - private HttpServletRequest request; + private Object request = new Object(); private Repository repository; @@ -89,8 +89,12 @@ public class GitReceivePackFactoryTest { ReceivePack receivePack = new ReceivePack(repository); when(wrappedReceivePackFactory.create(request, repository)).thenReturn(receivePack); - factory = new GitReceivePackFactory(handler, null); - factory.setWrapped(wrappedReceivePackFactory); + factory = new BaseReceivePackFactory(handler, null) { + @Override + protected ReceivePack createBasicReceivePack(Object request, Repository repository) throws ServiceNotEnabledException, ServiceNotAuthorizedException { + return wrappedReceivePackFactory.create(request, repository); + } + }; } private Repository createRepositoryForTesting() throws GitAPIException, IOException { @@ -105,6 +109,7 @@ public class GitReceivePackFactoryTest { assertThat(receivePack.getPreReceiveHook(), instanceOf(GitReceiveHook.class)); assertThat(receivePack.getPostReceiveHook(), instanceOf(GitReceiveHook.class)); assertTrue(receivePack.isAllowNonFastForwards()); + verify(wrappedReceivePackFactory).create(request, repository); } @Test diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolverTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolverTest.java new file mode 100644 index 0000000000..6ac4cdb54b --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/GitRepositoryContextResolverTest.java @@ -0,0 +1,45 @@ +package sonia.scm.protocolcommand.git; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.repository.NamespaceAndName; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryLocationResolver; +import sonia.scm.repository.RepositoryManager; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GitRepositoryContextResolverTest { + + private static final Repository REPOSITORY = new Repository("id", "git", "space", "X"); + + @Mock + RepositoryManager repositoryManager; + @Mock + RepositoryLocationResolver locationResolver; + + @InjectMocks + GitRepositoryContextResolver resolver; + + @Test + void shouldResolveCorrectRepository() throws IOException { + when(repositoryManager.get(new NamespaceAndName("space", "X"))).thenReturn(REPOSITORY); + Path repositoryPath = File.createTempFile("test", "scm").toPath(); + when(locationResolver.getPath("id")).thenReturn(repositoryPath); + + RepositoryContext context = resolver.resolve(new String[] {"git", "repo/space/X/something/else"}); + + assertThat(context.getRepository()).isSameAs(REPOSITORY); + assertThat(context.getDirectory()).isEqualTo(repositoryPath.resolve("data")); + } +} diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java index da26ebaf20..70c34fa122 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkdirFactoryTest.java @@ -27,12 +27,16 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + // keep this so that it will not be garbage collected (Transport keeps this in a week reference) + private ScmTransportProtocol proto; + @Before public void bindScmProtocol() { HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class)); HookEventFacade hookEventFacade = new HookEventFacade(of(mock(RepositoryManager.class)), hookContextFactory); GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class); - Transport.register(new ScmTransportProtocol(of(hookEventFacade), of(gitRepositoryHandler))); + proto = new ScmTransportProtocol(of(hookEventFacade), of(gitRepositoryHandler)); + Transport.register(proto); } @Test diff --git a/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js new file mode 100644 index 0000000000..31106593c4 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/BackendErrorNotification.js @@ -0,0 +1,123 @@ +// @flow +import React from "react"; +import { BackendError } from "./errors"; +import Notification from "./Notification"; + +import { translate } from "react-i18next"; + +type Props = { error: BackendError, t: string => string }; + +class BackendErrorNotification extends React.Component { + constructor(props: Props) { + super(props); + } + + render() { + return ( + +
+

{this.renderErrorName()}

+

{this.renderErrorDescription()}

+

{this.renderViolations()}

+ {this.renderMetadata()} +
+ + ); + } + + renderErrorName = () => { + const { error, t } = this.props; + const translation = t("errors." + error.errorCode + ".displayName"); + if (translation === error.errorCode) { + return error.message; + } + return translation; + }; + + renderErrorDescription = () => { + const { error, t } = this.props; + const translation = t("errors." + error.errorCode + ".description"); + if (translation === error.errorCode) { + return ""; + } + return translation; + }; + + renderViolations = () => { + const { error, t } = this.props; + if (error.violations) { + return ( + <> +

+ {t("errors.violations")} +

+
    + {error.violations.map((violation, index) => { + return ( +
  • + {violation.path}: {violation.message} +
    • + ); + })} +
+ + ); + } + }; + + renderMetadata = () => { + const { error, t } = this.props; + return ( + <> + {this.renderContext()} + {this.renderMoreInformationLink()} +
+
+ {t("errors.transactionId")} {error.transactionId} +
+
+ {t("errors.errorCode")} {error.errorCode} +
+
+ + ); + }; + + renderContext = () => { + const { error, t} = this.props; + if (error.context) { + return ( + <> +

+ {t("errors.context")} +

+
    + {error.context.map((context, index) => { + return ( +
  • + {context.type}: {context.id} +
    • + ); + })} +
+ + ); + } + }; + + renderMoreInformationLink = () => { + const { error, t } = this.props; + if (error.url) { + return ( +

+ {t("errors.moreInfo")}{" "} + + {error.errorCode} + +

+ ); + } + }; +} + +export default translate("plugins")(BackendErrorNotification); diff --git a/scm-ui-components/packages/ui-components/src/ErrorNotification.js b/scm-ui-components/packages/ui-components/src/ErrorNotification.js index 6645db5f60..b8acf89733 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorNotification.js +++ b/scm-ui-components/packages/ui-components/src/ErrorNotification.js @@ -1,28 +1,41 @@ //@flow import React from "react"; import { translate } from "react-i18next"; +import { BackendError, ForbiddenError, UnauthorizedError } from "./errors"; import Notification from "./Notification"; -import {UNAUTHORIZED_ERROR} from "./apiclient"; +import BackendErrorNotification from "./BackendErrorNotification"; type Props = { t: string => string, error?: Error }; -class ErrorNotification extends React.Component { +class ErrorNotification extends React.Component { render() { const { t, error } = this.props; if (error) { - if (error === UNAUTHORIZED_ERROR) { + if (error instanceof BackendError) { + return + } else if (error instanceof UnauthorizedError) { return ( - {t("error-notification.prefix")}: {t("error-notification.timeout")} - {" "} - {t("error-notification.loginLink")} + {t("error-notification.prefix")}:{" "} + {t("error-notification.timeout")}{" "} + + {t("error-notification.loginLink")} + ); - } else { + } else if (error instanceof ForbiddenError) { + return ( + + {t("error-notification.prefix")}:{" "} + {t("error-notification.forbidden")} + + ) + } else + { return ( {t("error-notification.prefix")}: {error.message} @@ -34,4 +47,4 @@ class ErrorNotification extends React.Component { } } -export default translate("commons")(ErrorNotification); +export default translate("commons")(ErrorNotification); diff --git a/scm-ui-components/packages/ui-components/src/ErrorPage.js b/scm-ui-components/packages/ui-components/src/ErrorPage.js index 196319681c..b86f374263 100644 --- a/scm-ui-components/packages/ui-components/src/ErrorPage.js +++ b/scm-ui-components/packages/ui-components/src/ErrorPage.js @@ -1,6 +1,7 @@ //@flow import React from "react"; import ErrorNotification from "./ErrorNotification"; +import { BackendError, ForbiddenError } from "./errors"; type Props = { error: Error, @@ -10,18 +11,26 @@ type Props = { class ErrorPage extends React.Component { render() { - const { title, subtitle, error } = this.props; + const { title, error } = this.props; return (

{title}

-

{subtitle}

+ {this.renderSubtitle()}
); } + + renderSubtitle = () => { + const { error, subtitle } = this.props; + if (error instanceof BackendError || error instanceof ForbiddenError) { + return null; + } + return

{subtitle}

+ } } export default ErrorPage; diff --git a/scm-ui-components/packages/ui-components/src/apiclient.js b/scm-ui-components/packages/ui-components/src/apiclient.js index 25a108877a..77ebe8cf4e 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.js @@ -1,9 +1,7 @@ // @flow -import {contextPath} from "./urls"; - -export const NOT_FOUND_ERROR = new Error("not found"); -export const UNAUTHORIZED_ERROR = new Error("unauthorized"); -export const CONFLICT_ERROR = new Error("conflict"); +import { contextPath } from "./urls"; +import { createBackendError, ForbiddenError, isBackendError, UnauthorizedError } from "./errors"; +import type { BackendErrorContent } from "./errors"; const fetchOptions: RequestOptions = { credentials: "same-origin", @@ -12,19 +10,24 @@ const fetchOptions: RequestOptions = { } }; -function handleStatusCode(response: Response) { - if (!response.ok) { - switch (response.status) { - case 401: - throw UNAUTHORIZED_ERROR; - case 404: - throw NOT_FOUND_ERROR; - case 409: - throw CONFLICT_ERROR; - default: - throw new Error("server returned status code " + response.status); - } + +function handleFailure(response: Response) { + if (!response.ok) { + if (isBackendError(response)) { + return response.json() + .then((content: BackendErrorContent) => { + throw createBackendError(content, response.status); + }); + } else { + if (response.status === 401) { + throw new UnauthorizedError("Unauthorized", 401); + } else if (response.status === 403) { + throw new ForbiddenError("Forbidden", 403); + } + + throw new Error("server returned status code " + response.status); + } } return response; } @@ -42,7 +45,7 @@ export function createUrl(url: string) { class ApiClient { get(url: string): Promise { - return fetch(createUrl(url), fetchOptions).then(handleStatusCode); + return fetch(createUrl(url), fetchOptions).then(handleFailure); } post(url: string, payload: any, contentType: string = "application/json") { @@ -58,7 +61,7 @@ class ApiClient { method: "HEAD" }; options = Object.assign(options, fetchOptions); - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } delete(url: string): Promise { @@ -66,7 +69,7 @@ class ApiClient { method: "DELETE" }; options = Object.assign(options, fetchOptions); - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } httpRequestWithJSONBody( @@ -83,7 +86,7 @@ class ApiClient { // $FlowFixMe options.headers["Content-Type"] = contentType; - return fetch(createUrl(url), options).then(handleStatusCode); + return fetch(createUrl(url), options).then(handleFailure); } } diff --git a/scm-ui-components/packages/ui-components/src/apiclient.test.js b/scm-ui-components/packages/ui-components/src/apiclient.test.js index bf3358fe95..8827ca72e4 100644 --- a/scm-ui-components/packages/ui-components/src/apiclient.test.js +++ b/scm-ui-components/packages/ui-components/src/apiclient.test.js @@ -1,65 +1,78 @@ // @flow -import {apiClient, createUrl} from "./apiclient"; -import fetchMock from "fetch-mock"; +import { apiClient, createUrl } from "./apiclient"; +import {fetchMock} from "fetch-mock"; +import { BackendError } from "./errors"; + +describe("create url", () => { + it("should not change absolute urls", () => { + expect(createUrl("https://www.scm-manager.org")).toBe( + "https://www.scm-manager.org" + ); + }); + + it("should add prefix for api", () => { + expect(createUrl("/users")).toBe("/api/v2/users"); + expect(createUrl("users")).toBe("/api/v2/users"); + }); +}); + + +describe("error handling tests", () => { + + const earthNotFoundError = { + transactionId: "42t", + errorCode: "42e", + message: "earth not found", + context: [{ + type: "planet", + id: "earth" + }] + }; -describe("apiClient", () => { afterEach(() => { fetchMock.reset(); fetchMock.restore(); }); - describe("create url", () => { - it("should not change absolute urls", () => { - expect(createUrl("https://www.scm-manager.org")).toBe( - "https://www.scm-manager.org" - ); + it("should create a normal error, if the content type is not scmm-error", (done) => { + + fetchMock.getOnce("/api/v2/error", { + status: 404 }); - it("should add prefix for api", () => { - expect(createUrl("/users")).toBe("/api/v2/users"); - expect(createUrl("users")).toBe("/api/v2/users"); - }); + apiClient.get("/error") + .catch((err: Error) => { + expect(err.name).toEqual("Error"); + expect(err.message).toContain("404"); + done(); + }); }); - describe("error handling", () => { - const error = { - message: "Error!!" - }; - - it("should append default error message for 401 if none provided", () => { - fetchMock.mock("api/v2/foo", 401); - return apiClient - .get("foo") - .catch(err => { - expect(err.message).toEqual("unauthorized"); - }); + it("should create an backend error, if the content type is scmm-error", (done) => { + fetchMock.getOnce("/api/v2/error", { + status: 404, + headers: { + "Content-Type": "application/vnd.scmm-error+json;v=2" + }, + body: earthNotFoundError }); - it("should append error message for 401 if provided", () => { - fetchMock.mock("api/v2/foo", {"status": 401, body: error}); - return apiClient - .get("foo") - .catch(err => { - expect(err.message).toEqual("Error!!"); - }); - }); + apiClient.get("/error") + .catch((err: BackendError) => { - it("should append default error message for 401 if none provided", () => { - fetchMock.mock("api/v2/foo", 404); - return apiClient - .get("foo") - .catch(err => { - expect(err.message).toEqual("not found"); - }); - }); + expect(err).toBeInstanceOf(BackendError); - it("should append error message for 404 if provided", () => { - fetchMock.mock("api/v2/foo", {"status": 404, body: error}); - return apiClient - .get("foo") - .catch(err => { - expect(err.message).toEqual("Error!!"); - }); - }); + expect(err.message).toEqual("earth not found"); + expect(err.statusCode).toBe(404); + + expect(err.transactionId).toEqual("42t"); + expect(err.errorCode).toEqual("42e"); + expect(err.context).toEqual([{ + type: "planet", + id: "earth" + }]); + done(); + }); }); + }); diff --git a/scm-ui-components/packages/ui-components/src/errors.js b/scm-ui-components/packages/ui-components/src/errors.js new file mode 100644 index 0000000000..13e4996cf7 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/errors.js @@ -0,0 +1,72 @@ +// @flow +type Context = { type: string, id: string }[]; +type Violation = { path: string, message: string }; + +export type BackendErrorContent = { + transactionId: string, + errorCode: string, + message: string, + url?: string, + context: Context, + violations: Violation[] +}; + +export class BackendError extends Error { + transactionId: string; + errorCode: string; + url: ?string; + context: Context = []; + statusCode: number; + violations: Violation[]; + + constructor(content: BackendErrorContent, name: string, statusCode: number) { + super(content.message); + this.name = name; + this.transactionId = content.transactionId; + this.errorCode = content.errorCode; + this.url = content.url; + this.context = content.context; + this.statusCode = statusCode; + this.violations = content.violations; + } +} + +export class UnauthorizedError extends Error { + statusCode: number; + constructor(message: string, statusCode: number) { + super(message); + this.statusCode = statusCode; + } +} + +export class ForbiddenError extends Error { + statusCode: number; + constructor(message: string, statusCode: number) { + super(message); + this.statusCode = statusCode; + } +} + +export class NotFoundError extends BackendError { + constructor(content: BackendErrorContent, statusCode: number) { + super(content, "NotFoundError", statusCode); + } +} +export function createBackendError( + content: BackendErrorContent, + statusCode: number +) { + switch (statusCode) { + case 404: + return new NotFoundError(content, statusCode); + default: + return new BackendError(content, "BackendError", statusCode); + } +} + +export function isBackendError(response: Response) { + return ( + response.headers.get("Content-Type") === + "application/vnd.scmm-error+json;v=2" + ); +} diff --git a/scm-ui-components/packages/ui-components/src/errors.test.js b/scm-ui-components/packages/ui-components/src/errors.test.js new file mode 100644 index 0000000000..3a886e7143 --- /dev/null +++ b/scm-ui-components/packages/ui-components/src/errors.test.js @@ -0,0 +1,35 @@ +// @flow + +import { BackendError, UnauthorizedError, createBackendError, NotFoundError } from "./errors"; + +describe("test createBackendError", () => { + + const earthNotFoundError = { + transactionId: "42t", + errorCode: "42e", + message: "earth not found", + context: [{ + type: "planet", + id: "earth" + }] + }; + + it("should return a default backend error", () => { + const err = createBackendError(earthNotFoundError, 500); + expect(err).toBeInstanceOf(BackendError); + expect(err.name).toBe("BackendError"); + }); + + it("should return an unauthorized error for status code 403", () => { + const err = createBackendError(earthNotFoundError, 403); + expect(err).toBeInstanceOf(UnauthorizedError); + expect(err.name).toBe("UnauthorizedError"); + }); + + it("should return an not found error for status code 404", () => { + const err = createBackendError(earthNotFoundError, 404); + expect(err).toBeInstanceOf(NotFoundError); + expect(err.name).toBe("NotFoundError"); + }); + +}); diff --git a/scm-ui-components/packages/ui-components/src/index.js b/scm-ui-components/packages/ui-components/src/index.js index 7bd844f0a8..39f3769441 100644 --- a/scm-ui-components/packages/ui-components/src/index.js +++ b/scm-ui-components/packages/ui-components/src/index.js @@ -26,7 +26,8 @@ export { getPageFromMatch } from "./urls"; export { default as Autocomplete} from "./Autocomplete"; export { default as BranchSelector } from "./BranchSelector"; -export { apiClient, NOT_FOUND_ERROR, UNAUTHORIZED_ERROR, CONFLICT_ERROR } from "./apiclient.js"; +export { apiClient } from "./apiclient.js"; +export * from "./errors"; export * from "./avatar"; export * from "./buttons"; diff --git a/scm-ui/public/locales/de/commons.json b/scm-ui/public/locales/de/commons.json index f32d764ce8..51a493fc40 100644 --- a/scm-ui/public/locales/de/commons.json +++ b/scm-ui/public/locales/de/commons.json @@ -23,7 +23,8 @@ "prefix": "Fehler", "loginLink": "Erneute Anmeldung", "timeout": "Die Session ist abgelaufen.", - "wrong-login-credentials": "Ungültige Anmeldedaten" + "wrong-login-credentials": "Ungültige Anmeldedaten", + "forbidden": "Sie haben nicht die Berechtigung, diesen Datensatz zu sehen" }, "loading": { "alt": "Lade ..." diff --git a/scm-ui/public/locales/de/config.json b/scm-ui/public/locales/de/config.json index 5767a2b376..b67bf90262 100644 --- a/scm-ui/public/locales/de/config.json +++ b/scm-ui/public/locales/de/config.json @@ -9,7 +9,8 @@ "config-form": { "submit": "Speichern", "submit-success-notification": "Einstellungen wurden erfolgreich geändert!", - "no-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Bearbeiten der Einstellungen!" + "no-read-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Lesen der Einstellungen!", + "no-write-permission-notification": "Hinweis: Es fehlen Berechtigungen zum Bearbeiten der Einstellungen!" }, "proxy-settings": { "name": "Proxy Einstellungen", diff --git a/scm-ui/public/locales/en/commons.json b/scm-ui/public/locales/en/commons.json index fed749a200..cc4edde82c 100644 --- a/scm-ui/public/locales/en/commons.json +++ b/scm-ui/public/locales/en/commons.json @@ -23,7 +23,8 @@ "prefix": "Error", "loginLink": "You can login here again.", "timeout": "The session has expired", - "wrong-login-credentials": "Invalid credentials" + "wrong-login-credentials": "Invalid credentials", + "forbidden": "You don't have permission to view this entity" }, "loading": { "alt": "Loading ..." diff --git a/scm-ui/public/locales/en/config.json b/scm-ui/public/locales/en/config.json index b08c5c2d1b..1b42878015 100644 --- a/scm-ui/public/locales/en/config.json +++ b/scm-ui/public/locales/en/config.json @@ -9,7 +9,8 @@ "config-form": { "submit": "Submit", "submit-success-notification": "Configuration changed successfully!", - "no-permission-notification": "Please note: You do not have the permission to edit the config!" + "no-read-permission-notification": "Please note: You do not have the permission to see the config!", + "no-write-permission-notification": "Please note: You do not have the permission to edit the config!" }, "proxy-settings": { "name": "Proxy Settings", diff --git a/scm-ui/src/config/components/form/ConfigForm.js b/scm-ui/src/config/components/form/ConfigForm.js index dc3f20c95d..7b650ccbfd 100644 --- a/scm-ui/src/config/components/form/ConfigForm.js +++ b/scm-ui/src/config/components/form/ConfigForm.js @@ -14,6 +14,7 @@ type Props = { config?: Config, loading?: boolean, t: string => string, + configReadPermission: boolean, configUpdatePermission: boolean }; @@ -84,16 +85,30 @@ class ConfigForm extends React.Component { }; render() { - const { loading, t, configUpdatePermission } = this.props; + const { + loading, + t, + configReadPermission, + configUpdatePermission + } = this.props; const config = this.state.config; let noPermissionNotification = null; + if (!configReadPermission) { + return ( + + ); + } + if (this.state.showNotification) { noPermissionNotification = ( this.onClose()} /> ); diff --git a/scm-ui/src/config/containers/GlobalConfig.js b/scm-ui/src/config/containers/GlobalConfig.js index eac8e27bee..fd3ee04098 100644 --- a/scm-ui/src/config/containers/GlobalConfig.js +++ b/scm-ui/src/config/containers/GlobalConfig.js @@ -1,7 +1,7 @@ // @flow import React from "react"; import { translate } from "react-i18next"; -import { Title, ErrorPage, Loading } from "@scm-manager/ui-components"; +import { Title, Loading, ErrorNotification } from "@scm-manager/ui-components"; import { fetchConfig, getFetchConfigFailure, @@ -35,6 +35,7 @@ type Props = { }; type State = { + configReadPermission: boolean, configChanged: boolean }; @@ -43,13 +44,18 @@ class GlobalConfig extends React.Component { super(props); this.state = { + configReadPermission: true, configChanged: false }; } componentDidMount() { this.props.configReset(); - this.props.fetchConfig(this.props.configLink); + if (this.props.configLink) { + this.props.fetchConfig(this.props.configLink); + } else { + this.setState({configReadPermission: false}); + } } modifyConfig = (config: Config) => { @@ -73,18 +79,8 @@ class GlobalConfig extends React.Component { }; render() { - const { t, error, loading, config, configUpdatePermission } = this.props; + const { t, loading } = this.props; - if (error) { - return ( - - ); - } if (loading) { return ; } @@ -92,16 +88,39 @@ class GlobalConfig extends React.Component { return (
- {this.renderConfigChangedNotification()} - <ConfigForm - submitForm={config => this.modifyConfig(config)} - config={config} - loading={loading} - configUpdatePermission={configUpdatePermission} - /> + {this.renderError()} + {this.renderContent()} </div> ); } + + renderError = () => { + const { error } = this.props; + if (error) { + return <ErrorNotification error={error} />; + } + return null; + }; + + renderContent = () => { + const { error, loading, config, configUpdatePermission } = this.props; + const { configReadPermission } = this.state; + if (!error) { + return ( + <> + {this.renderConfigChangedNotification()} + <ConfigForm + submitForm={config => this.modifyConfig(config)} + config={config} + loading={loading} + configUpdatePermission={configUpdatePermission} + configReadPermission={configReadPermission} + /> + </> + ); + } + return null; + }; } const mapDispatchToProps = dispatch => { diff --git a/scm-ui/src/containers/Login.js b/scm-ui/src/containers/Login.js index 3ee9b141ef..4ae9b5a622 100644 --- a/scm-ui/src/containers/Login.js +++ b/scm-ui/src/containers/Login.js @@ -15,8 +15,7 @@ import { InputField, SubmitButton, ErrorNotification, - Image, - UNAUTHORIZED_ERROR + Image, UnauthorizedError } from "@scm-manager/ui-components"; import classNames from "classnames"; import { getLoginLink } from "../modules/indexResource"; @@ -95,7 +94,7 @@ class Login extends React.Component<Props, State> { areCredentialsInvalid() { const { t, error } = this.props; - if (error === UNAUTHORIZED_ERROR) { + if (error instanceof UnauthorizedError) { return new Error(t("error-notification.wrong-login-credentials")); } else { return error; diff --git a/scm-ui/src/groups/containers/Groups.js b/scm-ui/src/groups/containers/Groups.js index 7bc31a8b79..78e373e261 100644 --- a/scm-ui/src/groups/containers/Groups.js +++ b/scm-ui/src/groups/containers/Groups.js @@ -78,13 +78,7 @@ class Groups extends React.Component<Props> { <GroupTable groups={groups} /> {this.renderPaginator()} {this.renderCreateButton()} - <PageActions> - <Button - label={t("create-group-button.label")} - link="/groups/add" - color="primary" - /> - </PageActions> + {this.renderPageActionCreateButton()} </Page> ); } @@ -99,7 +93,25 @@ class Groups extends React.Component<Props> { renderCreateButton() { if (this.props.canAddGroups) { - return <CreateGroupButton />; + return ( + <CreateGroupButton /> + ); + } else { + return; + } + } + + renderPageActionCreateButton() { + if (this.props.canAddGroups) { + return ( + <PageActions> + <Button + label={this.props.t("create-group-button.label")} + link="/groups/add" + color="primary" + /> + </PageActions> + ); } else { return; } diff --git a/scm-ui/src/groups/modules/groups.js b/scm-ui/src/groups/modules/groups.js index 483b5b3798..bbaccf6c4a 100644 --- a/scm-ui/src/groups/modules/groups.js +++ b/scm-ui/src/groups/modules/groups.js @@ -54,8 +54,8 @@ export function fetchGroupsByLink(link: string) { .then(data => { dispatch(fetchGroupsSuccess(data)); }) - .catch(err => { - dispatch(fetchGroupsFailure(link, err)); + .catch(error => { + dispatch(fetchGroupsFailure(link, error)); }); }; } @@ -104,8 +104,8 @@ function fetchGroup(link: string, name: string) { .then(data => { dispatch(fetchGroupSuccess(data)); }) - .catch(err => { - dispatch(fetchGroupFailure(name, err)); + .catch(error => { + dispatch(fetchGroupFailure(name, error)); }); }; } @@ -149,12 +149,8 @@ export function createGroup(link: string, group: Group, callback?: () => void) { callback(); } }) - .catch(err => { - dispatch( - createGroupFailure( - err - ) - ); + .catch(error => { + dispatch(createGroupFailure(error)); }); }; } @@ -199,13 +195,8 @@ export function modifyGroup(group: Group, callback?: () => void) { .then(() => { dispatch(fetchGroupByLink(group)); }) - .catch(err => { - dispatch( - modifyGroupFailure( - group, - err - ) - ); + .catch(error => { + dispatch(modifyGroupFailure(group, error)); }); }; } @@ -257,8 +248,8 @@ export function deleteGroup(group: Group, callback?: () => void) { callback(); } }) - .catch(err => { - dispatch(deleteGroupFailure(group, err)); + .catch(error => { + dispatch(deleteGroupFailure(group, error)); }); }; } @@ -342,7 +333,7 @@ function listReducer(state: any = {}, action: any = {}) { ...state, entries: groupNames, entry: { - groupCreatePermission: action.payload._links.create ? true : false, + groupCreatePermission: !!action.payload._links.create, page: action.payload.page, pageTotal: action.payload.pageTotal, _links: action.payload._links diff --git a/scm-ui/src/modules/auth.js b/scm-ui/src/modules/auth.js index 5d15107406..4c0d22b23d 100644 --- a/scm-ui/src/modules/auth.js +++ b/scm-ui/src/modules/auth.js @@ -2,7 +2,7 @@ import type { Me } from "@scm-manager/ui-types"; import * as types from "./types"; -import { apiClient, UNAUTHORIZED_ERROR } from "@scm-manager/ui-components"; +import { apiClient, UnauthorizedError } from "@scm-manager/ui-components"; import { isPending } from "./pending"; import { getFailure } from "./failure"; import { @@ -152,7 +152,7 @@ export const login = ( dispatch(loginPending()); return apiClient .post(loginLink, login_data) - .then(response => { + .then(() => { dispatch(fetchIndexResourcesPending()); return callFetchIndexResources(); }) @@ -178,7 +178,7 @@ export const fetchMe = (link: string) => { dispatch(fetchMeSuccess(me)); }) .catch((error: Error) => { - if (error === UNAUTHORIZED_ERROR) { + if (error instanceof UnauthorizedError) { dispatch(fetchMeUnauthenticated()); } else { dispatch(fetchMeFailure(error)); diff --git a/scm-ui/src/modules/auth.test.js b/scm-ui/src/modules/auth.test.js index 7236f803a8..1d7cb5096e 100644 --- a/scm-ui/src/modules/auth.test.js +++ b/scm-ui/src/modules/auth.test.js @@ -179,9 +179,7 @@ describe("auth actions", () => { }); it("should dispatch fetch me unauthorized", () => { - fetchMock.getOnce("/api/v2/me", { - status: 401 - }); + fetchMock.getOnce("/api/v2/me", 401); const expectedActions = [ { type: FETCH_ME_PENDING }, diff --git a/scm-ui/src/repos/containers/Overview.js b/scm-ui/src/repos/containers/Overview.js index ac044b5225..1d672aafb1 100644 --- a/scm-ui/src/repos/containers/Overview.js +++ b/scm-ui/src/repos/containers/Overview.js @@ -73,13 +73,7 @@ class Overview extends React.Component<Props> { error={error} > {this.renderList()} - <PageActions> - <Button - label={t("overview.createButton")} - link="/repos/create" - color="primary" - /> - </PageActions> + {this.renderPageActionCreateButton()} </Page> ); } @@ -107,6 +101,22 @@ class Overview extends React.Component<Props> { } return null; } + + renderPageActionCreateButton() { + const { showCreateButton, t } = this.props; + if (showCreateButton) { + return ( + <PageActions> + <Button + label={t("overview.createButton")} + link="/repos/create" + color="primary" + /> + </PageActions> + ); + } + return null; + } } const getPageFromProps = props => { diff --git a/scm-ui/src/repos/containers/RepositoryRoot.js b/scm-ui/src/repos/containers/RepositoryRoot.js index 8608b9c957..08d6aa9380 100644 --- a/scm-ui/src/repos/containers/RepositoryRoot.js +++ b/scm-ui/src/repos/containers/RepositoryRoot.js @@ -12,13 +12,13 @@ import { Route, Switch } from "react-router-dom"; import type { Repository } from "@scm-manager/ui-types"; import { - ErrorPage, + CollapsibleErrorPage, Loading, Navigation, SubNavigation, NavLink, Page, - Section + Section, ErrorPage } from "@scm-manager/ui-components"; import { translate } from "react-i18next"; import RepositoryDetails from "../components/RepositoryDetails"; @@ -82,13 +82,11 @@ class RepositoryRoot extends React.Component<Props> { const { loading, error, indexLinks, repository, t } = this.props; if (error) { - return ( - <ErrorPage - title={t("repositoryRoot.errorTitle")} - subtitle={t("repositoryRoot.errorSubtitle")} - error={error} - /> - ); + return <ErrorPage + title={t("repositoryRoot.errorTitle")} + subtitle={t("repositoryRoot.errorSubtitle")} + error={error} + /> } if (!repository || loading) { diff --git a/scm-ui/src/repos/modules/repos.js b/scm-ui/src/repos/modules/repos.js index aa77b4553b..fa89dc42a6 100644 --- a/scm-ui/src/repos/modules/repos.js +++ b/scm-ui/src/repos/modules/repos.js @@ -229,8 +229,8 @@ export function modifyRepo(repository: Repository, callback?: () => void) { .then(() => { dispatch(fetchRepoByLink(repository)); }) - .catch(err => { - dispatch(modifyRepoFailure(repository, err)); + .catch(error => { + dispatch(modifyRepoFailure(repository, error)); }); }; } diff --git a/scm-ui/src/repos/modules/repositoryTypes.js b/scm-ui/src/repos/modules/repositoryTypes.js index 2cdbd1194d..d96d24b612 100644 --- a/scm-ui/src/repos/modules/repositoryTypes.js +++ b/scm-ui/src/repos/modules/repositoryTypes.js @@ -37,10 +37,7 @@ function fetchRepositoryTypes(dispatch: any) { .then(repositoryTypes => { dispatch(fetchRepositoryTypesSuccess(repositoryTypes)); }) - .catch(err => { - const error = new Error( - `failed to fetch repository types: ${err.message}` - ); + .catch(error => { dispatch(fetchRepositoryTypesFailure(error)); }); } diff --git a/scm-ui/src/repos/permissions/modules/permissions.js b/scm-ui/src/repos/permissions/modules/permissions.js index cb6b013c61..8c4161e907 100644 --- a/scm-ui/src/repos/permissions/modules/permissions.js +++ b/scm-ui/src/repos/permissions/modules/permissions.js @@ -451,7 +451,10 @@ function deletePermissionFromState( ) { let newPermission = []; for (let i = 0; i < oldPermissions.length; i++) { - if (oldPermissions[i] !== permission) { + if ( + oldPermissions[i].name !== permission.name || + oldPermissions[i].groupPermission !== permission.groupPermission + ) { newPermission.push(oldPermissions[i]); } } diff --git a/scm-ui/src/repos/sources/containers/Content.js b/scm-ui/src/repos/sources/containers/Content.js index 2f3f5ba853..639b24df8e 100644 --- a/scm-ui/src/repos/sources/containers/Content.js +++ b/scm-ui/src/repos/sources/containers/Content.js @@ -11,6 +11,7 @@ import SourcesView from "./SourcesView"; import HistoryView from "./HistoryView"; import { getSources } from "../modules/sources"; import { connect } from "react-redux"; +import { ExtensionPoint } from "@scm-manager/ui-extensions"; type Props = { loading: boolean, @@ -103,7 +104,7 @@ class Content extends React.Component<Props, State> { showMoreInformation() { const collapsed = this.state.collapsed; - const { classes, file, revision, t } = this.props; + const { classes, file, revision, t, repository } = this.props; const date = <DateFromNow date={file.lastModified} />; const description = file.description ? ( <p> @@ -120,12 +121,7 @@ class Content extends React.Component<Props, State> { const fileSize = file.directory ? "" : <FileSize bytes={file.length} />; if (!collapsed) { return ( - <div - className={classNames( - "panel-block", - classes.hasBackground - )} - > + <div className={classNames("panel-block", classes.hasBackground)}> <table className={classNames("table", classes.hasBackground)}> <tbody> <tr> @@ -148,6 +144,11 @@ class Content extends React.Component<Props, State> { <td>{t("sources.content.description")}</td> <td className="is-word-break">{description}</td> </tr> + <ExtensionPoint + name="repos.content.metadata" + renderAll={true} + props={{ file, repository, revision }} + /> </tbody> </table> </div> diff --git a/scm-ui/src/users/containers/Users.js b/scm-ui/src/users/containers/Users.js index 10dedde32d..3d88523479 100644 --- a/scm-ui/src/users/containers/Users.js +++ b/scm-ui/src/users/containers/Users.js @@ -78,13 +78,7 @@ class Users extends React.Component<Props> { <UserTable users={users} /> {this.renderPaginator()} {this.renderCreateButton()} - <PageActions> - <Button - label={t("users.createButton")} - link="/users/add" - color="primary" - /> - </PageActions> + {this.renderPageActionCreateButton()} </Page> ); } @@ -105,6 +99,23 @@ class Users extends React.Component<Props> { return; } } + + renderPageActionCreateButton() { + const { t } = this.props; + if (this.props.canAddUsers) { + return ( + <PageActions> + <Button + label={t("users.createButton")} + link="/users/add" + color="primary" + /> + </PageActions> + ); + } else { + return; + } + } } const getPageFromProps = props => { diff --git a/scm-ui/src/users/modules/users.js b/scm-ui/src/users/modules/users.js index ab330d9ffd..a93d082f58 100644 --- a/scm-ui/src/users/modules/users.js +++ b/scm-ui/src/users/modules/users.js @@ -35,6 +35,8 @@ export const DELETE_USER_FAILURE = `${DELETE_USER}_${types.FAILURE_SUFFIX}`; const CONTENT_TYPE_USER = "application/vnd.scmm-user+json;v=2"; +// TODO i18n for error messages + // fetch users export function fetchUsers(link: string) { @@ -55,8 +57,8 @@ export function fetchUsersByLink(link: string) { .then(data => { dispatch(fetchUsersSuccess(data)); }) - .catch(err => { - dispatch(fetchUsersFailure(link, err)); + .catch(error => { + dispatch(fetchUsersFailure(link, error)); }); }; } @@ -105,8 +107,8 @@ function fetchUser(link: string, name: string) { .then(data => { dispatch(fetchUserSuccess(data)); }) - .catch(err => { - dispatch(fetchUserFailure(name, err)); + .catch(error => { + dispatch(fetchUserFailure(name, error)); }); }; } @@ -151,7 +153,9 @@ export function createUser(link: string, user: User, callback?: () => void) { callback(); } }) - .catch(err => dispatch(createUserFailure(err))); + .catch(error => + dispatch(createUserFailure(error)) + ); }; } @@ -250,8 +254,8 @@ export function deleteUser(user: User, callback?: () => void) { callback(); } }) - .catch(err => { - dispatch(deleteUserFailure(user, err)); + .catch(error => { + dispatch(deleteUserFailure(user, error)); }); }; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java index 63582e10b8..0b8b44f308 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ResteasyValidationExceptionMapper.java @@ -2,9 +2,9 @@ package sonia.scm.api.v2; import org.jboss.resteasy.api.validation.ResteasyViolationException; import sonia.scm.api.v2.resources.ResteasyViolationExceptionToErrorDtoMapper; +import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; @@ -23,7 +23,7 @@ public class ResteasyValidationExceptionMapper implements ExceptionMapper<Restea public Response toResponse(ResteasyViolationException exception) { return Response .status(Response.Status.BAD_REQUEST) - .type(MediaType.APPLICATION_JSON_TYPE) + .type(VndMediaType.ERROR_TYPE) .entity(mapper.map(exception)) .build(); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java index 991aeedaeb..17883e41cb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/ScmConstraintValidationExceptionMapper.java @@ -2,9 +2,9 @@ package sonia.scm.api.v2; import sonia.scm.ScmConstraintViolationException; import sonia.scm.api.v2.resources.ScmViolationExceptionToErrorDtoMapper; +import sonia.scm.web.VndMediaType; import javax.inject.Inject; -import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; @@ -23,7 +23,7 @@ public class ScmConstraintValidationExceptionMapper implements ExceptionMapper<S public Response toResponse(ScmConstraintViolationException exception) { return Response .status(Response.Status.BAD_REQUEST) - .type(MediaType.APPLICATION_JSON_TYPE) + .type(VndMediaType.ERROR_TYPE) .entity(mapper.map(exception)) .build(); } 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 71b1127ad8..d19c1a9e03 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 @@ -80,8 +80,6 @@ public class BranchRootResource { .build(); } catch (CommandNotSupportedException ex) { return Response.status(Response.Status.BAD_REQUEST).build(); - } catch (NotFoundException e) { - return Response.status(Response.Status.NOT_FOUND).build(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java index 8e015e8b60..881e9251f0 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionDtoToRepositoryPermissionMapper.java @@ -5,18 +5,8 @@ import org.mapstruct.Mapper; import org.mapstruct.MappingTarget; import sonia.scm.repository.RepositoryPermission; -@Mapper(collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE) +@Mapper( collectionMappingStrategy = CollectionMappingStrategy.TARGET_IMMUTABLE) public abstract class RepositoryPermissionDtoToRepositoryPermissionMapper { public abstract RepositoryPermission map(RepositoryPermissionDto permissionDto); - - /** - * this method is needed to modify an existing permission object - * - * @param target the target permission - * @param repositoryPermissionDto the source dto - * @return the mapped target permission object - */ - public abstract void modify(@MappingTarget RepositoryPermission target, RepositoryPermissionDto repositoryPermissionDto); - } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java index dd66e6e5f2..d149f55401 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResource.java @@ -177,7 +177,11 @@ public class RepositoryPermissionRootResource { .filter(filterPermission(permissionName)) .findFirst() .orElseThrow(() -> notFound(entity(RepositoryPermission.class, namespace).in(Repository.class, namespace + "/" + name))); - dtoToModelMapper.modify(existingPermission, permission); + RepositoryPermission newPermission = dtoToModelMapper.map(permission); + if (!repository.removePermission(existingPermission)) { + throw new IllegalStateException(String.format("could not delete modified permission %s from repository %s/%s", existingPermission, namespace, name)); + } + repository.addPermission(newPermission); manager.modify(repository); log.info("the permission with name: {} is updated.", permissionName); return Response.noContent().build(); @@ -210,7 +214,7 @@ public class RepositoryPermissionRootResource { .findFirst() .ifPresent(repository::removePermission); manager.modify(repository); - log.info("the permission with name: {} is updated.", permissionName); + log.info("the permission with name: {} is deleted.", permissionName); return Response.noContent().build(); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java index e8c303e0f8..5cc9df38f8 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/RepositoryResource.java @@ -154,6 +154,7 @@ public class RepositoryResource { private Repository processUpdate(RepositoryDto repositoryDto, Repository existing) { Repository changedRepository = dtoToRepositoryMapper.map(repositoryDto, existing.getId()); + changedRepository.setPermissions(existing.getPermissions()); return changedRepository; } diff --git a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml index 0ec51141b4..6b7f7bd99f 100644 --- a/scm-webapp/src/main/resources/META-INF/scm/permissions.xml +++ b/scm-webapp/src/main/resources/META-INF/scm/permissions.xml @@ -51,6 +51,9 @@ <permission> <value>group:*</value> </permission> + <permission> + <value>permission:*</value> + </permission> <permission> <value>configuration:list</value> </permission> diff --git a/scm-webapp/src/main/resources/locales/de/plugins.json b/scm-webapp/src/main/resources/locales/de/plugins.json index def3d0b093..0822be9cdd 100644 --- a/scm-webapp/src/main/resources/locales/de/plugins.json +++ b/scm-webapp/src/main/resources/locales/de/plugins.json @@ -34,6 +34,12 @@ "description": "Darf Gruppen administrieren." } }, + "permission": { + "*": { + "displayName": "Berechtigungen verwalten", + "description": "Darf Berechtigungen verwalten (benötigt zusätzlich 'Benutzer administrieren' bzw. 'Gruppen administrieren')." + } + }, "configuration": { "list": { "displayName": "Basis für Administration", @@ -55,15 +61,15 @@ "verbs": { "repository": { "read": { - "displayName": "Lesen", + "displayName": "Repository Lesen", "description": "Darf das Repository im SCM-Manager sehen." }, "modify": { - "displayName": "Modifizieren", + "displayName": "Repository Modifizieren", "description": "Darf die Eigenschaften des Repository verändern." }, "delete": { - "displayName": "Löschen", + "displayName": "Repository Löschen", "description": "Darf das Repository löschen." }, "pull": { @@ -87,5 +93,55 @@ "description": "Darf im Repository Kontext alles ausführen. Dies beinhaltet alle Repository Berechtigungen." } } + }, + "errors": { + "context": "Kontext", + "errorCode": "Fehlercode", + "transactionId": "Transaktions-ID", + "moreInfo": "Für mehr Informationen, siehe", + "AGR7UzkhA1": { + "displayName": "Nicht gefunden", + "description": "Der gewünschte Datensatz konnte nicht gefunden werden. Möglicherweise wurde er in einer weiteren Session gelöscht." + }, + "FtR7UznKU1": { + "displayName": "Existiert bereits", + "description": "Ein Datensatz mit den gegebenen Schlüsselwerten existiert bereits" + }, + "9BR7qpDAe1": { + "displayName": "Passwortänderung nicht erlaubt", + "description": "Sie haben nicht die Berechtigung, das Passwort zu ändern" + }, + "2wR7UzpPG1": { + "displayName": "Konkurrierende Änderungen", + "description": "Der Datensatz wurde konkurrierend von einem anderen Benutzer oder einem anderen Prozess modifiziert. Bitte laden sie die Daten erneut." + }, + "9SR8G0kmU1": { + "displayName": "Feature nicht unterstützt", + "description": "Das Versionsverwaltungssystem dieses Repositories unterstützt das angefragte Feature nicht." + }, + "CmR8GCJb31": { + "displayName": "Interner Serverfehler", + "description": "Im Server ist ein interner Fehler aufgetreten. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "92RCCCMHO1": { + "displayName": "Eine interne URL wurde nicht gefunden", + "description": "Ein interner Serveraufruf konnte nicht verarbeitet werden. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "2VRCrvpL71": { + "displayName": "Ungültiges Datenformat", + "description": "Die zum Server gesendeten Daten konnten nicht verarbeitet werden. Bitte prüfen Sie die eingegebenen Werte oder wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "8pRBYDURx1": { + "displayName": "Ungültiger Datentyp", + "description": "Die zum Server gesendeten Daten hatten einen ungültigen Typen. Bitte wenden Sie sich an ihren Administrator für weitere Hinweise." + }, + "1wR7ZBe7H1": { + "displayName": "Ungültige Eingabe", + "description": "Die eingegebenen Daten konnten nicht validiert werden. Bitte korrigieren Sie die Eingaben und senden Sie sie erneut." + }, + "3zR9vPNIE1": { + "displayName": "Ungültige Eingabe", + "description": "Die eingegebenen Daten konnten nicht validiert werden. Bitte korrigieren Sie die Eingaben und senden Sie sie erneut." + } } } diff --git a/scm-webapp/src/main/resources/locales/en/plugins.json b/scm-webapp/src/main/resources/locales/en/plugins.json index bf771def44..cc9902565b 100644 --- a/scm-webapp/src/main/resources/locales/en/plugins.json +++ b/scm-webapp/src/main/resources/locales/en/plugins.json @@ -34,6 +34,12 @@ "description": "May administer all groups" } }, + "permission": { + "*": { + "displayName": "Administer permissions", + "description": "May administer permissions (additionally needs 'administer users' and/or 'administer groups')." + } + }, "configuration": { "list": { "displayName": "Basic administration", @@ -55,23 +61,23 @@ "verbs": { "repository": { "read": { - "displayName": "read", + "displayName": "read repository", "description": "May see the repository inside the SCM-Manager" }, "modify": { - "displayName": "modify", - "description": "May modify the properties of the repository" + "displayName": "modify repository metadata", + "description": "May modify the basic properties of the repository" }, "delete": { - "displayName": "delete", + "displayName": "delete repository", "description": "May delete the repository" }, "pull": { - "displayName": "pull/checkout", + "displayName": "pull/checkout repository", "description": "May pull/checkout the repository" }, "push": { - "displayName": "push/commit", + "displayName": "push/commit repository", "description": "May change the content of the repository (push/commit)" }, "permissionRead": { @@ -83,9 +89,59 @@ "description": "May modify the permissions of the repository" }, "*": { - "displayName": "overall", + "displayName": "own repository", "description": "May change everything for the repository (includes all other permissions)" } } + }, + "errors": { + "context": "Context", + "errorCode": "Error Code", + "transactionId": "Transaction ID", + "moreInfo": "For more information, see", + "AGR7UzkhA1": { + "displayName": "Not found", + "description": "The requested entity could not be found. It may have been deleted in another session." + }, + "FtR7UznKU1": { + "displayName": "Already exists", + "description": "There is already an entity with the same key values." + }, + "9BR7qpDAe1": { + "displayName": "Password change not allowed", + "description": "You do not have the permission to change the password." + }, + "2wR7UzpPG1": { + "displayName": "Concurrent modifications", + "description": "The entity has been modified concurrently by another user or another process. Please reload the entity." + }, + "9SR8G0kmU1": { + "displayName": "Feature not supported", + "description": "The version control system for this repository does not support the requested feature." + }, + "CmR8GCJb31": { + "displayName": "Internal server error", + "description": "The server encountered an internal error. Please contact your administrator for further assistance." + }, + "92RCCCMHO1": { + "displayName": "An internal URL could not be found", + "description": "An internal request could not be handled by the server. Please contact your administrator for further assistance." + }, + "2VRCrvpL71": { + "displayName": "Illegal data format", + "description": "The data sent to the server could not be handled. Please check the values you have entered or contact your administrator for further assistance." + }, + "8pRBYDURx1": { + "displayName": "Illegal data type", + "description": "The data sent to the server had an illegal data type. Please contact your administrator for further assistance." + }, + "1wR7ZBe7H1": { + "displayName": "Illegal input", + "description": "The values could not be validated. Please correct your input and try again." + }, + "3zR9vPNIE1": { + "displayName": "Illegal input", + "description": "The values could not be validated. Please correct your input and try again." + } } } diff --git a/scm-webapp/src/main/resources/logback.default.xml b/scm-webapp/src/main/resources/logback.default.xml index a4e2eed965..8cd5bd450e 100644 --- a/scm-webapp/src/main/resources/logback.default.xml +++ b/scm-webapp/src/main/resources/logback.default.xml @@ -51,7 +51,8 @@ </appender> <logger name="sonia.scm" level="TRACE" /> - + <logger name="com.cloudogu.scm" level="TRACE" /> + <logger name="sonia.scm.security.AuthorizationCollector" level="DEBUG" /> <logger name="sonia.scm.web.filter.AutoLoginFilter" level="DEBUG" /> <logger name="sonia.scm.security.XsrfProtectionFilter" level="DEBUG" /> diff --git a/scm-webapp/src/main/resources/logback.release.xml b/scm-webapp/src/main/resources/logback.release.xml index 03ac987992..2cb43762bb 100644 --- a/scm-webapp/src/main/resources/logback.release.xml +++ b/scm-webapp/src/main/resources/logback.release.xml @@ -74,7 +74,8 @@ </appender> <logger name="sonia.scm" level="INFO" /> - + <logger name="com.cloudogu.scm" level="INFO" /> + <!-- suppress massive gzip logging --> <logger name="sonia.scm.filter.GZipFilter" level="WARN" /> <logger name="sonia.scm.filter.GZipResponseStream" level="WARN" /> diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java index 4472acb2c5..fb533ad280 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/RepositoryPermissionRootResourceTest.java @@ -301,11 +301,18 @@ public class RepositoryPermissionRootResourceTest extends RepositoryTestBase { @Test public void shouldGetUpdatedPermissions() throws URISyntaxException { - createUserWithRepositoryAndPermissions(TEST_PERMISSIONS, PERMISSION_WRITE); - RepositoryPermission modifiedPermission = TEST_PERMISSIONS.get(0); - // modify the type to owner - modifiedPermission.setVerbs(new ArrayList<>(singletonList("*"))); - ImmutableList<RepositoryPermission> expectedPermissions = ImmutableList.copyOf(TEST_PERMISSIONS); + ArrayList<RepositoryPermission> permissions = Lists + .newArrayList( + new RepositoryPermission("user_write", asList("*"), false), + new RepositoryPermission("user_read", singletonList("read"), false), + new RepositoryPermission("user_owner", singletonList("*"), false), + new RepositoryPermission("group_read", singletonList("read"), true), + new RepositoryPermission("group_write", asList("read", "modify"), true), + new RepositoryPermission("group_owner", singletonList("*"), true) + ); + createUserWithRepositoryAndPermissions(permissions, PERMISSION_WRITE); + RepositoryPermission modifiedPermission = permissions.get(0); + ImmutableList<RepositoryPermission> expectedPermissions = ImmutableList.copyOf(permissions); assertExpectedRequest(requestPUTPermission .content("{\"name\" : \"" + modifiedPermission.getName() + "\" , \"verbs\" : [\"*\"], \"groupPermission\" : false}") .path(PATH_OF_ALL_PERMISSIONS + modifiedPermission.getName()) diff --git a/scm-webapp/src/test/resources/logback-test.xml b/scm-webapp/src/test/resources/logback-test.xml index ca578f9112..2f3490f57c 100644 --- a/scm-webapp/src/test/resources/logback-test.xml +++ b/scm-webapp/src/test/resources/logback-test.xml @@ -51,7 +51,8 @@ </appender> <logger name="sonia.scm" level="DEBUG" /> - + <logger name="com.cloudogu.scm" level="DEBUG" /> + <!-- suppress massive gzip logging --> <logger name="sonia.scm.filter.GZipFilter" level="WARN" /> <logger name="sonia.scm.filter.GZipResponseStream" level="WARN" /> @@ -86,4 +87,4 @@ <appender-ref ref="STDOUT" /> </root> -</configuration> \ No newline at end of file +</configuration>