From 2baa61eb095386c96ff09690711df26536ca516b Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 13 Oct 2020 20:40:00 +0000 Subject: [PATCH 1/9] Bump junit from 4.13 to 4.13.1 Bumps [junit](https://github.com/junit-team/junit4) from 4.13 to 4.13.1. - [Release notes](https://github.com/junit-team/junit4/releases) - [Changelog](https://github.com/junit-team/junit4/blob/main/doc/ReleaseNotes4.13.1.md) - [Commits](https://github.com/junit-team/junit4/compare/r4.13...r4.13.1) Signed-off-by: dependabot[bot] --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0a7d225c3b..506c11920f 100644 --- a/pom.xml +++ b/pom.xml @@ -429,7 +429,7 @@ junit junit - 4.13 + 4.13.1 test From e3cbdf6a930bbf6851dcc585b94e2ec6fe404ca3 Mon Sep 17 00:00:00 2001 From: snyk-bot Date: Tue, 13 Oct 2020 21:57:02 +0000 Subject: [PATCH 2/9] fix: pom.xml to reduce vulnerabilities The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JAVA-JUNIT-1017047 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 0a7d225c3b..506c11920f 100644 --- a/pom.xml +++ b/pom.xml @@ -429,7 +429,7 @@ junit junit - 4.13 + 4.13.1 test From 2c640009cce8f20c2de7f98aff2c339f108541f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 08:34:32 +0200 Subject: [PATCH 3/9] Fix anonymous migration for deleted repositories --- CHANGELOG.md | 4 + .../repository/PublicFlagUpdateStep.java | 10 +- .../repository/PublicFlagUpdateStepTest.java | 117 ++++++++++-------- 3 files changed, 78 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97644147c9..e94d216088 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file. 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 +### Fixed +- Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) + ## [2.7.0] - 2020-10-12 ### Added - Users can create API keys with limited permissions ([#1359](https://github.com/scm-manager/scm-manager/pull/1359)) diff --git a/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java index 310ce25c5c..8a18442afa 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java +++ b/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.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.update.repository; import org.slf4j.Logger; @@ -89,9 +89,11 @@ public class PublicFlagUpdateStep implements UpdateStep { .filter(V1Repository::isPublic) .forEach(v1Repository -> { Repository v2Repository = repositoryDAO.get(v1Repository.getId()); - LOG.info(String.format("Add RepositoryRole 'READ' to _anonymous user for repository: %s - %s/%s", v2Repository.getId(), v2Repository.getNamespace(), v2Repository.getName())); - v2Repository.addPermission(new RepositoryPermission(v2AnonymousUser.getId(), "READ", false)); - repositoryDAO.modify(v2Repository); + if (v2Repository != null) { + LOG.info(String.format("Add RepositoryRole 'READ' to _anonymous user for repository: %s - %s/%s", v2Repository.getId(), v2Repository.getNamespace(), v2Repository.getName())); + v2Repository.addPermission(new RepositoryPermission(v2AnonymousUser.getId(), "READ", false)); + repositoryDAO.modify(v2Repository); + } }); } diff --git a/scm-webapp/src/test/java/sonia/scm/update/repository/PublicFlagUpdateStepTest.java b/scm-webapp/src/test/java/sonia/scm/update/repository/PublicFlagUpdateStepTest.java index 54c5460c6e..23eda6d38e 100644 --- a/scm-webapp/src/test/java/sonia/scm/update/repository/PublicFlagUpdateStepTest.java +++ b/scm-webapp/src/test/java/sonia/scm/update/repository/PublicFlagUpdateStepTest.java @@ -25,6 +25,7 @@ package sonia.scm.update.repository; 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; @@ -77,63 +78,81 @@ class PublicFlagUpdateStepTest { //prepare backup xml V1RepositoryFileSystem.createV1Home(tempDir); Files.move(tempDir.resolve("config").resolve("repositories.xml"), tempDir.resolve("config").resolve("repositories.xml.v1.backup")); - when(repositoryDAO.get((String) any())).thenReturn(REPOSITORY); } @Test - void shouldDeleteOldAnonymousUserIfExists() throws JAXBException { - User anonymous = new User("anonymous"); - when(userDAO.getAll()).thenReturn(Collections.singleton(anonymous)); - doReturn(anonymous).when(userDAO).get("anonymous"); - doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); - - updateStep.doUpdate(); - - verify(userDAO).delete(anonymous); - } - - @Test - void shouldNotTryToDeleteOldAnonymousUserIfNotExists() throws JAXBException { - when(userDAO.getAll()).thenReturn(Collections.emptyList()); - doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); - - updateStep.doUpdate(); - - verify(userDAO, never()).delete(any()); - } - - @Test - void shouldCreateNewAnonymousUserIfNotExists() throws JAXBException { - doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); - when(userDAO.getAll()).thenReturn(Collections.singleton(new User("trillian"))); - - updateStep.doUpdate(); - - verify(userDAO).add(SCMContext.ANONYMOUS); - } - - @Test - void shouldNotCreateNewAnonymousUserIfAlreadyExists() throws JAXBException { - doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); - when(userDAO.getAll()).thenReturn(Collections.singleton(new User("_anonymous"))); - - updateStep.doUpdate(); - - verify(userDAO, never()).add(SCMContext.ANONYMOUS); - } - - @Test - void shouldMigratePublicFlagToAnonymousRepositoryPermission() throws JAXBException { + void shouldNotFailForDeletedRepository() throws JAXBException { when(userDAO.getAll()).thenReturn(Collections.emptyList()); when(userDAO.get("_anonymous")).thenReturn(SCMContext.ANONYMOUS); updateStep.doUpdate(); - verify(repositoryDAO, times(2)).modify(repositoryCaptor.capture()); + verify(repositoryDAO, never()).modify(any()); + } - RepositoryPermission migratedRepositoryPermission = repositoryCaptor.getValue().getPermissions().iterator().next(); - assertThat(migratedRepositoryPermission.getName()).isEqualTo(SCMContext.USER_ANONYMOUS); - assertThat(migratedRepositoryPermission.getRole()).isEqualTo("READ"); - assertThat(migratedRepositoryPermission.isGroupPermission()).isFalse(); + @Nested + class WithExistingRepository { + + @BeforeEach + void mockRepository() { + when(repositoryDAO.get((String) any())).thenReturn(REPOSITORY); + } + + @Test + void shouldDeleteOldAnonymousUserIfExists() throws JAXBException { + User anonymous = new User("anonymous"); + when(userDAO.getAll()).thenReturn(Collections.singleton(anonymous)); + doReturn(anonymous).when(userDAO).get("anonymous"); + doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); + + updateStep.doUpdate(); + + verify(userDAO).delete(anonymous); + } + + @Test + void shouldNotTryToDeleteOldAnonymousUserIfNotExists() throws JAXBException { + when(userDAO.getAll()).thenReturn(Collections.emptyList()); + doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); + + updateStep.doUpdate(); + + verify(userDAO, never()).delete(any()); + } + + @Test + void shouldCreateNewAnonymousUserIfNotExists() throws JAXBException { + doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); + when(userDAO.getAll()).thenReturn(Collections.singleton(new User("trillian"))); + + updateStep.doUpdate(); + + verify(userDAO).add(SCMContext.ANONYMOUS); + } + + @Test + void shouldNotCreateNewAnonymousUserIfAlreadyExists() throws JAXBException { + doReturn(SCMContext.ANONYMOUS).when(userDAO).get(SCMContext.USER_ANONYMOUS); + when(userDAO.getAll()).thenReturn(Collections.singleton(new User("_anonymous"))); + + updateStep.doUpdate(); + + verify(userDAO, never()).add(SCMContext.ANONYMOUS); + } + + @Test + void shouldMigratePublicFlagToAnonymousRepositoryPermission() throws JAXBException { + when(userDAO.getAll()).thenReturn(Collections.emptyList()); + when(userDAO.get("_anonymous")).thenReturn(SCMContext.ANONYMOUS); + + updateStep.doUpdate(); + + verify(repositoryDAO, times(2)).modify(repositoryCaptor.capture()); + + RepositoryPermission migratedRepositoryPermission = repositoryCaptor.getValue().getPermissions().iterator().next(); + assertThat(migratedRepositoryPermission.getName()).isEqualTo(SCMContext.USER_ANONYMOUS); + assertThat(migratedRepositoryPermission.getRole()).isEqualTo("READ"); + assertThat(migratedRepositoryPermission.isGroupPermission()).isFalse(); + } } } From c9410a63929c0d4f90dc9ab17b101802019a65f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 08:56:07 +0200 Subject: [PATCH 4/9] Add logging --- .../sonia/scm/update/repository/PublicFlagUpdateStep.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java b/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java index 8a18442afa..e80fca770d 100644 --- a/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java +++ b/scm-webapp/src/main/java/sonia/scm/update/repository/PublicFlagUpdateStep.java @@ -90,9 +90,11 @@ public class PublicFlagUpdateStep implements UpdateStep { .forEach(v1Repository -> { Repository v2Repository = repositoryDAO.get(v1Repository.getId()); if (v2Repository != null) { - LOG.info(String.format("Add RepositoryRole 'READ' to _anonymous user for repository: %s - %s/%s", v2Repository.getId(), v2Repository.getNamespace(), v2Repository.getName())); + LOG.info("Add RepositoryRole 'READ' to _anonymous user for repository: {} - {}/{}", v2Repository.getId(), v2Repository.getNamespace(), v2Repository.getName()); v2Repository.addPermission(new RepositoryPermission(v2AnonymousUser.getId(), "READ", false)); repositoryDAO.modify(v2Repository); + } else { + LOG.info("Repository no longer found for id {}; could not set permission for former anonymous mode", v1Repository.getId()); } }); } From d273b19f683adb8fc1ba8b966a46793a7ae1cdcf Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 14 Oct 2020 10:47:03 +0200 Subject: [PATCH 5/9] add null check on parsing svn properties in browsecommand --- CHANGELOG.md | 1 + .../scm/repository/spi/SvnBrowseCommand.java | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e94d216088..e14e58db6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### 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)) ## [2.7.0] - 2020-10-12 ### Added 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); } From 07a85ef9c16b5e0e0d23d1f1bbbc9c7fd4ddf827 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 11:03:42 +0200 Subject: [PATCH 6/9] Check token content before handling them This adds plausibility checks before handling tokens as for example jwt or api keys. Doing so we generate less error logs and therefore we cause less confusion. --- CHANGELOG.md | 2 ++ .../java/sonia/scm/security/ApiKeyRealm.java | 13 ++++++++++++- .../java/sonia/scm/security/BearerRealm.java | 17 +++++++++++++++-- .../scm/web/security/TokenRefreshFilter.java | 6 +++++- .../sonia/scm/security/ApiKeyRealmTest.java | 9 +++++++++ .../scm/security/ApiKeyTokenHandlerTest.java | 7 +++++++ .../sonia/scm/security/BearerRealmTest.java | 10 ++++++++++ .../web/security/TokenRefreshFilterTest.java | 11 ++++++++--- 8 files changed, 68 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e94d216088..314f94b374 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Fixed - Null Pointer Exception on anonymous migration with deleted repositories ([#1371](https://github.com/scm-manager/scm-manager/pull/1371)) +### 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/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java index ed2954eb32..d9c1536e0e 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 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/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..151d3e2c10 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("JhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ"); + + 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"); + } } From f35fddc505f80468fee8163d44c3d4317a311cfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 11:28:21 +0200 Subject: [PATCH 7/9] Add debug log for successful login --- scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java | 1 + 1 file changed, 1 insertion(+) 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 d9c1536e0e..672f972dab 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java +++ b/scm-webapp/src/main/java/sonia/scm/security/ApiKeyRealm.java @@ -85,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)) From ab6e5fd6d6ff0547b5cd52674f4b288f46f0a23b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 14 Oct 2020 11:43:47 +0200 Subject: [PATCH 8/9] Fix unit test --- .../test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 151d3e2c10..ac0ee44040 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/ApiKeyTokenHandlerTest.java @@ -64,7 +64,7 @@ class ApiKeyTokenHandlerTest { @Test void shouldParseRealWorldExample() { - Optional token = handler.readToken("JhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ"); + Optional token = handler.readToken("eyJhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ"); assertThat(token).get().extracting("user").isEqualTo("horst"); } From 0847a3eda0a94873848c592a3abc531ff2f6f159 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Wed, 14 Oct 2020 12:02:05 +0200 Subject: [PATCH 9/9] add unit tests --- .../spi/AbstractSvnCommandTestBase.java | 4 +- .../repository/spi/SvnBrowseCommandTest.java | 67 ++++++++++++++++--- 2 files changed, 57 insertions(+), 14 deletions(-) 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()