diff --git a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthClientServiceImpl.java b/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthClientServiceImpl.java index bb5d434cae..b13e534bfd 100644 --- a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthClientServiceImpl.java +++ b/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 { - 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"); + /** + * Inner private method for refreshToken. If 'forceRefresh' is false then only fetch a new token if + * the prior token is not expired, otherwise return the prior token. If 'forceRefresh' is true + * then always fetch a new token. + * + * @param forceRefresh determines whether to force a refresh or check for token expiry + * @return either the prior AccessTokenResponse or a new one + */ + private AccessTokenResponse refreshTokenInner(boolean forceRefresh) + throws OAuthException, IOException, OAuthResponseException { + AccessTokenResponse accessTokenResponse = null; + + synchronized (refreshTokenProcessLock) { + AccessTokenResponse lastAccessToken; + try { + 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; - 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)); + notifyAccessTokenResponse(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; } diff --git a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnector.java b/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnector.java index 5fe999f2a6..79bd3796b1 100644 --- a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnector.java +++ b/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) (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) (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; diff --git a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnectorRFC8628.java b/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnectorRFC8628.java index 0fd6a7a0dc..800d9fc523 100644 --- a/bundles/org.openhab.core.auth.oauth2client/src/main/java/org/openhab/core/auth/oauth2client/internal/OAuthConnectorRFC8628.java +++ b/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); }