[http] Fix authentication and request issues (smarthomej#517)

Authentications need to be removed when the thing is reconfigured, state content requests need to use the proper content type Signed-off-by: Jan N. Klug <[email protected]>
J-N-K · Aug 14, 2023 · 72702e4 · 72702e4
1 parent cfea610
commit 72702e4
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 19 deletions.
diff --git a/...mej.binding.http/src/main/java/org/smarthomej/binding/http/internal/HttpThingHandler.java b/...mej.binding.http/src/main/java/org/smarthomej/binding/http/internal/HttpThingHandler.java
@@ -23,6 +23,7 @@
 import java.util.Base64;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -180,10 +181,23 @@ public void initialize() {
         config.headers.removeIf(String::isBlank);
 
         // configure authentication
-        if (!config.username.isEmpty() || !config.password.isEmpty()) {
-            try {
-                AuthenticationStore authStore = rateLimitedHttpClient.getAuthenticationStore();
-                URI uri = new URI(config.baseURL);
+        try {
+            AuthenticationStore authStore = rateLimitedHttpClient.getAuthenticationStore();
+            URI uri = new URI(config.baseURL);
+
+            // clear old auths if available
+            Authentication.Result authResult = authStore.findAuthenticationResult(uri);
+            if (authResult != null) {
+                authStore.removeAuthenticationResult(authResult);
+            }
+            for (String authType : List.of("Basic", "Digest")) {
+                Authentication authentication = authStore.findAuthentication(authType, uri, Authentication.ANY_REALM);
+                if (authentication != null) {
+                    authStore.removeAuthentication(authentication);
+                }
+            }
+
+            if (!config.username.isEmpty() || !config.password.isEmpty()) {
                 switch (config.authMode) {
                     case BASIC_PREEMPTIVE:
                         config.headers.add("Authorization=Basic " + Base64.getEncoder()
@@ -213,14 +227,12 @@ public void initialize() {
                         logger.warn("Unknown authentication method '{}' for thing '{}'", config.authMode,
                                 thing.getUID());
                 }
-            } catch (URISyntaxException e) {
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
-                        "failed to create authentication: baseUrl is invalid");
+            } else {
+                logger.debug("No authentication configured for thing '{}'", thing.getUID());
             }
-        } else {
-            logger.debug("No authentication configured for thing '{}'", thing.getUID());
+        } catch (URISyntaxException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Cannot create URI from baseUrl.");
         }
-
         // create channels
         thing.getChannels().forEach(this::createChannel);
 
@@ -317,11 +329,9 @@ private void createChannel(Channel channel) {
             // we need a key consisting of stateContent and URL, only if both are equal, we can use the same cache
             String key = channelConfig.stateContent + "$" + stateUrl;
             channelUrls.put(channelUID, key);
-            Objects.requireNonNull(
-                    urlHandlers
-                            .computeIfAbsent(key,
-                                    k -> new RefreshingUrlCache(scheduler, rateLimitedHttpClient, stateUrl, config,
-                                            channelConfig.stateContent, this)))
+            Objects.requireNonNull(urlHandlers.computeIfAbsent(key,
+                    k -> new RefreshingUrlCache(scheduler, rateLimitedHttpClient, stateUrl, config,
+                            channelConfig.stateContent, config.contentType, this)))
                     .addConsumer(itemValueConverter::process);
         }
 

diff --git a/...g.http/src/main/java/org/smarthomej/binding/http/internal/http/RateLimitedHttpClient.java b/...g.http/src/main/java/org/smarthomej/binding/http/internal/http/RateLimitedHttpClient.java
@@ -206,7 +206,7 @@ public RequestQueueEntry(URI finalUrl, HttpMethod method, String content, @Nulla
          */
         public void completeFuture(HttpClient httpClient) {
             Request request = httpClient.newRequest(finalUrl).method(method);
-            if (method != HttpMethod.GET && !content.isEmpty()) {
+            if ((method == HttpMethod.POST || method == HttpMethod.PUT) && !content.isEmpty()) {
                 if (contentType == null) {
                     request.content(new StringContentProvider(content));
                 } else {

diff --git a/...ding.http/src/main/java/org/smarthomej/binding/http/internal/http/RefreshingUrlCache.java b/...ding.http/src/main/java/org/smarthomej/binding/http/internal/http/RefreshingUrlCache.java
@@ -57,13 +57,15 @@ public class RefreshingUrlCache {
     private final Map<String, String> headers;
     private final HttpMethod httpMethod;
     private final String httpContent;
+    private final @Nullable String httpContentType;
     private final HttpStatusListener httpStatusListener;
 
     private final ScheduledFuture<?> future;
     private @Nullable ContentWrapper lastContent;
 
     public RefreshingUrlCache(ScheduledExecutorService executor, RateLimitedHttpClient httpClient, String url,
-            HttpThingConfig thingConfig, String httpContent, HttpStatusListener httpStatusListener) {
+            HttpThingConfig thingConfig, String httpContent, @Nullable String httpContentType,
+            HttpStatusListener httpStatusListener) {
         this.httpClient = httpClient;
         this.url = url;
         this.strictErrorHandling = thingConfig.strictErrorHandling;
@@ -72,6 +74,7 @@ public RefreshingUrlCache(ScheduledExecutorService executor, RateLimitedHttpClie
         this.httpMethod = thingConfig.stateMethod;
         this.headers = thingConfig.getHeaders();
         this.httpContent = httpContent;
+        this.httpContentType = httpContentType;
         this.httpStatusListener = httpStatusListener;
         fallbackEncoding = thingConfig.encoding;
 
@@ -94,7 +97,7 @@ private void refresh(boolean isRetry) {
             URI uri = Util.uriFromString(String.format(this.url, new Date()));
             logger.trace("Requesting refresh (retry={}) from '{}' with timeout {}ms", isRetry, uri, timeout);
 
-            httpClient.newRequest(uri, httpMethod, httpContent, null).thenAccept(request -> {
+            httpClient.newRequest(uri, httpMethod, httpContent, httpContentType).thenAccept(request -> {
                 request.timeout(timeout, TimeUnit.MILLISECONDS);
                 headers.forEach(request::header);
 

diff --git a/...thomej.binding.http/src/test/java/org/smarthomej/binding/http/RefreshingUrlCacheTest.java b/...thomej.binding.http/src/test/java/org/smarthomej/binding/http/RefreshingUrlCacheTest.java
@@ -253,7 +253,7 @@ public void testDateIsFormattedInURL() {
      */
     private RefreshingUrlCache getUrlCache(String content) {
         RefreshingUrlCache urlCache = new RefreshingUrlCache(scheduler, rateLimitedHttpClient, url, thingConfig,
-                content, statusListener);
+                content, null, statusListener);
         urlCache.addConsumer(contentWrappers::add);
 
         return urlCache;