Skip to content

Commit

Permalink
fix: accessToken scopes clean serialization and default as empty list (…
Browse files Browse the repository at this point in the history 
…#1125)

* fix: acessToken scopes clean serialization and default as empty list

* cleanup and more tests

* 🦉 Updates from OwlBot post-processor
  • Loading branch information
@TimurSadykov
TimurSadykov authored Jan 11, 2023
1 parent 240c26b commit f55d41f
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 62 deletions.
16 changes: 13 additions & 3 deletions oauth2_http/java/com/google/auth/oauth2/AccessToken.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import com.google.common.base.MoreObjects;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
Expand All @@ -54,7 +55,7 @@ public class AccessToken implements Serializable {
public AccessToken(String tokenValue, Date expirationTime) {
this.tokenValue = tokenValue;
this.expirationTimeMillis = (expirationTime == null) ? null : expirationTime.getTime();
this.scopes = null;
this.scopes = new ArrayList<>();
}

private AccessToken(Builder builder) {
Expand Down Expand Up @@ -135,7 +136,7 @@ public boolean equals(Object obj) {
public static class Builder {
private String tokenValue;
private Date expirationTime;
private List<String> scopes;
private List<String> scopes = new ArrayList<>();

protected Builder() {}

Expand Down Expand Up @@ -163,9 +164,18 @@ public Builder setTokenValue(String tokenValue) {
}

public Builder setScopes(String scopes) {
if (scopes != null) {
if (scopes != null && scopes.trim().length() > 0) {
this.scopes = Arrays.asList(scopes.split(" "));
}
return this;
}

public Builder setScopes(List<String> scopes) {
if (scopes == null) {
this.scopes = new ArrayList<>();
} else {
this.scopes = scopes;
}

return this;
}
Expand Down
1 change: 0 additions & 1 deletion oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
* overriding the state and environment for testing purposes.
*/
class DefaultCredentialsProvider {

static final DefaultCredentialsProvider DEFAULT = new DefaultCredentialsProvider();
static final String CREDENTIAL_ENV_VAR = "GOOGLE_APPLICATION_CREDENTIALS";
static final String QUOTA_PROJECT_ENV_VAR = "GOOGLE_CLOUD_QUOTA_PROJECT";
Expand Down
15 changes: 15 additions & 0 deletions oauth2_http/java/com/google/auth/oauth2/OAuth2Utils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

Expand Down Expand Up @@ -168,6 +169,20 @@ static String validateOptionalString(Map<String, Object> map, String key, String
return (String) value;
}

/** Return the specified list of strings from JSON or throw a helpful error message. */
static List<String> validateOptionalListString(
Map<String, Object> map, String key, String errorPrefix) throws IOException {
Object value = map.get(key);
if (value == null) {
return null;
}
if (!(value instanceof List)) {
throw new IOException(
String.format(VALUE_WRONG_TYPE_MESSAGE, errorPrefix, "List<String>", key));
}
return (List<String>) value;
}

/** Return the specified integer from JSON or throw a helpful error message. */
static int validateInt32(Map<String, Object> map, String key, String errorPrefix)
throws IOException {
Expand Down
15 changes: 7 additions & 8 deletions oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.io.IOException;
import java.net.URI;
import java.net.URL;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Date;
import java.util.List;
Expand Down Expand Up @@ -205,8 +206,8 @@ public UserCredentials getCredentials(String userId) throws IOException {
Long expirationMillis =
OAuth2Utils.validateLong(tokenJson, "expiration_time_millis", TOKEN_STORE_ERROR);
Date expirationTime = new Date(expirationMillis);
String scopes =
OAuth2Utils.validateOptionalString(
List<String> scopes =
OAuth2Utils.validateOptionalListString(
tokenJson, OAuth2Utils.TOKEN_RESPONSE_SCOPE, FETCH_TOKEN_ERROR);
AccessToken accessToken =
AccessToken.newBuilder()
Expand Down Expand Up @@ -362,20 +363,18 @@ public void storeCredentials(String userId, UserCredentials credentials) throws
String acessTokenValue = null;
String scopes = null;
Date expiresBy = null;
List<String> grantedScopes = new ArrayList<>();

if (accessToken != null) {
acessTokenValue = accessToken.getTokenValue();
expiresBy = accessToken.getExpirationTime();
List<String> grantedScopes = accessToken.getScopes();

if (grantedScopes != null) {
scopes = String.join(" ", grantedScopes);
}
grantedScopes = accessToken.getScopes();
}
String refreshToken = credentials.getRefreshToken();
GenericJson tokenStateJson = new GenericJson();
tokenStateJson.setFactory(OAuth2Utils.JSON_FACTORY);
tokenStateJson.put("access_token", acessTokenValue);
tokenStateJson.put(OAuth2Utils.TOKEN_RESPONSE_SCOPE, scopes);
tokenStateJson.put(OAuth2Utils.TOKEN_RESPONSE_SCOPE, grantedScopes);
tokenStateJson.put("expiration_time_millis", expiresBy.getTime());
if (refreshToken != null) {
tokenStateJson.put("refresh_token", refreshToken);
Expand Down
63 changes: 55 additions & 8 deletions oauth2_http/javatests/com/google/auth/oauth2/AccessTokenTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,17 @@

package com.google.auth.oauth2;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand All @@ -49,15 +52,16 @@ public class AccessTokenTest extends BaseSerializationTest {

private static final String TOKEN = "AccessToken";
private static final Date EXPIRATION_DATE = new Date();
private static final String SCOPES = "scope1 scope2";
private static final List<String> SCOPES = Arrays.asList("scope1", "scope2");
private static final String SCOPES_STRING = "scope1 scope2";

@Test
public void constructor() {
AccessToken accessToken = new AccessToken(TOKEN, EXPIRATION_DATE);
assertEquals(TOKEN, accessToken.getTokenValue());
assertEquals(EXPIRATION_DATE, accessToken.getExpirationTime());
assertEquals(EXPIRATION_DATE.getTime(), (long) accessToken.getExpirationTimeMillis());
assertEquals(null, accessToken.getScopes());
assertEquals(new ArrayList<>(), accessToken.getScopes());
}

@Test
Expand All @@ -66,12 +70,41 @@ public void builder() {
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes(SCOPES)
.setScopes(SCOPES_STRING)
.build();
assertEquals(TOKEN, accessToken.getTokenValue());
assertEquals(EXPIRATION_DATE, accessToken.getExpirationTime());
assertEquals(EXPIRATION_DATE.getTime(), (long) accessToken.getExpirationTimeMillis());
assertArrayEquals(SCOPES.split(" "), accessToken.getScopes().toArray());
assertEquals(SCOPES, accessToken.getScopes());
assertNotSame(SCOPES, accessToken.getScopes());

// scopes list
accessToken =
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes(SCOPES)
.build();
assertEquals(SCOPES, accessToken.getScopes());
assertSame(SCOPES, accessToken.getScopes());

// single scope
accessToken =
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes("dummy")
.build();
assertEquals(Arrays.asList("dummy"), accessToken.getScopes());

// empty scope
accessToken =
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes(" ")
.build();
assertEquals(new ArrayList<>(), accessToken.getScopes());
}

@Test
Expand All @@ -87,6 +120,7 @@ public void equals_true() throws IOException {
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setTokenValue(TOKEN)
.setScopes(SCOPES)
.build();

Expand All @@ -107,7 +141,7 @@ public void equals_false_scopes() throws IOException {
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes("scope1")
.setScopes(Arrays.asList("scope1"))
.build();

assertFalse(accessToken.equals(otherAccessToken));
Expand Down Expand Up @@ -165,7 +199,7 @@ public void toString_containsFields() {
String expectedToString =
String.format(
"AccessToken{tokenValue=%s, expirationTimeMillis=%d, scopes=%s}",
TOKEN, EXPIRATION_DATE.getTime(), Arrays.asList(SCOPES.split(" ")));
TOKEN, EXPIRATION_DATE.getTime(), SCOPES);
assertEquals(expectedToString, accessToken.toString());
}

Expand All @@ -190,14 +224,27 @@ public void hashCode_equals() throws IOException {

@Test
public void serialize() throws IOException, ClassNotFoundException {
AccessToken emptyScopes =
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes("")
.build();

AccessToken deserializedAccessToken = serializeAndDeserialize(emptyScopes);
assertEquals(emptyScopes, deserializedAccessToken);
assertEquals(emptyScopes.hashCode(), deserializedAccessToken.hashCode());
assertEquals(emptyScopes.toString(), deserializedAccessToken.toString());
assertEquals(new ArrayList<>(), deserializedAccessToken.getScopes());

AccessToken accessToken =
AccessToken.newBuilder()
.setExpirationTime(EXPIRATION_DATE)
.setTokenValue(TOKEN)
.setScopes(SCOPES)
.build();

AccessToken deserializedAccessToken = serializeAndDeserialize(accessToken);
deserializedAccessToken = serializeAndDeserialize(accessToken);
assertEquals(accessToken, deserializedAccessToken);
assertEquals(accessToken.hashCode(), deserializedAccessToken.hashCode());
assertEquals(accessToken.toString(), deserializedAccessToken.toString());
Expand Down
Loading

0 comments on commit f55d41f

Please sign in to comment.