OAuth RFC-8628 various post-fixes (#4699)

* add timeout to oauth calls

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>

bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthClientServiceImpl.java

@@ -66,6 +66,8 @@ public class OAuthClientServiceImpl implements OAuthClientService {
 
     private final transient Logger logger = LoggerFactory.getLogger(OAuthClientServiceImpl.class);
 
+    private final Object refreshTokenProcessLock = new Object();
+
     private @NonNullByDefault({}) OAuthStoreHandler storeHandler;
 
     // Constructor params - static
@@ -291,49 +293,72 @@ public class OAuthClientServiceImpl implements OAuthClientService {
     }
 
     @Override
-    public synchronized AccessTokenResponse refreshToken() throws OAuthException, IOException, OAuthResponseException {
+    public AccessTokenResponse refreshToken() throws OAuthException, IOException, OAuthResponseException {
         if (isClosed()) {
             throw new OAuthException(EXCEPTION_MESSAGE_CLOSED);
         }
+        return refreshTokenInner(true);
+    }
 
-        AccessTokenResponse lastAccessToken;
+    /**
-        try {
+     * Inner private method for refreshToken. If 'forceRefresh' is false then only fetch a new token if
-            lastAccessToken = storeHandler.loadAccessTokenResponse(handle);
+     * the prior token is not expired, otherwise return the prior token. If 'forceRefresh' is true
-        } catch (GeneralSecurityException e) {
+     * then always fetch a new token.
-            throw new OAuthException("Cannot decrypt access token from store", e);
+     *
-        }
+     * @param forceRefresh determines whether to force a refresh or check for token expiry
-        if (lastAccessToken == null) {
+     * @return either the prior AccessTokenResponse or a new one
-            throw new OAuthException(
+     */
-                    "Cannot refresh token because last access token is not available from handle: " + handle);
+    private AccessTokenResponse refreshTokenInner(boolean forceRefresh)
-        }
+            throws OAuthException, IOException, OAuthResponseException {
-        if (lastAccessToken.getRefreshToken() == null) {
+        AccessTokenResponse accessTokenResponse = null;
-            throw new OAuthException("Cannot refresh token because last access token did not have a refresh token");
+
-        }
+        synchronized (refreshTokenProcessLock) {
-        String tokenUrl = persistedParams.tokenUrl;
+            AccessTokenResponse lastAccessToken;
-        if (tokenUrl == null) {
+            try {
-            throw new OAuthException("tokenUrl is required but null");
+                lastAccessToken = storeHandler.loadAccessTokenResponse(handle);
+            } catch (GeneralSecurityException e) {
+                throw new OAuthException("Cannot decrypt access token from store", e);
+            }
+            if (lastAccessToken == null) {
+                throw new OAuthException(
+                        "Cannot refresh token because last access token is not available from handle: " + handle);
+            }
+            if (lastAccessToken.getRefreshToken() == null) {
+                throw new OAuthException("Cannot refresh token because last access token did not have a refresh token");
+            }
+            String tokenUrl = persistedParams.tokenUrl;
+            if (tokenUrl == null) {
+                throw new OAuthException("tokenUrl is required but null");
+            }
+
+            if (forceRefresh || lastAccessToken.isExpired(Instant.now(), tokenExpiresInSeconds)) {
+                GsonBuilder gsonBuilder = this.gsonBuilder;
+                OAuthConnector connector = gsonBuilder == null ? new OAuthConnector(httpClientFactory, extraAuthFields)
+                        : new OAuthConnector(httpClientFactory, extraAuthFields, gsonBuilder);
+                accessTokenResponse = connector.grantTypeRefreshToken(tokenUrl, lastAccessToken.getRefreshToken(),
+                        persistedParams.clientId, persistedParams.clientSecret, persistedParams.scope,
+                        Boolean.TRUE.equals(persistedParams.supportsBasicAuth));
+
+                // The service may not return the refresh token so use the last refresh token otherwise it's not stored.
+                String refreshToken = accessTokenResponse.getRefreshToken();
+                if (refreshToken == null || refreshToken.isBlank()) {
+                    accessTokenResponse.setRefreshToken(lastAccessToken.getRefreshToken());
+                }
+
+                // store it
+                storeHandler.saveAccessTokenResponse(handle, accessTokenResponse);
+            } else {
+                // No need to refresh the token, just return the last one
+                return lastAccessToken;
+            }
         }
 
-        GsonBuilder gsonBuilder = this.gsonBuilder;
+        notifyAccessTokenResponse(accessTokenResponse);
-        OAuthConnector connector = gsonBuilder == null ? new OAuthConnector(httpClientFactory, extraAuthFields)
-                : new OAuthConnector(httpClientFactory, extraAuthFields, gsonBuilder);
-        AccessTokenResponse accessTokenResponse = connector.grantTypeRefreshToken(tokenUrl,
-                lastAccessToken.getRefreshToken(), persistedParams.clientId, persistedParams.clientSecret,
-                persistedParams.scope, Boolean.TRUE.equals(persistedParams.supportsBasicAuth));
-
-        // The service may not return the refresh token so use the last refresh token otherwise it's not stored.
-        String refreshToken = accessTokenResponse.getRefreshToken();
-        if (refreshToken == null || refreshToken.isBlank()) {
-            accessTokenResponse.setRefreshToken(lastAccessToken.getRefreshToken());
-        }
-        // store it
-        storeHandler.saveAccessTokenResponse(handle, accessTokenResponse);
-        accessTokenRefreshListeners.forEach(l -> l.onAccessTokenResponse(accessTokenResponse));
         return accessTokenResponse;
     }
 
     @Override
-    public synchronized @Nullable AccessTokenResponse getAccessTokenResponse()
+    public @Nullable AccessTokenResponse getAccessTokenResponse()
             throws OAuthException, IOException, OAuthResponseException {
         if (isClosed()) {
             throw new OAuthException(EXCEPTION_MESSAGE_CLOSED);
@@ -351,7 +376,7 @@ public class OAuthClientServiceImpl implements OAuthClientService {
 
         if (lastAccessToken.isExpired(Instant.now(), tokenExpiresInSeconds)
                 && lastAccessToken.getRefreshToken() != null) {
-            return refreshToken();
+            return refreshTokenInner(false);
         }
         return lastAccessToken;
     }

bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnector.java

@@ -23,6 +23,7 @@ import java.time.ZoneId;
 import java.time.format.DateTimeParseException;
 import java.util.Base64;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -47,6 +48,8 @@ import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
 import com.google.gson.JsonDeserializer;
+import com.google.gson.JsonElement;
+import com.google.gson.JsonObject;
 import com.google.gson.JsonSyntaxException;
 
 /**
@@ -61,6 +64,7 @@ import com.google.gson.JsonSyntaxException;
 public class OAuthConnector {
 
     private static final String HTTP_CLIENT_CONSUMER_NAME = "OAuthConnector";
+    private static final int TIMEOUT_SECONDS = 10;
 
     protected final HttpClientFactory httpClientFactory;
 
@@ -87,6 +91,29 @@ public class OAuthConnector {
         this.extraFields = extraFields;
         gson = gsonBuilder.setDateFormat(DateTimeType.DATE_PATTERN_JSON_COMPAT)
                 .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
+                .registerTypeAdapter(OAuthResponseException.class,
+                        (JsonDeserializer<OAuthResponseException>) (json, typeOfT, context) -> {
+                            OAuthResponseException result = new OAuthResponseException();
+                            JsonObject jsonObject = json.getAsJsonObject();
+                            JsonElement jsonElement;
+                            jsonElement = jsonObject.get("error");
+                            if (jsonElement != null) {
+                                result.setError(jsonElement.getAsString());
+                            }
+                            jsonElement = jsonObject.get("error_description");
+                            if (jsonElement != null) {
+                                result.setErrorDescription(jsonElement.getAsString());
+                            }
+                            jsonElement = jsonObject.get("error_uri");
+                            if (jsonElement != null) {
+                                result.setErrorUri(jsonElement.getAsString());
+                            }
+                            jsonElement = jsonObject.get("state");
+                            if (jsonElement != null) {
+                                result.setState(jsonElement.getAsString());
+                            }
+                            return result;
+                        })
                 .registerTypeAdapter(Instant.class, (JsonDeserializer<Instant>) (json, typeOfT, context) -> {
                     try {
                         return Instant.parse(json.getAsString());
@@ -273,7 +300,8 @@ public class OAuthConnector {
     }
 
     private Request getMethod(HttpClient httpClient, String tokenUrl) {
-        Request request = httpClient.newRequest(tokenUrl).method(HttpMethod.POST);
+        Request request = httpClient.newRequest(tokenUrl).method(HttpMethod.POST).timeout(TIMEOUT_SECONDS,
+                TimeUnit.SECONDS);
         request.header(HttpHeader.ACCEPT, "application/json");
         request.header(HttpHeader.ACCEPT_CHARSET, StandardCharsets.UTF_8.name());
         return request;

bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnectorRFC8628.java

@@ -40,6 +40,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import com.google.gson.GsonBuilder;
+import com.google.gson.JsonSyntaxException;
 
 /**
  * The {@link OAuthConnectorRFC8628} extends {@link OAuthConnector} to implement
@@ -292,9 +293,10 @@ public class OAuthConnectorRFC8628 extends OAuthConnector implements AutoCloseab
         request.param(PARAM_SCOPE, scopeParameter);
         logger.trace("fetchDeviceCodeResponse() request: {}", request.getURI());
 
+        String content = null;
         try {
             ContentResponse response = request.send();
-            String content = response.getContentAsString();
+            content = response.getContentAsString();
             logger.trace("fetchDeviceCodeResponse() response: {}", content);
 
             if (response.getStatus() == HttpStatus.OK_200) {
@@ -310,6 +312,9 @@ public class OAuthConnectorRFC8628 extends OAuthConnector implements AutoCloseab
                 }
             }
             throw new OAuthException("fetchDeviceCodeResponse() error: " + response);
+        } catch (JsonSyntaxException e) {
+            logger.warn("fetchDeviceCodeResponse() error parsing content:{}", content);
+            throw new OAuthException("fetchDeviceCodeResponse() error", e);
         } catch (InterruptedException | TimeoutException | ExecutionException e) {
             throw new OAuthException("fetchDeviceCodeResponse() error", e);
         }
@@ -341,9 +346,10 @@ public class OAuthConnectorRFC8628 extends OAuthConnector implements AutoCloseab
         request.param(PARAM_DEVICE_CODE, dcr.getDeviceCode());
         logger.trace("fetchAccessTokenResponse() request: {}", request.getURI());
 
+        String content = null;
         try {
             ContentResponse response = request.send();
-            String content = response.getContentAsString();
+            content = response.getContentAsString();
             logger.trace("fetchAccessTokenResponse() response: {}", content);
 
             switch (response.getStatus()) {
@@ -367,6 +373,9 @@ public class OAuthConnectorRFC8628 extends OAuthConnector implements AutoCloseab
              * completed the verification process
              */
             return null;
+        } catch (JsonSyntaxException e) {
+            logger.warn("fetchAccessTokenResponse() error parsing content:{}", content);
+            throw new OAuthException("fetchAccessTokenResponse() error", e);
         } catch (InterruptedException | TimeoutException | ExecutionException e) {
             throw new OAuthException("fetchAccessTokenResponse() error", e);
         }