Make auth expiration for LFS configurable (#1697)

When SCM-Manager is used behind a reverse proxy like
Nginx it may be the case, that lfs PUT requests are
buffered by the reverse proxy and will be sent to the
SCM-Manager after the whole file has been received. Due
to the expiration time of 5 minutes for the authentivation
token that had been requested by Git before the upload
has been started, this request from the proxy to
SCM-Manager fails if the upload from the client to the
reverse proxy took longer than these 5 minutes.

To solve this, we make this expiration time configurable,
so that whenever you have very large files or small
bandwidth the expiration timeout can be increased.
This commit is contained in:
René Pfeuffer
2021-06-16 09:14:52 +02:00
committed by GitHub
parent 97b32f3918
commit b6d343bf09
15 changed files with 325 additions and 152 deletions

View File

@@ -48,6 +48,7 @@ import sonia.scm.repository.RepositoryManager;
import sonia.scm.store.ConfigurationStore;
import sonia.scm.store.ConfigurationStoreFactory;
import sonia.scm.web.GitVndMediaType;
import sonia.scm.web.JsonMockHttpRequest;
import sonia.scm.web.RestDispatcher;
import javax.servlet.http.HttpServletResponse;
@@ -127,10 +128,12 @@ public class GitConfigResourceTest {
String responseString = response.getContentAsString();
assertTrue(responseString.contains("\"disabled\":false"));
assertTrue(responseString.contains("\"gcExpression\":\"valid Git GC Cron Expression\""));
assertTrue(responseString.contains("\"self\":{\"href\":\"/v2/config/git"));
assertTrue(responseString.contains("\"update\":{\"href\":\"/v2/config/git"));
assertThat(responseString)
.contains("\"disabled\":false")
.contains("\"gcExpression\":\"valid Git GC Cron Expression\"")
.contains("\"self\":{\"href\":\"/v2/config/git")
.contains("\"update\":{\"href\":\"/v2/config/git")
.contains("\"lfsWriteAuthorizationExpirationInMinutes\":5");
}
@Test
@@ -324,9 +327,9 @@ public class GitConfigResourceTest {
}
private MockHttpResponse put() throws URISyntaxException {
MockHttpRequest request = MockHttpRequest.put("/" + GitConfigResource.GIT_CONFIG_PATH_V2)
JsonMockHttpRequest request = JsonMockHttpRequest.put("/" + GitConfigResource.GIT_CONFIG_PATH_V2)
.contentType(GitVndMediaType.GIT_CONFIG)
.content("{\"disabled\":true, \"defaultBranch\":\"main\"}".getBytes());
.json("{'disabled':true, 'defaultBranch':'main', 'lfsWriteAuthorizationExpirationInMinutes':5}");
MockHttpResponse response = new MockHttpResponse();
dispatcher.invoke(request, response);
@@ -337,6 +340,7 @@ public class GitConfigResourceTest {
GitConfig config = new GitConfig();
config.setGcExpression("valid Git GC Cron Expression");
config.setDisabled(false);
config.setLfsWriteAuthorizationExpirationInMinutes(5);
return config;
}
}

View File

@@ -0,0 +1,188 @@
/*
* 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.web.lfs;
import org.apache.shiro.authz.UnauthorizedException;
import org.github.sdorra.jse.ShiroExtension;
import org.github.sdorra.jse.SubjectAware;
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.mockito.Answers;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import sonia.scm.repository.GitConfig;
import sonia.scm.repository.GitRepositoryHandler;
import sonia.scm.repository.Repository;
import sonia.scm.repository.RepositoryTestData;
import sonia.scm.security.AccessToken;
import sonia.scm.security.AccessTokenBuilder;
import sonia.scm.security.AccessTokenBuilderFactory;
import java.util.concurrent.TimeUnit;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ExtendWith(MockitoExtension.class)
@ExtendWith(ShiroExtension.class)
class LfsAccessTokenFactoryTest {
@Mock
private AccessTokenBuilderFactory tokenBuilderFactory;
@Mock(answer = Answers.RETURNS_SELF)
private AccessTokenBuilder tokenBuilder;
@Mock
private GitRepositoryHandler handler;
@InjectMocks
private LfsAccessTokenFactory factory;
@Mock
private AccessToken createdTokenFromMock;
private Repository repository = RepositoryTestData.createHeartOfGold();
@BeforeEach
void initRepository() {
repository.setId("42");
}
@Nested
class WithPermissions {
@BeforeEach
void initTokenBuilder() {
when(tokenBuilderFactory.create()).thenReturn(tokenBuilder);
when(tokenBuilder.build()).thenReturn(createdTokenFromMock);
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read,pull:42"
)
void shouldCreateReadToken() {
AccessToken readAccessToken = factory.createReadAccessToken(repository);
assertThat(readAccessToken).isSameAs(createdTokenFromMock);
verify(tokenBuilder).expiresIn(5, TimeUnit.MINUTES);
verify(tokenBuilder).scope(argThat(scope -> {
assertThat(scope.iterator()).toIterable()
.containsExactly("repository:read:42", "repository:pull:42");
return true;
}));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read,pull,push:42"
)
void shouldCreateReadTokenWithPushIfPermitted() {
AccessToken readAccessToken = factory.createReadAccessToken(repository);
assertThat(readAccessToken).isSameAs(createdTokenFromMock);
verify(tokenBuilder).expiresIn(5, TimeUnit.MINUTES);
verify(tokenBuilder).scope(argThat(scope -> {
assertThat(scope.iterator()).toIterable()
.containsExactly("repository:read:42", "repository:pull:42", "repository:push:42");
return true;
}));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read,pull,push:42"
)
void shouldCreateWriteToken() {
GitConfig config = new GitConfig();
config.setLfsWriteAuthorizationExpirationInMinutes(23);
when(handler.getConfig()).thenReturn(config);
AccessToken writeAccessToken = factory.createWriteAccessToken(repository);
assertThat(writeAccessToken).isSameAs(createdTokenFromMock);
verify(tokenBuilder).expiresIn(23, TimeUnit.MINUTES);
verify(tokenBuilder).scope(argThat(scope -> {
assertThat(scope.iterator()).toIterable()
.containsExactly("repository:read:42", "repository:pull:42", "repository:push:42");
return true;
}));
}
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read:42"
)
void shouldFailToCreateReadTokenWithoutPullPermission() {
assertThrows(UnauthorizedException.class, () -> factory.createReadAccessToken(repository));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:pull:42"
)
void shouldFailToCreateReadTokenWithoutReadPermission() {
assertThrows(UnauthorizedException.class, () -> factory.createReadAccessToken(repository));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:pull,push:42"
)
void shouldFailToCreateWriteTokenWithoutReadPermission() {
assertThrows(UnauthorizedException.class, () -> factory.createWriteAccessToken(repository));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read,push:42"
)
void shouldFailToCreateWriteTokenWithoutPullPermission() {
assertThrows(UnauthorizedException.class, () -> factory.createWriteAccessToken(repository));
}
@Test
@SubjectAware(
value = "trillian",
permissions = "repository:read,pull:42"
)
void shouldFailToCreateWriteTokenWithoutPushPermission() {
assertThrows(UnauthorizedException.class, () -> factory.createWriteAccessToken(repository));
}
}