Skip to content

fix(auth): fix JSpecify nullability in UserAuthorizer and TokenStore #14150

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 2 commits into from
Aug 20, 2026
Merged

fix(auth): fix JSpecify nullability in UserAuthorizer and TokenStore #14150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ece0440
fix(auth): fix JSpecify nullability in UserAuthorizer and TokenStore
lqiu96 Aug 20, 2026
f402ea7
chore: Apply suggestions from code review
lqiu96 Aug 20, 2026
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
3 changes: 2 additions & 1 deletion google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/MemoryTokensStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@
import java.util.HashMap;
import java.util.Map;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/** Represents an in-memory storage of tokens. */
@NullMarked
public class MemoryTokensStorage implements TokenStore {
private final Map<String, String> tokensStorage = new HashMap<>();

@Override
public String load(String id) throws IOException {
public @Nullable String load(String id) throws IOException {
return tokensStorage.get(id);
}

Expand Down
3 changes: 2 additions & 1 deletion google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/TokenStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import java.io.IOException;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/** Interface for long term storage of tokens */
@NullMarked
Expand All @@ -45,7 +46,7 @@ public interface TokenStore {
* @return The loaded token data.
* @throws IOException An error loading the token data from storage.
*/
String load(String id) throws IOException;
@Nullable String load(String id) throws IOException;

/**
* Put the token data into storage for the given ID.
Expand Down
70 changes: 32 additions & 38 deletions google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -86,7 +87,7 @@ public enum ClientAuthenticationType {
private final HttpTransportFactory transportFactory;
private final URI tokenServerUri;
private final URI userAuthUri;
private final PKCEProvider pkce;
private final @Nullable PKCEProvider pkce;
private final ClientAuthenticationType clientAuthenticationType;

/** Internal constructor. See {@link Builder}. */
Expand Down Expand Up @@ -147,7 +148,7 @@ public URI getCallbackUri() {
* @param baseUri The URI to resolve the callback URI relative to.
* @return The resolved URI.
*/
public URI getCallbackUri(URI baseUri) {
public URI getCallbackUri(@Nullable URI baseUri) {
if (callbackUri.isAbsolute()) {
return callbackUri;
}
Expand Down Expand Up @@ -184,7 +185,8 @@ public ClientAuthenticationType getClientAuthenticationType() {
* @param baseUri The URI to resolve the OAuth2 callback URI relative to.
* @return The URL that can be navigated or redirected to.
*/
public URL getAuthorizationUrl(String userId, String state, URI baseUri) {
public URL getAuthorizationUrl(
@Nullable String userId, @Nullable String state, @Nullable URI baseUri) {
return this.getAuthorizationUrl(userId, state, baseUri, null);
}

Expand All @@ -198,9 +200,9 @@ public URL getAuthorizationUrl(String userId, String state, URI baseUri) {
* @return The URL that can be navigated or redirected to.
*/
public URL getAuthorizationUrl(
String userId,
String state,
URI baseUri,
@Nullable String userId,
@Nullable String state,
@Nullable URI baseUri,
Comment on lines +203 to +205

@lqiu96 lqiu96 Aug 20, 2026 •
edited
Loading

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.

Added these annotations because there of the null checks below (L215 and L220)

@Nullable Map<String, String> additionalParameters) {
URI resolvedCallbackUri = getCallbackUri(baseUri);
String scopesString = Joiner.on(' ').join(scopes);
Expand All @@ -221,9 +223,7 @@ public URL getAuthorizationUrl(
url.put("include_granted_scopes", true);

if (additionalParameters != null) {
for (Map.Entry<String, String> entry : additionalParameters.entrySet()) {
url.put(entry.getKey(), entry.getValue());
}
url.putAll(additionalParameters);
}

if (pkce != null) {
Expand All @@ -240,12 +240,8 @@ public URL getAuthorizationUrl(
* @return The loaded credentials or null if there are no valid approved credentials.
* @throws IOException If there is error retrieving or loading the credentials.
*/
@Nullable
public UserCredentials getCredentials(String userId) throws IOException {
public @Nullable UserCredentials getCredentials(String userId) throws IOException {
Preconditions.checkNotNull(userId);
if (tokenStore == null) {
throw new IllegalStateException("Method cannot be called if token store is not specified.");
}
Comment on lines -246 to -248

@lqiu96 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.

constructor creates a default one if builder passes in null

String tokenData = tokenStore.load(userId);
if (tokenData == null) {
return null;
Expand Down Expand Up @@ -288,8 +284,9 @@ public UserCredentials getCredentials(String userId) throws IOException {
* @return the UserCredentials instance created from the authorization code.
* @throws IOException An error from the server API call to get the tokens.
*/
public UserCredentials getCredentialsFromCode(String code, URI baseUri) throws IOException {
return getCredentialsFromCode(code, baseUri, null);
public UserCredentials getCredentialsFromCode(String code, @Nullable URI baseUri)
throws IOException {
return getCredentialsFromCode(code, baseUri, Collections.emptyMap());
}

/**
Expand All @@ -303,10 +300,12 @@ public UserCredentials getCredentialsFromCode(String code, URI baseUri) throws I
* @throws IOException An error from the server API call to get the tokens.
*/
public UserCredentials getCredentialsFromCode(
String code, URI baseUri, @Nullable Map<String, String> additionalParameters)
String code, @Nullable URI baseUri, @Nullable Map<String, String> additionalParameters)
throws IOException {
Map<String, String> effectiveAdditionalParameters =
additionalParameters != null ? additionalParameters : Collections.emptyMap();
TokenResponseWithConfig tokenResponseWithConfig =
getCredentialsFromCodeInternal(code, baseUri, additionalParameters);
getCredentialsFromCodeInternal(code, baseUri, effectiveAdditionalParameters);
return UserCredentials.newBuilder()
.setClientId(tokenResponseWithConfig.getClientId())
.setClientSecret(tokenResponseWithConfig.getClientSecret())
Expand All @@ -330,8 +329,11 @@ public UserCredentials getCredentialsFromCode(
* @throws IOException If an error occurs during the token exchange process.
*/
public TokenResponseWithConfig getTokenResponseFromAuthCodeExchange(
String code, URI callbackUri, Map<String, String> additionalParameters) throws IOException {
return getCredentialsFromCodeInternal(code, callbackUri, additionalParameters);
String code, @Nullable URI callbackUri, @Nullable Map<String, String> additionalParameters)
throws IOException {
Map<String, String> effectiveAdditionalParameters =
additionalParameters != null ? additionalParameters : Collections.emptyMap();
return getCredentialsFromCodeInternal(code, callbackUri, effectiveAdditionalParameters);
}

/**
Expand All @@ -343,8 +345,8 @@ public TokenResponseWithConfig getTokenResponseFromAuthCodeExchange(
* @return UserCredentials instance created from the authorization code.
* @throws IOException An error from the server API call to get the tokens or store the tokens.
*/
public UserCredentials getAndStoreCredentialsFromCode(String userId, String code, URI baseUri)
throws IOException {
public UserCredentials getAndStoreCredentialsFromCode(
String userId, String code, @Nullable URI baseUri) throws IOException {
Preconditions.checkNotNull(userId);
Preconditions.checkNotNull(code);
UserCredentials credentials = getCredentialsFromCode(code, baseUri);
Expand All @@ -361,9 +363,6 @@ public UserCredentials getAndStoreCredentialsFromCode(String userId, String code
*/
public void revokeAuthorization(String userId) throws IOException {
Preconditions.checkNotNull(userId);
if (tokenStore == null) {
throw new IllegalStateException("Method cannot be called if token store is not specified.");
}
String tokenData = tokenStore.load(userId);
if (tokenData == null) {
return;
Expand Down Expand Up @@ -414,9 +413,6 @@ public void revokeAuthorization(String userId) throws IOException {
* @throws IOException An error storing the credentials.
*/
public void storeCredentials(String userId, UserCredentials credentials) throws IOException {
if (tokenStore == null) {
throw new IllegalStateException("Cannot store tokens if tokenStore is not specified.");
}
AccessToken accessToken = credentials.getAccessToken();
String acessTokenValue = null;
Date expiresBy = null;
Expand Down Expand Up @@ -451,7 +447,8 @@ protected void monitorCredentials(String userId, UserCredentials credentials) {
}

private TokenResponseWithConfig getCredentialsFromCodeInternal(
String code, URI baseUri, Map<String, String> additionalParameters) throws IOException {
String code, @Nullable URI baseUri, Map<String, String> additionalParameters)
throws IOException {
Preconditions.checkNotNull(code);
URI resolvedCallbackUri = getCallbackUri(baseUri);

Expand All @@ -461,11 +458,7 @@ private TokenResponseWithConfig getCredentialsFromCodeInternal(
tokenData.put("redirect_uri", resolvedCallbackUri);
tokenData.put("grant_type", "authorization_code");

if (additionalParameters != null) {
for (Map.Entry<String, String> entry : additionalParameters.entrySet()) {
tokenData.put(entry.getKey(), entry.getValue());
}
}
tokenData.putAll(additionalParameters);

if (pkce != null) {
tokenData.put("code_verifier", pkce.getCodeVerifier());
Expand Down Expand Up @@ -565,7 +558,7 @@ public static class Builder {
private URI userAuthUri;
private Collection<String> scopes;
private HttpTransportFactory transportFactory;
private PKCEProvider pkce;
private @Nullable PKCEProvider pkce;
private ClientAuthenticationType clientAuthenticationType;

protected Builder() {}
Expand Down Expand Up @@ -676,14 +669,15 @@ public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) {
* @return this {@code Builder} object
*/
@CanIgnoreReturnValue
public Builder setPKCEProvider(PKCEProvider pkce) {
public Builder setPKCEProvider(@Nullable PKCEProvider pkce) {
if (pkce != null) {
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.");
"PKCE provider contained null implementations. PKCE object must implement all"
+ " PKCEProvider methods.");
}
}
this.pkce = pkce;
Expand Down Expand Up @@ -732,7 +726,7 @@ public HttpTransportFactory getHttpTransportFactory() {
return transportFactory;
}

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

Expand Down
57 changes: 57 additions & 0 deletions ...le-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,63 @@ void getCallbackUri_relativeToBase() {
assertEquals(expectedCallbackURI, absoluteCallbackURI);
}

@Test
void getCallbackUri_absoluteCallback_nullBaseUri() {
final URI callbackURI = URI.create("http://example.com/bar");
UserAuthorizer authorizer =
UserAuthorizer.newBuilder()
.setClientId(CLIENT_ID)
.setScopes(DUMMY_SCOPES)
.setCallbackUri(callbackURI)
.build();

URI resultCallbackURI = authorizer.getCallbackUri(null);

assertEquals(callbackURI, resultCallbackURI);
}

@Test
void getCallbackUri_relativeCallback_nullBaseUri_throwsIllegalStateException() {
final URI callbackURI = URI.create("/bar");
UserAuthorizer authorizer =
UserAuthorizer.newBuilder()
.setClientId(CLIENT_ID)
.setScopes(DUMMY_SCOPES)
.setCallbackUri(callbackURI)
.build();

assertThrows(IllegalStateException.class, () -> authorizer.getCallbackUri(null));
}

@Test
void getAuthorizationUrl_nullBaseUri() throws IOException {
final String protocol = "https";
final String host = "accounts.test.com";
final String path = "/o/o/oauth2/auth";
final URI authUri = URI.create(protocol + "://" + host + path);
final URI absoluteCallbackUri = URI.create("http://example.com/oauth2callback");
UserAuthorizer authorizer =
UserAuthorizer.newBuilder()
.setClientId(CLIENT_ID)
.setScopes(DUMMY_SCOPES)
.setCallbackUri(absoluteCallbackUri)
.setUserAuthUri(authUri)
.build();

URL authorizationUrl = authorizer.getAuthorizationUrl(USER_ID, "state", null);

assertEquals(protocol, authorizationUrl.getProtocol());
assertEquals(path, authorizationUrl.getPath());
assertEquals(host, authorizationUrl.getHost());
String query = authorizationUrl.getQuery();
Map<String, String> parameters = TestUtils.parseQuery(query);
assertEquals("state", parameters.get("state"));
assertEquals(USER_ID, parameters.get("login_hint"));
assertEquals(absoluteCallbackUri.toString(), parameters.get("redirect_uri"));
assertEquals(CLIENT_ID_VALUE, parameters.get("client_id"));
assertEquals(DUMMY_SCOPE, parameters.get("scope"));
}

@Test
void getAuthorizationUrl() throws IOException {
final String customState = "custom_state";
Expand Down
Loading