feat: adds support for 3PI credentials #464
Conversation
…le/url based external credentials
google-cla
bot
added
the
cla: yes
lsirac
changed the title
|
|
||
| private static final JsonFactory JSON_FACTORY = JacksonFactory.getDefaultInstance(); | ||
|
|
||
| public static final String UTF_8 = "UTF-8"; |
There was a problem hiding this comment.
Please see comments on original PR
oauth2_http/javatests/com/google/auth/oauth2/IdentityPoolCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java
Outdated
Show resolved
Hide resolved
| subjectTokenType, | ||
| tokenUrl, | ||
| tokenInfoUrl, | ||
| new IdentityPoolCredentialSource(credentialSourceMap), |
There was a problem hiding this comment.
Should knowledge of this inner class belong only to the IdentityPoolCredentials?
| if (serviceAccountImpersonationUrl == null) { | ||
| return accessToken; | ||
| } |
There was a problem hiding this comment.
Would a caller of this need to know if this was skipped?
There was a problem hiding this comment.
What do you mean? Are you suggesting I log something here or?
There was a problem hiding this comment.
This is an internal method so it probably doesn't actually matter.
I was just wondering if somewhere that calls this method might want to conditionally do something else if we did not actually attempt impersonation.
| } else { | ||
| credentialLocation = (String) credentialSourceMap.get("url"); | ||
| credentialSourceType = IdentityPoolCredentialSourceType.URL; | ||
| } |
There was a problem hiding this comment.
Should we fail early if this input is invalid and doesn't contain file or url?
There was a problem hiding this comment.
Good question, I'm not sure. The input here should be from a file generated by gCloud, and should be valid.
oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java
Outdated
Show resolved
Hide resolved
chingor13
dismissed
elharo’s stale review
We can address constant nits in the final PR to master
No description provided.