Skip to content

Commit 86e5ee1

Browse files
committed
chore: Address PR comments
1 parent 205031e commit 86e5ee1

13 files changed

+56
-34
lines changed

oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ public class ComputeEngineCredentials extends GoogleCredentials
128128
private final BindingEnforcement bindingEnforcement;
129129

130130
private transient HttpTransportFactory transportFactory;
131-
private transient String serviceAccountEmail;
132131

133132
private String universeDomainFromMetadata = null;
134133

@@ -346,9 +345,8 @@ private String getUniverseDomainFromMetadata() throws IOException {
346345
@Override
347346
public AccessToken refreshAccessToken() throws IOException {
348347
// Retrieve the default service account email prior to retrieving the access token
349-
if (serviceAccountEmail == null) {
350-
serviceAccountEmail = getDefaultServiceAccount();
351-
principal = serviceAccountEmail;
348+
if (principal == null) {
349+
principal = getDefaultServiceAccount();
352350
}
353351

354352
HttpResponse response =
@@ -695,14 +693,14 @@ public static Builder newBuilder() {
695693
@Override
696694
// todo(#314) getAccount should not throw a RuntimeException
697695
public String getAccount() {
698-
if (serviceAccountEmail == null) {
696+
if (principal == null) {
699697
try {
700-
serviceAccountEmail = getDefaultServiceAccount();
698+
principal = getDefaultServiceAccount();
701699
} catch (IOException ex) {
702700
throw new RuntimeException("Failed to get service account", ex);
703701
}
704702
}
705-
return serviceAccountEmail;
703+
return principal;
706704
}
707705

708706
/**

oauth2_http/java/com/google/auth/oauth2/DefaultCredentialsProvider.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,10 @@ private final GoogleCredentials tryGetComputeCredentials(HttpTransportFactory tr
311311
}
312312
boolean runningOnComputeEngine = ComputeEngineCredentials.isOnGce(transportFactory, this);
313313
checkedComputeEngine = true;
314-
ComputeEngineCredentials.Builder builder = ComputeEngineCredentials.newBuilder();
315314
if (runningOnComputeEngine) {
316-
return builder.setHttpTransportFactory(transportFactory).build();
315+
return ComputeEngineCredentials.newBuilder()
316+
.setHttpTransportFactory(transportFactory)
317+
.build();
317318
}
318319
return null;
319320
}

oauth2_http/java/com/google/auth/oauth2/GoogleCredentials.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import com.google.common.base.MoreObjects.ToStringHelper;
4343
import com.google.common.base.Strings;
4444
import com.google.common.collect.ImmutableList;
45+
import com.google.common.collect.ImmutableMap;
4546
import com.google.errorprone.annotations.CanIgnoreReturnValue;
4647
import java.io.IOException;
4748
import java.io.InputStream;
@@ -76,7 +77,7 @@ enum GoogleCredentialsInfo {
7677
COMPUTE_ENGINE_CREDENTIALS("Compute Engine Credentials", null);
7778

7879
private final String credentialName;
79-
private final String fileType;
80+
@Nullable private final String fileType;
8081

8182
GoogleCredentialsInfo(String credentialName, String fileType) {
8283
this.credentialName = credentialName;
@@ -98,7 +99,8 @@ String getFileType() {
9899
// User-friendly name of the Credential class
99100
String name;
100101
// Identity of the credential
101-
String principal;
102+
// Note: This field is marked transient to prevent it from being serialized
103+
transient String principal;
102104

103105
private final String universeDomain;
104106
private final boolean isExplicitUniverseDomain;
@@ -554,9 +556,11 @@ GoogleCredentials withSource(String source) {
554556
* <li>principal - Identity used for the credential
555557
* </ul>
556558
*
557-
* Missing fields values are not included in the mapping
559+
* Unknown field values are not included in the mapping (e.g. ComputeCredentials may not know the
560+
* principal value until after a call to MDS is made and will be excluded if `getCredentialInfo`
561+
* is called prior to retrieving that value)
558562
*
559-
* @return Map of information regarding how the Credential was initialized
563+
* @return ImmutableMap of information regarding how the Credential was initialized
560564
*/
561565
public Map<String, String> getCredentialInfo() {
562566
Map<String, String> infoMap = new HashMap<>();
@@ -569,7 +573,7 @@ public Map<String, String> getCredentialInfo() {
569573
if (!Strings.isNullOrEmpty(principal)) {
570574
infoMap.put("Principal", principal);
571575
}
572-
return infoMap;
576+
return ImmutableMap.copyOf(infoMap);
573577
}
574578

575579
public static class Builder extends OAuth2Credentials.Builder {

oauth2_http/java/com/google/auth/oauth2/ServiceAccountJwtAccessCredentials.java

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import com.google.auth.Credentials;
4242
import com.google.auth.RequestMetadataCallback;
4343
import com.google.auth.ServiceAccountSigner;
44+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
4445
import com.google.common.annotations.VisibleForTesting;
4546
import com.google.common.base.MoreObjects;
4647
import com.google.common.base.Throwables;
@@ -296,17 +297,14 @@ public static ServiceAccountJwtAccessCredentials fromStream(
296297
if (fileType == null) {
297298
throw new IOException("Error reading credentials from stream, 'type' field not specified.");
298299
}
299-
if (GoogleCredentials.GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS
300-
.getFileType()
301-
.equals(fileType)) {
300+
if (GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS.getFileType().equals(fileType)) {
302301
return fromJson(fileContents, defaultAudience);
303302
}
304303
throw new IOException(
305304
String.format(
306305
"Error reading credentials from stream, 'type' value '%s' not recognized."
307306
+ " Expecting '%s'.",
308-
fileType,
309-
GoogleCredentials.GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS.getFileType()));
307+
fileType, GoogleCredentialsInfo.SERVICE_ACCOUNT_CREDENTIALS.getFileType()));
310308
}
311309

312310
private LoadingCache<JwtClaims, JwtCredentials> createCache() {

oauth2_http/java/com/google/auth/oauth2/UserCredentials.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ private UserCredentials(Builder builder) {
9696
this.tokenServerUri =
9797
(builder.tokenServerUri == null) ? OAuth2Utils.TOKEN_SERVER_URI : builder.tokenServerUri;
9898
this.transportFactoryClassName = this.transportFactory.getClass().getName();
99+
if (builder.getAccount() != null) {
100+
this.principal = builder.getAccount();
101+
}
99102

100103
this.name = GoogleCredentialsInfo.USER_CREDENTIALS.getCredentialName();
101104

@@ -132,6 +135,10 @@ static UserCredentials fromJson(Map<String, Object> json, HttpTransportFactory t
132135
// currently "token_uri" is not a default field and needs to be added to json file manually
133136
String tokenUrl = (String) json.get("token_uri");
134137
URI tokenUri = (tokenUrl == null || tokenUrl.isEmpty()) ? null : URI.create(tokenUrl);
138+
String account = null;
139+
if (json.containsKey("account")) {
140+
account = (String) json.get("account");
141+
}
135142
return UserCredentials.newBuilder()
136143
.setClientId(clientId)
137144
.setClientSecret(clientSecret)
@@ -140,6 +147,7 @@ static UserCredentials fromJson(Map<String, Object> json, HttpTransportFactory t
140147
.setHttpTransportFactory(transportFactory)
141148
.setTokenServerUri(tokenUri)
142149
.setQuotaProjectId(quotaProjectId)
150+
.setAccount(account)
143151
.build();
144152
}
145153

@@ -409,6 +417,7 @@ public static class Builder extends GoogleCredentials.Builder {
409417
private String clientSecret;
410418
private String refreshToken;
411419
private URI tokenServerUri;
420+
private String account;
412421
private HttpTransportFactory transportFactory;
413422

414423
protected Builder() {}
@@ -420,6 +429,7 @@ protected Builder(UserCredentials credentials) {
420429
this.refreshToken = credentials.refreshToken;
421430
this.transportFactory = credentials.transportFactory;
422431
this.tokenServerUri = credentials.tokenServerUri;
432+
this.account = credentials.account;
423433
}
424434

425435
@CanIgnoreReturnValue
@@ -452,6 +462,12 @@ public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) {
452462
return this;
453463
}
454464

465+
@CanIgnoreReturnValue
466+
Builder setAccount(String account) {
467+
this.account = account;
468+
return this;
469+
}
470+
455471
@Override
456472
@CanIgnoreReturnValue
457473
public Builder setAccessToken(AccessToken token) {
@@ -496,6 +512,10 @@ public URI getTokenServerUri() {
496512
return tokenServerUri;
497513
}
498514

515+
String getAccount() {
516+
return account;
517+
}
518+
499519
public HttpTransportFactory getHttpTransportFactory() {
500520
return transportFactory;
501521
}

oauth2_http/javatests/com/google/auth/oauth2/AwsCredentialsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.google.api.client.util.Clock;
4646
import com.google.auth.TestUtils;
4747
import com.google.auth.oauth2.ExternalAccountCredentialsTest.MockExternalAccountCredentialsTransportFactory;
48+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
4849
import java.io.IOException;
4950
import java.io.InputStream;
5051
import java.net.URI;
@@ -1346,8 +1347,7 @@ static InputStream writeAwsCredentialsStream(String stsUrl, String regionUrl, St
13461347
json.put("subject_token_type", "subjectTokenType");
13471348
json.put("token_url", stsUrl);
13481349
json.put("token_info_url", "tokenInfoUrl");
1349-
json.put(
1350-
"type", GoogleCredentials.GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
1350+
json.put("type", GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
13511351

13521352
GenericJson credentialSource = new GenericJson();
13531353
credentialSource.put("environment_id", "aws1");

oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountAuthorizedUserCredentialsTest.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.google.auth.TestUtils;
4848
import com.google.auth.http.AuthHttpConstants;
4949
import com.google.auth.http.HttpTransportFactory;
50+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
5051
import com.google.common.collect.ImmutableList;
5152
import com.google.common.collect.ImmutableMap;
5253
import com.google.common.io.BaseEncoding;
@@ -1241,9 +1242,7 @@ public void serialize() throws IOException, ClassNotFoundException {
12411242
static GenericJson buildJsonCredentials() {
12421243
GenericJson json = new GenericJson();
12431244
json.put(
1244-
"type",
1245-
GoogleCredentials.GoogleCredentialsInfo.EXTERNAL_ACCOUNT_AUTHORIZED_USER_CREDENTIALS
1246-
.getFileType());
1245+
"type", GoogleCredentialsInfo.EXTERNAL_ACCOUNT_AUTHORIZED_USER_CREDENTIALS.getFileType());
12471246
json.put("audience", AUDIENCE);
12481247
json.put("refresh_token", REFRESH_TOKEN);
12491248
json.put("client_id", CLIENT_ID);

oauth2_http/javatests/com/google/auth/oauth2/GdchCredentialsTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.google.api.client.testing.http.FixedClock;
4747
import com.google.api.client.util.Clock;
4848
import com.google.auth.TestUtils;
49+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
4950
import java.io.File;
5051
import java.io.IOException;
5152
import java.io.InputStream;
@@ -915,7 +916,7 @@ static GenericJson writeGdchServiceAccountJson(
915916
if (tokenServerUri != null) {
916917
json.put("token_uri", tokenServerUri.toString());
917918
}
918-
json.put("type", GoogleCredentials.GoogleCredentialsInfo.GDCH_CREDENTIALS.getFileType());
919+
json.put("type", GoogleCredentialsInfo.GDCH_CREDENTIALS.getFileType());
919920
return json;
920921
}
921922

oauth2_http/javatests/com/google/auth/oauth2/IdentityPoolCredentialsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import com.google.auth.TestUtils;
4545
import com.google.auth.http.HttpTransportFactory;
4646
import com.google.auth.mtls.X509Provider;
47+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
4748
import java.io.ByteArrayInputStream;
4849
import java.io.File;
4950
import java.io.IOException;
@@ -1197,8 +1198,7 @@ static InputStream writeIdentityPoolCredentialsStream(
11971198
json.put("subject_token_type", "subjectTokenType");
11981199
json.put("token_url", tokenUrl);
11991200
json.put("token_info_url", "tokenInfoUrl");
1200-
json.put(
1201-
"type", GoogleCredentials.GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
1201+
json.put("type", GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
12021202

12031203
if (serviceAccountImpersonationUrl != null) {
12041204
json.put("service_account_impersonation_url", serviceAccountImpersonationUrl);

oauth2_http/javatests/com/google/auth/oauth2/PluggableAuthCredentialsTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.google.auth.TestUtils;
4141
import com.google.auth.http.HttpTransportFactory;
4242
import com.google.auth.oauth2.ExecutableHandler.ExecutableOptions;
43+
import com.google.auth.oauth2.GoogleCredentials.GoogleCredentialsInfo;
4344
import java.io.IOException;
4445
import java.io.InputStream;
4546
import java.io.NotSerializableException;
@@ -628,8 +629,7 @@ static InputStream writeCredentialsStream(String tokenUrl) throws IOException {
628629
json.put("subject_token_type", "subjectTokenType");
629630
json.put("token_url", tokenUrl);
630631
json.put("token_info_url", "tokenInfoUrl");
631-
json.put(
632-
"type", GoogleCredentials.GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
632+
json.put("type", GoogleCredentialsInfo.EXTERNAL_ACCOUNT_CREDENTIALS.getFileType());
633633

634634
GenericJson credentialSource = new GenericJson();
635635
GenericJson executable = new GenericJson();

0 commit comments

Comments
 (0)