FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(auth): fix remaining nullability in UserAuthorizer and Builder by lqiu96 · Pull Request #14158 · googleapis/google-cloud-java · GitHub

/ google-cloud-java Public

fix(auth): fix remaining nullability in UserAuthorizer and Builder #14158

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
lqiu96 merged 10 commits into from
Aug 21, 2026
Conversation 13 Commits 10 Checks 284 Files changed
Merged

fix(auth): fix remaining nullability in UserAuthorizer and Builder #14158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
04d869c
fix(auth): fix remaining nullability in UserAuthorizer and Builder
lqiu96 Aug 20, 2026
5473d28
chore: Update google-auth-library-java/oauth2_http/java/com/google/au…
lqiu96 Aug 20, 2026
aa5cdf9
fix(auth): fix extra closing brace in UserAuthorizer
lqiu96 Aug 20, 2026
4415d89
fix(auth): initialize default values in UserAuthorizer.Builder
lqiu96 Aug 20, 2026
aaf1235
chore: Add static field
lqiu96 Aug 20, 2026
9d91637
fix(auth): allow nullable setters in UserAuthorizer.Builder
lqiu96 Aug 20, 2026
7fcbf4a
fix(auth): make UserAuthorizer.Builder setters non-null without runti…
lqiu96 Aug 20, 2026
c74668d
test(auth): remove redundant builder nullness and copying unit tests
lqiu96 Aug 20, 2026
670cc5e
chore: Remove unnessary clientid non-null check
lqiu96 Aug 21, 2026
4483370
Merge branch 'fix_auth_user_authorizer_nullability' of github.com:goo…
lqiu96 Aug 21, 2026
File filter

Filter by extension

Filter by extension .java  (3) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
      • ClientId.java
      • UserAuthorizer.java
2 changes: 1 addition & 1 deletion google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/ClientId.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static ClientId fromJson(Map<String, Object> json) throws IOException {
@SuppressWarnings("unchecked")
Map<String, Object> detail = (Map<String, Object>) rawDetail;
String clientId = OAuth2Utils.validateString(detail, FIELD_CLIENT_ID, JSON_PARSE_ERROR);
if (clientId == null || clientId.length() == 0) {
if (clientId.isEmpty()) {

lqiu96 Aug 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

OAuth2Utils.validateString ensures that it's non-null
throw new IOException(
"Unable to parse ClientId. Field '" + FIELD_CLIENT_ID + "' is required.");
}
Expand Down
88 changes: 44 additions & 44 deletions google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
import com.google.api.client.json.JsonObjectParser;
import com.google.api.client.util.GenericData;
import com.google.api.client.util.Joiner;
import com.google.api.client.util.Preconditions;
import com.google.auth.http.HttpTransportFactory;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.io.IOException;
Expand Down Expand Up @@ -76,8 +76,8 @@ public enum ClientAuthenticationType {

static final URI DEFAULT_CALLBACK_URI = URI.create("/oauth2callback");

private final String TOKEN_STORE_ERROR = "Error parsing stored token data.";
private final String FETCH_TOKEN_ERROR = "Error reading result of Token API:";
private static final String TOKEN_STORE_ERROR = "Error parsing stored token data.";
private static final String FETCH_TOKEN_ERROR = "Error reading result of Token API:";

private final ClientId clientId;
private final Collection<String> scopes;
Expand Down Expand Up @@ -249,7 +249,7 @@ public URL getAuthorizationUrl(
GenericJson tokenJson = OAuth2Utils.parseJson(tokenData);
String accessTokenValue =
OAuth2Utils.validateString(tokenJson, "access_token", TOKEN_STORE_ERROR);
Long expirationMillis =
long expirationMillis =
OAuth2Utils.validateLong(tokenJson, "expiration_time_millis", TOKEN_STORE_ERROR);
Date expirationTime = new Date(expirationMillis);
Comment thread
lqiu96 marked this conversation as resolved.
List<String> scopes =
Expand Down Expand Up @@ -414,21 +414,23 @@ public void revokeAuthorization(String userId) throws IOException {
*/
public void storeCredentials(String userId, UserCredentials credentials) throws IOException {
AccessToken accessToken = credentials.getAccessToken();
String acessTokenValue = null;
String accessTokenValue = null;
Date expiresBy = null;
List<String> grantedScopes = new ArrayList<>();

if (accessToken != null) {
acessTokenValue = accessToken.getTokenValue();
accessTokenValue = accessToken.getTokenValue();
expiresBy = accessToken.getExpirationTime();
grantedScopes = accessToken.getScopes();
}
String refreshToken = credentials.getRefreshToken();
GenericJson tokenStateJson = new GenericJson();
tokenStateJson.setFactory(OAuth2Utils.JSON_FACTORY);
tokenStateJson.put("access_token", acessTokenValue);
tokenStateJson.put("access_token", accessTokenValue);
tokenStateJson.put(OAuth2Utils.TOKEN_RESPONSE_SCOPE, grantedScopes);
tokenStateJson.put("expiration_time_millis", expiresBy.getTime());
if (expiresBy != null) {
tokenStateJson.put("expiration_time_millis", expiresBy.getTime());
}
Comment on lines +431 to +433

lqiu96 Aug 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

we can't ensure that expiresBy is non-null because accessToken may null here
if (refreshToken != null) {
tokenStateJson.put("refresh_token", refreshToken);
}
Expand Down Expand Up @@ -498,7 +500,7 @@ private TokenResponseWithConfig getCredentialsFromCodeInternal(
String accessTokenValue =
OAuth2Utils.validateString(parsedTokens, "access_token", FETCH_TOKEN_ERROR);
int expiresInSecs = OAuth2Utils.validateInt32(parsedTokens, "expires_in", FETCH_TOKEN_ERROR);
Date expirationTime = new Date(new Date().getTime() + expiresInSecs * 1000);
Date expirationTime = new Date(new Date().getTime() + expiresInSecs * 1000L);
String scopes =
OAuth2Utils.validateOptionalString(
parsedTokens, OAuth2Utils.TOKEN_RESPONSE_SCOPE, FETCH_TOKEN_ERROR);
Expand Down Expand Up @@ -551,15 +553,15 @@ public Builder toBuilder() {

public static class Builder {

private ClientId clientId;
private TokenStore tokenStore;
private URI callbackUri;
private URI tokenServerUri;
private URI userAuthUri;
private Collection<String> scopes;
private HttpTransportFactory transportFactory;
private @Nullable ClientId clientId;
private @Nullable TokenStore tokenStore;
private @Nullable URI callbackUri;
private @Nullable URI tokenServerUri;
private @Nullable URI userAuthUri;
private @Nullable Collection<String> scopes;
private @Nullable HttpTransportFactory transportFactory;
private @Nullable PKCEProvider pkce;
private ClientAuthenticationType clientAuthenticationType;
private @Nullable ClientAuthenticationType clientAuthenticationType;

protected Builder() {}

Expand Down Expand Up @@ -674,7 +676,6 @@ public Builder setPKCEProvider(@Nullable PKCEProvider pkce) {
if (pkce.getCodeChallenge() == null
|| pkce.getCodeVerifier() == null
|| pkce.getCodeChallengeMethod() == null) {

throw new IllegalArgumentException(
"PKCE provider contained null implementations. PKCE object must implement all"
+ " PKCEProvider methods.");
Expand All @@ -698,39 +699,39 @@ public Builder setClientAuthenticationType(ClientAuthenticationType clientAuthen
return this;
}

public ClientId getClientId() {
public @Nullable ClientId getClientId() {
return clientId;
}

public TokenStore getTokenStore() {
public @Nullable TokenStore getTokenStore() {
return tokenStore;
}

public Collection<String> getScopes() {
public @Nullable Collection<String> getScopes() {
return scopes;
}

public URI getTokenServerUri() {
public @Nullable URI getTokenServerUri() {
return tokenServerUri;
}

public URI getCallbackUri() {
public @Nullable URI getCallbackUri() {
return callbackUri;
}

public URI getUserAuthUri() {
public @Nullable URI getUserAuthUri() {
return userAuthUri;
}

public HttpTransportFactory getHttpTransportFactory() {
public @Nullable HttpTransportFactory getHttpTransportFactory() {
return transportFactory;
}

public @Nullable PKCEProvider getPKCEProvider() {
return pkce;
}

public ClientAuthenticationType getClientAuthenticationType() {
public @Nullable ClientAuthenticationType getClientAuthenticationType() {
return clientAuthenticationType;
}

Expand Down Expand Up @@ -771,18 +772,18 @@ public UserAuthorizer build() {
public static class TokenResponseWithConfig {

private final String clientId;
private final String clientSecret;
private final String refreshToken;
private final @Nullable String clientSecret;
private final @Nullable String refreshToken;
private final AccessToken accessToken;
private URI tokenServerUri;
private final URI tokenServerUri;
private final HttpTransportFactory httpTransportFactory;

private TokenResponseWithConfig(Builder builder) {
this.clientId = builder.clientId;
this.clientId = Preconditions.checkNotNull(builder.clientId);
this.clientSecret = builder.clientSecret;
this.accessToken = builder.accessToken;
this.httpTransportFactory = builder.httpTransportFactory;
this.tokenServerUri = builder.tokenServerUri;
this.accessToken = Preconditions.checkNotNull(builder.accessToken);
this.httpTransportFactory = Preconditions.checkNotNull(builder.httpTransportFactory);
this.tokenServerUri = Preconditions.checkNotNull(builder.tokenServerUri);
Comment on lines +782 to +786

lqiu96 Aug 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

these are passed in from UserAuthorizer which enforces them to be non-null

lqiu96 Aug 21, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

TokenResponseWithConfig#Builder's setters are all package-private and internal usages of this ensures that clientId, accessToken, httpTransportFactory, and tokenServerUri are non-null (default value is created)
this.refreshToken = builder.refreshToken;
}

Expand All @@ -800,7 +801,7 @@ public String getClientId() {
*
* @return The client secret.
*/
public String getClientSecret() {
public @Nullable String getClientSecret() {
return clientSecret;
}

Expand Down Expand Up @@ -836,8 +837,7 @@ public URI getTokenServerUri() {
*
* @return The refresh token, or null if not granted.
*/
@Nullable
public String getRefreshToken() {
public @Nullable String getRefreshToken() {
Comment on lines 804 to +840

lqiu96 Aug 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Turns out Oauth2Utils#validateOptionalString can return null if it doesn't exist. So these two fields use it and it can be null
return refreshToken;
}

Expand All @@ -846,12 +846,12 @@ static Builder newBuilder() {
}

static class Builder {
private String clientId;
private String clientSecret;
private String refreshToken;
private AccessToken accessToken;
private URI tokenServerUri;
private HttpTransportFactory httpTransportFactory;
private @Nullable String clientId;
private @Nullable String clientSecret;
private @Nullable String refreshToken;
private @Nullable AccessToken accessToken;
private @Nullable URI tokenServerUri;
private @Nullable HttpTransportFactory httpTransportFactory;

@CanIgnoreReturnValue
Builder setClientId(String clientId) {
Expand All @@ -860,13 +860,13 @@ Builder setClientId(String clientId) {
}

@CanIgnoreReturnValue
Builder setClientSecret(String clientSecret) {
Builder setClientSecret(@Nullable String clientSecret) {
this.clientSecret = clientSecret;
return this;
}

@CanIgnoreReturnValue
Builder setRefreshToken(String refreshToken) {
Builder setRefreshToken(@Nullable String refreshToken) {
this.refreshToken = refreshToken;
return this;
}
Expand Down
60 changes: 60 additions & 0 deletions ...le-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -946,4 +946,64 @@ void testTokenResponseWithConfig_noRefreshToken() {
assertEquals(httpTransportFactory, tokenResponse.getHttpTransportFactory());
assertNull(tokenResponse.getRefreshToken());
}

@Test
void testTokenResponseWithConfig_noClientSecret() {
String clientId = "testClientId";
AccessToken accessToken = new AccessToken("token", new Date());
URI tokenServerUri = URI.create("https://example.com/token");
HttpTransportFactory httpTransportFactory = new MockTokenServerTransportFactory();

TokenResponseWithConfig tokenResponse =
TokenResponseWithConfig.newBuilder()
.setClientId(clientId)
.setClientSecret(null)
.setAccessToken(accessToken)
.setTokenServerUri(tokenServerUri)
.setHttpTransportFactory(httpTransportFactory)
.build();

assertEquals(clientId, tokenResponse.getClientId());
assertNull(tokenResponse.getClientSecret());
assertEquals(accessToken, tokenResponse.getAccessToken());
assertEquals(tokenServerUri, tokenResponse.getTokenServerUri());
assertEquals(httpTransportFactory, tokenResponse.getHttpTransportFactory());
assertNull(tokenResponse.getRefreshToken());
}

@Test
void build_withDefaults() {
UserAuthorizer authorizer =
UserAuthorizer.newBuilder().setClientId(CLIENT_ID).setScopes(DUMMY_SCOPES).build();

assertNotNull(authorizer.getTokenStore());
assertEquals(DUMMY_SCOPES, authorizer.getScopes());
assertEquals(UserAuthorizer.DEFAULT_CALLBACK_URI, authorizer.getCallbackUri());
assertEquals(
UserAuthorizer.ClientAuthenticationType.CLIENT_SECRET_POST,
authorizer.getClientAuthenticationType());
}

@Test
void storeCredentials_nullAccessToken() throws IOException {
TokenStore store = new MemoryTokensStorage();
UserAuthorizer authorizer =
UserAuthorizer.newBuilder()
.setClientId(CLIENT_ID)
.setScopes(DUMMY_SCOPES)
.setTokenStore(store)
.build();

UserCredentials credentials =
UserCredentials.newBuilder()
.setClientId(CLIENT_ID_VALUE)
.setClientSecret(CLIENT_SECRET)
.setRefreshToken(REFRESH_TOKEN)
.build();

authorizer.storeCredentials(USER_ID, credentials);
String loaded = store.load(USER_ID);
assertNotNull(loaded);
assertTrue(loaded.contains(REFRESH_TOKEN));
}
Comment thread
lqiu96 marked this conversation as resolved.
}
Loading
Back | FazBrowse Home | New Git URL