Skip to content

fix: respect token_uri from json in UserCredentials creation. #1630

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.

Sign up for GitHub

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

Merged
zhumin8 merged 3 commits into from
Feb 3, 2025
Merged

fix: respect token_uri from json in UserCredentials creation. #1630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04cf1c2
fix: respect token_uri from json in UserCredentials creation.
zhumin8 Jan 24, 2025
47e1b53
fix format
zhumin8 Jan 24, 2025
f7930a7
add comment.
zhumin8 Jan 28, 2025
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
5 changes: 4 additions & 1 deletion oauth2_http/java/com/google/auth/oauth2/UserCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,16 @@ static UserCredentials fromJson(Map<String, Object> json, HttpTransportFactory t
"Error reading user credential from JSON, "
+ " expecting 'client_id', 'client_secret' and 'refresh_token'.");
}
// currently "token_uri" is not a default field and needs to be added to json file manually
String tokenUrl = (String) json.get("token_uri");
URI tokenUri = (tokenUrl == null || tokenUrl.isEmpty()) ? null : URI.create(tokenUrl);
Comment on lines +128 to +129
Copy link
Member

@lqiu96 lqiu96 Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qq, is this a value that exists? I was looking at my credentials.json file in the well known ADC path and I don't think I see an entry for tokenUri. Perhaps I needed additional flags for gcloud application-default login?

Copy link
Contributor Author

@zhumin8 zhumin8 Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this exists in the standard credentials.json file. Rather iiuc, we are providing a workaround that if user edit their json file and add this field, we use value from it. (ref: http://b/382442857#comment73)

Copy link
Member

@lqiu96 lqiu96 Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add that as a quick comment above? I'm not entirely sure that this is going to be permanent/ how long we intend to support this.

And given the prior stuff about malicious URIs, perhaps a warning about modifying the token_uri/ using a JSON with a malicious token_uri?

Copy link
Contributor Author

@zhumin8 zhumin8 Jan 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Although in this case, user has to explicitly provide this. @sai-sunder-s do we need to add similar warning here?

Copy link
Contributor

@sai-sunder-s sai-sunder-s Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think we need a warning here because even if url is malicious, we will be sending it a token that is built purely using the data in the json.

return UserCredentials.newBuilder()
.setClientId(clientId)
.setClientSecret(clientSecret)
.setRefreshToken(refreshToken)
.setAccessToken(null)
.setHttpTransportFactory(transportFactory)
.setTokenServerUri(null)
.setTokenServerUri(tokenUri)
.setQuotaProjectId(quotaProjectId)
.build();
}
Expand Down
39 changes: 35 additions & 4 deletions oauth2_http/javatests/com/google/auth/oauth2/UserCredentialsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,44 @@ public void fromJson_hasAccessToken() throws IOException {
MockTokenServerTransportFactory transportFactory = new MockTokenServerTransportFactory();
transportFactory.transport.addClient(CLIENT_ID, CLIENT_SECRET);
transportFactory.transport.addRefreshToken(REFRESH_TOKEN, ACCESS_TOKEN);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, null);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, null, null);

GoogleCredentials credentials = UserCredentials.fromJson(json, transportFactory);

Map<String, List<String>> metadata = credentials.getRequestMetadata(CALL_URI);
TestUtils.assertContainsBearerToken(metadata, ACCESS_TOKEN);
}

@Test
public void fromJson_hasTokenUri() throws IOException {
String tokenUrl = "token.url.xyz";
MockTokenServerTransportFactory transportFactory = new MockTokenServerTransportFactory();
transportFactory.transport.addClient(CLIENT_ID, CLIENT_SECRET);
transportFactory.transport.addRefreshToken(REFRESH_TOKEN, ACCESS_TOKEN);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, null, tokenUrl);

UserCredentials credentials = UserCredentials.fromJson(json, transportFactory);
assertEquals(URI.create(tokenUrl), credentials.toBuilder().getTokenServerUri());
}

@Test
public void fromJson_emptyTokenUri() throws IOException {
String tokenUrl = "";
MockTokenServerTransportFactory transportFactory = new MockTokenServerTransportFactory();
transportFactory.transport.addClient(CLIENT_ID, CLIENT_SECRET);
transportFactory.transport.addRefreshToken(REFRESH_TOKEN, ACCESS_TOKEN);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, null, tokenUrl);

UserCredentials credentials = UserCredentials.fromJson(json, transportFactory);
assertEquals(OAuth2Utils.TOKEN_SERVER_URI, credentials.toBuilder().getTokenServerUri());
}

@Test
public void fromJson_hasQuotaProjectId() throws IOException {
MockTokenServerTransportFactory transportFactory = new MockTokenServerTransportFactory();
transportFactory.transport.addClient(CLIENT_ID, CLIENT_SECRET);
transportFactory.transport.addRefreshToken(REFRESH_TOKEN, ACCESS_TOKEN);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, QUOTA_PROJECT);
GenericJson json = writeUserJson(CLIENT_ID, CLIENT_SECRET, REFRESH_TOKEN, QUOTA_PROJECT, null);

GoogleCredentials credentials = UserCredentials.fromJson(json, transportFactory);

Expand Down Expand Up @@ -865,7 +889,11 @@ public void userCredentials_toBuilder_copyEveryAttribute() {
}

static GenericJson writeUserJson(
String clientId, String clientSecret, String refreshToken, String quotaProjectId) {
String clientId,
String clientSecret,
String refreshToken,
String quotaProjectId,
String tokenUrl) {
GenericJson json = new GenericJson();
if (clientId != null) {
json.put("client_id", clientId);
Expand All @@ -879,14 +907,17 @@ static GenericJson writeUserJson(
if (quotaProjectId != null) {
json.put("quota_project_id", quotaProjectId);
}
if (tokenUrl != null) {
json.put("token_uri", tokenUrl);
}
json.put("type", GoogleCredentials.USER_FILE_TYPE);
return json;
}

static InputStream writeUserStream(
String clientId, String clientSecret, String refreshToken, String quotaProjectId)
throws IOException {
GenericJson json = writeUserJson(clientId, clientSecret, refreshToken, quotaProjectId);
GenericJson json = writeUserJson(clientId, clientSecret, refreshToken, quotaProjectId, null);
return TestUtils.jsonToInputStream(json);
}

Expand Down
Loading