From 9f48875d57e72d2c1fc94eb4aa536c1b0bc2e6f4 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 17 Oct 2019 09:23:37 +0200 Subject: [PATCH 01/18] POC --- scm-plugins/scm-git-plugin/pom.xml | 6 ++ .../main/java/sonia/scm/LFSAuthCommand.java | 101 ++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java diff --git a/scm-plugins/scm-git-plugin/pom.xml b/scm-plugins/scm-git-plugin/pom.xml index 0456370c99..0cace5e94d 100644 --- a/scm-plugins/scm-git-plugin/pom.xml +++ b/scm-plugins/scm-git-plugin/pom.xml @@ -40,6 +40,12 @@ 2.6 + + org.jboss.resteasy + resteasy-jackson2-provider + ${resteasy.version} + + diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java new file mode 100644 index 0000000000..0864b84dfd --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java @@ -0,0 +1,101 @@ +package sonia.scm; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.module.jaxb.JaxbAnnotationModule; +import sonia.scm.plugin.Extension; +import sonia.scm.protocolcommand.CommandInterpreter; +import sonia.scm.protocolcommand.CommandInterpreterFactory; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.RepositoryContextResolver; +import sonia.scm.protocolcommand.ScmCommandProtocol; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenBuilderFactory; + +import javax.inject.Inject; +import javax.xml.bind.annotation.XmlElement; +import java.io.ByteArrayOutputStream; +import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.Date; +import java.util.Optional; +import java.util.concurrent.TimeUnit; + +@Extension +public class LFSAuthCommand implements CommandInterpreterFactory { + + private final AccessTokenBuilderFactory tokenBuilderFactory; + + @Inject + public LFSAuthCommand(AccessTokenBuilderFactory tokenBuilderFactory) { + this.tokenBuilderFactory = tokenBuilderFactory; + } + + @Override + public Optional canHandle(String command) { + return command.startsWith("git-lfs-authenticate") ? Optional.of(new CommandInterpreter() { + @Override + public String[] getParsedArgs() { + return new String[0]; + } + + @Override + public ScmCommandProtocol getProtocolHandler() { + return (context, repositoryContext) -> { + AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + + ObjectMapper objectMapper = new ObjectMapper(); + objectMapper.registerModule(new JaxbAnnotationModule()); + objectMapper.setDateFormat(new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'")); + LfsAuthResponse response = new LfsAuthResponse("http://localhost:8081/scm/repo/scmadmin/lfs.git/info/lfs/", new LfsAuthHeader(accessToken.compact()), Instant.now().plus(5, ChronoUnit.MINUTES)); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + objectMapper.writeValue(buffer, response); + context.getOutputStream().write(buffer.toString().getBytes()); + }; + } + + @Override + public RepositoryContextResolver getRepositoryContextResolver() { + return args -> new RepositoryContext(null, null); + } + }) : Optional.empty(); + } + + private class LfsAuthResponse { + private final String href; + private final LfsAuthHeader header; + @XmlElement(name = "expires_at") + private final Date expiresAt; + + public LfsAuthResponse(String href, LfsAuthHeader header, Instant expiresAt) { + this.href = href; + this.header = header; + this.expiresAt = Date.from(expiresAt); + } + + public String getHref() { + return href; + } + + public LfsAuthHeader getHeader() { + return header; + } + + public Date getExpiresAt() { + return expiresAt; + } + } + + private class LfsAuthHeader { + @XmlElement(name = "Authorization") + private final String authorization; + + public LfsAuthHeader(String authorization) { + this.authorization = authorization; + } + + public String getAuthorization() { + return "Bearer " + authorization; + } + } +} From 6cdb81d263fa77e2751312d3373efedd33c43cfb Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 18 Oct 2019 10:22:45 +0200 Subject: [PATCH 02/18] POC --- .../main/java/sonia/scm/LFSAuthCommand.java | 21 ++++++++++--- .../scm/web/lfs/ScmBlobLfsRepository.java | 31 ++++++++++++++++--- .../web/lfs/servlet/LfsServletFactory.java | 7 +++-- 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java index 0864b84dfd..17a05ebe60 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java @@ -2,12 +2,14 @@ package sonia.scm; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.module.jaxb.JaxbAnnotationModule; +import sonia.scm.config.ScmConfiguration; import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandInterpreter; import sonia.scm.protocolcommand.CommandInterpreterFactory; -import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.protocolcommand.RepositoryContextResolver; import sonia.scm.protocolcommand.ScmCommandProtocol; +import sonia.scm.protocolcommand.git.GitRepositoryContextResolver; +import sonia.scm.repository.Repository; import sonia.scm.security.AccessToken; import sonia.scm.security.AccessTokenBuilderFactory; @@ -21,14 +23,20 @@ import java.util.Date; import java.util.Optional; import java.util.concurrent.TimeUnit; +import static java.lang.String.format; + @Extension public class LFSAuthCommand implements CommandInterpreterFactory { private final AccessTokenBuilderFactory tokenBuilderFactory; + private final GitRepositoryContextResolver gitRepositoryContextResolver; + private final ScmConfiguration configuration; @Inject - public LFSAuthCommand(AccessTokenBuilderFactory tokenBuilderFactory) { + public LFSAuthCommand(AccessTokenBuilderFactory tokenBuilderFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) { this.tokenBuilderFactory = tokenBuilderFactory; + this.gitRepositoryContextResolver = gitRepositoryContextResolver; + this.configuration = configuration; } @Override @@ -36,7 +44,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { return command.startsWith("git-lfs-authenticate") ? Optional.of(new CommandInterpreter() { @Override public String[] getParsedArgs() { - return new String[0]; + return new String[] {command.split("\\s+")[1]}; } @Override @@ -44,10 +52,13 @@ public class LFSAuthCommand implements CommandInterpreterFactory { return (context, repositoryContext) -> { AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + Repository repository = repositoryContext.getRepository(); + String url = format("%s/repo/%s/%s.git/info/lfs/", configuration.getBaseUrl(), repository.getNamespace(), repository.getName()); + ObjectMapper objectMapper = new ObjectMapper(); objectMapper.registerModule(new JaxbAnnotationModule()); objectMapper.setDateFormat(new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'")); - LfsAuthResponse response = new LfsAuthResponse("http://localhost:8081/scm/repo/scmadmin/lfs.git/info/lfs/", new LfsAuthHeader(accessToken.compact()), Instant.now().plus(5, ChronoUnit.MINUTES)); + LfsAuthResponse response = new LfsAuthResponse(url, new LfsAuthHeader(accessToken.compact()), Instant.now().plus(5, ChronoUnit.MINUTES)); ByteArrayOutputStream buffer = new ByteArrayOutputStream(); objectMapper.writeValue(buffer, response); context.getOutputStream().write(buffer.toString().getBytes()); @@ -56,7 +67,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { @Override public RepositoryContextResolver getRepositoryContextResolver() { - return args -> new RepositoryContext(null, null); + return gitRepositoryContextResolver; } }) : Optional.empty(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index 46a58f6f07..06574ddd8d 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -1,12 +1,21 @@ package sonia.scm.web.lfs; +import org.eclipse.jgit.lfs.Protocol; import org.eclipse.jgit.lfs.lib.AnyLongObjectId; import org.eclipse.jgit.lfs.server.LargeFileRepository; import org.eclipse.jgit.lfs.server.Response; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenBuilderFactory; import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; import java.io.IOException; +import java.sql.Date; +import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.HashMap; +import java.util.concurrent.TimeUnit; /** * This LargeFileRepository is used for jGit-Servlet implementation. Under the jgit LFS Servlet hood, the @@ -18,6 +27,7 @@ import java.io.IOException; public class ScmBlobLfsRepository implements LargeFileRepository { private final BlobStore blobStore; + private final AccessTokenBuilderFactory tokenBuilderFactory; /** * This URI is used to determine the actual URI for Upload / Download. Must be full URI (or rewritable by reverse @@ -28,14 +38,15 @@ public class ScmBlobLfsRepository implements LargeFileRepository { /** * Creates a {@link ScmBlobLfsRepository} for the provided repository. * - * @param blobStore The SCM Blobstore used for this @{@link LargeFileRepository}. - * @param baseUri This URI is used to determine the actual URI for Upload / Download. Must be full URI (or - * rewritable by reverse proxy). + * @param blobStore The SCM Blobstore used for this @{@link LargeFileRepository}. + * @param tokenBuilderFactory + * @param baseUri This URI is used to determine the actual URI for Upload / Download. Must be full URI (or */ - public ScmBlobLfsRepository(BlobStore blobStore, String baseUri) { + public ScmBlobLfsRepository(BlobStore blobStore, AccessTokenBuilderFactory tokenBuilderFactory, String baseUri) { this.blobStore = blobStore; + this.tokenBuilderFactory = tokenBuilderFactory; this.baseUri = baseUri; } @@ -82,9 +93,19 @@ public class ScmBlobLfsRepository implements LargeFileRepository { //LFS protocol has to provide the information on where to put or get the actual content, i. e. //the actual URI for up- and download. - Response.Action a = new Response.Action(); + ExpiringAction a = new ExpiringAction(); a.href = baseUri + id.getName(); + AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + a.header = new HashMap<>(); + a.header.put("Authorization", "Bearer " + accessToken.compact()); + Instant expire = Instant.now().plus(5, ChronoUnit.MINUTES); + a.expires_at = new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'").format(Date.from(expire)); + return a; } + + private static class ExpiringAction extends Response.Action { + public String expires_at; + } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java index f4eed34678..f174f6e8aa 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java @@ -7,6 +7,7 @@ import org.eclipse.jgit.lfs.server.fs.FileLfsServlet; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.repository.Repository; +import sonia.scm.security.AccessTokenBuilderFactory; import sonia.scm.store.BlobStore; import sonia.scm.util.HttpUtil; import sonia.scm.web.lfs.LfsBlobStoreFactory; @@ -30,10 +31,12 @@ public class LfsServletFactory { private static final Logger logger = LoggerFactory.getLogger(LfsServletFactory.class); private final LfsBlobStoreFactory lfsBlobStoreFactory; + private final AccessTokenBuilderFactory tokenBuilderFactory; @Inject - public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory) { + public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory, AccessTokenBuilderFactory tokenBuilderFactory) { this.lfsBlobStoreFactory = lfsBlobStoreFactory; + this.tokenBuilderFactory = tokenBuilderFactory; } /** @@ -47,7 +50,7 @@ public class LfsServletFactory { BlobStore blobStore = lfsBlobStoreFactory.getLfsBlobStore(repository); String baseUri = buildBaseUri(repository, request); - LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(blobStore, baseUri); + LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(blobStore, tokenBuilderFactory, baseUri); return new ScmLfsProtocolServlet(largeFileRepository); } From 162a1d9e0504257eebab7ca426aa2c41dcab89f2 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 18 Oct 2019 13:33:53 +0200 Subject: [PATCH 03/18] POC --- .../scm/web/lfs/ScmBlobLfsRepository.java | 23 +++++++++++++------ .../web/lfs/servlet/LfsServletFactory.java | 2 +- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index 06574ddd8d..088fb2b5d4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -1,11 +1,13 @@ package sonia.scm.web.lfs; -import org.eclipse.jgit.lfs.Protocol; import org.eclipse.jgit.lfs.lib.AnyLongObjectId; import org.eclipse.jgit.lfs.server.LargeFileRepository; import org.eclipse.jgit.lfs.server.Response; +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; import sonia.scm.security.AccessToken; import sonia.scm.security.AccessTokenBuilderFactory; +import sonia.scm.security.Scope; import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; @@ -34,17 +36,19 @@ public class ScmBlobLfsRepository implements LargeFileRepository { * proxy). */ private final String baseUri; + private final Repository repository; /** * Creates a {@link ScmBlobLfsRepository} for the provided repository. * + * @param repository * @param blobStore The SCM Blobstore used for this @{@link LargeFileRepository}. * @param tokenBuilderFactory * @param baseUri This URI is used to determine the actual URI for Upload / Download. Must be full URI (or */ - public ScmBlobLfsRepository(BlobStore blobStore, AccessTokenBuilderFactory tokenBuilderFactory, String baseUri) { - + public ScmBlobLfsRepository(Repository repository, BlobStore blobStore, AccessTokenBuilderFactory tokenBuilderFactory, String baseUri) { + this.repository = repository; this.blobStore = blobStore; this.tokenBuilderFactory = tokenBuilderFactory; this.baseUri = baseUri; @@ -53,13 +57,13 @@ public class ScmBlobLfsRepository implements LargeFileRepository { @Override public Response.Action getDownloadAction(AnyLongObjectId id) { - return getAction(id); + return getAction(id, Scope.valueOf(RepositoryPermissions.read(repository).asShiroString(), RepositoryPermissions.pull(repository).asShiroString())); } @Override public Response.Action getUploadAction(AnyLongObjectId id, long size) { - return getAction(id); + return getAction(id, Scope.valueOf(RepositoryPermissions.read(repository).asShiroString(), RepositoryPermissions.pull(repository).asShiroString(), RepositoryPermissions.push(repository).asShiroString())); } @Override @@ -88,7 +92,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { /** * Constructs the Download / Upload actions to be supplied to the client. */ - private Response.Action getAction(AnyLongObjectId id) { + private Response.Action getAction(AnyLongObjectId id, Scope scope) { //LFS protocol has to provide the information on where to put or get the actual content, i. e. //the actual URI for up- and download. @@ -96,7 +100,12 @@ public class ScmBlobLfsRepository implements LargeFileRepository { ExpiringAction a = new ExpiringAction(); a.href = baseUri + id.getName(); - AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + AccessToken accessToken = + tokenBuilderFactory + .create() + .expiresIn(5, TimeUnit.MINUTES) + .scope(scope) + .build(); a.header = new HashMap<>(); a.header.put("Authorization", "Bearer " + accessToken.compact()); Instant expire = Instant.now().plus(5, ChronoUnit.MINUTES); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java index f174f6e8aa..bfc8a12ba8 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java @@ -50,7 +50,7 @@ public class LfsServletFactory { BlobStore blobStore = lfsBlobStoreFactory.getLfsBlobStore(repository); String baseUri = buildBaseUri(repository, request); - LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(blobStore, tokenBuilderFactory, baseUri); + LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(repository, blobStore, tokenBuilderFactory, baseUri); return new ScmLfsProtocolServlet(largeFileRepository); } From a2c42060e523b56976d4fa52b69adae33fe9a9b7 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 18 Oct 2019 16:17:22 +0200 Subject: [PATCH 04/18] POC --- .../src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index 088fb2b5d4..a67953ee11 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -114,6 +114,8 @@ public class ScmBlobLfsRepository implements LargeFileRepository { return a; } + @SuppressWarnings({"squid:ClassVariableVisibilityCheck", "squid:S00116"}) + // This class is used for json serialization, only private static class ExpiringAction extends Response.Action { public String expires_at; } From d43080a3eafc42bf499ecde241351a4e11705f51 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 13:24:58 +0200 Subject: [PATCH 05/18] Extract common functionality --- .../sonia/scm/web/lfs/ExpiringAction.java | 19 +++++ .../scm/{ => web/lfs}/LFSAuthCommand.java | 74 +++++-------------- .../scm/web/lfs/ScmBlobLfsRepository.java | 33 +++------ 3 files changed, 48 insertions(+), 78 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java rename scm-plugins/scm-git-plugin/src/main/java/sonia/scm/{ => web/lfs}/LFSAuthCommand.java (52%) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java new file mode 100644 index 0000000000..b68640f24b --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java @@ -0,0 +1,19 @@ +package sonia.scm.web.lfs; + +import org.eclipse.jgit.lfs.server.Response; +import sonia.scm.security.AccessToken; + +import java.text.SimpleDateFormat; +import java.util.Collections; + +@SuppressWarnings({"squid:S00116"}) +// This class is used for json serialization, only +class ExpiringAction extends Response.Action { + public final String expires_at; + + ExpiringAction(String href, AccessToken accessToken) { + this.expires_at = new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'").format(accessToken.getExpiration()); + this.href = href; + this.header = Collections.singletonMap("Authorization", "Bearer " + accessToken.compact()); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java similarity index 52% rename from scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java rename to scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 17a05ebe60..9bf3d3a4f9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -1,11 +1,11 @@ -package sonia.scm; +package sonia.scm.web.lfs; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.module.jaxb.JaxbAnnotationModule; import sonia.scm.config.ScmConfiguration; import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandInterpreter; import sonia.scm.protocolcommand.CommandInterpreterFactory; +import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.protocolcommand.RepositoryContextResolver; import sonia.scm.protocolcommand.ScmCommandProtocol; import sonia.scm.protocolcommand.git.GitRepositoryContextResolver; @@ -14,12 +14,7 @@ import sonia.scm.security.AccessToken; import sonia.scm.security.AccessTokenBuilderFactory; import javax.inject.Inject; -import javax.xml.bind.annotation.XmlElement; import java.io.ByteArrayOutputStream; -import java.text.SimpleDateFormat; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.Date; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -30,13 +25,16 @@ public class LFSAuthCommand implements CommandInterpreterFactory { private final AccessTokenBuilderFactory tokenBuilderFactory; private final GitRepositoryContextResolver gitRepositoryContextResolver; - private final ScmConfiguration configuration; + private final ObjectMapper objectMapper; + private final String baseUrl; @Inject public LFSAuthCommand(AccessTokenBuilderFactory tokenBuilderFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) { this.tokenBuilderFactory = tokenBuilderFactory; this.gitRepositoryContextResolver = gitRepositoryContextResolver; - this.configuration = configuration; + + objectMapper = new ObjectMapper(); + baseUrl = configuration.getBaseUrl(); } @Override @@ -44,69 +42,33 @@ public class LFSAuthCommand implements CommandInterpreterFactory { return command.startsWith("git-lfs-authenticate") ? Optional.of(new CommandInterpreter() { @Override public String[] getParsedArgs() { + // we are interested only in the 'repo' argument, so we discard the rest return new String[] {command.split("\\s+")[1]}; } @Override public ScmCommandProtocol getProtocolHandler() { return (context, repositoryContext) -> { - AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); - - Repository repository = repositoryContext.getRepository(); - String url = format("%s/repo/%s/%s.git/info/lfs/", configuration.getBaseUrl(), repository.getNamespace(), repository.getName()); - - ObjectMapper objectMapper = new ObjectMapper(); - objectMapper.registerModule(new JaxbAnnotationModule()); - objectMapper.setDateFormat(new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'")); - LfsAuthResponse response = new LfsAuthResponse(url, new LfsAuthHeader(accessToken.compact()), Instant.now().plus(5, ChronoUnit.MINUTES)); + ExpiringAction response = createResponse(repositoryContext); ByteArrayOutputStream buffer = new ByteArrayOutputStream(); objectMapper.writeValue(buffer, response); context.getOutputStream().write(buffer.toString().getBytes()); }; } + private ExpiringAction createResponse(RepositoryContext repositoryContext) { + AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + + Repository repository = repositoryContext.getRepository(); + String url = format("%s/repo/%s/%s.git/info/lfs/", baseUrl, repository.getNamespace(), repository.getName()); + + return new ExpiringAction(url, accessToken); + } + @Override public RepositoryContextResolver getRepositoryContextResolver() { return gitRepositoryContextResolver; } }) : Optional.empty(); } - - private class LfsAuthResponse { - private final String href; - private final LfsAuthHeader header; - @XmlElement(name = "expires_at") - private final Date expiresAt; - - public LfsAuthResponse(String href, LfsAuthHeader header, Instant expiresAt) { - this.href = href; - this.header = header; - this.expiresAt = Date.from(expiresAt); - } - - public String getHref() { - return href; - } - - public LfsAuthHeader getHeader() { - return header; - } - - public Date getExpiresAt() { - return expiresAt; - } - } - - private class LfsAuthHeader { - @XmlElement(name = "Authorization") - private final String authorization; - - public LfsAuthHeader(String authorization) { - this.authorization = authorization; - } - - public String getAuthorization() { - return "Bearer " + authorization; - } - } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index a67953ee11..ee1fb15d94 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -12,11 +12,6 @@ import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; import java.io.IOException; -import java.sql.Date; -import java.text.SimpleDateFormat; -import java.time.Instant; -import java.time.temporal.ChronoUnit; -import java.util.HashMap; import java.util.concurrent.TimeUnit; /** @@ -38,12 +33,14 @@ public class ScmBlobLfsRepository implements LargeFileRepository { private final String baseUri; private final Repository repository; + private AccessToken accessToken; + /** * Creates a {@link ScmBlobLfsRepository} for the provided repository. * - * @param repository + * @param repository The current scm repository this LFS repository is used for. * @param blobStore The SCM Blobstore used for this @{@link LargeFileRepository}. - * @param tokenBuilderFactory + * @param tokenBuilderFactory The token builder used to create short lived access tokens. * @param baseUri This URI is used to determine the actual URI for Upload / Download. Must be full URI (or */ @@ -97,26 +94,18 @@ public class ScmBlobLfsRepository implements LargeFileRepository { //LFS protocol has to provide the information on where to put or get the actual content, i. e. //the actual URI for up- and download. - ExpiringAction a = new ExpiringAction(); - a.href = baseUri + id.getName(); + return new ExpiringAction(baseUri + id.getName(), getAccessToken(scope)); + } - AccessToken accessToken = - tokenBuilderFactory + private AccessToken getAccessToken(Scope scope) { + if (accessToken == null) { + accessToken = tokenBuilderFactory .create() .expiresIn(5, TimeUnit.MINUTES) .scope(scope) .build(); - a.header = new HashMap<>(); - a.header.put("Authorization", "Bearer " + accessToken.compact()); - Instant expire = Instant.now().plus(5, ChronoUnit.MINUTES); - a.expires_at = new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'").format(Date.from(expire)); - - return a; + } + return accessToken; } - @SuppressWarnings({"squid:ClassVariableVisibilityCheck", "squid:S00116"}) - // This class is used for json serialization, only - private static class ExpiringAction extends Response.Action { - public String expires_at; - } } From c067ce1a5c9d77b523ffd2fd2b018fa7a30997f1 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 13:47:59 +0200 Subject: [PATCH 06/18] Extract token generation --- .../sonia/scm/web/lfs/LFSAuthCommand.java | 73 +++++++++++-------- .../scm/web/lfs/LfsAccessTokenFactory.java | 43 +++++++++++ .../scm/web/lfs/ScmBlobLfsRepository.java | 44 +++++------ .../web/lfs/servlet/LfsServletFactory.java | 14 ++-- 4 files changed, 108 insertions(+), 66 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 9bf3d3a4f9..860c7495e8 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -11,26 +11,24 @@ import sonia.scm.protocolcommand.ScmCommandProtocol; import sonia.scm.protocolcommand.git.GitRepositoryContextResolver; import sonia.scm.repository.Repository; import sonia.scm.security.AccessToken; -import sonia.scm.security.AccessTokenBuilderFactory; import javax.inject.Inject; import java.io.ByteArrayOutputStream; import java.util.Optional; -import java.util.concurrent.TimeUnit; import static java.lang.String.format; @Extension public class LFSAuthCommand implements CommandInterpreterFactory { - private final AccessTokenBuilderFactory tokenBuilderFactory; + private final LfsAccessTokenFactory tokenFactory; private final GitRepositoryContextResolver gitRepositoryContextResolver; private final ObjectMapper objectMapper; private final String baseUrl; @Inject - public LFSAuthCommand(AccessTokenBuilderFactory tokenBuilderFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) { - this.tokenBuilderFactory = tokenBuilderFactory; + public LFSAuthCommand(LfsAccessTokenFactory tokenFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) { + this.tokenFactory = tokenFactory; this.gitRepositoryContextResolver = gitRepositoryContextResolver; objectMapper = new ObjectMapper(); @@ -39,36 +37,49 @@ public class LFSAuthCommand implements CommandInterpreterFactory { @Override public Optional canHandle(String command) { - return command.startsWith("git-lfs-authenticate") ? Optional.of(new CommandInterpreter() { - @Override - public String[] getParsedArgs() { - // we are interested only in the 'repo' argument, so we discard the rest - return new String[] {command.split("\\s+")[1]}; - } + if (command.startsWith("git-lfs-authenticate")) { + return Optional.of(new LfsAuthCommandInterpreter(command)); + } else { + return Optional.empty(); + } + } - @Override - public ScmCommandProtocol getProtocolHandler() { - return (context, repositoryContext) -> { - ExpiringAction response = createResponse(repositoryContext); - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - objectMapper.writeValue(buffer, response); - context.getOutputStream().write(buffer.toString().getBytes()); - }; - } + private class LfsAuthCommandInterpreter implements CommandInterpreter { - private ExpiringAction createResponse(RepositoryContext repositoryContext) { - AccessToken accessToken = tokenBuilderFactory.create().expiresIn(5, TimeUnit.MINUTES).build(); + private final String command; - Repository repository = repositoryContext.getRepository(); - String url = format("%s/repo/%s/%s.git/info/lfs/", baseUrl, repository.getNamespace(), repository.getName()); + public LfsAuthCommandInterpreter(String command) { + this.command = command; + } - return new ExpiringAction(url, accessToken); - } + @Override + public String[] getParsedArgs() { + // we are interested only in the 'repo' argument, so we discard the rest + return new String[]{command.split("\\s+")[1]}; + } - @Override - public RepositoryContextResolver getRepositoryContextResolver() { - return gitRepositoryContextResolver; - } - }) : Optional.empty(); + @Override + public ScmCommandProtocol getProtocolHandler() { + return (context, repositoryContext) -> { + ExpiringAction response = createResponse(repositoryContext); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + objectMapper.writeValue(buffer, response); + context.getOutputStream().write(buffer.toString().getBytes()); + }; + } + + private ExpiringAction createResponse(RepositoryContext repositoryContext) { + Repository repository = repositoryContext.getRepository(); + + String url = format("%s/repo/%s/%s.git/info/lfs/", baseUrl, repository.getNamespace(), repository.getName()); + AccessToken accessToken = tokenFactory.getReadAccessToken(repository); + + return new ExpiringAction(url, accessToken); + } + + @Override + public RepositoryContextResolver getRepositoryContextResolver() { + return gitRepositoryContextResolver; + } } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java new file mode 100644 index 0000000000..d7c7b70234 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java @@ -0,0 +1,43 @@ +package sonia.scm.web.lfs; + +import sonia.scm.repository.Repository; +import sonia.scm.repository.RepositoryPermissions; +import sonia.scm.security.AccessToken; +import sonia.scm.security.AccessTokenBuilderFactory; +import sonia.scm.security.Scope; + +import javax.inject.Inject; +import java.util.concurrent.TimeUnit; + +public class LfsAccessTokenFactory { + + private final AccessTokenBuilderFactory tokenBuilderFactory; + + @Inject + LfsAccessTokenFactory(AccessTokenBuilderFactory tokenBuilderFactory) { + this.tokenBuilderFactory = tokenBuilderFactory; + } + + AccessToken getReadAccessToken(Repository repository) { + return createToken( + Scope.valueOf( + RepositoryPermissions.read(repository).asShiroString(), + RepositoryPermissions.pull(repository).asShiroString())); + } + + AccessToken getWriteAccessToken(Repository repository) { + return createToken( + Scope.valueOf( + RepositoryPermissions.read(repository).asShiroString(), + RepositoryPermissions.pull(repository).asShiroString(), + RepositoryPermissions.push(repository).asShiroString())); + } + + private AccessToken createToken(Scope scope) { + return tokenBuilderFactory + .create() + .expiresIn(5, TimeUnit.MINUTES) + .scope(scope) + .build(); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index ee1fb15d94..2ce819e766 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -4,15 +4,11 @@ import org.eclipse.jgit.lfs.lib.AnyLongObjectId; import org.eclipse.jgit.lfs.server.LargeFileRepository; import org.eclipse.jgit.lfs.server.Response; import sonia.scm.repository.Repository; -import sonia.scm.repository.RepositoryPermissions; import sonia.scm.security.AccessToken; -import sonia.scm.security.AccessTokenBuilderFactory; -import sonia.scm.security.Scope; import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; import java.io.IOException; -import java.util.concurrent.TimeUnit; /** * This LargeFileRepository is used for jGit-Servlet implementation. Under the jgit LFS Servlet hood, the @@ -24,7 +20,7 @@ import java.util.concurrent.TimeUnit; public class ScmBlobLfsRepository implements LargeFileRepository { private final BlobStore blobStore; - private final AccessTokenBuilderFactory tokenBuilderFactory; + private final LfsAccessTokenFactory tokenFactory; /** * This URI is used to determine the actual URI for Upload / Download. Must be full URI (or rewritable by reverse @@ -33,6 +29,10 @@ public class ScmBlobLfsRepository implements LargeFileRepository { private final String baseUri; private final Repository repository; + /** + * A {@link ScmBlobLfsRepository} is created for either download or upload, not both. Therefore we can cache the + * access token and do not have to create them anew for each action. + */ private AccessToken accessToken; /** @@ -40,27 +40,31 @@ public class ScmBlobLfsRepository implements LargeFileRepository { * * @param repository The current scm repository this LFS repository is used for. * @param blobStore The SCM Blobstore used for this @{@link LargeFileRepository}. - * @param tokenBuilderFactory The token builder used to create short lived access tokens. + * @param tokenFactory The token builder for subsequent LFS requests. * @param baseUri This URI is used to determine the actual URI for Upload / Download. Must be full URI (or */ - public ScmBlobLfsRepository(Repository repository, BlobStore blobStore, AccessTokenBuilderFactory tokenBuilderFactory, String baseUri) { + public ScmBlobLfsRepository(Repository repository, BlobStore blobStore, LfsAccessTokenFactory tokenFactory, String baseUri) { this.repository = repository; this.blobStore = blobStore; - this.tokenBuilderFactory = tokenBuilderFactory; + this.tokenFactory = tokenFactory; this.baseUri = baseUri; } @Override public Response.Action getDownloadAction(AnyLongObjectId id) { - - return getAction(id, Scope.valueOf(RepositoryPermissions.read(repository).asShiroString(), RepositoryPermissions.pull(repository).asShiroString())); + if (accessToken == null) { + accessToken = tokenFactory.getReadAccessToken(repository); + } + return getAction(id, accessToken); } @Override public Response.Action getUploadAction(AnyLongObjectId id, long size) { - - return getAction(id, Scope.valueOf(RepositoryPermissions.read(repository).asShiroString(), RepositoryPermissions.pull(repository).asShiroString(), RepositoryPermissions.push(repository).asShiroString())); + if (accessToken == null) { + accessToken = tokenFactory.getWriteAccessToken(repository); + } + return getAction(id, accessToken); } @Override @@ -89,23 +93,11 @@ public class ScmBlobLfsRepository implements LargeFileRepository { /** * Constructs the Download / Upload actions to be supplied to the client. */ - private Response.Action getAction(AnyLongObjectId id, Scope scope) { + private Response.Action getAction(AnyLongObjectId id, AccessToken token) { //LFS protocol has to provide the information on where to put or get the actual content, i. e. //the actual URI for up- and download. - return new ExpiringAction(baseUri + id.getName(), getAccessToken(scope)); + return new ExpiringAction(baseUri + id.getName(), token); } - - private AccessToken getAccessToken(Scope scope) { - if (accessToken == null) { - accessToken = tokenBuilderFactory - .create() - .expiresIn(5, TimeUnit.MINUTES) - .scope(scope) - .build(); - } - return accessToken; - } - } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java index bfc8a12ba8..cc79d8e5c4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java @@ -4,12 +4,10 @@ import com.google.common.annotations.VisibleForTesting; import org.eclipse.jgit.lfs.server.LargeFileRepository; import org.eclipse.jgit.lfs.server.LfsProtocolServlet; import org.eclipse.jgit.lfs.server.fs.FileLfsServlet; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import sonia.scm.repository.Repository; -import sonia.scm.security.AccessTokenBuilderFactory; import sonia.scm.store.BlobStore; import sonia.scm.util.HttpUtil; +import sonia.scm.web.lfs.LfsAccessTokenFactory; import sonia.scm.web.lfs.LfsBlobStoreFactory; import sonia.scm.web.lfs.ScmBlobLfsRepository; @@ -28,15 +26,13 @@ import javax.servlet.http.HttpServletRequest; @Singleton public class LfsServletFactory { - private static final Logger logger = LoggerFactory.getLogger(LfsServletFactory.class); - private final LfsBlobStoreFactory lfsBlobStoreFactory; - private final AccessTokenBuilderFactory tokenBuilderFactory; + private final LfsAccessTokenFactory tokenFactory; @Inject - public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory, AccessTokenBuilderFactory tokenBuilderFactory) { + public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory, LfsAccessTokenFactory tokenFactory) { this.lfsBlobStoreFactory = lfsBlobStoreFactory; - this.tokenBuilderFactory = tokenBuilderFactory; + this.tokenFactory = tokenFactory; } /** @@ -50,7 +46,7 @@ public class LfsServletFactory { BlobStore blobStore = lfsBlobStoreFactory.getLfsBlobStore(repository); String baseUri = buildBaseUri(repository, request); - LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(repository, blobStore, tokenBuilderFactory, baseUri); + LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(repository, blobStore, tokenFactory, baseUri); return new ScmLfsProtocolServlet(largeFileRepository); } From 2a998060f4effe1b231e8c659221f7afb3f105dc Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 13:50:49 +0200 Subject: [PATCH 07/18] Cleanup --- .../sonia/scm/web/lfs/LFSAuthCommand.java | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 860c7495e8..59502b800f 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -14,6 +14,7 @@ import sonia.scm.security.AccessToken; import javax.inject.Inject; import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.util.Optional; import static java.lang.String.format; @@ -21,6 +22,8 @@ import static java.lang.String.format; @Extension public class LFSAuthCommand implements CommandInterpreterFactory { + private static final String LFS_INFO_URL_PATTERN = "%s/repo/%s/%s.git/info/lfs/"; + private final LfsAccessTokenFactory tokenFactory; private final GitRepositoryContextResolver gitRepositoryContextResolver; private final ObjectMapper objectMapper; @@ -48,7 +51,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { private final String command; - public LfsAuthCommandInterpreter(String command) { + LfsAuthCommandInterpreter(String command) { this.command = command; } @@ -61,25 +64,30 @@ public class LFSAuthCommand implements CommandInterpreterFactory { @Override public ScmCommandProtocol getProtocolHandler() { return (context, repositoryContext) -> { - ExpiringAction response = createResponse(repositoryContext); - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - objectMapper.writeValue(buffer, response); - context.getOutputStream().write(buffer.toString().getBytes()); + ExpiringAction response = createResponseObject(repositoryContext); + String buffer = serializeResponse(response); + context.getOutputStream().write(buffer.getBytes()); }; } - private ExpiringAction createResponse(RepositoryContext repositoryContext) { - Repository repository = repositoryContext.getRepository(); - - String url = format("%s/repo/%s/%s.git/info/lfs/", baseUrl, repository.getNamespace(), repository.getName()); - AccessToken accessToken = tokenFactory.getReadAccessToken(repository); - - return new ExpiringAction(url, accessToken); - } - @Override public RepositoryContextResolver getRepositoryContextResolver() { return gitRepositoryContextResolver; } + + private ExpiringAction createResponseObject(RepositoryContext repositoryContext) { + Repository repository = repositoryContext.getRepository(); + + String url = format(LFS_INFO_URL_PATTERN, baseUrl, repository.getNamespace(), repository.getName()); + AccessToken accessToken = tokenFactory.getReadAccessToken(repository); + + return new ExpiringAction(url, accessToken); + } + + private String serializeResponse(ExpiringAction response) throws IOException { + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + objectMapper.writeValue(buffer, response); + return buffer.toString(); + } } } From 9f6674cce48180bf10e46ed1787b5b16013feffe Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 13:54:27 +0200 Subject: [PATCH 08/18] Add permission checks --- .../main/java/sonia/scm/web/lfs/LFSAuthCommand.java | 2 +- .../java/sonia/scm/web/lfs/LfsAccessTokenFactory.java | 11 ++++++++--- .../java/sonia/scm/web/lfs/ScmBlobLfsRepository.java | 4 ++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 59502b800f..9f029f28c0 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -79,7 +79,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { Repository repository = repositoryContext.getRepository(); String url = format(LFS_INFO_URL_PATTERN, baseUrl, repository.getNamespace(), repository.getName()); - AccessToken accessToken = tokenFactory.getReadAccessToken(repository); + AccessToken accessToken = tokenFactory.createReadAccessToken(repository); return new ExpiringAction(url, accessToken); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java index d7c7b70234..272d70fd6c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java @@ -18,14 +18,19 @@ public class LfsAccessTokenFactory { this.tokenBuilderFactory = tokenBuilderFactory; } - AccessToken getReadAccessToken(Repository repository) { + AccessToken createReadAccessToken(Repository repository) { + RepositoryPermissions.pull(repository).check(); + RepositoryPermissions.read(repository).check(); return createToken( Scope.valueOf( RepositoryPermissions.read(repository).asShiroString(), RepositoryPermissions.pull(repository).asShiroString())); } - AccessToken getWriteAccessToken(Repository repository) { + AccessToken createWriteAccessToken(Repository repository) { + RepositoryPermissions.read(repository).check(); + RepositoryPermissions.pull(repository).check(); + RepositoryPermissions.push(repository).check(); return createToken( Scope.valueOf( RepositoryPermissions.read(repository).asShiroString(), @@ -36,7 +41,7 @@ public class LfsAccessTokenFactory { private AccessToken createToken(Scope scope) { return tokenBuilderFactory .create() - .expiresIn(5, TimeUnit.MINUTES) + .expiresIn(1, TimeUnit.MINUTES) .scope(scope) .build(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index 2ce819e766..5d606379a1 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -54,7 +54,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { @Override public Response.Action getDownloadAction(AnyLongObjectId id) { if (accessToken == null) { - accessToken = tokenFactory.getReadAccessToken(repository); + accessToken = tokenFactory.createReadAccessToken(repository); } return getAction(id, accessToken); } @@ -62,7 +62,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { @Override public Response.Action getUploadAction(AnyLongObjectId id, long size) { if (accessToken == null) { - accessToken = tokenFactory.getWriteAccessToken(repository); + accessToken = tokenFactory.createWriteAccessToken(repository); } return getAction(id, accessToken); } From f9ecae129ea630d844ddf18d0492decf2b72980a Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 15:55:07 +0200 Subject: [PATCH 09/18] Add unit test --- .../sonia/scm/web/lfs/LFSAuthCommand.java | 6 +- .../sonia/scm/web/lfs/LFSAuthCommandTest.java | 92 +++++++++++++++++++ 2 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 9f029f28c0..11d36dadc2 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -27,7 +27,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { private final LfsAccessTokenFactory tokenFactory; private final GitRepositoryContextResolver gitRepositoryContextResolver; private final ObjectMapper objectMapper; - private final String baseUrl; + private final ScmConfiguration configuration; @Inject public LFSAuthCommand(LfsAccessTokenFactory tokenFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) { @@ -35,7 +35,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { this.gitRepositoryContextResolver = gitRepositoryContextResolver; objectMapper = new ObjectMapper(); - baseUrl = configuration.getBaseUrl(); + this.configuration = configuration; } @Override @@ -78,7 +78,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { private ExpiringAction createResponseObject(RepositoryContext repositoryContext) { Repository repository = repositoryContext.getRepository(); - String url = format(LFS_INFO_URL_PATTERN, baseUrl, repository.getNamespace(), repository.getName()); + String url = format(LFS_INFO_URL_PATTERN, configuration.getBaseUrl(), repository.getNamespace(), repository.getName()); AccessToken accessToken = tokenFactory.createReadAccessToken(repository); return new ExpiringAction(url, accessToken); diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java new file mode 100644 index 0000000000..0627bab822 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java @@ -0,0 +1,92 @@ +package sonia.scm.web.lfs; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.config.ScmConfiguration; +import sonia.scm.protocolcommand.CommandContext; +import sonia.scm.protocolcommand.CommandInterpreter; +import sonia.scm.protocolcommand.RepositoryContext; +import sonia.scm.protocolcommand.git.GitRepositoryContextResolver; +import sonia.scm.repository.Repository; +import sonia.scm.security.AccessToken; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.Date; +import java.util.Optional; + +import static java.time.Instant.parse; +import static java.util.Date.from; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; + +@ExtendWith(MockitoExtension.class) +class LFSAuthCommandTest { + + static final Repository REPOSITORY = new Repository("1", "git", "space", "X"); + static final Date EXPIRATION = from(parse("2007-05-03T10:15:30.00Z")); + + @Mock + LfsAccessTokenFactory tokenFactory; + @Mock + GitRepositoryContextResolver gitRepositoryContextResolver; + @Mock + ScmConfiguration configuration; + + @InjectMocks + LFSAuthCommand lfsAuthCommand; + + @BeforeEach + void initAuthorizationToken() { + AccessToken accessToken = mock(AccessToken.class); + lenient().when(this.tokenFactory.createReadAccessToken(REPOSITORY)).thenReturn(accessToken); + lenient().when(this.tokenFactory.createWriteAccessToken(REPOSITORY)).thenReturn(accessToken); + lenient().when(accessToken.getExpiration()).thenReturn(EXPIRATION); + lenient().when(accessToken.compact()).thenReturn("ACCESS_TOKEN"); + } + + @BeforeEach + void initConfig() { + lenient().when(configuration.getBaseUrl()).thenReturn("http://example.com"); + } + + @Test + void shouldHandleGitLfsAuthenticate() { + Optional commandInterpreter = lfsAuthCommand.canHandle("git-lfs-authenticate repo/space/X upload"); + assertThat(commandInterpreter).isPresent(); + } + + @Test + void shouldNotHandleOtherCommands() { + Optional commandInterpreter = lfsAuthCommand.canHandle("git-lfs-something repo/space/X upload"); + assertThat(commandInterpreter).isEmpty(); + } + + @Test + void shouldExtractRepositoryArgument() { + CommandInterpreter commandInterpreter = lfsAuthCommand.canHandle("git-lfs-authenticate repo/space/X\t upload").get(); + assertThat(commandInterpreter.getParsedArgs()).containsOnly("repo/space/X"); + } + + @Test + void shouldCreateJsonResponse() throws IOException { + CommandInterpreter commandInterpreter = lfsAuthCommand.canHandle("git-lfs-authenticate repo/space/X\t upload").get(); + CommandContext commandContext = createCommandContext(); + commandInterpreter.getProtocolHandler().handle(commandContext, createRepositoryContext()); + assertThat(commandContext.getOutputStream().toString()) + .isEqualTo("{\"href\":\"http://example.com/repo/space/X.git/info/lfs/\",\"header\":{\"Authorization\":\"Bearer ACCESS_TOKEN\"},\"expires_at\":\"2007-05-03T12:05:30Z\"}"); + } + + private CommandContext createCommandContext() { + return new CommandContext(null, null, null, new ByteArrayOutputStream(), null); + } + + private RepositoryContext createRepositoryContext() { + return new RepositoryContext(REPOSITORY, null); + } +} From 2a501a9dbb1ef45ea0fd89fd4fb53cf5babf6a01 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Sat, 19 Oct 2019 16:31:54 +0200 Subject: [PATCH 10/18] Simplify test --- .../lfs/servlet/LfsServletFactoryTest.java | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/servlet/LfsServletFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/servlet/LfsServletFactoryTest.java index 09a431ea43..f386dc2125 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/servlet/LfsServletFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/servlet/LfsServletFactoryTest.java @@ -11,41 +11,26 @@ import static org.junit.Assert.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -/** - * Created by omilke on 18.05.2017. - */ public class LfsServletFactoryTest { + private static final String NAMESPACE = "space"; + private static final String NAME = "git-lfs-demo"; + private static final Repository REPOSITORY = new Repository("", "GIT", NAMESPACE, NAME); + @Test - public void buildBaseUri() { - - String repositoryNamespace = "space"; - String repositoryName = "git-lfs-demo"; - - String result = LfsServletFactory.buildBaseUri(new Repository("", "GIT", repositoryNamespace, repositoryName), RequestWithUri(repositoryName, true)); - assertThat(result, is(equalTo("http://localhost:8081/scm/repo/space/git-lfs-demo.git/info/lfs/objects/"))); - - - //result will be with dot-git suffix, ide - result = LfsServletFactory.buildBaseUri(new Repository("", "GIT", repositoryNamespace, repositoryName), RequestWithUri(repositoryName, false)); + public void shouldBuildBaseUri() { + String result = LfsServletFactory.buildBaseUri(REPOSITORY, requestWithUri("git-lfs-demo")); assertThat(result, is(equalTo("http://localhost:8081/scm/repo/space/git-lfs-demo.git/info/lfs/objects/"))); } - private HttpServletRequest RequestWithUri(String repositoryName, boolean withDotGitSuffix) { + private HttpServletRequest requestWithUri(String repositoryName) { HttpServletRequest mockedRequest = mock(HttpServletRequest.class); - final String suffix; - if (withDotGitSuffix) { - suffix = ".git"; - } else { - suffix = ""; - } - //build from valid live request data when(mockedRequest.getRequestURL()).thenReturn( - new StringBuffer(String.format("http://localhost:8081/scm/repo/%s%s/info/lfs/objects/batch", repositoryName, suffix))); - when(mockedRequest.getRequestURI()).thenReturn(String.format("/scm/repo/%s%s/info/lfs/objects/batch", repositoryName, suffix)); + new StringBuffer(String.format("http://localhost:8081/scm/repo/%s/info/lfs/objects/batch", repositoryName))); + when(mockedRequest.getRequestURI()).thenReturn(String.format("/scm/repo/%s/info/lfs/objects/batch", repositoryName)); when(mockedRequest.getContextPath()).thenReturn("/scm"); return mockedRequest; From 647464618a352087fce41dd7e646f5ebaf9c1c2a Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 21 Oct 2019 10:24:53 +0200 Subject: [PATCH 11/18] Fix time format --- .../main/java/sonia/scm/web/lfs/ExpiringAction.java | 12 +++++++++--- .../java/sonia/scm/web/lfs/LFSAuthCommandTest.java | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java index b68640f24b..28a6ee1016 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java @@ -5,14 +5,20 @@ import sonia.scm.security.AccessToken; import java.text.SimpleDateFormat; import java.util.Collections; +import java.util.TimeZone; -@SuppressWarnings({"squid:S00116"}) -// This class is used for json serialization, only class ExpiringAction extends Response.Action { + private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); + static { + DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("GMT")); + } + + @SuppressWarnings({"squid:S00116"}) + // This class is used for json serialization, only public final String expires_at; ExpiringAction(String href, AccessToken accessToken) { - this.expires_at = new SimpleDateFormat("yyyy-MM-dd'T'HH:MM:ss'Z'").format(accessToken.getExpiration()); + this.expires_at = DATE_FORMAT.format(accessToken.getExpiration()); this.href = href; this.header = Collections.singletonMap("Authorization", "Bearer " + accessToken.compact()); } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java index 0627bab822..1c3d55bc91 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/LFSAuthCommandTest.java @@ -79,7 +79,7 @@ class LFSAuthCommandTest { CommandContext commandContext = createCommandContext(); commandInterpreter.getProtocolHandler().handle(commandContext, createRepositoryContext()); assertThat(commandContext.getOutputStream().toString()) - .isEqualTo("{\"href\":\"http://example.com/repo/space/X.git/info/lfs/\",\"header\":{\"Authorization\":\"Bearer ACCESS_TOKEN\"},\"expires_at\":\"2007-05-03T12:05:30Z\"}"); + .isEqualTo("{\"href\":\"http://example.com/repo/space/X.git/info/lfs/\",\"header\":{\"Authorization\":\"Bearer ACCESS_TOKEN\"},\"expires_at\":\"2007-05-03T10:15:30Z\"}"); } private CommandContext createCommandContext() { From 287ee9efe119aecc18ae228056ee6d2fd426b9f8 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 21 Oct 2019 10:40:16 +0200 Subject: [PATCH 12/18] Simplify stream --- .../src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 11d36dadc2..42dd7226b9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -1,6 +1,7 @@ package sonia.scm.web.lfs; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Charsets; import sonia.scm.config.ScmConfiguration; import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandInterpreter; @@ -13,7 +14,6 @@ import sonia.scm.repository.Repository; import sonia.scm.security.AccessToken; import javax.inject.Inject; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.util.Optional; @@ -66,7 +66,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { return (context, repositoryContext) -> { ExpiringAction response = createResponseObject(repositoryContext); String buffer = serializeResponse(response); - context.getOutputStream().write(buffer.getBytes()); + context.getOutputStream().write(buffer.getBytes(Charsets.UTF_8)); }; } @@ -85,9 +85,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { } private String serializeResponse(ExpiringAction response) throws IOException { - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - objectMapper.writeValue(buffer, response); - return buffer.toString(); + return objectMapper.writeValueAsString(response); } } } From 36d2723c50e7f181c4bf7f1ffe237d66deaeac1e Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 21 Oct 2019 11:11:44 +0200 Subject: [PATCH 13/18] Add unit test --- .../scm/web/lfs/ScmBlobLfsRepository.java | 10 +- .../scm/web/lfs/ScmBlobLfsRepositoryTest.java | 98 +++++++++++++++++++ 2 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/ScmBlobLfsRepositoryTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index 5d606379a1..c646a194b7 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -52,7 +52,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { } @Override - public Response.Action getDownloadAction(AnyLongObjectId id) { + public ExpiringAction getDownloadAction(AnyLongObjectId id) { if (accessToken == null) { accessToken = tokenFactory.createReadAccessToken(repository); } @@ -60,7 +60,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { } @Override - public Response.Action getUploadAction(AnyLongObjectId id, long size) { + public ExpiringAction getUploadAction(AnyLongObjectId id, long size) { if (accessToken == null) { accessToken = tokenFactory.createWriteAccessToken(repository); } @@ -68,14 +68,14 @@ public class ScmBlobLfsRepository implements LargeFileRepository { } @Override - public Response.Action getVerifyAction(AnyLongObjectId id) { + public ExpiringAction getVerifyAction(AnyLongObjectId id) { //validation is optional. We do not support it. return null; } @Override - public long getSize(AnyLongObjectId id) throws IOException { + public long getSize(AnyLongObjectId id) { //this needs to be size of what is will be written into the response of the download. Clients are likely to // verify it. @@ -93,7 +93,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { /** * Constructs the Download / Upload actions to be supplied to the client. */ - private Response.Action getAction(AnyLongObjectId id, AccessToken token) { + private ExpiringAction getAction(AnyLongObjectId id, AccessToken token) { //LFS protocol has to provide the information on where to put or get the actual content, i. e. //the actual URI for up- and download. diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/ScmBlobLfsRepositoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/ScmBlobLfsRepositoryTest.java new file mode 100644 index 0000000000..eefa4314c2 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/web/lfs/ScmBlobLfsRepositoryTest.java @@ -0,0 +1,98 @@ +package sonia.scm.web.lfs; + +import org.eclipse.jgit.lfs.lib.LongObjectId; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.repository.Repository; +import sonia.scm.security.AccessToken; +import sonia.scm.store.BlobStore; + +import java.util.Date; + +import static java.time.Instant.parse; +import static java.util.Date.from; +import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.jgit.lfs.lib.LongObjectId.fromString; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class ScmBlobLfsRepositoryTest { + + static final Repository REPOSITORY = new Repository("1", "git", "space", "X"); + static final Date EXPIRATION = from(parse("2007-05-03T10:15:30.00Z")); + static final LongObjectId OBJECT_ID = fromString("976ed944c37cc5d1606af316937edb9d286ecf6c606af316937edb9d286ecf6c"); + + @Mock + BlobStore blobStore; + @Mock + LfsAccessTokenFactory tokenFactory; + + ScmBlobLfsRepository lfsRepository; + + @BeforeEach + void initializeLfsRepository() { + lfsRepository = new ScmBlobLfsRepository(REPOSITORY, blobStore, tokenFactory, "http://scm.org/"); + } + + @BeforeEach + void initAuthorizationToken() { + AccessToken readToken = createToken("READ_TOKEN"); + lenient().when(this.tokenFactory.createReadAccessToken(REPOSITORY)) + .thenReturn(readToken); + AccessToken writeToken = createToken("WRITE_TOKEN"); + lenient().when(this.tokenFactory.createWriteAccessToken(REPOSITORY)) + .thenReturn(writeToken); + } + + AccessToken createToken(String mockedValue) { + AccessToken accessToken = mock(AccessToken.class); + lenient().when(accessToken.getExpiration()).thenReturn(EXPIRATION); + lenient().when(accessToken.compact()).thenReturn(mockedValue); + return accessToken; + } + + @Test + void shouldTakeExpirationFromToken() { + ExpiringAction downloadAction = lfsRepository.getDownloadAction(OBJECT_ID); + assertThat(downloadAction.expires_at).isEqualTo("2007-05-03T10:15:30Z"); + } + + @Test + void shouldContainReadTokenForDownlo() { + ExpiringAction downloadAction = lfsRepository.getDownloadAction(OBJECT_ID); + assertThat(downloadAction.header.get("Authorization")).isEqualTo("Bearer READ_TOKEN"); + } + + @Test + void shouldContainWriteTokenForUpload() { + ExpiringAction downloadAction = lfsRepository.getUploadAction(OBJECT_ID, 42L); + assertThat(downloadAction.header.get("Authorization")).isEqualTo("Bearer WRITE_TOKEN"); + } + + @Test + void shouldContainUrl() { + ExpiringAction downloadAction = lfsRepository.getDownloadAction(OBJECT_ID); + assertThat(downloadAction.href).isEqualTo("http://scm.org/976ed944c37cc5d1606af316937edb9d286ecf6c606af316937edb9d286ecf6c"); + } + + @Test + void shouldCreateTokenForDownloadActionOnlyOnce() { + lfsRepository.getDownloadAction(OBJECT_ID); + lfsRepository.getDownloadAction(OBJECT_ID); + verify(tokenFactory, times(1)).createReadAccessToken(REPOSITORY); + } + + @Test + void shouldCreateTokenForUploadActionOnlyOnce() { + lfsRepository.getUploadAction(OBJECT_ID, 42L); + lfsRepository.getUploadAction(OBJECT_ID, 42L); + verify(tokenFactory, times(1)).createWriteAccessToken(REPOSITORY); + } +} + From 5185a68eebf08cd08797726a96c26ed0ec108885 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Mon, 21 Oct 2019 13:10:48 +0200 Subject: [PATCH 14/18] Formatter is not thread safe --- .../main/java/sonia/scm/web/lfs/ExpiringAction.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java index 28a6ee1016..223bb3151e 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java @@ -3,23 +3,26 @@ package sonia.scm.web.lfs; import org.eclipse.jgit.lfs.server.Response; import sonia.scm.security.AccessToken; +import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Collections; import java.util.TimeZone; class ExpiringAction extends Response.Action { - private static final SimpleDateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); - static { - DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("GMT")); - } @SuppressWarnings({"squid:S00116"}) // This class is used for json serialization, only public final String expires_at; ExpiringAction(String href, AccessToken accessToken) { - this.expires_at = DATE_FORMAT.format(accessToken.getExpiration()); + this.expires_at = createDateFormat().format(accessToken.getExpiration()); this.href = href; this.header = Collections.singletonMap("Authorization", "Bearer " + accessToken.compact()); } + + private DateFormat createDateFormat() { + SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); + dateFormat.setTimeZone(TimeZone.getTimeZone("GMT")); + return dateFormat; + } } From e885d130beb5d56ba0026fe18ae7e1eb201c9bf4 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 22 Oct 2019 10:13:46 +0200 Subject: [PATCH 15/18] add push permissions to read access token This is required because the read access token is used to obtain the write access token. --- .../sonia/scm/web/lfs/LFSAuthCommand.java | 7 +++ .../scm/web/lfs/LfsAccessTokenFactory.java | 53 +++++++++++++------ .../scm/web/lfs/ScmBlobLfsRepository.java | 9 ++-- .../web/lfs/servlet/LfsServletFactory.java | 6 +++ 4 files changed, 57 insertions(+), 18 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java index 42dd7226b9..02f461bc5c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java @@ -2,6 +2,8 @@ package sonia.scm.web.lfs; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Charsets; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.config.ScmConfiguration; import sonia.scm.plugin.Extension; import sonia.scm.protocolcommand.CommandInterpreter; @@ -22,6 +24,8 @@ import static java.lang.String.format; @Extension public class LFSAuthCommand implements CommandInterpreterFactory { + private static final Logger LOG = LoggerFactory.getLogger(LFSAuthCommand.class); + private static final String LFS_INFO_URL_PATTERN = "%s/repo/%s/%s.git/info/lfs/"; private final LfsAccessTokenFactory tokenFactory; @@ -41,6 +45,7 @@ public class LFSAuthCommand implements CommandInterpreterFactory { @Override public Optional canHandle(String command) { if (command.startsWith("git-lfs-authenticate")) { + LOG.trace("create command for input: {}", command); return Optional.of(new LfsAuthCommandInterpreter(command)); } else { return Optional.empty(); @@ -65,6 +70,8 @@ public class LFSAuthCommand implements CommandInterpreterFactory { public ScmCommandProtocol getProtocolHandler() { return (context, repositoryContext) -> { ExpiringAction response = createResponseObject(repositoryContext); + // we buffer the response and write it with a single write, + // because otherwise the ssh connection is not closed String buffer = serializeResponse(response); context.getOutputStream().write(buffer.getBytes(Charsets.UTF_8)); }; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java index 272d70fd6c..ba24d0fd35 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java @@ -1,16 +1,24 @@ package sonia.scm.web.lfs; +import com.github.sdorra.ssp.PermissionCheck; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.security.AccessToken; import sonia.scm.security.AccessTokenBuilderFactory; +import sonia.scm.security.Permission; import sonia.scm.security.Scope; import javax.inject.Inject; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.TimeUnit; public class LfsAccessTokenFactory { + private static final Logger LOG = LoggerFactory.getLogger(LfsAccessTokenFactory.class); + private final AccessTokenBuilderFactory tokenBuilderFactory; @Inject @@ -19,29 +27,44 @@ public class LfsAccessTokenFactory { } AccessToken createReadAccessToken(Repository repository) { - RepositoryPermissions.pull(repository).check(); - RepositoryPermissions.read(repository).check(); - return createToken( - Scope.valueOf( - RepositoryPermissions.read(repository).asShiroString(), - RepositoryPermissions.pull(repository).asShiroString())); + PermissionCheck read = RepositoryPermissions.read(repository); + read.check(); + + PermissionCheck pull = RepositoryPermissions.pull(repository); + pull.check(); + + List permissions = new ArrayList<>(); + permissions.add(read.asShiroString()); + permissions.add(pull.asShiroString()); + + PermissionCheck push = RepositoryPermissions.push(repository); + if (push.isPermitted()) { + // we have to add push permissions, + // because this token is also used to obtain the write access token + permissions.add(push.asShiroString()); + } + + return createToken(Scope.valueOf(permissions)); } AccessToken createWriteAccessToken(Repository repository) { - RepositoryPermissions.read(repository).check(); - RepositoryPermissions.pull(repository).check(); - RepositoryPermissions.push(repository).check(); - return createToken( - Scope.valueOf( - RepositoryPermissions.read(repository).asShiroString(), - RepositoryPermissions.pull(repository).asShiroString(), - RepositoryPermissions.push(repository).asShiroString())); + PermissionCheck read = RepositoryPermissions.read(repository); + read.check(); + + PermissionCheck pull = RepositoryPermissions.pull(repository); + pull.check(); + + PermissionCheck push = RepositoryPermissions.push(repository); + push.check(); + + return createToken(Scope.valueOf(read.asShiroString(), pull.asShiroString(), push.asShiroString())); } private AccessToken createToken(Scope scope) { + LOG.trace("create access token with scope: {}", scope); return tokenBuilderFactory .create() - .expiresIn(1, TimeUnit.MINUTES) + .expiresIn(5, TimeUnit.MINUTES) .scope(scope) .build(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java index c646a194b7..b70b948c9b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java @@ -2,14 +2,13 @@ package sonia.scm.web.lfs; import org.eclipse.jgit.lfs.lib.AnyLongObjectId; import org.eclipse.jgit.lfs.server.LargeFileRepository; -import org.eclipse.jgit.lfs.server.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.repository.Repository; import sonia.scm.security.AccessToken; import sonia.scm.store.Blob; import sonia.scm.store.BlobStore; -import java.io.IOException; - /** * This LargeFileRepository is used for jGit-Servlet implementation. Under the jgit LFS Servlet hood, the * SCM-Repository API is used to implement the Repository. @@ -19,6 +18,8 @@ import java.io.IOException; */ public class ScmBlobLfsRepository implements LargeFileRepository { + private static final Logger LOG = LoggerFactory.getLogger(ScmBlobLfsRepository.class); + private final BlobStore blobStore; private final LfsAccessTokenFactory tokenFactory; @@ -54,6 +55,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { @Override public ExpiringAction getDownloadAction(AnyLongObjectId id) { if (accessToken == null) { + LOG.trace("create access token to download lfs object {} from repository {}", id, repository.getNamespaceAndName()); accessToken = tokenFactory.createReadAccessToken(repository); } return getAction(id, accessToken); @@ -62,6 +64,7 @@ public class ScmBlobLfsRepository implements LargeFileRepository { @Override public ExpiringAction getUploadAction(AnyLongObjectId id, long size) { if (accessToken == null) { + LOG.trace("create access token to upload lfs object {} to repository {}", id, repository.getNamespaceAndName()); accessToken = tokenFactory.createWriteAccessToken(repository); } return getAction(id, accessToken); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java index cc79d8e5c4..3b200ade36 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java @@ -4,6 +4,8 @@ import com.google.common.annotations.VisibleForTesting; import org.eclipse.jgit.lfs.server.LargeFileRepository; import org.eclipse.jgit.lfs.server.LfsProtocolServlet; import org.eclipse.jgit.lfs.server.fs.FileLfsServlet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.repository.Repository; import sonia.scm.store.BlobStore; import sonia.scm.util.HttpUtil; @@ -26,6 +28,8 @@ import javax.servlet.http.HttpServletRequest; @Singleton public class LfsServletFactory { + private static final Logger LOG = LoggerFactory.getLogger(LfsServletFactory.class); + private final LfsBlobStoreFactory lfsBlobStoreFactory; private final LfsAccessTokenFactory tokenFactory; @@ -43,6 +47,7 @@ public class LfsServletFactory { * @return The {@link LfsProtocolServlet} to provide the LFS Batch API for a SCM Repository. */ public LfsProtocolServlet createProtocolServletFor(Repository repository, HttpServletRequest request) { + LOG.trace("create lfs protocol servlet for repository {}", repository.getNamespaceAndName()); BlobStore blobStore = lfsBlobStoreFactory.getLfsBlobStore(repository); String baseUri = buildBaseUri(repository, request); @@ -58,6 +63,7 @@ public class LfsServletFactory { * @return The {@link FileLfsServlet} to provide the LFS Upload / Download API for a SCM Repository. */ public HttpServlet createFileLfsServletFor(Repository repository, HttpServletRequest request) { + LOG.trace("create lfs file servlet for repository {}", repository.getNamespaceAndName()); return new ScmFileTransferServlet(lfsBlobStoreFactory.getLfsBlobStore(repository)); } From 52f471b5dd3f4080c9f8059b7cbf682d3d8e2897 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 22 Oct 2019 10:50:49 +0200 Subject: [PATCH 16/18] fix lfs authentication via ssh command and enabled xsrf protection --- .../scm/web/lfs/LfsAccessTokenFactory.java | 1 - .../scm/security/XsrfAccessTokenEnricher.java | 56 +++--- .../security/XsrfAccessTokenEnricherTest.java | 163 ++++++++++-------- 3 files changed, 125 insertions(+), 95 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java index ba24d0fd35..c290b513e1 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java @@ -7,7 +7,6 @@ import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryPermissions; import sonia.scm.security.AccessToken; import sonia.scm.security.AccessTokenBuilderFactory; -import sonia.scm.security.Permission; import sonia.scm.security.Scope; import javax.inject.Inject; diff --git a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java index 617950ddea..b12a43ffe6 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java @@ -1,19 +1,19 @@ /** * Copyright (c) 2014, Sebastian Sdorra * All rights reserved. - * + *

* Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + *

* 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. + * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + *

* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE @@ -24,17 +24,19 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + *

* http://bitbucket.org/sdorra/scm-manager - * */ package sonia.scm.security; import com.google.common.annotations.VisibleForTesting; + import java.util.UUID; import javax.inject.Inject; import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; + +import com.google.inject.OutOfScopeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.config.ScmConfiguration; @@ -46,9 +48,9 @@ import sonia.scm.util.HttpUtil; * add the xsrf field, if the authentication request is issued from the web interface and xsrf protection is * enabled. The xsrf field will be validated on every request by the {@link XsrfAccessTokenValidator}. Xsrf protection * can be disabled with {@link ScmConfiguration#setEnabledXsrfProtection(boolean)}. - * - * @see Issue 793 + * * @author Sebastian Sdorra + * @see Issue 793 * @since 2.0.0 */ @Extension @@ -58,14 +60,14 @@ public class XsrfAccessTokenEnricher implements AccessTokenEnricher { * the logger for XsrfAccessTokenEnricher */ private static final Logger LOG = LoggerFactory.getLogger(XsrfAccessTokenEnricher.class); - + private final ScmConfiguration configuration; private final Provider requestProvider; /** * Constructs a new instance. - * - * @param configuration scm main configuration + * + * @param configuration scm main configuration * @param requestProvider http request provider */ @Inject @@ -73,12 +75,11 @@ public class XsrfAccessTokenEnricher implements AccessTokenEnricher { this.configuration = configuration; this.requestProvider = requestProvider; } - + @Override public void enrich(AccessTokenBuilder builder) { if (configuration.isEnabledXsrfProtection()) { - if (HttpUtil.isWUIRequest(requestProvider.get())) { - LOG.debug("received wui token claim, enrich jwt with xsrf key"); + if (isEnrichable()) { builder.custom(Xsrf.TOKEN_KEY, createToken()); } else { LOG.trace("skip xsrf enrichment, because jwt session is started from a non wui client"); @@ -87,11 +88,26 @@ public class XsrfAccessTokenEnricher implements AccessTokenEnricher { LOG.trace("xsrf is disabled, skip xsrf enrichment"); } } - + + private boolean isEnrichable() { + try { + HttpServletRequest request = requestProvider.get(); + if (HttpUtil.isWUIRequest(request)) { + LOG.debug("received wui token claim, enrich jwt with xsrf key"); + return true; + } else { + LOG.trace("skip xsrf enrichment, because jwt session is started from a non wui client"); + } + } catch (OutOfScopeException ex) { + LOG.trace("skip xsrf enrichment, because no request scope is available"); + } + return false; + } + @VisibleForTesting String createToken() { // TODO create interface and use a better method return UUID.randomUUID().toString(); } - + } diff --git a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java index 37d853011d..0d8960b0ee 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java @@ -1,19 +1,19 @@ /** * Copyright (c) 2014, Sebastian Sdorra * All rights reserved. - * + *

* Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: - * + *

* 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. + * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * + * contributors may be used to endorse or promote products derived from this + * software without specific prior written permission. + *

* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE @@ -24,103 +24,118 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + *

* http://bitbucket.org/sdorra/scm-manager - * */ package sonia.scm.security; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import com.google.inject.OutOfScopeException; +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.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; import sonia.scm.config.ScmConfiguration; import sonia.scm.util.HttpUtil; +import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; /** * Unit tests for {@link XsrfAccessTokenEnricher}. - * + * * @author Sebastian Sdorra */ -@RunWith(MockitoJUnitRunner.class) -public class XsrfAccessTokenEnricherTest { +@ExtendWith(MockitoExtension.class) +class XsrfAccessTokenEnricherTest { @Mock private HttpServletRequest request; @Mock private AccessTokenBuilder builder; - + private ScmConfiguration configuration; - + private XsrfAccessTokenEnricher enricher; - - /** - * Prepare object under test. - */ - @Before - public void prepareObjectUnderTest() { + + @BeforeEach + void createConfiguration() { configuration = new ScmConfiguration(); - enricher = new XsrfAccessTokenEnricher(configuration, () -> request) { + } + + @Test + @SuppressWarnings("unchecked") + void testWithoutRequestScope() { + // prepare + Provider requestProvider = mock(Provider.class); + when(requestProvider.get()).thenThrow(new OutOfScopeException("request scope is not available")); + configuration.setEnabledXsrfProtection(true); + XsrfAccessTokenEnricher enricher = createEnricher(requestProvider); + + // execute + enricher.enrich(builder); + + // assert + verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); + } + + private XsrfAccessTokenEnricher createEnricher(Provider requestProvider) { + return new XsrfAccessTokenEnricher(configuration, requestProvider) { @Override String createToken() { return "42"; } }; } - - /** - * Tests {@link XsrfAccessTokenEnricher#enrich(java.util.Map)}. - */ - @Test - public void testEnrich() { - // prepare - configuration.setEnabledXsrfProtection(true); - when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); - - // execute - enricher.enrich(builder); - - // assert - verify(builder).custom(Xsrf.TOKEN_KEY, "42"); - } - - /** - * Tests {@link XsrfAccessTokenEnricher#enrich(java.util.Map)} with disabled xsrf protection. - */ - @Test - public void testEnrichWithDisabledXsrf() { - // prepare - configuration.setEnabledXsrfProtection(false); - // execute - enricher.enrich(builder); - - // assert - verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); - } - - /** - * Tests {@link XsrfAccessTokenEnricher#enrich(java.util.Map)} with disabled xsrf protection. - */ - @Test - public void testEnrichWithNonWuiClient() { - // prepare - configuration.setEnabledXsrfProtection(true); - - // execute - enricher.enrich(builder); - - // assert - verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); - } + @Nested + class WithRequestMock { + @BeforeEach + void setupEnricher() { + enricher = createEnricher(() -> request); + } + + @Test + void testEnrich() { + // prepare + configuration.setEnabledXsrfProtection(true); + when(request.getHeader(HttpUtil.HEADER_SCM_CLIENT)).thenReturn(HttpUtil.SCM_CLIENT_WUI); + + // execute + enricher.enrich(builder); + + // assert + verify(builder).custom(Xsrf.TOKEN_KEY, "42"); + } + + @Test + void testEnrichWithDisabledXsrf() { + // prepare + configuration.setEnabledXsrfProtection(false); + + // execute + enricher.enrich(builder); + + // assert + verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); + } + + @Test + void testEnrichWithNonWuiClient() { + // prepare + configuration.setEnabledXsrfProtection(true); + + // execute + enricher.enrich(builder); + + // assert + verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); + } + } } From 10fbf50263cf580d365e2b26531a2647064733c3 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 22 Oct 2019 11:23:34 +0200 Subject: [PATCH 17/18] fix wrong OutOfScopeException detection --- .../scm/security/XsrfAccessTokenEnricher.java | 9 +++++++-- .../security/XsrfAccessTokenEnricherTest.java | 17 ++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java index b12a43ffe6..ed7093c09c 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java +++ b/scm-webapp/src/main/java/sonia/scm/security/XsrfAccessTokenEnricher.java @@ -37,6 +37,7 @@ import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; import com.google.inject.OutOfScopeException; +import com.google.inject.ProvisionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.config.ScmConfiguration; @@ -98,8 +99,12 @@ public class XsrfAccessTokenEnricher implements AccessTokenEnricher { } else { LOG.trace("skip xsrf enrichment, because jwt session is started from a non wui client"); } - } catch (OutOfScopeException ex) { - LOG.trace("skip xsrf enrichment, because no request scope is available"); + } catch (ProvisionException ex) { + if (ex.getCause() instanceof OutOfScopeException) { + LOG.trace("skip xsrf enrichment, because no request scope is available"); + } else { + throw ex; + } } return false; } diff --git a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java index 0d8960b0ee..5fbaaeb64c 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/XsrfAccessTokenEnricherTest.java @@ -31,6 +31,7 @@ package sonia.scm.security; import com.google.inject.OutOfScopeException; +import com.google.inject.ProvisionException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -43,6 +44,7 @@ import sonia.scm.util.HttpUtil; import javax.inject.Provider; import javax.servlet.http.HttpServletRequest; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; /** @@ -73,7 +75,7 @@ class XsrfAccessTokenEnricherTest { void testWithoutRequestScope() { // prepare Provider requestProvider = mock(Provider.class); - when(requestProvider.get()).thenThrow(new OutOfScopeException("request scope is not available")); + when(requestProvider.get()).thenThrow(new ProvisionException("failed to provision", new OutOfScopeException("no request scope is available"))); configuration.setEnabledXsrfProtection(true); XsrfAccessTokenEnricher enricher = createEnricher(requestProvider); @@ -84,6 +86,19 @@ class XsrfAccessTokenEnricherTest { verify(builder, never()).custom(Xsrf.TOKEN_KEY, "42"); } + @Test + @SuppressWarnings("unchecked") + void testWithProvisionException() { + // prepare + Provider requestProvider = mock(Provider.class); + when(requestProvider.get()).thenThrow(new ProvisionException("failed to provision")); + configuration.setEnabledXsrfProtection(true); + XsrfAccessTokenEnricher enricher = createEnricher(requestProvider); + + // execute + assertThrows(ProvisionException.class, () -> enricher.enrich(builder)); + } + private XsrfAccessTokenEnricher createEnricher(Provider requestProvider) { return new XsrfAccessTokenEnricher(configuration, requestProvider) { @Override From af2a9961a6b05cb1f7667e94d90b032277f7068d Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 22 Oct 2019 12:01:38 +0000 Subject: [PATCH 18/18] Close branch feature/lfs_over_ssh