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

Author: lqiu96

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/MemoryTokensStorage.java

@@ -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);
   }
 

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/TokenStore.java

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

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/UserAuthorizer.java

@@ -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;
@@ -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}. */
@@ -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;
     }
@@ -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);
   }
 
@@ -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,
       @Nullable Map<String, String> additionalParameters) {
     URI resolvedCallbackUri = getCallbackUri(baseUri);
     String scopesString = Joiner.on(' ').join(scopes);
@@ -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) {
@@ -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.");
-    }
     String tokenData = tokenStore.load(userId);
     if (tokenData == null) {
       return null;
@@ -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());
   }
 
   /**
@@ -303,8 +300,11 @@ 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 {
+    if (additionalParameters == null) {
+      additionalParameters = Collections.emptyMap();
+    }
     TokenResponseWithConfig tokenResponseWithConfig =
         getCredentialsFromCodeInternal(code, baseUri, additionalParameters);
     return UserCredentials.newBuilder()
@@ -330,7 +330,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 {
+      String code, @Nullable URI callbackUri, @Nullable Map<String, String> additionalParameters)
+      throws IOException {
+    if (additionalParameters == null) {
+      additionalParameters = Collections.emptyMap();
+    }
     return getCredentialsFromCodeInternal(code, callbackUri, additionalParameters);
   }
 
@@ -343,8 +347,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);
@@ -361,9 +365,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;
@@ -414,9 +415,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;
@@ -451,7 +449,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);
 
@@ -461,11 +460,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());
@@ -565,7 +560,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() {}
@@ -676,14 +671,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;
@@ -732,7 +728,7 @@ public HttpTransportFactory getHttpTransportFactory() {
       return transportFactory;
     }
 
-    public PKCEProvider getPKCEProvider() {
+    public @Nullable PKCEProvider getPKCEProvider() {
       return pkce;
     }
 

google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/UserAuthorizerTest.java

@@ -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";