Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve check for validity of token #76

Merged
merged 4 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/main/java/dev/openfga/sdk/api/auth/AccessToken.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,48 @@
import static dev.openfga.sdk.util.StringUtil.isNullOrWhitespace;

import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Random;

class AccessToken {
private static final int TOKEN_EXPIRY_BUFFER_THRESHOLD_IN_SEC = 300;
private static final int TOKEN_EXPIRY_JITTER_IN_SEC =
300; // We add some jitter so that token refreshes are less likely to collide

private final Random random = new Random();
private Instant expiresAt;

private final Random random = new Random();
private String token;

public boolean isValid() {
return !isNullOrWhitespace(token)
&& (expiresAt == null
|| expiresAt.isAfter(Instant.now()
.minusSeconds(TOKEN_EXPIRY_BUFFER_THRESHOLD_IN_SEC)
.minusSeconds(random.nextLong() % TOKEN_EXPIRY_JITTER_IN_SEC)));
if (isNullOrWhitespace(token)) {
return false;
}

// Is expiry is null then the token will not expire so should be considered always valid
if (expiresAt == null) {
return true;
}

// A token should be considered valid until 5 minutes before the expiry with some jitter
// to account for multiple calls to `isValid` at the same time and prevent multiple refresh calls
Instant expiresWithLeeway = expiresAt
.minusSeconds(TOKEN_EXPIRY_BUFFER_THRESHOLD_IN_SEC)
.minusSeconds(random.nextInt(TOKEN_EXPIRY_JITTER_IN_SEC))
.truncatedTo(ChronoUnit.SECONDS);

return Instant.now().truncatedTo(ChronoUnit.SECONDS).isBefore(expiresWithLeeway);
}

public String getToken() {
return token;
}

public void setExpiresAt(Instant expiresAt) {
this.expiresAt = expiresAt;
if (expiresAt != null) {
// Truncate to seconds to zero out the milliseconds to keep comparison simpler
this.expiresAt = expiresAt.truncatedTo(ChronoUnit.SECONDS);
}
}

public void setToken(String token) {
Expand Down
30 changes: 23 additions & 7 deletions src/test/java/dev/openfga/sdk/api/auth/AccessTokenTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,32 @@ class AccessTokenTest {

private static Stream<Arguments> expTimeAndResults() {
return Stream.of(
Arguments.of(Instant.now().plus(1, ChronoUnit.HOURS), true),
Arguments.of(Instant.now().minus(1, ChronoUnit.HOURS), false),
Arguments.of(Instant.now().minus(10, ChronoUnit.MINUTES), false),
Arguments.of(Instant.now().plus(10, ChronoUnit.MINUTES), true),
Arguments.of(Instant.now(), true));
Arguments.of("Expires in 1 hour should be valid", Instant.now().plus(1, ChronoUnit.HOURS), true),
Arguments.of(
"Expires in 15 minutes should be valid", Instant.now().plus(15, ChronoUnit.MINUTES), true),
Arguments.of("No expiry value should be valid", null, true),
Arguments.of(
"Expired 1 hour ago should not be valid", Instant.now().minus(1, ChronoUnit.HOURS), false),
Arguments.of(
"Expired 10 minutes ago should not be valid",
Instant.now().minus(10, ChronoUnit.MINUTES),
false),
Arguments.of(
"Expired 5 minutes ago should not be valid",
Instant.now().minus(5, ChronoUnit.MINUTES),
false),
Arguments.of(
"Expires in 5 minutes should not be valid",
Instant.now().plus(5, ChronoUnit.MINUTES),
false),
Arguments.of(
"Expires in 1 minute should not be valid", Instant.now().plus(1, ChronoUnit.MINUTES), false),
Arguments.of("Expires now should not be valid", Instant.now(), false));
}

@MethodSource("expTimeAndResults")
@ParameterizedTest
public void testTokenValid(Instant exp, boolean valid) {
@ParameterizedTest(name = "{0}")
public void testTokenValid(String name, Instant exp, boolean valid) {
AccessToken accessToken = new AccessToken();
accessToken.setToken("token");
accessToken.setExpiresAt(exp);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.hamcrest.Matchers.*;
import static org.hamcrest.core.StringContains.containsString;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;

import com.fasterxml.jackson.databind.ObjectMapper;
Expand Down Expand Up @@ -148,7 +149,7 @@ public void createStore_withClientCredentials() throws Exception {
containsString(String.format("client_secret=%s", clientSecret)),
containsString(String.format("audience=%s", apiAudience)),
containsString(String.format("grant_type=%s", "client_credentials"))))
.doReturn(200, String.format("{\"access_token\":\"%s\"}", apiToken));
.doReturn(200, String.format("{\"access_token\":\"%s\",\"expires_in\":\"%s\"}", apiToken, 3600));
mockHttpClient
.onPost("https://localhost/stores")
.withBody(is(expectedBody))
Expand Down Expand Up @@ -180,6 +181,62 @@ public void createStore_withClientCredentials() throws Exception {
assertEquals(DEFAULT_STORE_NAME, response2.getName());
}

@Test
public void createStore_withClientCredentialsWithRefresh() throws Exception {
// Given
String apiTokenIssuer = "oauth2.server";
String clientId = "some-client-id";
String clientSecret = "some-client-secret";
String apiToken = "some-generated-token";
String apiAudience = "some-audience";
clientConfiguration.credentials(new Credentials(new ClientCredentials()
.clientId(clientId)
.clientSecret(clientSecret)
.apiTokenIssuer(apiTokenIssuer)
.apiAudience(apiAudience)));
fga.setConfiguration(clientConfiguration);

String expectedBody = String.format("{\"name\":\"%s\"}", DEFAULT_STORE_NAME);
String requestBody = String.format("{\"id\":\"%s\",\"name\":\"%s\"}", DEFAULT_STORE_ID, DEFAULT_STORE_NAME);
mockHttpClient
.onPost(String.format("https://%s/oauth/token", apiTokenIssuer))
.withBody(allOf(
containsString(String.format("client_id=%s", clientId)),
containsString(String.format("client_secret=%s", clientSecret)),
containsString(String.format("audience=%s", apiAudience)),
containsString(String.format("grant_type=%s", "client_credentials"))))
.doReturn(200, String.format("{\"access_token\":\"%s\",\"expires_in\":\"%s\"}", apiToken, 1));
mockHttpClient
.onPost("https://localhost/stores")
ewanharris marked this conversation as resolved.
Show resolved Hide resolved
.withBody(is(expectedBody))
.withHeader("Authorization", String.format("Bearer %s", apiToken))
.doReturn(201, requestBody);
CreateStoreRequest request = new CreateStoreRequest().name(DEFAULT_STORE_NAME);

// When
// We call two times to ensure the token is cached after the first request.
CreateStoreResponse response1 = fga.createStore(request).get();
CreateStoreResponse response2 = fga.createStore(request).get();

// Then
// OAuth2 server should be called 1 time.
mockHttpClient
.verify()
.post(String.format("https://%s/oauth/token", apiTokenIssuer))
.called(2);
// OpenFGA server should be called 2 times.
mockHttpClient
.verify()
.post("https://localhost/stores")
.withBody(is(expectedBody))
.withHeader("Authorization", String.format("Bearer %s", apiToken))
.called(2);
assertEquals(DEFAULT_STORE_ID, response1.getId());
assertEquals(DEFAULT_STORE_NAME, response1.getName());
assertEquals(DEFAULT_STORE_ID, response2.getId());
assertEquals(DEFAULT_STORE_NAME, response2.getName());
}

/**
* List all stores.
*/
Expand Down