From 56331c420113caf10730a66d818a051a417e492c Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Mon, 2 Nov 2020 17:13:13 +0100 Subject: [PATCH 1/8] use api key scope when creating access tokens from api keys instead of full user scope --- .../src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index 5221142fc0..5e6bda9000 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -156,7 +156,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessToken build() { if (SecurityUtils.getSubject().getPrincipals().getRealmNames().contains(ApiKeyRealm.NAME)) { - throw new AuthorizationException("Cannot create access token for api keys"); + scope = Scope.valueOf(SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class)); } String id = keyGenerator.createKey(); From 5c42c3b49d4465f8e84a2a35b4fc292f51a124d9 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 3 Nov 2020 11:42:44 +0100 Subject: [PATCH 2/8] update unit test --- .../sonia/scm/security/JwtAccessTokenBuilderTest.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java index 4e25b8e869..c30a8a338d 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -39,6 +39,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import java.util.Collection; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -135,15 +136,22 @@ class JwtAccessTokenBuilderTest { @Nested class FromApiKeyRealm { + private Scope scope; + @BeforeEach void mockApiKeyRealm() { + scope = Scope.valueOf("dummy:scope:*"); lenient().when(principalCollection.getRealmNames()).thenReturn(singleton("ApiTokenRealm")); + lenient().when(principalCollection.oneByType(Scope.class)).thenReturn(scope); } @Test void testRejectedRequest() { JwtAccessTokenBuilder builder = factory.create().subject("dent"); - assertThrows(AuthorizationException.class, builder::build); + final JwtAccessToken accessToken = builder.build(); + assertThat(accessToken).isNotNull(); + assertThat(accessToken.getSubject()).isEqualTo("dent"); + assertThat((Collection) accessToken.getCustom("scope").get()).containsExactly("dummy:scope:*"); } } From 621ed5b827cc3696c003aafd24bce92a50d32ebd Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 3 Nov 2020 14:43:06 +0100 Subject: [PATCH 3/8] update unit test name --- .../test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java index c30a8a338d..071ca6df48 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -146,7 +146,7 @@ class JwtAccessTokenBuilderTest { } @Test - void testRejectedRequest() { + void testSimpleRequest() { JwtAccessTokenBuilder builder = factory.create().subject("dent"); final JwtAccessToken accessToken = builder.build(); assertThat(accessToken).isNotNull(); From 36b6b221a45f0d62fc18b2b390bb3dd96bbeb6d1 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Tue, 3 Nov 2020 14:47:55 +0100 Subject: [PATCH 4/8] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbd392b0f..9166878d00 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 - Do not expose subversion commit with id 0 ([#1395](https://github.com/scm-manager/scm-manager/pull/1395)) +- Repositories can now be cloned with api keys ([#1407](https://github.com/scm-manager/scm-manager/pull/1407)) ## [2.8.0] - 2020-10-27 ### Added From b5f042ad15bc29d6af78368635a70d2add16377c Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Wed, 4 Nov 2020 09:38:29 +0100 Subject: [PATCH 5/8] use preexisiting scope by default but prevent overriding of builder scope and update unit tests --- .../sonia/scm/security/JwtAccessTokenBuilder.java | 12 +++++++----- .../scm/security/JwtAccessTokenBuilderTest.java | 8 +++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index 5e6bda9000..257aa10467 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -40,9 +40,7 @@ import java.time.Clock; import java.time.Instant; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; /** @@ -71,7 +69,6 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { private Instant refreshExpiration; private String parentKeyId; private Scope scope = Scope.empty(); - private Set groups = new HashSet<>(); private final Map custom = Maps.newHashMap(); @@ -155,8 +152,13 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessToken build() { - if (SecurityUtils.getSubject().getPrincipals().getRealmNames().contains(ApiKeyRealm.NAME)) { - scope = Scope.valueOf(SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class)); + final Scope principalScope = SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class); + if (principalScope != null) { + if (scope != null && !scope.isEmpty()) { + throw new AuthorizationException(String.format("cannot merge builder scope (%s) with principal scope (%s)", scope, principalScope)); + } + LOG.debug("using existing scope for new access token: {}", principalScope); + scope = principalScope; } String id = keyGenerator.createKey(); diff --git a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java index 071ca6df48..1888bac08c 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/JwtAccessTokenBuilderTest.java @@ -146,13 +146,19 @@ class JwtAccessTokenBuilderTest { } @Test - void testSimpleRequest() { + void shouldCreateJwtAndUsePreviousScope() { JwtAccessTokenBuilder builder = factory.create().subject("dent"); final JwtAccessToken accessToken = builder.build(); assertThat(accessToken).isNotNull(); assertThat(accessToken.getSubject()).isEqualTo("dent"); assertThat((Collection) accessToken.getCustom("scope").get()).containsExactly("dummy:scope:*"); } + + @Test + void shouldThrowExceptionWhenScopeAlreadyDefinedInBuilder() { + JwtAccessTokenBuilder builder = factory.create().scope(Scope.valueOf("an:incompatible:scope")).subject("dent"); + assertThrows(AuthorizationException.class, builder::build); + } } @Nested From a53cdc902b8e385bca11a08c8da5021a13117fb8 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Wed, 4 Nov 2020 09:47:59 +0100 Subject: [PATCH 6/8] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b925289154..142fdebf52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Internal server error for git sub modules without tree object ([#1397](https://github.com/scm-manager/scm-manager/pull/1397)) - Do not expose subversion commit with id 0 ([#1395](https://github.com/scm-manager/scm-manager/pull/1395)) -- Repositories can now be cloned with api keys ([#1407](https://github.com/scm-manager/scm-manager/pull/1407)) +- Cloning of Mercurial repositories with api keys ([#1407](https://github.com/scm-manager/scm-manager/pull/1407)) - Disable cloning repositories via ssh for anonymous users ([#1403](https://github.com/scm-manager/scm-manager/pull/1403)) - Support anonymous file download through rest api for non-browser clients (e.g. curl or postman) when anonymous mode is set to protocol-only ([#1402](https://github.com/scm-manager/scm-manager/pull/1402)) - SVN diff with property changes ([#1400](https://github.com/scm-manager/scm-manager/pull/1400)) From a6667b0ffd947254942015a69a1fb6991c3660ee Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Wed, 4 Nov 2020 10:00:28 +0100 Subject: [PATCH 7/8] update integration test --- .../test/java/sonia/scm/it/ApiKeyITCase.java | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/scm-it/src/test/java/sonia/scm/it/ApiKeyITCase.java b/scm-it/src/test/java/sonia/scm/it/ApiKeyITCase.java index b9f75a9dec..24ce46581f 100644 --- a/scm-it/src/test/java/sonia/scm/it/ApiKeyITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/ApiKeyITCase.java @@ -30,6 +30,8 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import sonia.scm.it.utils.RepositoryUtil; import sonia.scm.it.utils.RestUtil; import sonia.scm.it.utils.TestData; @@ -39,16 +41,30 @@ import sonia.scm.web.VndMediaType; import javax.ws.rs.core.MediaType; import java.io.IOException; +import java.util.Collection; import java.util.Objects; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static sonia.scm.it.utils.RepositoryUtil.addAndCommitRandomFile; import static sonia.scm.it.utils.RestUtil.given; +import static sonia.scm.it.utils.ScmTypes.availableScmTypes; import static sonia.scm.it.utils.TestData.WRITE; +@RunWith(Parameterized.class) public class ApiKeyITCase { + @Parameterized.Parameters(name = "{0}") + public static Collection createParameters() { + return availableScmTypes(); + } + + private final String repositoryType; + + public ApiKeyITCase(String repositoryType) { + this.repositoryType = repositoryType; + } + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); @@ -56,7 +72,7 @@ public class ApiKeyITCase { public void prepareEnvironment() { TestData.createDefault(); TestData.createNotAdminUser("user", "user"); - TestData.createUserPermission("user", WRITE, "git"); + TestData.createUserPermission("user", WRITE, repositoryType); } @After @@ -68,7 +84,7 @@ public class ApiKeyITCase { public void shouldCloneWithRestrictedApiKey() throws IOException { String passphrase = registerApiKey(); - RepositoryClient client = RepositoryUtil.createRepositoryClient("git", temporaryFolder.newFolder(), "user", passphrase); + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), "user", passphrase); assertEquals(1, Objects.requireNonNull(client.getWorkingCopy().list()).length); } @@ -77,7 +93,7 @@ public class ApiKeyITCase { public void shouldFailToCommit() throws IOException { String passphrase = registerApiKey(); - RepositoryClient client = RepositoryUtil.createRepositoryClient("git", temporaryFolder.newFolder(), "user", passphrase); + RepositoryClient client = RepositoryUtil.createRepositoryClient(repositoryType, temporaryFolder.newFolder(), "user", passphrase); assertThrows(RepositoryClientException.class, () -> addAndCommitRandomFile(client, "user")); } From 4144e295994a09ae211a4758e965b8d09a712943 Mon Sep 17 00:00:00 2001 From: Konstantin Schaper Date: Thu, 5 Nov 2020 11:27:11 +0100 Subject: [PATCH 8/8] fix scope check --- .../src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java index 257aa10467..2455a01eba 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/security/JwtAccessTokenBuilder.java @@ -153,7 +153,7 @@ public final class JwtAccessTokenBuilder implements AccessTokenBuilder { @Override public JwtAccessToken build() { final Scope principalScope = SecurityUtils.getSubject().getPrincipals().oneByType(Scope.class); - if (principalScope != null) { + if (principalScope != null && !principalScope.isEmpty()) { if (scope != null && !scope.isEmpty()) { throw new AuthorizationException(String.format("cannot merge builder scope (%s) with principal scope (%s)", scope, principalScope)); }