Add support for "expires_in" field in authentication response
Per the documentation [1], the "expires_in" field is preferred over
the "expires_at" field.
Refactor the implementation to hold the instant at which the token was
issued, along with the expiry period, and calculate the "expires_at"
value on demand.
[1] https://github.com/git-lfs/git-lfs/blob/master/docs/api/authentication.md
Change-Id: I14a05e5d71254ecacf36655a341d60febed8531e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/AuthInfo.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/AuthInfo.java
index 187ce6f..4336ed5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/AuthInfo.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/AuthInfo.java
@@ -14,12 +14,48 @@
package com.googlesource.gerrit.plugins.lfs;
-public class AuthInfo {
- public final String authToken;
- public final String expiresAt;
+import java.time.Instant;
- public AuthInfo(String authToken, String expiresAt) {
+public class AuthInfo {
+ private final String authToken;
+ private final Instant issued;
+ private final Long expiresIn;
+
+ /**
+ * @param authToken token
+ * @param issued time at which the token was issued
+ * @param expiresIn expiry duration in seconds
+ */
+ public AuthInfo(String authToken, Instant issued, Long expiresIn) {
this.authToken = authToken;
- this.expiresAt = expiresAt;
+ this.issued = issued;
+ this.expiresIn = expiresIn;
+ }
+
+ /**
+ * Get the auth token
+ *
+ * @return the auth token
+ */
+ public String authToken() {
+ return authToken;
+ }
+
+ /**
+ * Get a formatted timestamp of the time at which this authentication expires
+ *
+ * @return timestamp
+ */
+ public String expiresAt() {
+ return LfsDateTime.format(issued.plusSeconds(expiresIn));
+ }
+
+ /**
+ * Get the time duration after which this authentication expires
+ *
+ * @return the time duration in seconds
+ */
+ public Long expiresIn() {
+ return expiresIn;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/ExpiringAction.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/ExpiringAction.java
index 7d43d04..d3255a1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/ExpiringAction.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/ExpiringAction.java
@@ -21,10 +21,12 @@
public class ExpiringAction extends Response.Action {
public final String expiresAt;
+ public final Long expiresIn;
public ExpiringAction(String href, AuthInfo info) {
this.href = href;
- this.header = Collections.singletonMap(HDR_AUTHORIZATION, info.authToken);
- this.expiresAt = info.expiresAt;
+ this.header = Collections.singletonMap(HDR_AUTHORIZATION, info.authToken());
+ this.expiresAt = info.expiresAt();
+ this.expiresIn = info.expiresIn();
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
index 278447c..5f3b9c2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsAuthToken.java
@@ -16,6 +16,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
+import java.time.Instant;
import java.util.List;
import java.util.Optional;
@@ -55,27 +56,26 @@
}
public boolean verify() {
- return onTime(token.expiresAt) && verifyTokenValues();
+ return onTime(Instant.now()) && verifyTokenValues();
}
protected abstract boolean verifyTokenValues();
- static boolean onTime(String dateTime) {
- return LfsDateTime.now().compareTo(dateTime) <= 0;
+ public boolean onTime(Instant when) {
+ return when.isBefore(token.issued.plusSeconds(token.expiresIn));
}
}
- public final String expiresAt;
+ public final Instant issued;
+ public final Long expiresIn;
- protected LfsAuthToken(int expirationSeconds) {
- this(timeout(expirationSeconds));
+ protected LfsAuthToken(Instant issued, Long expiresIn) {
+ this.issued = issued;
+ this.expiresIn = expiresIn;
}
- protected LfsAuthToken(String expiresAt) {
- this.expiresAt = expiresAt;
- }
-
- static String timeout(int expirationSeconds) {
- return LfsDateTime.now(expirationSeconds);
+ protected LfsAuthToken(String issued, Long expiresIn) {
+ this.issued = Instant.parse(issued);
+ this.expiresIn = expiresIn;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
index ccedee5..4a1936c 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsDateTime.java
@@ -29,10 +29,6 @@
return FORMAT.format(Instant.now());
}
- public static String now(int secondsToAdd) {
- return FORMAT.format(Instant.now().plusSeconds(secondsToAdd));
- }
-
public static String format(Instant instant) {
return FORMAT.format(instant);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsSshRequestAuthorizer.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsSshRequestAuthorizer.java
index 2637a12..12c5552 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsSshRequestAuthorizer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/LfsSshRequestAuthorizer.java
@@ -17,6 +17,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
@@ -26,8 +27,8 @@
@Singleton
class LfsSshRequestAuthorizer {
static class SshAuthInfo extends AuthInfo {
- SshAuthInfo(String authToken, String expiresAt) {
- super(SSH_AUTH_PREFIX + authToken, expiresAt);
+ SshAuthInfo(String authToken, Instant issued, Long expiresIn) {
+ super(SSH_AUTH_PREFIX + authToken, issued, expiresIn);
}
}
@@ -36,12 +37,12 @@
static final String SSH_AUTH_PREFIX = "Ssh: ";
private final Processor processor;
- private final int expirationSeconds;
+ private final Long expiresIn;
@Inject
LfsSshRequestAuthorizer(Processor processor, LfsConfigurationFactory configFactory) {
this.processor = processor;
- int timeout = DEFAULT_SSH_TIMEOUT;
+ long timeout = DEFAULT_SSH_TIMEOUT;
try {
timeout =
configFactory
@@ -53,13 +54,13 @@
DEFAULT_SSH_TIMEOUT,
e);
}
- this.expirationSeconds = timeout;
+ this.expiresIn = timeout;
}
SshAuthInfo generateAuthInfo(CurrentUser user, String project, String operation) {
LfsSshAuthToken token =
- new LfsSshAuthToken(user.getUserName(), project, operation, expirationSeconds);
- return new SshAuthInfo(processor.serialize(token), token.expiresAt);
+ new LfsSshAuthToken(user.getUserName(), project, operation, Instant.now(), expiresIn);
+ return new SshAuthInfo(processor.serialize(token), token.issued, token.expiresIn);
}
Optional<String> getUserFromValidToken(String authToken, String project, String operation) {
@@ -89,18 +90,24 @@
values.add(token.user);
values.add(token.project);
values.add(token.operation);
- values.add(token.expiresAt);
+ values.add(LfsDateTime.format(token.issued));
+ values.add(String.valueOf(token.expiresIn));
return values;
}
@Override
protected Optional<LfsSshAuthToken> createToken(List<String> values) {
- if (values.size() != 4) {
+ if (values.size() != 5) {
return Optional.empty();
}
return Optional.of(
- new LfsSshAuthToken(values.get(0), values.get(1), values.get(2), values.get(3)));
+ new LfsSshAuthToken(
+ values.get(0),
+ values.get(1),
+ values.get(2),
+ values.get(3),
+ Long.valueOf(values.get(4))));
}
}
@@ -125,15 +132,15 @@
private final String project;
private final String operation;
- LfsSshAuthToken(String user, String project, String operation, int expirationSeconds) {
- super(expirationSeconds);
+ LfsSshAuthToken(String user, String project, String operation, Instant issued, Long expiresIn) {
+ super(issued, expiresIn);
this.user = user;
this.project = project;
this.operation = operation;
}
- LfsSshAuthToken(String user, String project, String operation, String expiresAt) {
- super(expiresAt);
+ LfsSshAuthToken(String user, String project, String operation, String issued, Long expiresIn) {
+ super(issued, expiresIn);
this.user = user;
this.project = project;
this.operation = operation;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizer.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizer.java
index f27eb62..7e982b5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizer.java
@@ -19,6 +19,8 @@
import com.googlesource.gerrit.plugins.lfs.AuthInfo;
import com.googlesource.gerrit.plugins.lfs.LfsAuthToken;
import com.googlesource.gerrit.plugins.lfs.LfsCipher;
+import com.googlesource.gerrit.plugins.lfs.LfsDateTime;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
@@ -34,9 +36,10 @@
this.processor = processor;
}
- public AuthInfo generateAuthInfo(String operation, AnyLongObjectId id, int expirationSeconds) {
- LfsFsAuthToken token = new LfsFsAuthToken(operation, id, expirationSeconds);
- return new AuthInfo(processor.serialize(token), token.expiresAt);
+ public AuthInfo generateAuthInfo(
+ String operation, AnyLongObjectId id, Instant now, Long expiresIn) {
+ LfsFsAuthToken token = new LfsFsAuthToken(operation, id, now, expiresIn);
+ return new AuthInfo(processor.serialize(token), token.issued, token.expiresIn);
}
public boolean verifyAuthInfo(String authToken, String operation, AnyLongObjectId id) {
@@ -59,18 +62,23 @@
List<String> values = new ArrayList<>(3);
values.add(token.operation);
values.add(token.id.getName());
- values.add(token.expiresAt);
+ values.add(LfsDateTime.format(token.issued));
+ values.add(String.valueOf(token.expiresIn));
return values;
}
@Override
protected Optional<LfsFsAuthToken> createToken(List<String> values) {
- if (values.size() != 3) {
+ if (values.size() != 4) {
return Optional.empty();
}
return Optional.of(
- new LfsFsAuthToken(values.get(0), LongObjectId.fromString(values.get(1)), values.get(2)));
+ new LfsFsAuthToken(
+ values.get(0),
+ LongObjectId.fromString(values.get(1)),
+ values.get(2),
+ Long.valueOf(values.get(3))));
}
}
@@ -94,14 +102,14 @@
private final String operation;
private final AnyLongObjectId id;
- LfsFsAuthToken(String operation, AnyLongObjectId id, int expirationSeconds) {
- super(expirationSeconds);
+ LfsFsAuthToken(String operation, AnyLongObjectId id, Instant issued, Long expiresIn) {
+ super(issued, expiresIn);
this.operation = operation;
this.id = id;
}
- LfsFsAuthToken(String operation, AnyLongObjectId id, String expiresAt) {
- super(expiresAt);
+ LfsFsAuthToken(String operation, AnyLongObjectId id, String issued, Long expiresIn) {
+ super(issued, expiresIn);
this.operation = operation;
this.id = id;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LocalLargeFileRepository.java b/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LocalLargeFileRepository.java
index 4d897d4..477061e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LocalLargeFileRepository.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/lfs/fs/LocalLargeFileRepository.java
@@ -32,6 +32,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.time.Instant;
import org.eclipse.jgit.lfs.lib.AnyLongObjectId;
import org.eclipse.jgit.lfs.server.Response;
import org.eclipse.jgit.lfs.server.fs.FileLfsRepository;
@@ -46,7 +47,7 @@
private final String servletUrlPattern;
private final LfsFsRequestAuthorizer authorizer;
- private final int expirationSeconds;
+ private final Long expiresIn;
@Inject
LocalLargeFileRepository(
@@ -61,10 +62,11 @@
getOrCreateDataDir(configFactory.getGlobalConfig(), backend, defaultDataDir));
this.authorizer = authorizer;
this.servletUrlPattern = "/" + getContentPath(backend) + "*";
- this.expirationSeconds =
- configFactory
- .getGlobalConfig()
- .getInt(backend.type.name(), backend.name, "expirationSeconds", DEFAULT_TIMEOUT);
+ this.expiresIn =
+ (long)
+ configFactory
+ .getGlobalConfig()
+ .getInt(backend.type.name(), backend.name, "expirationSeconds", DEFAULT_TIMEOUT);
}
public String getServletUrlPattern() {
@@ -74,14 +76,14 @@
@Override
public Response.Action getDownloadAction(AnyLongObjectId id) {
Response.Action action = super.getDownloadAction(id);
- AuthInfo authInfo = authorizer.generateAuthInfo(DOWNLOAD, id, expirationSeconds);
+ AuthInfo authInfo = authorizer.generateAuthInfo(DOWNLOAD, id, Instant.now(), expiresIn);
return new ExpiringAction(action.href, authInfo);
}
@Override
public Response.Action getUploadAction(AnyLongObjectId id, long size) {
Response.Action action = super.getUploadAction(id, size);
- AuthInfo authInfo = authorizer.generateAuthInfo(UPLOAD, id, expirationSeconds);
+ AuthInfo authInfo = authorizer.generateAuthInfo(UPLOAD, id, Instant.now(), expiresIn);
return new ExpiringAction(action.href, authInfo);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
index 01a4c72..f41307b 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/LfsAuthTokenTest.java
@@ -15,7 +15,6 @@
package com.googlesource.gerrit.plugins.lfs;
import static com.google.common.truth.Truth.assertThat;
-import static com.googlesource.gerrit.plugins.lfs.LfsAuthToken.Verifier.onTime;
import java.time.Instant;
import java.util.ArrayList;
@@ -27,55 +26,41 @@
private final LfsCipher cipher = new LfsCipher();
@Test
- public void testExpiredTime() throws Exception {
- // test that even 1ms expiration is enough
- assertThat(onTime(LfsDateTime.format(now().minusMillis(1)))).isFalse();
- }
-
- @Test
- public void testOnTime() throws Exception {
- // if there is at least 1ms before there is no timeout
- assertThat(onTime(LfsDateTime.format(now().plusMillis(1)))).isTrue();
- }
-
- @Test
public void testTokenSerializationDeserialization() throws Exception {
TestTokenProessor processor = new TestTokenProessor(cipher);
- TestToken token = new TestToken(0);
+ TestToken token = new TestToken(Instant.now(), 0L);
String serialized = processor.serialize(token);
assertThat(serialized).isNotEmpty();
Optional<TestToken> deserialized = processor.deserialize(serialized);
assertThat(deserialized.isPresent()).isTrue();
- assertThat(token.expiresAt).isEqualTo(deserialized.get().expiresAt);
+ assertThat(token.issued).isEqualTo(deserialized.get().issued);
}
@Test
public void testTokenOnTime() throws Exception {
- TestToken token = new TestToken(1);
+ Instant when = Instant.now();
+ TestToken token = new TestToken(when, 1L);
TestTokenVerifier verifier = new TestTokenVerifier(token);
- assertThat(verifier.verify()).isTrue();
+ assertThat(verifier.onTime(when.plusMillis(999))).isTrue();
}
@Test
public void testTokenExpired() throws Exception {
- TestToken token = new TestToken(-1);
+ Instant when = Instant.now();
+ TestToken token = new TestToken(when, 1L);
TestTokenVerifier verifier = new TestTokenVerifier(token);
- assertThat(verifier.verify()).isFalse();
- }
-
- private Instant now() {
- return Instant.now();
+ assertThat(verifier.onTime(when.plusMillis(1001))).isFalse();
}
private class TestToken extends LfsAuthToken {
- TestToken(int expirationSeconds) {
- super(expirationSeconds);
+ TestToken(Instant now, Long expiresIn) {
+ super(now, expiresIn);
}
- TestToken(String expiresAt) {
- super(expiresAt);
+ TestToken(String expiresAt, Long expiresIn) {
+ super(expiresAt, expiresIn);
}
}
@@ -87,13 +72,14 @@
@Override
protected List<String> getValues(TestToken token) {
List<String> values = new ArrayList<>(2);
- values.add(token.expiresAt);
+ values.add(LfsDateTime.format(token.issued));
+ values.add(String.valueOf(token.expiresIn));
return values;
}
@Override
protected Optional<TestToken> createToken(List<String> values) {
- return Optional.of(new TestToken(values.get(0)));
+ return Optional.of(new TestToken(values.get(0), Long.valueOf(values.get(1))));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizerTest.java b/src/test/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizerTest.java
index 38c310f..8cd7225 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/lfs/fs/LfsFsRequestAuthorizerTest.java
@@ -20,6 +20,7 @@
import com.googlesource.gerrit.plugins.lfs.AuthInfo;
import com.googlesource.gerrit.plugins.lfs.LfsCipher;
import com.googlesource.gerrit.plugins.lfs.fs.LfsFsRequestAuthorizer.Processor;
+import java.time.Instant;
import org.eclipse.jgit.lfs.lib.LongObjectId;
import org.junit.Test;
@@ -29,22 +30,22 @@
@Test
public void testVerifyAuthInfo() throws Exception {
- AuthInfo info = auth.generateAuthInfo("o", zeroId(), 1);
- assertThat(auth.verifyAuthInfo(info.authToken, "o", zeroId())).isTrue();
+ AuthInfo info = auth.generateAuthInfo("o", zeroId(), Instant.now(), 1L);
+ assertThat(auth.verifyAuthInfo(info.authToken(), "o", zeroId())).isTrue();
}
@Test
public void testVerifyAgainstDifferentOperation() throws Exception {
- AuthInfo info = auth.generateAuthInfo("o", zeroId(), 1);
- assertThat(auth.verifyAuthInfo(info.authToken, "p", zeroId())).isFalse();
+ AuthInfo info = auth.generateAuthInfo("o", zeroId(), Instant.now(), 1L);
+ assertThat(auth.verifyAuthInfo(info.authToken(), "p", zeroId())).isFalse();
}
@Test
public void testVerifyAgainstDifferentObjectId() throws Exception {
- AuthInfo info = auth.generateAuthInfo("o", zeroId(), 1);
+ AuthInfo info = auth.generateAuthInfo("o", zeroId(), Instant.now(), 1L);
assertThat(
auth.verifyAuthInfo(
- info.authToken,
+ info.authToken(),
"o",
LongObjectId.fromString(
"123456789012345678901234567890" + "123456789012345678901234567890" + "1234")))
