Merge branch 'develop' into feature/user_converter

This commit is contained in:
Eduard Heimbuch
2020-10-14 15:45:07 +02:00
14 changed files with 228 additions and 89 deletions

View File

@@ -5,6 +5,14 @@ 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))
- 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
- Users can create API keys with limited permissions ([#1359](https://github.com/scm-manager/scm-manager/pull/1359))

View File

@@ -429,7 +429,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13</version>
<version>4.13.1</version>
<scope>test</scope>
</dependency>

View File

@@ -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);
}

View File

@@ -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;
/**

View File

@@ -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<FileObject> foList, String name)
{
private FileObject getFileObject(Collection<FileObject> foList, String name) {
return foList.stream()
.filter(f -> name.equals(f.getName()))
.findFirst()

View File

@@ -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))

View File

@@ -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

View File

@@ -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,13 @@ 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("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());
}
});
}

View File

@@ -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) {

View File

@@ -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);

View File

@@ -61,4 +61,11 @@ class ApiKeyTokenHandlerTest {
assertThat(token).isEmpty();
}
@Test
void shouldParseRealWorldExample() {
Optional<ApiKeyTokenHandler.Token> token = handler.readToken("eyJhcGlLZXlJZCI6IkE2U0ROWmV0MjEiLCJ1c2VyIjoiaG9yc3QiLCJwYXNzcGhyYXNlIjoiWGNKQ01PMnZuZ1JaOEhVU21BSVoifQ");
assertThat(token).get().extracting("user").isEqualTo("horst");
}
}

View File

@@ -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();
}
}

View File

@@ -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();
}
}
}

View File

@@ -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");
}
}