Merged in feature/lfs_over_ssh (pull request #336)

Feature LFS over ssh

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ExpiringAction.java

@@ -0,0 +1,28 @@
+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 {
+
+  @SuppressWarnings({"squid:S00116"})
+  // This class is used for json serialization, only
+  public final String expires_at;
+
+  ExpiringAction(String href, AccessToken accessToken) {
+    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;
+  }
+}

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LFSAuthCommand.java

@@ -0,0 +1,98 @@
+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;
+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 javax.inject.Inject;
+import java.io.IOException;
+import java.util.Optional;
+
+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;
+  private final GitRepositoryContextResolver gitRepositoryContextResolver;
+  private final ObjectMapper objectMapper;
+  private final ScmConfiguration configuration;
+
+  @Inject
+  public LFSAuthCommand(LfsAccessTokenFactory tokenFactory, GitRepositoryContextResolver gitRepositoryContextResolver, ScmConfiguration configuration) {
+    this.tokenFactory = tokenFactory;
+    this.gitRepositoryContextResolver = gitRepositoryContextResolver;
+
+    objectMapper = new ObjectMapper();
+    this.configuration = configuration;
+  }
+
+  @Override
+  public Optional<CommandInterpreter> 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();
+    }
+  }
+
+  private class LfsAuthCommandInterpreter implements CommandInterpreter {
+
+    private final String command;
+
+    LfsAuthCommandInterpreter(String command) {
+      this.command = command;
+    }
+
+    @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) -> {
+        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));
+      };
+    }
+
+    @Override
+    public RepositoryContextResolver getRepositoryContextResolver() {
+      return gitRepositoryContextResolver;
+    }
+
+    private ExpiringAction createResponseObject(RepositoryContext repositoryContext) {
+      Repository repository = repositoryContext.getRepository();
+
+      String url = format(LFS_INFO_URL_PATTERN, configuration.getBaseUrl(), repository.getNamespace(), repository.getName());
+      AccessToken accessToken = tokenFactory.createReadAccessToken(repository);
+
+      return new ExpiringAction(url, accessToken);
+    }
+
+    private String serializeResponse(ExpiringAction response) throws IOException {
+      return objectMapper.writeValueAsString(response);
+    }
+  }
+}

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/LfsAccessTokenFactory.java

@@ -0,0 +1,70 @@
+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.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
+  LfsAccessTokenFactory(AccessTokenBuilderFactory tokenBuilderFactory) {
+    this.tokenBuilderFactory = tokenBuilderFactory;
+  }
+
+  AccessToken createReadAccessToken(Repository repository) {
+    PermissionCheck read = RepositoryPermissions.read(repository);
+    read.check();
+
+    PermissionCheck pull = RepositoryPermissions.pull(repository);
+    pull.check();
+
+    List<String> 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) {
+    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(5, TimeUnit.MINUTES)
+      .scope(scope)
+      .build();
+  }
+}

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/ScmBlobLfsRepository.java

@@ -2,12 +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.
@@ -17,49 +18,67 @@ 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;
 
   /**
    * This URI is used to determine the actual URI for Upload / Download. Must be full URI (or rewritable by reverse
    * proxy).
    */
   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;
 
   /**
    * 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 repository          The current scm repository this LFS repository is used for.
+   * @param blobStore           The SCM Blobstore used for this @{@link LargeFileRepository}.
+   * @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(BlobStore blobStore, String baseUri) {
-
+  public ScmBlobLfsRepository(Repository repository, BlobStore blobStore, LfsAccessTokenFactory tokenFactory, String baseUri) {
+    this.repository = repository;
     this.blobStore = blobStore;
+    this.tokenFactory = tokenFactory;
     this.baseUri = baseUri;
   }
 
   @Override
-  public Response.Action getDownloadAction(AnyLongObjectId id) {
-
-    return getAction(id);
+  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);
   }
 
   @Override
-  public Response.Action getUploadAction(AnyLongObjectId id, long size) {
-
-    return getAction(id);
+  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);
   }
 
   @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.
@@ -77,14 +96,11 @@ public class ScmBlobLfsRepository implements LargeFileRepository {
   /**
    * Constructs the Download / Upload actions to be supplied to the client.
    */
-  private Response.Action getAction(AnyLongObjectId id) {
+  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.
 
-    Response.Action a = new Response.Action();
-    a.href = baseUri + id.getName();
-
-    return a;
+    return new ExpiringAction(baseUri + id.getName(), token);
   }
 }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/lfs/servlet/LfsServletFactory.java

@@ -9,6 +9,7 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.repository.Repository;
 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;
 
@@ -27,13 +28,15 @@ import javax.servlet.http.HttpServletRequest;
 @Singleton
 public class LfsServletFactory {
 
-  private static final Logger logger = LoggerFactory.getLogger(LfsServletFactory.class);
+  private static final Logger LOG = LoggerFactory.getLogger(LfsServletFactory.class);
 
   private final LfsBlobStoreFactory lfsBlobStoreFactory;
+  private final LfsAccessTokenFactory tokenFactory;
 
   @Inject
-  public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory) {
+  public LfsServletFactory(LfsBlobStoreFactory lfsBlobStoreFactory, LfsAccessTokenFactory tokenFactory) {
     this.lfsBlobStoreFactory = lfsBlobStoreFactory;
+    this.tokenFactory = tokenFactory;
   }
 
   /**
@@ -44,10 +47,11 @@ 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);
 
-    LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(blobStore, baseUri);
+    LargeFileRepository largeFileRepository = new ScmBlobLfsRepository(repository, blobStore, tokenFactory, baseUri);
     return new ScmLfsProtocolServlet(largeFileRepository);
   }
 
@@ -59,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));
   }
 

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> commandInterpreter = lfsAuthCommand.canHandle("git-lfs-authenticate repo/space/X upload");
+    assertThat(commandInterpreter).isPresent();
+  }
+
+  @Test
+  void shouldNotHandleOtherCommands() {
+    Optional<CommandInterpreter> 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-03T10:15:30Z\"}");
+  }
+
+  private CommandContext createCommandContext() {
+    return new CommandContext(null, null, null, new ByteArrayOutputStream(), null);
+  }
+
+  private RepositoryContext createRepositoryContext() {
+    return new RepositoryContext(REPOSITORY, null);
+  }
+}

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

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;