[netatmo] Make OAuth2 token refresh RFC compliant

bundles/org.openhab.binding.netatmo/README.md

@@ -48,9 +48,6 @@ The Account bridge has the following configuration elements:
 | clientSecret      | String | Yes      | Client Secret provided for the application you created                                                                 |
 | webHookUrl        | String | No       | Protocol, public IP and port to access openHAB server from Internet                                                    |
 | reconnectInterval | Number | No       | The reconnection interval to Netatmo API (in s)                                                                        |
-| refreshToken      | String | Yes*     | The refresh token provided by Netatmo API after the granting process. Can be saved in case of file based configuration |
-
-(*) Strictly said this parameter is not mandatory at first run, until you grant your binding on Netatmo Connect. Once present, you'll not have to grant again.
 
 **Supported channels for the Account bridge thing:**
 
@@ -67,7 +64,6 @@ The Account bridge has the following configuration elements:
 1. Go to the authorization page of your server. `http://<your openHAB address>:8080/netatmo/connect/<_CLIENT_ID_>`. Your newly added bridge should be listed there (no need for you to expose your openHAB server outside your local network for this).
 1. Press the _"Authorize Thing"_ button. This will take you either to the login page of Netatmo Connect or directly to the authorization screen. Login and/or authorize the application. You will be returned and the entry should go green.
 1. The bridge configuration will be updated with a refresh token and go _ONLINE_. The refresh token is used to re-authorize the bridge with Netatmo Connect Web API whenever required. So you can consult this token by opening the Thing page in MainUI, this is the value of the advanced parameter named “Refresh Token”.
-1. If you're using file based .things config file, copy the provided refresh token in the **refreshToken** parameter of your thing definition (example below).
 
 Now that you have got your bridge _ONLINE_ you can now start a scan with the binding to auto discover your things.
 
@@ -646,7 +642,7 @@ All these channels are read only.
 ### things/netatmo.things
 
 ```java
-Bridge netatmo:account:myaccount "Netatmo Account" [clientId="xxxxx", clientSecret="yyyy", refreshToken="zzzzz"] {
+Bridge netatmo:account:myaccount "Netatmo Account" [clientId="xxxxx", clientSecret="yyyy"] {
     Bridge weather-station inside "Inside Weather Station" [id="70:ee:aa:aa:aa:aa"] {
         outdoor outside   "Outside Module" [id="02:00:00:aa:aa:aa"] {
             Channels:

bundles/org.openhab.binding.netatmo/pom.xml

@@ -1,4 +1,6 @@
-<?xml version="1.0" encoding="UTF-8" standalone="no"?><project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 
   <modelVersion>4.0.0</modelVersion>
 

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/AuthenticationApi.java

@@ -17,6 +17,7 @@
 
 import java.net.URI;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -51,69 +52,74 @@ public class AuthenticationApi extends RestManager {
     private final ScheduledExecutorService scheduler;
 
     private Optional<ScheduledFuture<?>> refreshTokenJob = Optional.empty();
-    private Optional<AccessTokenResponse> tokenResponse = Optional.empty();
+    private List<Scope> grantedScope = List.of();
+    private @Nullable String authorization;
 
     public AuthenticationApi(ApiBridgeHandler bridge, ScheduledExecutorService scheduler) {
         super(bridge, FeatureArea.NONE);
         this.scheduler = scheduler;
     }
 
-    public String authorize(ApiHandlerConfiguration credentials, @Nullable String code, @Nullable String redirectUri)
-            throws NetatmoException {
+    public void authorize(ApiHandlerConfiguration credentials, String refreshToken, @Nullable String code,
+            @Nullable String redirectUri) throws NetatmoException {
         if (!(credentials.clientId.isBlank() || credentials.clientSecret.isBlank())) {
             Map<String, String> params = new HashMap<>(Map.of(SCOPE, FeatureArea.ALL_SCOPES));
-            String refreshToken = credentials.refreshToken;
+
             if (!refreshToken.isBlank()) {
                 params.put(REFRESH_TOKEN, refreshToken);
-            } else {
-                if (code != null && redirectUri != null) {
-                    params.putAll(Map.of(REDIRECT_URI, redirectUri, CODE, code));
-                }
+            } else if (code != null && redirectUri != null) {
+                params.putAll(Map.of(REDIRECT_URI, redirectUri, CODE, code));
             }
             if (params.size() > 1) {
-                return requestToken(credentials.clientId, credentials.clientSecret, params);
+                requestToken(credentials.clientId, credentials.clientSecret, params);
+                return;
             }
         }
         throw new IllegalArgumentException("Inconsistent configuration state, please file a bug report.");
     }
 
-    private String requestToken(String id, String secret, Map<String, String> entries) throws NetatmoException {
-        Map<String, String> payload = new HashMap<>(entries);
-        payload.put(GRANT_TYPE, payload.keySet().contains(CODE) ? AUTHORIZATION_CODE : REFRESH_TOKEN);
-        payload.putAll(Map.of(CLIENT_ID, id, CLIENT_SECRET, secret));
+    private void requestToken(String clientId, String secret, Map<String, String> entries) throws NetatmoException {
         disconnect();
+
+        Map<String, String> payload = new HashMap<>(entries);
+        payload.putAll(Map.of(GRANT_TYPE, payload.keySet().contains(CODE) ? AUTHORIZATION_CODE : REFRESH_TOKEN,
+                CLIENT_ID, clientId, CLIENT_SECRET, secret));
+
         AccessTokenResponse response = post(TOKEN_URI, AccessTokenResponse.class, payload);
+
         refreshTokenJob = Optional.of(scheduler.schedule(() -> {
             try {
-                requestToken(id, secret, Map.of(REFRESH_TOKEN, response.getRefreshToken()));
+                requestToken(clientId, secret, Map.of(REFRESH_TOKEN, response.getRefreshToken()));
             } catch (NetatmoException e) {
                 logger.warn("Unable to refresh access token : {}", e.getMessage());
             }
-        }, Math.round(response.getExpiresIn() * 0.8), TimeUnit.SECONDS));
-        tokenResponse = Optional.of(response);
-        return response.getRefreshToken();
+        }, Math.round(response.getExpiresIn() * 0.9), TimeUnit.SECONDS));
+
+        grantedScope = response.getScope();
+        authorization = "Bearer %s".formatted(response.getAccessToken());
+        apiBridge.storeRefreshToken(response.getRefreshToken());
     }
 
     public void disconnect() {
-        tokenResponse = Optional.empty();
+        authorization = null;
+        grantedScope = List.of();
     }
 
     public void dispose() {
         refreshTokenJob.ifPresent(job -> job.cancel(true));
         refreshTokenJob = Optional.empty();
     }
 
-    public @Nullable String getAuthorization() {
-        return tokenResponse.map(at -> String.format("Bearer %s", at.getAccessToken())).orElse(null);
+    public Optional<String> getAuthorization() {
+        return Optional.ofNullable(authorization);
     }
 
     public boolean matchesScopes(Set<Scope> requiredScopes) {
-        return requiredScopes.isEmpty() // either we do not require any scope, either connected and all scopes available
-                || (isConnected() && tokenResponse.map(at -> at.getScope().containsAll(requiredScopes)).orElse(false));
+        return requiredScopes.isEmpty() || grantedScope.containsAll(requiredScopes);
     }
 
     public boolean isConnected() {
-        return tokenResponse.isPresent();
+        return authorization != null;
     }
 
     public static UriBuilder getAuthorizationBuilder(String clientId) {

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/api/RestManager.java

@@ -41,7 +41,7 @@ public abstract class RestManager {
     private static final UriBuilder API_URI_BUILDER = getApiBaseBuilder().path(PATH_API);
 
     private final Set<Scope> requiredScopes;
-    private final ApiBridgeHandler apiBridge;
+    protected final ApiBridgeHandler apiBridge;
 
     public RestManager(ApiBridgeHandler apiBridge, FeatureArea features) {
         this.requiredScopes = features.scopes;

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/config/ApiHandlerConfiguration.java

@@ -23,15 +23,13 @@
 @NonNullByDefault
 public class ApiHandlerConfiguration {
     public static final String CLIENT_ID = "clientId";
-    public static final String REFRESH_TOKEN = "refreshToken";
 
     public String clientId = "";
     public String clientSecret = "";
-    public String refreshToken = "";
     public String webHookUrl = "";
     public int reconnectInterval = 300;
 
-    public ConfigurationLevel check() {
+    public ConfigurationLevel check(String refreshToken) {
         if (clientId.isBlank()) {
             return ConfigurationLevel.EMPTY_CLIENT_ID;
         } else if (clientSecret.isBlank()) {

bundles/org.openhab.binding.netatmo/src/main/java/org/openhab/binding/netatmo/internal/handler/ApiBridgeHandler.java

@@ -16,10 +16,14 @@
 import static org.openhab.binding.netatmo.internal.NetatmoBindingConstants.*;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Constructor;
 import java.net.URI;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.time.LocalDateTime;
 import java.util.ArrayDeque;
 import java.util.Collection;
@@ -69,7 +73,7 @@
 import org.openhab.binding.netatmo.internal.discovery.NetatmoDiscoveryService;
 import org.openhab.binding.netatmo.internal.servlet.GrantServlet;
 import org.openhab.binding.netatmo.internal.servlet.WebhookServlet;
-import org.openhab.core.config.core.Configuration;
+import org.openhab.core.OpenHAB;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.thing.Bridge;
 import org.openhab.core.thing.ChannelUID;
@@ -93,76 +97,82 @@
 @NonNullByDefault
 public class ApiBridgeHandler extends BaseBridgeHandler {
     private static final int TIMEOUT_S = 20;
+    private static final String REFRESH_TOKEN = "refreshToken";
 
     private final Logger logger = LoggerFactory.getLogger(ApiBridgeHandler.class);
+    private final AuthenticationApi connectApi = new AuthenticationApi(this, scheduler);
+    private final Map<Class<? extends RestManager>, RestManager> managers = new HashMap<>();
+    private final Deque<LocalDateTime> requestsTimestamps = new ArrayDeque<>(200);
     private final BindingConfiguration bindingConf;
-    private final AuthenticationApi connectApi;
     private final HttpClient httpClient;
     private final NADeserializer deserializer;
     private final HttpService httpService;
+    private final ChannelUID requestCountChannelUID;
+    private final Path tokenFile;
 
     private Optional<ScheduledFuture<?>> connectJob = Optional.empty();
-    private Map<Class<? extends RestManager>, RestManager> managers = new HashMap<>();
-    private @Nullable WebhookServlet webHookServlet;
-    private @Nullable GrantServlet grantServlet;
-    private Deque<LocalDateTime> requestsTimestamps;
-    private final ChannelUID requestCountChannelUID;
+    private Optional<WebhookServlet> webHookServlet = Optional.empty();
+    private Optional<GrantServlet> grantServlet = Optional.empty();
 
     public ApiBridgeHandler(Bridge bridge, HttpClient httpClient, NADeserializer deserializer,
             BindingConfiguration configuration, HttpService httpService) {
         super(bridge);
         this.bindingConf = configuration;
-        this.connectApi = new AuthenticationApi(this, scheduler);
         this.httpClient = httpClient;
         this.deserializer = deserializer;
         this.httpService = httpService;
-        this.requestsTimestamps = new ArrayDeque<>(200);
-        this.requestCountChannelUID = new ChannelUID(getThing().getUID(), GROUP_MONITORING, CHANNEL_REQUEST_COUNT);
+        this.requestCountChannelUID = new ChannelUID(thing.getUID(), GROUP_MONITORING, CHANNEL_REQUEST_COUNT);
+
+        Path homeFolder = Paths.get(OpenHAB.getUserDataFolder(), BINDING_ID);
+        if (Files.notExists(homeFolder)) {
+            try {
+                Files.createDirectory(homeFolder);
+            } catch (IOException e) {
+                logger.warn("Unable to create {} folder : {}", homeFolder.toString(), e.getMessage());
+            }
+        }
+        tokenFile = homeFolder.resolve(REFRESH_TOKEN + "_" + thing.getUID().toString().replace(":", "_"));
     }
 
     @Override
     public void initialize() {
         logger.debug("Initializing Netatmo API bridge handler.");
         updateStatus(ThingStatus.UNKNOWN);
-        GrantServlet servlet = new GrantServlet(this, httpService);
-        servlet.startListening();
-        grantServlet = servlet;
         scheduler.execute(() -> openConnection(null, null));
     }
 
     public void openConnection(@Nullable String code, @Nullable String redirectUri) {
         ApiHandlerConfiguration configuration = getConfiguration();
-        ConfigurationLevel level = configuration.check();
+
+        String refreshToken = readRefreshToken();
+
+        ConfigurationLevel level = configuration.check(refreshToken);
         switch (level) {
             case EMPTY_CLIENT_ID:
             case EMPTY_CLIENT_SECRET:
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, level.message);
                 break;
             case REFRESH_TOKEN_NEEDED:
                 if (code == null || redirectUri == null) {
+                    GrantServlet servlet = new GrantServlet(this, httpService);
+                    servlet.startListening();
+                    grantServlet = Optional.of(servlet);
                     updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, level.message);
                     break;
                 } // else we can proceed to get the token refresh
             case COMPLETED:
                 try {
                     logger.debug("Connecting to Netatmo API.");
 
-                    String refreshToken = connectApi.authorize(configuration, code, redirectUri);
-
-                    if (configuration.refreshToken.isBlank()) {
-                        Configuration thingConfig = editConfiguration();
-                        thingConfig.put(ApiHandlerConfiguration.REFRESH_TOKEN, refreshToken);
-                        updateConfiguration(thingConfig);
-                        configuration = getConfiguration();
-                    }
+                    connectApi.authorize(configuration, refreshToken, code, redirectUri);
 
                     if (!configuration.webHookUrl.isBlank()) {
                         SecurityApi securityApi = getRestManager(SecurityApi.class);
                         if (securityApi != null) {
                             WebhookServlet servlet = new WebhookServlet(this, httpService, deserializer, securityApi,
                                     configuration.webHookUrl);
                             servlet.startListening();
-                            this.webHookServlet = servlet;
+                            this.webHookServlet = Optional.of(servlet);
                         }
                     }
 
@@ -180,6 +190,30 @@ public void openConnection(@Nullable String code, @Nullable String redirectUri)
         }
     }
 
+    private String readRefreshToken() {
+        if (Files.exists(tokenFile)) {
+            try {
+                return Files.readString(tokenFile);
+            } catch (IOException e) {
+                logger.warn("Unable to read token file {} : {}", tokenFile.toString(), e.getMessage());
+            }
+        }
+        return "";
+    }
+
+    public void storeRefreshToken(String refreshToken) {
+        if (refreshToken.isBlank()) {
+            logger.trace("Blank refresh token received - ignored");
+        } else {
+            logger.trace("Updating refresh token in {} : {}", tokenFile.toString(), refreshToken);
+            try {
+                Files.write(tokenFile, refreshToken.getBytes());
+            } catch (IOException e) {
+                logger.warn("Error saving refresh token to {} : {}", tokenFile.toString(), e.getMessage());
+            }
+        }
+    }
+
     public ApiHandlerConfiguration getConfiguration() {
         return getConfigAs(ApiHandlerConfiguration.class);
     }
@@ -199,14 +233,13 @@ private void freeConnectJob() {
     @Override
     public void dispose() {
         logger.debug("Shutting down Netatmo API bridge handler.");
-        WebhookServlet localWebHook = this.webHookServlet;
-        if (localWebHook != null) {
-            localWebHook.dispose();
-        }
-        GrantServlet localGrant = this.grantServlet;
-        if (localGrant != null) {
-            localGrant.dispose();
-        }
+
+        webHookServlet.ifPresent(servlet -> servlet.dispose());
+        webHookServlet = Optional.empty();
+
+        grantServlet.ifPresent(servlet -> servlet.dispose());
+        grantServlet = Optional.empty();
+
         connectApi.dispose();
         freeConnectJob();
         super.dispose();
@@ -243,10 +276,7 @@ public synchronized <T> T executeUri(URI uri, HttpMethod method, Class<T> clazz,
 
             Request request = httpClient.newRequest(uri).method(method).timeout(TIMEOUT_S, TimeUnit.SECONDS);
 
-            String auth = connectApi.getAuthorization();
-            if (auth != null) {
-                request.header(HttpHeader.AUTHORIZATION, auth);
-            }
+            connectApi.getAuthorization().ifPresent(auth -> request.header(HttpHeader.AUTHORIZATION, auth));
 
             if (payload != null && contentType != null
                     && (HttpMethod.POST.equals(method) || HttpMethod.PUT.equals(method))) {
@@ -384,6 +414,6 @@ public Collection<Class<? extends ThingHandlerService>> getServices() {
     }
 
     public Optional<WebhookServlet> getWebHookServlet() {
-        return Optional.ofNullable(webHookServlet);
+        return webHookServlet;
     }
 }

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/config/config.xml

@@ -17,13 +17,6 @@
 			<context>password</context>
 		</parameter>
 
-		<parameter name="refreshToken" type="text">
-			<label>@text/config.refreshToken.label</label>
-			<description>@text/config.refreshToken.description</description>
-			<context>password</context>
-			<advanced>true</advanced>
-		</parameter>
-
 		<parameter name="webHookUrl" type="text" required="false">
 			<label>@text/config.webHookUrl.label</label>
 			<description>@text/config.webHookUrl.description</description>

bundles/org.openhab.binding.netatmo/src/main/resources/OH-INF/i18n/netatmo.properties

@@ -421,8 +421,6 @@ config.clientId.label = Client ID
 config.clientId.description = Client ID provided for the application you created on http://dev.netatmo.com/createapp
 config.clientSecret.label = Client Secret
 config.clientSecret.description = Client Secret provided for the application you created.
-config.refreshToken.label = Refresh Token
-config.refreshToken.description = Refresh token provided by the oAuth2 authentication process.
 config.webHookUrl.label = Webhook Address
 config.webHookUrl.description = Protocol, public IP or hostname and port to access openHAB server from Internet.
 config.reconnectInterval.label = Reconnect Interval