diff --git a/CHANGELOG.md b/CHANGELOG.md index e94d216088..e6810df880 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Added +- Generation of email addresses for users, where none is configured ([#1370](https://github.com/scm-manager/scm-manager/pull/1370)) + +## [2.7.1] - 2020-10-14 ### Fixed - Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) +- Null Pointer Exception on parsing SVN properties ([#1373](https://github.com/scm-manager/scm-manager/pull/1373)) + +### Changed +- Reduced logging for invalid JWT or api keys ([#1374](https://github.com/scm-manager/scm-manager/pull/1374)) ## [2.7.0] - 2020-10-12 ### Added diff --git a/docs/en/development/intellij-idea-configuration.md b/docs/en/development/intellij-idea-configuration.md index f740281518..d415b4d206 100644 --- a/docs/en/development/intellij-idea-configuration.md +++ b/docs/en/development/intellij-idea-configuration.md @@ -11,6 +11,12 @@ title: Intellij IDEA Configuration ### Settings +* Build, Execution, Deployment / Compiler + * Add runtime assertions for non-null-annotated methods and parameters (must be checked) + * Configure annotation ... (of "Add runtime assertions...") + * Nullable annotations: select (✓) `javax.annotation.Nullable` + * NotNull annotations: select (✓) `javax.annotation.Nonnull` and check Instrument + * Run Configurations / Edit Configuration * Add Maven * Name: run-backend diff --git a/pom.xml b/pom.xml index 506c11920f..7ce1d3c1c0 100644 --- a/pom.xml +++ b/pom.xml @@ -913,7 +913,7 @@ 3.1.0 2.1.1 - 4.5.7.Final + 4.5.8.Final 1.19.4 2.11.2 4.2.3 diff --git a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java index 7bb64a9454..6f1414b8e0 100644 --- a/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java +++ b/scm-core/src/main/java/sonia/scm/config/ScmConfiguration.java @@ -80,6 +80,14 @@ public class ScmConfiguration implements Configuration { */ public static final String DEFAULT_LOGIN_INFO_URL = "https://login-info.scm-manager.org/api/v1/login-info"; + /** + * Default e-mail domain name that will be used whenever we have to generate an e-mail address for a user that has no + * mail address configured. + * + * @since 2.8.0 + */ + public static final String DEFAULT_MAIL_DOMAIN_NAME = "scm-manager.local"; + /** * Default plugin url from version 1.0 */ @@ -187,6 +195,8 @@ public class ScmConfiguration implements Configuration { @XmlElement(name = "login-info-url") private String loginInfoUrl = DEFAULT_LOGIN_INFO_URL; + @XmlElement(name = "mail-domain-name") + private String mailDomainName = DEFAULT_MAIL_DOMAIN_NAME; /** * Calls the {@link sonia.scm.ConfigChangedListener#configChanged(Object)} @@ -227,6 +237,7 @@ public class ScmConfiguration implements Configuration { this.namespaceStrategy = other.namespaceStrategy; this.loginInfoUrl = other.loginInfoUrl; this.releaseFeedUrl = other.releaseFeedUrl; + this.mailDomainName = other.mailDomainName; } /** @@ -291,6 +302,15 @@ public class ScmConfiguration implements Configuration { return releaseFeedUrl; } + /** + * Returns the mail domain, that will be used to create e-mail addresses for users without one whenever one is required. + * @return default mail domain + * @since 2.8.0 + */ + public String getMailDomainName() { + return mailDomainName; + } + /** * Returns a set of glob patterns for urls which should excluded from * proxy settings. @@ -471,6 +491,16 @@ public class ScmConfiguration implements Configuration { this.releaseFeedUrl = releaseFeedUrl; } + /** + * Sets the mail host, that will be used to create e-mail addresses for users without one whenever one is required. + * + * @param mailDomainName The default mail domain to use + * @since 2.8.0 + */ + public void setMailDomainName(String mailDomainName) { + this.mailDomainName = mailDomainName; + } + /** * Set glob patterns for urls which are should be excluded from proxy * settings. diff --git a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java index 06a3b489a1..6cbf2caa8c 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/MergeCommandBuilder.java @@ -30,7 +30,9 @@ import sonia.scm.repository.spi.MergeCommand; import sonia.scm.repository.spi.MergeCommandRequest; import sonia.scm.repository.spi.MergeConflictResult; import sonia.scm.repository.util.AuthorUtil; +import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.util.Set; /** @@ -78,8 +80,12 @@ public class MergeCommandBuilder { private final MergeCommand mergeCommand; private final MergeCommandRequest request = new MergeCommandRequest(); - MergeCommandBuilder(MergeCommand mergeCommand) { + @Nullable + private final EMail eMail; + + MergeCommandBuilder(MergeCommand mergeCommand, @Nullable EMail eMail) { this.mergeCommand = mergeCommand; + this.eMail = eMail; } /** @@ -209,7 +215,7 @@ public class MergeCommandBuilder { * @return The result of the merge. */ public MergeCommandResult executeMerge() { - AuthorUtil.setAuthorIfNotAvailable(request); + AuthorUtil.setAuthorIfNotAvailable(request, eMail); Preconditions.checkArgument(request.isValid(), "revision to merge and target revision is required"); return mergeCommand.merge(request); } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java index aeccb24b99..f2b11a00a0 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModifyCommandBuilder.java @@ -35,8 +35,10 @@ import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.spi.ModifyCommandRequest; import sonia.scm.repository.util.AuthorUtil; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; import sonia.scm.util.IOUtil; +import javax.annotation.Nullable; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -51,7 +53,6 @@ import java.util.function.Consumer; * default a {@link sonia.scm.AlreadyExistsException} will be thrown) *
  • modify existing files ({@link #modifyFile(String)}
    • *
  • delete existing files ({@link #deleteFile(String)}
    • - *
  • move/rename existing files ({@link #moveFile(String, String)}
    • * * * You can collect multiple changes before they are executed with a call to {@link #execute()}. @@ -75,11 +76,15 @@ public class ModifyCommandBuilder { private final ModifyCommand command; private final File workdir; + @Nullable + private final EMail eMail; + private final ModifyCommandRequest request = new ModifyCommandRequest(); - ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider) { + ModifyCommandBuilder(ModifyCommand command, WorkdirProvider workdirProvider, @Nullable EMail eMail) { this.command = command; this.workdir = workdirProvider.createNewWorkdir(); + this.eMail = eMail; } /** @@ -124,7 +129,7 @@ public class ModifyCommandBuilder { * @return The revision of the new commit. */ public String execute() { - AuthorUtil.setAuthorIfNotAvailable(request); + AuthorUtil.setAuthorIfNotAvailable(request, eMail); try { Preconditions.checkArgument(request.isValid(), "commit message and at least one request are required"); return command.execute(request); diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java index 15f84a4ecd..aa2a41782d 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryService.java @@ -34,7 +34,9 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.io.Closeable; import java.io.IOException; import java.util.Set; @@ -84,30 +86,36 @@ public final class RepositoryService implements Closeable { private final PreProcessorUtil preProcessorUtil; private final RepositoryServiceProvider provider; private final Repository repository; - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "java:S3740"}) private final Set protocolProviders; private final WorkdirProvider workdirProvider; + @Nullable + private final EMail eMail; + /** * Constructs a new {@link RepositoryService}. This constructor should only * be called from the {@link RepositoryServiceFactory}. * @param cacheManager cache manager * @param provider implementation for {@link RepositoryServiceProvider} * @param repository the repository - * @param workdirProvider + * @param workdirProvider provider for workdirs + * @param eMail utility to compute email addresses if missing */ RepositoryService(CacheManager cacheManager, - RepositoryServiceProvider provider, Repository repository, + RepositoryServiceProvider provider, + Repository repository, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, - WorkdirProvider workdirProvider - ) { + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider, + @Nullable EMail eMail) { this.cacheManager = cacheManager; this.provider = provider; this.repository = repository; this.preProcessorUtil = preProcessorUtil; this.protocolProviders = protocolProviders; this.workdirProvider = workdirProvider; + this.eMail = eMail; } /** @@ -397,7 +405,7 @@ public final class RepositoryService implements Closeable { LOG.debug("create merge command for repository {}", repository.getNamespaceAndName()); - return new MergeCommandBuilder(provider.getMergeCommand()); + return new MergeCommandBuilder(provider.getMergeCommand(), eMail); } /** @@ -418,7 +426,7 @@ public final class RepositoryService implements Closeable { LOG.debug("create modify command for repository {}", repository.getNamespaceAndName()); - return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider); + return new ModifyCommandBuilder(provider.getModifyCommand(), workdirProvider, eMail); } /** @@ -448,7 +456,7 @@ public final class RepositoryService implements Closeable { .map(this::createProviderInstanceForRepository); } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "java:S3740"}) private ScmProtocol createProviderInstanceForRepository(ScmProtocolProvider protocolProvider) { return protocolProvider.get(repository); } diff --git a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java index b030d1cb0c..2c13cb63be 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/RepositoryServiceFactory.java @@ -42,7 +42,6 @@ import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.config.ScmConfiguration; import sonia.scm.event.ScmEventBus; -import sonia.scm.repository.BranchCreatedEvent; import sonia.scm.repository.ClearRepositoryCacheEvent; import sonia.scm.repository.NamespaceAndName; import sonia.scm.repository.PostReceiveRepositoryHookEvent; @@ -58,7 +57,9 @@ import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.security.PublicKeyCreatedEvent; import sonia.scm.security.PublicKeyDeletedEvent; import sonia.scm.security.ScmSecurityException; +import sonia.scm.user.EMail; +import javax.annotation.Nullable; import java.util.Set; import static sonia.scm.ContextEntry.ContextBuilder.entity; @@ -115,7 +116,17 @@ public final class RepositoryServiceFactory { private static final Logger logger = LoggerFactory.getLogger(RepositoryServiceFactory.class); - //~--- constructors --------------------------------------------------------- + private final CacheManager cacheManager; + private final RepositoryManager repositoryManager; + private final Set resolvers; + private final PreProcessorUtil preProcessorUtil; + @SuppressWarnings({"rawtypes", "java:S3740"}) + private final Set protocolProviders; + private final WorkdirProvider workdirProvider; + + @Nullable + private final EMail eMail; + /** * Constructs a new {@link RepositoryServiceFactory}. This constructor @@ -127,40 +138,67 @@ public final class RepositoryServiceFactory { * @param repositoryManager manager for repositories * @param resolvers a set of {@link RepositoryServiceResolver} * @param preProcessorUtil helper object for pre processor handling - * @param protocolProviders - * @param workdirProvider + * @param protocolProviders providers for repository protocols + * @param workdirProvider provider for working directories + * + * @deprecated use {@link RepositoryServiceFactory#RepositoryServiceFactory(CacheManager, RepositoryManager, Set, PreProcessorUtil, Set, WorkdirProvider, EMail)} instead * @since 1.21 */ - @Inject + @Deprecated public RepositoryServiceFactory(ScmConfiguration configuration, CacheManager cacheManager, RepositoryManager repositoryManager, Set resolvers, PreProcessorUtil preProcessorUtil, - @SuppressWarnings("rawtypes") Set protocolProviders, WorkdirProvider workdirProvider) { + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider) { this( - configuration, cacheManager, repositoryManager, resolvers, - preProcessorUtil, protocolProviders, workdirProvider, ScmEventBus.getInstance() + cacheManager, repositoryManager, resolvers, + preProcessorUtil, protocolProviders, workdirProvider, null, ScmEventBus.getInstance() + ); + } + + /** + * Constructs a new {@link RepositoryServiceFactory}. This constructor + * should not be called manually, it should only be used by the injection + * container. + * + * @param cacheManager cache manager + * @param repositoryManager manager for repositories + * @param resolvers a set of {@link RepositoryServiceResolver} + * @param preProcessorUtil helper object for pre processor handling + * @param protocolProviders providers for repository protocols + * @param workdirProvider provider for working directories + * @param eMail handling user emails + * @since 2.8.0 + */ + @Inject + public RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, + Set resolvers, PreProcessorUtil preProcessorUtil, + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider, EMail eMail) { + this( + cacheManager, repositoryManager, resolvers, + preProcessorUtil, protocolProviders, workdirProvider, + eMail, ScmEventBus.getInstance() ); } @VisibleForTesting - RepositoryServiceFactory(ScmConfiguration configuration, - CacheManager cacheManager, RepositoryManager repositoryManager, + @SuppressWarnings("java:S107") // to keep backward compatibility, we can not reduce amount of parameters + RepositoryServiceFactory(CacheManager cacheManager, RepositoryManager repositoryManager, Set resolvers, PreProcessorUtil preProcessorUtil, - Set protocolProviders, WorkdirProvider workdirProvider, - ScmEventBus eventBus) { - this.configuration = configuration; + @SuppressWarnings({"rawtypes", "java:S3740"}) Set protocolProviders, + WorkdirProvider workdirProvider, @Nullable EMail eMail, ScmEventBus eventBus) { this.cacheManager = cacheManager; this.repositoryManager = repositoryManager; this.resolvers = resolvers; this.preProcessorUtil = preProcessorUtil; this.protocolProviders = protocolProviders; this.workdirProvider = workdirProvider; + this.eMail = eMail; eventBus.register(new CacheClearHook(cacheManager)); } - //~--- methods -------------------------------------------------------------- - /** * Creates a new RepositoryService for the given repository. * @@ -246,7 +284,7 @@ public final class RepositoryServiceFactory { } service = new RepositoryService(cacheManager, provider, repository, - preProcessorUtil, protocolProviders, workdirProvider); + preProcessorUtil, protocolProviders, workdirProvider, eMail); break; } @@ -259,8 +297,6 @@ public final class RepositoryServiceFactory { return service; } - //~--- inner classes -------------------------------------------------------- - /** * Hook and listener to clear all relevant repository caches. */ @@ -284,8 +320,6 @@ public final class RepositoryServiceFactory { this.caches.add(cacheManager.getCache(BranchesCommandBuilder.CACHE_NAME)); } - //~--- methods ------------------------------------------------------------ - /** * Clear caches on explicit repository cache clear event. * @@ -347,35 +381,4 @@ public final class RepositoryServiceFactory { } } - - //~--- fields --------------------------------------------------------------- - - /** - * cache manager - */ - private final CacheManager cacheManager; - - /** - * scm-manager configuration - */ - private final ScmConfiguration configuration; - - /** - * pre processor util - */ - private final PreProcessorUtil preProcessorUtil; - - /** - * repository manager - */ - private final RepositoryManager repositoryManager; - - /** - * service resolvers - */ - private final Set resolvers; - - private Set protocolProviders; - - private final WorkdirProvider workdirProvider; } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java index fa2c598f03..e5a6501604 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/AuthorUtil.java @@ -21,28 +21,35 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.util; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.Subject; import sonia.scm.repository.Person; +import sonia.scm.user.EMail; import sonia.scm.user.User; +import javax.annotation.Nullable; + public class AuthorUtil { public static void setAuthorIfNotAvailable(CommandWithAuthor request) { + setAuthorIfNotAvailable(request, null); + } + + public static void setAuthorIfNotAvailable(CommandWithAuthor request, @Nullable EMail eMail) { if (request.getAuthor() == null) { - request.setAuthor(createAuthorFromSubject()); + request.setAuthor(createAuthorFromSubject(eMail)); } } - private static Person createAuthorFromSubject() { + private static Person createAuthorFromSubject(@Nullable EMail eMail) { Subject subject = SecurityUtils.getSubject(); User user = subject.getPrincipals().oneByType(User.class); String name = user.getDisplayName(); - String email = user.getMail(); - return new Person(name, email); + String mailAddress = eMail != null ? eMail.getMailOrFallback(user) : user.getMail(); + return new Person(name, mailAddress); } public interface CommandWithAuthor { diff --git a/scm-core/src/main/java/sonia/scm/user/EMail.java b/scm-core/src/main/java/sonia/scm/user/EMail.java new file mode 100644 index 0000000000..e525b8d46c --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/user/EMail.java @@ -0,0 +1,71 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.user; + +import com.google.common.base.Strings; +import sonia.scm.config.ScmConfiguration; +import sonia.scm.util.ValidationUtil; + +import javax.inject.Inject; + +/** + * Email is able to resolve email addresses of users. + * + * @since 2.8.0 + */ +public class EMail { + + private final ScmConfiguration scmConfiguration; + + @Inject + public EMail(ScmConfiguration scmConfiguration) { + this.scmConfiguration = scmConfiguration; + } + + /** + * Returns the email address of the given user or a generated fallback address. + * @param user user to resolve address from + * @return email address or fallback + */ + public String getMailOrFallback(User user) { + if (Strings.isNullOrEmpty(user.getMail())) { + if (isMailUsedAsId(user)) { + return user.getId(); + } else { + return createFallbackMail(user); + } + } else { + return user.getMail(); + } + } + + private boolean isMailUsedAsId(User user) { + return ValidationUtil.isMailAddressValid(user.getId()); + } + + private String createFallbackMail(User user) { + return user.getId() + "@" + scmConfiguration.getMailDomainName(); + } +} diff --git a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java index 5583261e52..281e793cbb 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/ModifyCommandBuilderTest.java @@ -25,7 +25,12 @@ package sonia.scm.repository.api; import com.google.common.io.ByteSource; +import org.apache.shiro.subject.PrincipalCollection; +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.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; @@ -34,10 +39,12 @@ import org.mockito.Mock; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.jupiter.MockitoExtension; import org.mockito.stubbing.Answer; -import sonia.scm.repository.Person; +import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.spi.ModifyCommand; import sonia.scm.repository.spi.ModifyCommandRequest; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; +import sonia.scm.user.User; import java.io.ByteArrayInputStream; import java.io.File; @@ -50,15 +57,19 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class ModifyCommandBuilderTest { + private static final ScmConfiguration SCM_CONFIGURATION = new ScmConfiguration(); + @Mock ModifyCommand command; @Mock @@ -73,7 +84,7 @@ class ModifyCommandBuilderTest { void initWorkdir(@TempDir Path temp) throws IOException { workdir = Files.createDirectory(temp.resolve("workdir")); lenient().when(workdirProvider.createNewWorkdir()).thenReturn(workdir.toFile()); - commandBuilder = new ModifyCommandBuilder(command, workdirProvider); + commandBuilder = new ModifyCommandBuilder(command, workdirProvider, new EMail(SCM_CONFIGURATION)); } @BeforeEach @@ -89,136 +100,27 @@ class ModifyCommandBuilderTest { ); } - @Test - void shouldReturnTargetRevisionFromCommit() { - String targetRevision = initCommand() - .deleteFile("toBeDeleted") - .execute(); - - assertThat(targetRevision).isEqualTo("target"); - } - - @Test - void shouldExecuteDelete() throws IOException { - initCommand() - .deleteFile("toBeDeleted") - .execute(); - - verify(worker).delete("toBeDeleted"); - } - - @Test - void shouldExecuteCreateWithByteSourceContent() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithInputStreamContent() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithOverwriteFalseAsDefault() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); - - initCommand() - .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(overwriteCaptor.getValue()).isFalse(); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateWithOverwriteIfSet() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); - - initCommand() - .createFile("toBeCreated").setOverwrite(true).withData(new ByteArrayInputStream("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); - assertThat(overwriteCaptor.getValue()).isTrue(); - assertThat(contentCaptor).contains("content"); - } - - @Test - void shouldExecuteCreateMultipleTimes() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); - - initCommand() - .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) - .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes())) - .execute(); - - List createdNames = nameCaptor.getAllValues(); - assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1"); - assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2"); - assertThat(contentCaptor).contains("content_1", "content_2"); - } - - @Test - void shouldExecuteModify() throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - List contentCaptor = new ArrayList<>(); - doAnswer(new ExtractContent(contentCaptor)).when(worker).modify(nameCaptor.capture(), any()); - - initCommand() - .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(nameCaptor.getValue()).isEqualTo("toBeModified"); - assertThat(contentCaptor).contains("content"); - } - private ModifyCommandBuilder initCommand() { return commandBuilder .setBranch("branch") - .setCommitMessage("message") - .setAuthor(new Person()); + .setCommitMessage("message"); } - @Test - void shouldDeleteTemporaryFiles(@TempDir Path temp) throws IOException { - ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); - ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(File.class); - doNothing().when(worker).modify(nameCaptor.capture(), fileCaptor.capture()); + private void mockLoggedInUser(User loggedInUser) { + Subject subject = mock(Subject.class); + ThreadContext.bind(subject); + PrincipalCollection principals = mock(PrincipalCollection.class); + when(subject.getPrincipals()).thenReturn(principals); + when(principals.oneByType(User.class)).thenReturn(loggedInUser); + } - initCommand() - .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) - .execute(); - - assertThat(Files.list(temp)).isEmpty(); + @AfterEach + void unbindSubjec() { + ThreadContext.unbindSubject(); } private static class ExtractContent implements Answer { + private final List contentCaptor; public ExtractContent(List contentCaptor) { @@ -230,4 +132,171 @@ class ModifyCommandBuilderTest { return contentCaptor.add(Files.readAllLines(((File) invocation.getArgument(1)).toPath()).get(0)); } } + + @Nested + class WithUserWithMail { + + @BeforeEach + void initSubject() { + User loggedInUser = new User("dent", "Arthur", "dent@hitchhiker.com"); + mockLoggedInUser(loggedInUser); + } + + @Test + void shouldReturnTargetRevisionFromCommit() { + String targetRevision = initCommand() + .deleteFile("toBeDeleted") + .execute(); + + assertThat(targetRevision).isEqualTo("target"); + } + + @Test + void shouldExecuteDelete() throws IOException { + initCommand() + .deleteFile("toBeDeleted") + .execute(); + + verify(worker).delete("toBeDeleted"); + } + + @Test + void shouldExecuteCreateWithByteSourceContent() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithInputStreamContent() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithOverwriteFalseAsDefault() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isFalse(); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateWithOverwriteIfSet() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor overwriteCaptor = ArgumentCaptor.forClass(Boolean.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), overwriteCaptor.capture()); + + initCommand() + .createFile("toBeCreated").setOverwrite(true).withData(new ByteArrayInputStream("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeCreated"); + assertThat(overwriteCaptor.getValue()).isTrue(); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldExecuteCreateMultipleTimes() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).create(nameCaptor.capture(), any(), anyBoolean()); + + initCommand() + .createFile("toBeCreated_1").withData(new ByteArrayInputStream("content_1".getBytes())) + .createFile("toBeCreated_2").withData(new ByteArrayInputStream("content_2".getBytes())) + .execute(); + + List createdNames = nameCaptor.getAllValues(); + assertThat(createdNames.get(0)).isEqualTo("toBeCreated_1"); + assertThat(createdNames.get(1)).isEqualTo("toBeCreated_2"); + assertThat(contentCaptor).contains("content_1", "content_2"); + } + + @Test + void shouldExecuteModify() throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + List contentCaptor = new ArrayList<>(); + doAnswer(new ExtractContent(contentCaptor)).when(worker).modify(nameCaptor.capture(), any()); + + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(nameCaptor.getValue()).isEqualTo("toBeModified"); + assertThat(contentCaptor).contains("content"); + } + + @Test + void shouldDeleteTemporaryFiles(@TempDir Path temp) throws IOException { + ArgumentCaptor nameCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor fileCaptor = ArgumentCaptor.forClass(File.class); + doNothing().when(worker).modify(nameCaptor.capture(), fileCaptor.capture()); + + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + assertThat(Files.list(temp)).isEmpty(); + } + + @Test + void shouldUseMailFromUser() throws IOException { + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + verify(command).execute(argThat(modifyCommandRequest -> { + assertThat(modifyCommandRequest.getAuthor().getMail()).isEqualTo("dent@hitchhiker.com"); + return true; + })); + } + } + + @Nested + class WithUserWithoutMail { + + @BeforeEach + void initSubject() { + User loggedInUser = new User("dent", "Arthur", null); + mockLoggedInUser(loggedInUser); + } + + @Test + void shouldUseMailFromUser() throws IOException { + SCM_CONFIGURATION.setMailDomainName("heart-of-gold.local"); + initCommand() + .modifyFile("toBeModified").withData(ByteSource.wrap("content".getBytes())) + .execute(); + + verify(command).execute(argThat(modifyCommandRequest -> { + assertThat(modifyCommandRequest.getAuthor().getMail()).isEqualTo("dent@heart-of-gold.local"); + return true; + })); + } + } } diff --git a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java index 492d23dec8..d77d0d072b 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceFactoryTest.java @@ -46,6 +46,7 @@ import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.spi.RepositoryServiceProvider; import sonia.scm.repository.spi.RepositoryServiceResolver; import sonia.scm.repository.work.WorkdirProvider; +import sonia.scm.user.EMail; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -56,9 +57,6 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class RepositoryServiceFactoryTest { - @Mock - private ScmConfiguration configuration; - @Mock(answer = Answers.RETURNS_MOCKS) private CacheManager cacheManager; @@ -94,8 +92,9 @@ class RepositoryServiceFactoryTest { builder.add(repositoryServiceResolver); } return new RepositoryServiceFactory( - configuration, cacheManager, repositoryManager, builder.build(), - preProcessorUtil, ImmutableSet.of(), workdirProvider, eventBus + cacheManager, repositoryManager, builder.build(), + preProcessorUtil, ImmutableSet.of(), workdirProvider, + new EMail(new ScmConfiguration()), eventBus ); } diff --git a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java index 901666cb87..8394e3c582 100644 --- a/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/api/RepositoryServiceTest.java @@ -21,13 +21,15 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.api; import org.junit.Test; +import sonia.scm.config.ScmConfiguration; import sonia.scm.repository.Repository; import sonia.scm.repository.spi.HttpScmProtocol; import sonia.scm.repository.spi.RepositoryServiceProvider; +import sonia.scm.user.EMail; import javax.servlet.ServletConfig; import javax.servlet.http.HttpServletRequest; @@ -46,9 +48,11 @@ public class RepositoryServiceTest { private final RepositoryServiceProvider provider = mock(RepositoryServiceProvider.class); private final Repository repository = new Repository("", "git", "space", "repo"); + private final EMail eMail = new EMail(new ScmConfiguration()); + @Test public void shouldReturnMatchingProtocolsFromProvider() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); Stream supportedProtocols = repositoryService.getSupportedProtocols(); assertThat(sizeOf(supportedProtocols.collect(Collectors.toList()))).isEqualTo(1); @@ -56,7 +60,7 @@ public class RepositoryServiceTest { @Test public void shouldFindKnownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); HttpScmProtocol protocol = repositoryService.getProtocol(HttpScmProtocol.class); @@ -65,11 +69,9 @@ public class RepositoryServiceTest { @Test public void shouldFailForUnknownProtocol() { - RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null); + RepositoryService repositoryService = new RepositoryService(null, provider, repository, null, Collections.singleton(new DummyScmProtocolProvider()), null, eMail); - assertThrows(IllegalArgumentException.class, () -> { - repositoryService.getProtocol(UnknownScmProtocol.class); - }); + assertThrows(IllegalArgumentException.class, () -> repositoryService.getProtocol(UnknownScmProtocol.class)); } private static class DummyHttpProtocol extends HttpScmProtocol { diff --git a/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java new file mode 100644 index 0000000000..08a49cea44 --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/repository/util/AuthorUtilTest.java @@ -0,0 +1,114 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.repository.util; + +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.Answers; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.repository.Person; +import sonia.scm.user.EMail; +import sonia.scm.user.User; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class AuthorUtilTest { + + @Mock + private EMail eMail; + + @Mock(answer = Answers.RETURNS_DEEP_STUBS) + private Subject subject; + + @BeforeEach + void setUpSubject() { + ThreadContext.bind(subject); + } + + @AfterEach + void tearDownSubject() { + ThreadContext.unbindSubject(); + } + + @Test + void shouldCreateMailAddressFromEmail() { + User trillian = new User("trillian"); + when(subject.getPrincipals().oneByType(User.class)).thenReturn(trillian); + when(eMail.getMailOrFallback(trillian)).thenReturn("tricia@hitchhicker.com"); + + Command command = new Command(null); + AuthorUtil.setAuthorIfNotAvailable(command, eMail); + + assertThat(command.getAuthor().getMail()).isEqualTo("tricia@hitchhicker.com"); + } + + @Test + void shouldUseUsersMailAddressWithoutEMail() { + User trillian = new User("trillian", "Trillian", "trillian.mcmillan@hitchhiker.com"); + when(subject.getPrincipals().oneByType(User.class)).thenReturn(trillian); + + Command command = new Command(null); + AuthorUtil.setAuthorIfNotAvailable(command); + + assertThat(command.getAuthor().getMail()).isEqualTo("trillian.mcmillan@hitchhiker.com"); + } + + @Test + void shouldKeepExistingAuthor() { + Person person = new Person("Trillian McMillan", "trillian.mcmillian@hitchhiker.com"); + + Command command = new Command(person); + AuthorUtil.setAuthorIfNotAvailable(command); + + assertThat(command.getAuthor()).isSameAs(person); + } + + public static class Command implements AuthorUtil.CommandWithAuthor { + + private Person person; + + public Command(Person person) { + this.person = person; + } + + @Override + public Person getAuthor() { + return person; + } + + @Override + public void setAuthor(Person person) { + this.person = person; + } + } + +} diff --git a/scm-core/src/test/java/sonia/scm/user/EMailTest.java b/scm-core/src/test/java/sonia/scm/user/EMailTest.java new file mode 100644 index 0000000000..49842223ba --- /dev/null +++ b/scm-core/src/test/java/sonia/scm/user/EMailTest.java @@ -0,0 +1,62 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.user; + +import org.junit.jupiter.api.Test; +import sonia.scm.config.ScmConfiguration; + +import static org.assertj.core.api.Assertions.assertThat; + +class EMailTest { + + EMail eMail = new EMail(new ScmConfiguration()); + + @Test + void shouldUserUsersAddressIfAvailable() { + User user = new User("dent", "Arthur Dent", "arthur@hitchhiker.com"); + + String mailAddress = eMail.getMailOrFallback(user); + + assertThat(mailAddress).isEqualTo("arthur@hitchhiker.com"); + } + + @Test + void shouldCreateAddressIfNoneAvailable() { + User user = new User("dent", "Arthur Dent", ""); + + String mailAddress = eMail.getMailOrFallback(user); + + assertThat(mailAddress).isEqualTo("dent@scm-manager.local"); + } + + @Test + void shouldUserUsersIdIfItLooksLikeAnMailAddress() { + User user = new User("dent@hitchhiker.com", "Arthur Dent", ""); + + String mailAddress = eMail.getMailOrFallback(user); + + assertThat(mailAddress).isEqualTo("dent@hitchhiker.com"); + } +} diff --git a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java index 00a135dd82..e83a0dc365 100644 --- a/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java +++ b/scm-plugins/scm-svn-plugin/src/main/java/sonia/scm/repository/spi/SvnBrowseCommand.java @@ -193,23 +193,28 @@ public class SvnBrowseCommand extends AbstractSvnCommand repository.getDir(entry.getRelativePath(), revision, properties, (Collection) null); - String[] externals = properties.getStringValue(SVNProperty.EXTERNALS).split("\\r?\\n"); - for (String external : externals) { - String subRepoUrl = ""; - String subRepoPath = ""; - for (String externalPart : external.split(" ")) { - if (shouldSetExternal(externalPart)) { - subRepoUrl = externalPart; - } else if (!externalPart.contains("-r")) { - subRepoPath = externalPart; + String externals = properties.getStringValue(SVNProperty.EXTERNALS); + + if (!Strings.isNullOrEmpty(externals)) { + String[] splitExternals = externals.split("\\r?\\n"); + for (String external : splitExternals) { + String subRepoUrl = ""; + String subRepoPath = ""; + for (String externalPart : external.split(" ")) { + if (shouldSetExternal(externalPart)) { + subRepoUrl = externalPart; + } else if (!externalPart.contains("-r")) { + subRepoPath = externalPart; + } + } + + if (Util.isNotEmpty(external)) { + SubRepository subRepository = new SubRepository(subRepoUrl); + fileObject.addChild(createSubRepoDirectory(subRepository, subRepoPath)); } } - - if (Util.isNotEmpty(external)) { - SubRepository subRepository = new SubRepository(subRepoUrl); - fileObject.addChild(createSubRepoDirectory(subRepository, subRepoPath)); - } } + } catch (SVNException ex) { logger.error("could not fetch file properties", ex); } diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java index b6d9e4ba26..8228d42f39 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/AbstractSvnCommandTestBase.java @@ -21,15 +21,13 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.repository.spi; //~--- non-JDK imports -------------------------------------------------------- import org.junit.After; -//~--- JDK imports ------------------------------------------------------------ - import java.io.IOException; /** diff --git a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java index 3d3a3131e5..698fec8f37 100644 --- a/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java +++ b/scm-plugins/scm-svn-plugin/src/test/java/sonia/scm/repository/spi/SvnBrowseCommandTest.java @@ -24,10 +24,20 @@ package sonia.scm.repository.spi; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.tmatesoft.svn.core.SVNDepth; +import org.tmatesoft.svn.core.SVNException; +import org.tmatesoft.svn.core.SVNPropertyValue; +import org.tmatesoft.svn.core.SVNURL; +import org.tmatesoft.svn.core.wc.SVNClientManager; +import org.tmatesoft.svn.core.wc.SVNRevision; import sonia.scm.repository.BrowserResult; import sonia.scm.repository.FileObject; +import sonia.scm.repository.SubRepository; +import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.Iterator; @@ -36,14 +46,16 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; /** - * * @author Sebastian Sdorra */ -public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase -{ +public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase { + + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); @Test public void testBrowseWithFilePath() { @@ -83,7 +95,6 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase /** * Method description * - * * @throws IOException */ @Test @@ -260,14 +271,51 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase .containsExactly("e.txt"); } + @Test + public void shouldNotAddSubRepositoryIfNotSetInProperties() { + BrowserResult browserResult = new SvnBrowseCommand(createContext()).getBrowserResult(new BrowseCommandRequest()); + + boolean containsSubRepository = browserResult.getFile().getChildren() + .stream() + .anyMatch(c -> c.getSubRepository() != null); + + assertFalse(containsSubRepository); + } + + @Test + public void shouldAddSubRepositoryIfSetInProperties() throws IOException, SVNException { + String externalLink = "https://scm-manager.org/svn-repo"; + SvnContext svnContext = setProp("svn:externals", "external -r1 " + externalLink); + + BrowserResult browserResult = new SvnBrowseCommand(svnContext).getBrowserResult(new BrowseCommandRequest()); + + boolean containsSubRepository = browserResult.getFile().getChildren() + .stream() + .anyMatch(c -> c.getSubRepository().getRepositoryUrl().equals(externalLink)); + + assertTrue(containsSubRepository); + } + + private SvnContext setProp(String propName, String propValue) throws SVNException, IOException { + SvnContext context = createContext(); + SVNClientManager client = SVNClientManager.newInstance(); + + File workingCopyDirectory = temporaryFolder.newFolder("working-copy"); + + SVNURL url = SVNURL.fromFile(context.getDirectory()); + client.getUpdateClient().doCheckout(url, workingCopyDirectory, SVNRevision.HEAD, SVNRevision.HEAD, SVNDepth.INFINITY, true); + + client.getWCClient().doSetProperty(workingCopyDirectory, propName, SVNPropertyValue.create(propValue), true, SVNDepth.UNKNOWN, null, null); + client.getCommitClient().doCommit(new File[]{workingCopyDirectory}, false, "set prop", null, null, false, false, SVNDepth.UNKNOWN); + return context; + } + /** * Method description * - * * @return */ - private SvnBrowseCommand createCommand() - { + private SvnBrowseCommand createCommand() { return new SvnBrowseCommand(createContext()); } @@ -276,14 +324,11 @@ public class SvnBrowseCommandTest extends AbstractSvnCommandTestBase /** * Method description * - * * @param foList * @param name - * * @return */ - private FileObject getFileObject(Collection foList, String name) - { + private FileObject getFileObject(Collection foList, String name) { return foList.stream() .filter(f -> name.equals(f.getName())) .findFirst() diff --git a/scm-ui/ui-components/src/repos/CommitAuthor.tsx b/scm-ui/ui-components/src/repos/CommitAuthor.tsx new file mode 100644 index 0000000000..f845582267 --- /dev/null +++ b/scm-ui/ui-components/src/repos/CommitAuthor.tsx @@ -0,0 +1,60 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +import React, { FC } from "react"; +import { useTranslation } from "react-i18next"; +import Notification from "../Notification"; +import { Me } from "@scm-manager/ui-types"; +import { connect } from "react-redux"; + +type Props = { + // props from global state + me: Me; +}; + +const CommitAuthor: FC = ({ me }) => { + const [t] = useTranslation("repos"); + + const mail = me.mail ? me.mail : me.fallbackMail; + + return ( + <> + {!me.mail && {t("commit.commitAuthor.noMail")}} + + {t("commit.commitAuthor.author")} {`${me.displayName} <${mail}>`} + + + ); +}; + +const mapStateToProps = (state: any) => { + const { auth } = state; + const me = auth.me; + + return { + me + }; +}; + +export default connect(mapStateToProps)(CommitAuthor); diff --git a/scm-ui/ui-components/src/repos/index.ts b/scm-ui/ui-components/src/repos/index.ts index 36b26495b8..f33ffbc4cb 100644 --- a/scm-ui/ui-components/src/repos/index.ts +++ b/scm-ui/ui-components/src/repos/index.ts @@ -52,6 +52,7 @@ export { default as RepositoryAvatar } from "./RepositoryAvatar"; export { default as RepositoryEntry } from "./RepositoryEntry"; export { default as RepositoryEntryLink } from "./RepositoryEntryLink"; export { default as JumpToFileButton } from "./JumpToFileButton"; +export { default as CommitAuthor } from "./CommitAuthor"; export { File, diff --git a/scm-ui/ui-types/src/Config.ts b/scm-ui/ui-types/src/Config.ts index 667aa7b199..92b678e491 100644 --- a/scm-ui/ui-types/src/Config.ts +++ b/scm-ui/ui-types/src/Config.ts @@ -48,5 +48,6 @@ export type Config = { namespaceStrategy: string; loginInfoUrl: string; releaseFeedUrl: string; + mailDomainName: string; _links: Links; }; diff --git a/scm-ui/ui-types/src/Me.ts b/scm-ui/ui-types/src/Me.ts index 38a3a23278..d8595e0771 100644 --- a/scm-ui/ui-types/src/Me.ts +++ b/scm-ui/ui-types/src/Me.ts @@ -27,7 +27,8 @@ import { Links } from "./hal"; export type Me = { name: string; displayName: string; - mail: string; + mail?: string; + fallbackMail?: string; groups: string[]; _links: Links; }; diff --git a/scm-ui/ui-types/src/User.ts b/scm-ui/ui-types/src/User.ts index 43ff64b9fa..29009b5d0d 100644 --- a/scm-ui/ui-types/src/User.ts +++ b/scm-ui/ui-types/src/User.ts @@ -27,13 +27,13 @@ import { Links } from "./hal"; export type DisplayedUser = { id: string; displayName: string; - mail: string; + mail?: string; }; export type User = { displayName: string; name: string; - mail: string; + mail?: string; password: string; active: boolean; type?: string; diff --git a/scm-ui/ui-webapp/public/locales/de/config.json b/scm-ui/ui-webapp/public/locales/de/config.json index ad749c12c8..24830f2f4d 100644 --- a/scm-ui/ui-webapp/public/locales/de/config.json +++ b/scm-ui/ui-webapp/public/locales/de/config.json @@ -47,6 +47,7 @@ "skip-failed-authenticators": "Fehlgeschlagene Authentifizierer überspringen", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", + "mail-domain-name": "Fallback E-Mail Domain Name", "enabled-xsrf-protection": "XSRF Protection aktivieren", "namespace-strategy": "Namespace Strategie", "login-info-url": "Login Info URL" @@ -62,6 +63,7 @@ "dateFormatHelpText": "Moments Datumsformat. Zulässige Formate sind in der MomentJS Dokumentation beschrieben.", "pluginUrlHelpText": "Die URL der Plugin Center API. Beschreibung der Platzhalter: version = SCM-Manager Version; os = Betriebssystem; arch = Architektur", "releaseFeedUrlHelpText": "Die URL des RSS Release Feed des SCM-Manager. Darüber wird über die neue SCM-Manager Version informiert. Um diese Funktion zu deaktivieren lassen Sie dieses Feld leer.", + "mailDomainNameHelpText": "Dieser Domain Name wird genutzt, wenn für einen User eine E-Mail-Adresse benötigt wird, für den keine hinterlegt ist. Diese Domain wird nicht zum Versenden von E-Mails genutzt und auch keine anderweitige Verbindung aufgebaut.", "enableForwardingHelpText": "mod_proxy Port Weiterleitung aktivieren.", "disableGroupingGridHelpText": "Repository Gruppen deaktivieren. Nach einer Änderung an dieser Einstellung muss die Seite komplett neu geladen werden.", "allowAnonymousAccessHelpText": "Anonyme Benutzer haben Zugriff auf freigegebene Repositories.", diff --git a/scm-ui/ui-webapp/public/locales/de/repos.json b/scm-ui/ui-webapp/public/locales/de/repos.json index a1303555f4..ae0f222765 100644 --- a/scm-ui/ui-webapp/public/locales/de/repos.json +++ b/scm-ui/ui-webapp/public/locales/de/repos.json @@ -135,6 +135,12 @@ "sources": "Sources" } }, + "commit": { + "commitAuthor": { + "author": "Autor", + "noMail": "Für den aktuellen Benutzer existiert keine E-Mail-Adresse. Es wird die unten angezeigte generierte Adresse genutzt." + } + }, "repositoryForm": { "subtitle": "Repository bearbeiten", "submit": "Speichern", @@ -269,4 +275,4 @@ "clickHere": "Klicken Sie hier um Ihre Datei hochzuladen.", "dragAndDrop": "Sie können Ihre Datei auch direkt in die Dropzone ziehen." } -} +}, diff --git a/scm-ui/ui-webapp/public/locales/en/config.json b/scm-ui/ui-webapp/public/locales/en/config.json index 1150dd4e3d..6cb06ecb25 100644 --- a/scm-ui/ui-webapp/public/locales/en/config.json +++ b/scm-ui/ui-webapp/public/locales/en/config.json @@ -47,6 +47,7 @@ "skip-failed-authenticators": "Skip Failed Authenticators", "plugin-url": "Plugin Center URL", "release-feed-url": "Release Feed URL", + "mail-domain-name": "Fallback Mail Domain Name", "enabled-xsrf-protection": "Enabled XSRF Protection", "namespace-strategy": "Namespace Strategy", "login-info-url": "Login Info URL" @@ -62,6 +63,7 @@ "dateFormatHelpText": "Moments date format. Please have a look at the MomentJS documentation.", "pluginUrlHelpText": "The url of the Plugin Center API. Explanation of the placeholders: version = SCM-Manager Version; os = Operation System; arch = Architecture", "releaseFeedUrlHelpText": "The url of the RSS Release Feed for SCM-Manager. This provides up-to-date version information. To disable this feature just leave the url blank.", + "mailDomainNameHelpText": "This domain name will be used to create email addresses for users without one when needed. It will not be used to send mails nor will be accessed otherwise.", "enableForwardingHelpText": "Enable mod_proxy port forwarding.", "disableGroupingGridHelpText": "Disable repository Groups. A complete page reload is required after a change of this value.", "allowAnonymousAccessHelpText": "Anonymous users have access on granted repositories.", diff --git a/scm-ui/ui-webapp/public/locales/en/repos.json b/scm-ui/ui-webapp/public/locales/en/repos.json index bbc21d3cad..3b05c1f48c 100644 --- a/scm-ui/ui-webapp/public/locales/en/repos.json +++ b/scm-ui/ui-webapp/public/locales/en/repos.json @@ -135,6 +135,12 @@ "count_plural": "{{count}} Contributors" } }, + "commit": { + "commitAuthor": { + "author": "Author", + "noMail": "We have found no email address for your current user. We will use the generated address shown below." + } + }, "repositoryForm": { "subtitle": "Edit Repository", "submit": "Save", diff --git a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx index f412acf8e8..c284df3180 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/ConfigForm.tsx @@ -144,6 +144,7 @@ class ConfigForm extends React.Component { skipFailedAuthenticators={config.skipFailedAuthenticators} pluginUrl={config.pluginUrl} releaseFeedUrl={config.releaseFeedUrl} + mailDomainName={config.mailDomainName} enabledXsrfProtection={config.enabledXsrfProtection} namespaceStrategy={config.namespaceStrategy} onChange={(isValid, changedValue, name) => this.onChange(isValid, changedValue, name)} diff --git a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx index 580a3546da..e9ed50a3b5 100644 --- a/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx +++ b/scm-ui/ui-webapp/src/admin/components/form/GeneralSettings.tsx @@ -36,6 +36,7 @@ type Props = WithTranslation & { skipFailedAuthenticators: boolean; pluginUrl: string; releaseFeedUrl: string; + mailDomainName: string; enabledXsrfProtection: boolean; namespaceStrategy: string; namespaceStrategies?: NamespaceStrategies; @@ -51,6 +52,7 @@ class GeneralSettings extends React.Component { loginInfoUrl, pluginUrl, releaseFeedUrl, + mailDomainName, enabledXsrfProtection, anonymousMode, namespaceStrategy, @@ -129,7 +131,7 @@ class GeneralSettings extends React.Component {
    -
    +
    { helpText={t("help.releaseFeedUrlHelpText")} />
    +
    + +
    ); @@ -164,6 +175,9 @@ class GeneralSettings extends React.Component { handleReleaseFeedUrlChange = (value: string) => { this.props.onChange(true, value, "releaseFeedUrl"); }; + handleMailDomainNameChange = (value: string) => { + this.props.onChange(true, value, "mailDomainName"); + }; } export default withTranslation("config")(GeneralSettings); diff --git a/scm-ui/ui-webapp/src/users/components/UserForm.tsx b/scm-ui/ui-webapp/src/users/components/UserForm.tsx index cd0856fa84..9c78ef8476 100644 --- a/scm-ui/ui-webapp/src/users/components/UserForm.tsx +++ b/scm-ui/ui-webapp/src/users/components/UserForm.tsx @@ -113,8 +113,7 @@ class UserForm extends React.Component { this.editUserComponentsAreUnchanged() || this.state.mailValidationError || this.state.displayNameValidationError || - this.isFalsy(user.displayName) || - this.isFalsy(user.mail) + this.isFalsy(user.displayName) ); }; @@ -152,6 +151,7 @@ class UserForm extends React.Component { // edit existing user subtitle = ; } + return ( <> {subtitle} @@ -218,7 +218,7 @@ class UserForm extends React.Component { handleEmailChange = (mail: string) => { this.setState({ - mailValidationError: !validator.isMailValid(mail), + mailValidationError: !!mail && !validator.isMailValid(mail), user: { ...this.state.user, mail diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java index bec72a552b..f7025f141f 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ConfigDto.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import de.otto.edison.hal.HalRepresentation; @@ -59,6 +59,7 @@ public class ConfigDto extends HalRepresentation { private String namespaceStrategy; private String loginInfoUrl; private String releaseFeedUrl; + private String mailDomainName; @Override @SuppressWarnings("squid:S1185") // We want to have this method available in this package diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java index 968d892536..b4e011f7bb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDto.java @@ -21,9 +21,10 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; +import com.fasterxml.jackson.annotation.JsonInclude; import de.otto.edison.hal.Embedded; import de.otto.edison.hal.HalRepresentation; import de.otto.edison.hal.Links; @@ -40,7 +41,10 @@ public class MeDto extends HalRepresentation { private String name; private String displayName; + @JsonInclude(JsonInclude.Include.NON_NULL) private String mail; + @JsonInclude(JsonInclude.Include.NON_NULL) + private String fallbackMail; private Set groups; MeDto(Links links, Embedded embedded) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java index 2101ba2ffb..ff3914654d 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeDtoFactory.java @@ -24,12 +24,14 @@ package sonia.scm.api.v2.resources; +import com.google.common.base.Strings; import de.otto.edison.hal.Embedded; import de.otto.edison.hal.Links; import org.apache.shiro.SecurityUtils; import org.apache.shiro.subject.PrincipalCollection; import org.apache.shiro.subject.Subject; import sonia.scm.group.GroupCollector; +import sonia.scm.user.EMail; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.user.UserPermissions; @@ -46,12 +48,14 @@ public class MeDtoFactory extends HalAppenderMapper { private final ResourceLinks resourceLinks; private final UserManager userManager; private final GroupCollector groupCollector; + private final EMail eMail; @Inject - public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager, GroupCollector groupCollector) { + public MeDtoFactory(ResourceLinks resourceLinks, UserManager userManager, GroupCollector groupCollector, EMail eMail) { this.resourceLinks = resourceLinks; this.userManager = userManager; this.groupCollector = groupCollector; + this.eMail = eMail; } public MeDto create() { @@ -61,6 +65,7 @@ public class MeDtoFactory extends HalAppenderMapper { MeDto dto = createDto(user); mapUserProperties(user, dto); mapGroups(user, dto); + setGeneratedMail(user, dto); return dto; } @@ -79,6 +84,12 @@ public class MeDtoFactory extends HalAppenderMapper { return subject.getPrincipals(); } + private void setGeneratedMail(User user, MeDto dto) { + if (Strings.isNullOrEmpty(user.getMail())) { + dto.setFallbackMail(eMail.getMailOrFallback(user)); + } + } + private MeDto createDto(User user) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java index 0e4e9b34c2..027cc59621 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.fasterxml.jackson.annotation.JsonInclude; @@ -46,7 +46,8 @@ public class UserDto extends HalRepresentation { private String displayName; @JsonInclude(JsonInclude.Include.NON_NULL) private Instant lastModified; - @NotEmpty @Email + @JsonInclude(JsonInclude.Include.NON_NULL) + @Email private String mail; @Pattern(regexp = ValidationUtil.REGEX_NAME) private String name; diff --git a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java index ed2954eb32..672f972dab 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java @@ -30,6 +30,8 @@ import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.authc.credential.AllowAllCredentialsMatcher; import org.apache.shiro.authz.AuthorizationException; import org.apache.shiro.realm.AuthenticatingRealm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.plugin.Extension; import sonia.scm.repository.RepositoryRole; import sonia.scm.repository.RepositoryRoleManager; @@ -43,6 +45,8 @@ import static com.google.common.base.Preconditions.checkArgument; @Extension public class ApiKeyRealm extends AuthenticatingRealm { + private static final Logger LOG = LoggerFactory.getLogger(ApiKeyRealm.class); + private final ApiKeyService apiKeyService; private final DAORealmHelper helper; private final RepositoryRoleManager repositoryRoleManager; @@ -58,7 +62,14 @@ public class ApiKeyRealm extends AuthenticatingRealm { @Override public boolean supports(AuthenticationToken token) { - return token instanceof UsernamePasswordToken || token instanceof BearerToken; + if (token instanceof UsernamePasswordToken || token instanceof BearerToken) { + boolean containsDot = getPassword(token).contains("."); + if (containsDot) { + LOG.debug("Ignoring token with at least one dot ('.'); this is probably a JWT token"); + } + return !containsDot; + } + return false; } @Override @@ -74,6 +85,7 @@ public class ApiKeyRealm extends AuthenticatingRealm { private AuthenticationInfo buildAuthenticationInfo(AuthenticationToken token, ApiKeyService.CheckResult check) { RepositoryRole repositoryRole = determineRole(check); Scope scope = createScope(repositoryRole); + LOG.debug("login for user {} with api key limited to role {}", check.getUser(), check.getPermissionRole()); return helper .authenticationInfoBuilder(check.getUser()) .withSessionId(getPrincipal(token)) diff --git a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java index e037590db6..f554c92d04 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/BearerRealm.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.security; import com.google.common.annotations.VisibleForTesting; @@ -29,6 +29,8 @@ import org.apache.shiro.authc.AuthenticationInfo; import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.credential.AllowAllCredentialsMatcher; import org.apache.shiro.realm.AuthenticatingRealm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.group.GroupDAO; import sonia.scm.plugin.Extension; import sonia.scm.user.UserDAO; @@ -54,6 +56,7 @@ public class BearerRealm extends AuthenticatingRealm @VisibleForTesting static final String REALM = "BearerRealm"; + private static final Logger LOG = LoggerFactory.getLogger(BearerRealm.class); /** dao realm helper */ private final DAORealmHelper helper; @@ -76,7 +79,17 @@ public class BearerRealm extends AuthenticatingRealm setAuthenticationTokenClass(BearerToken.class); } - //~--- methods -------------------------------------------------------------- + @Override + public boolean supports(AuthenticationToken token) { + if (token instanceof BearerToken) { + boolean containsDot = ((BearerToken) token).getCredentials().contains("."); + if (!containsDot) { + LOG.debug("Ignoring token without a dot ('.'); this probably is an API key"); + } + return containsDot; + } + return false; + } /** * Validates the given bearer token and retrieves authentication data from diff --git a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java index 47da2d5718..be691da1d8 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/web/security/TokenRefreshFilter.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationException; @@ -90,6 +90,10 @@ public class TokenRefreshFilter extends HttpFilter { private void examineToken(HttpServletRequest request, HttpServletResponse response, BearerToken token) { AccessToken accessToken; + if (!token.getCredentials().contains(".")) { + LOG.trace("Ignoring token without dot. This probably is an API key, no JWT"); + return; + } try { accessToken = resolver.resolve(token); } catch (AuthenticationException e) { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java index 3e01a1e4cc..f938c7f655 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ConfigDtoToScmConfigurationMapperTest.java @@ -75,6 +75,7 @@ public class ConfigDtoToScmConfigurationMapperTest { assertTrue(config.isEnabledXsrfProtection()); assertEquals("username", config.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", config.getLoginInfoUrl()); + assertEquals("hitchhiker.mail", config.getMailDomainName()); } @Test @@ -113,6 +114,7 @@ public class ConfigDtoToScmConfigurationMapperTest { configDto.setEnabledXsrfProtection(true); configDto.setNamespaceStrategy("username"); configDto.setLoginInfoUrl("https://scm-manager.org/login-info"); + configDto.setMailDomainName("hitchhiker.mail"); return configDto; } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java index 901d625579..ea539c4631 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeDtoFactoryTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.api.v2.resources; import com.google.common.collect.ImmutableSet; @@ -38,9 +38,9 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; import sonia.scm.SCMContext; import sonia.scm.group.GroupCollector; +import sonia.scm.user.EMail; import sonia.scm.user.User; import sonia.scm.user.UserManager; -import sonia.scm.user.UserPermissions; import sonia.scm.user.UserTestData; import java.net.URI; @@ -65,13 +65,16 @@ class MeDtoFactoryTest { @Mock private Subject subject; + @Mock + private EMail eMail; + private MeDtoFactory meDtoFactory; @BeforeEach void setUpContext() { ThreadContext.bind(subject); ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); - meDtoFactory = new MeDtoFactory(resourceLinks, userManager, groupCollector); + meDtoFactory = new MeDtoFactory(resourceLinks, userManager, groupCollector, eMail); } @AfterEach @@ -235,4 +238,17 @@ class MeDtoFactoryTest { MeDto dto = meDtoFactory.create(); assertThat(dto.getLinks().getLinkBy("profile").get().getHref()).isEqualTo("http://hitchhiker.com/users/trillian"); } + + @Test + void shouldUserGeneratedMailOnlyWhenUserHasNone() { + User user = UserTestData.createTrillian(); + user.setMail(null); + prepareSubject(user); + when(eMail.getMailOrFallback(user)).thenReturn("trillian@hitchhiker.local"); + + MeDto dto = meDtoFactory.create(); + + assertThat(dto.getMail()).isNull(); + assertThat(dto.getFallbackMail()).isEqualTo("trillian@hitchhiker.local"); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java index cef98c4062..49769bf1b3 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ScmConfigurationToConfigDtoMapperTest.java @@ -106,6 +106,7 @@ public class ScmConfigurationToConfigDtoMapperTest { assertEquals("username", dto.getNamespaceStrategy()); assertEquals("https://scm-manager.org/login-info", dto.getLoginInfoUrl()); assertEquals("https://www.scm-manager.org/download/rss.xml", dto.getReleaseFeedUrl()); + assertEquals("scm-manager.local", dto.getMailDomainName()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("self").get().getHref()); assertEquals(expectedBaseUri.toString(), dto.getLinks().getLinkBy("update").get().getHref()); diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java index 48a014dfb2..0f4171b8d9 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyRealmTest.java @@ -96,6 +96,15 @@ class ApiKeyRealmTest { assertThrows(AuthorizationException.class, () -> realm.doGetAuthenticationInfo(token)); } + @Test + void shouldIgnoreTokensWithDots() { + BearerToken token = valueOf("this.is.no.api.token"); + + boolean supports = realm.supports(token); + + assertThat(supports).isFalse(); + } + void verifyScopeSet(String... permissions) { verify(authenticationInfoBuilder).withScope(argThat(scope -> { assertThat(scope).containsExactly(permissions); diff --git a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java index 390d5e6ba2..ac0ee44040 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java @@ -61,4 +61,11 @@ class ApiKeyTokenHandlerTest { assertThat(token).isEmpty(); } + + @Test + void shouldParseRealWorldExample() { + Optional token = handler.readToken("eyJhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ"); + + assertThat(token).get().extracting("user").isEqualTo("horst"); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java index c0ac82b7f6..c9d834cdbc 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/BearerRealmTest.java @@ -40,6 +40,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static sonia.scm.security.BearerToken.valueOf; /** * Unit tests for {@link BearerRealm}. @@ -96,4 +97,13 @@ class BearerRealmTest { void shouldThrowIllegalArgumentExceptionForWrongTypeOfToken() { assertThrows(IllegalArgumentException.class, () -> realm.doGetAuthenticationInfo(new UsernamePasswordToken())); } + + @Test + void shouldIgnoreTokensWithoutDot() { + BearerToken token = valueOf("this-is-no-jwt-token"); + + boolean supports = realm.supports(token); + + assertThat(supports).isFalse(); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java index d74a5eef7f..410eb1f368 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/web/security/TokenRefreshFilterTest.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.security; import org.apache.shiro.authc.AuthenticationToken; @@ -52,6 +52,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static sonia.scm.security.BearerToken.valueOf; @ExtendWith({MockitoExtension.class}) class TokenRefreshFilterTest { @@ -103,7 +104,7 @@ class TokenRefreshFilterTest { @Test void shouldNotRefreshNonJwtToken() throws IOException, ServletException { - BearerToken token = mock(BearerToken.class); + BearerToken token = createValidToken(); JwtAccessToken jwtToken = mock(JwtAccessToken.class); when(tokenGenerator.createToken(request)).thenReturn(token); when(resolver.resolve(token)).thenReturn(jwtToken); @@ -116,7 +117,7 @@ class TokenRefreshFilterTest { @Test void shouldRefreshIfRefreshable() throws IOException, ServletException { - BearerToken token = mock(BearerToken.class); + BearerToken token = createValidToken(); JwtAccessToken jwtToken = mock(JwtAccessToken.class); JwtAccessToken newJwtToken = mock(JwtAccessToken.class); when(tokenGenerator.createToken(request)).thenReturn(token); @@ -128,4 +129,8 @@ class TokenRefreshFilterTest { verify(issuer).authenticate(request, response, newJwtToken); verify(filterChain).doFilter(request, response); } + + BearerToken createValidToken() { + return valueOf("some.jwt.token"); + } }