fix wrong handling of redirect to unsecure connection

Author: theScrabi

owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequester.kt

@@ -28,6 +28,7 @@ import com.owncloud.android.lib.common.OwnCloudClient
 import com.owncloud.android.lib.common.http.HttpConstants
 import com.owncloud.android.lib.common.http.methods.nonwebdav.GetMethod
 import com.owncloud.android.lib.common.operations.RemoteOperationResult
+
 import com.owncloud.android.lib.resources.status.HttpScheme.HTTPS_SCHEME
 import com.owncloud.android.lib.resources.status.HttpScheme.HTTP_SCHEME
 import org.json.JSONObject
@@ -36,14 +37,20 @@ import java.util.concurrent.TimeUnit
 
 internal class StatusRequester {
 
-    private fun checkIfConnectionIsRedirectedToNoneSecure(
-        isConnectionSecure: Boolean,
+    /**
+     * This function is ment to detect if a redirect from a secure to an unsecure connection
+     * was made. If only connections from unsecure connections to unsecure connections were made
+     * this function should not return true, because if the whole redirect chain was unsecure
+     * we assume it was a debug setup.
+     */
+    fun isRedirectedToNonSecureConnection(
+        redirectedToUnsecureLocationBefore: Boolean,
         baseUrl: String,
         redirectedUrl: String
-    ): Boolean {
-        return isConnectionSecure ||
-                (baseUrl.startsWith(HTTPS_SCHEME) && redirectedUrl.startsWith(HTTP_SCHEME))
-    }
+    ) = redirectedToUnsecureLocationBefore
+            || (baseUrl.startsWith(HTTPS_SCHEME)
+            && (!redirectedUrl.startsWith(HTTPS_SCHEME))
+            && redirectedUrl.startsWith(HTTP_SCHEME))
 
     fun updateLocationWithRedirectPath(oldLocation: String, redirectedLocation: String): String {
         if (!redirectedLocation.startsWith("/"))
@@ -84,7 +91,7 @@ internal class StatusRequester {
             } else {
                 val nextLocation = updateLocationWithRedirectPath(currentLocation, result.redirectedLocation)
                 redirectedToUnsecureLocation =
-                    checkIfConnectionIsRedirectedToNoneSecure(
+                    isRedirectedToNonSecureConnection(
                         redirectedToUnsecureLocation,
                         currentLocation,
                         nextLocation

owncloudComLibrary/src/test/java/com/owncloud/android/lib/StatusRequestorTest.kt

@@ -26,39 +26,68 @@ package com.owncloud.android.lib
 
 import com.owncloud.android.lib.resources.status.StatusRequester
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Test
 
 class StatusRequestorTest {
-    private val requestor = StatusRequester()
+    private val requester = StatusRequester()
 
     @Test
     fun `update location - ok - absolute path`() {
-        val newLocation = requestor.updateLocationWithRedirectPath(TEST_DOMAIN, "$TEST_DOMAIN$SUB_PATH")
+        val newLocation = requester.updateLocationWithRedirectPath(TEST_DOMAIN, "$TEST_DOMAIN$SUB_PATH")
         assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
     }
 
     @Test
     fun `update location - ok - smaller absolute path`() {
-        val newLocation = requestor.updateLocationWithRedirectPath("$TEST_DOMAIN$SUB_PATH", TEST_DOMAIN)
+        val newLocation = requester.updateLocationWithRedirectPath("$TEST_DOMAIN$SUB_PATH", TEST_DOMAIN)
         assertEquals(TEST_DOMAIN, newLocation)
     }
 
     @Test
     fun `update location - ok - relative path`() {
-        val newLocation = requestor.updateLocationWithRedirectPath(TEST_DOMAIN, SUB_PATH)
+        val newLocation = requester.updateLocationWithRedirectPath(TEST_DOMAIN, SUB_PATH)
         assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
     }
 
     @Test
     fun `update location - ok - replace relative path`() {
-        val newLocation = requestor.updateLocationWithRedirectPath(
+        val newLocation = requester.updateLocationWithRedirectPath(
             "$TEST_DOMAIN/some/other/subdir", SUB_PATH
         )
         assertEquals("$TEST_DOMAIN$SUB_PATH", newLocation)
     }
 
+    @Test
+    fun `check redirect to unsecure connection - ok - redirect to http`() {
+        assertTrue(requester.isRedirectedToNonSecureConnection(
+            false, SECURE_DOMAIN, UNSECURE_DOMAIN))
+    }
+
+    @Test
+    fun `check redirect to unsecure connection - ko - redirect to https from http`() {
+        assertFalse(requester.isRedirectedToNonSecureConnection(
+            false, UNSECURE_DOMAIN, SECURE_DOMAIN))
+    }
+
+    @Test
+    fun `check redirect to unsecure connection - ko - from https to https`() {
+        assertFalse(requester.isRedirectedToNonSecureConnection(
+            false, SECURE_DOMAIN, SECURE_DOMAIN))
+    }
+
+    @Test
+    fun `check redirect to unsecure connection - ok - from https to https with previous http`() {
+        assertTrue(requester.isRedirectedToNonSecureConnection(
+            true, SECURE_DOMAIN, SECURE_DOMAIN))
+    }
+
     companion object {
         const val TEST_DOMAIN = "https://cloud.somewhere.com"
         const val SUB_PATH = "/subdir"
+
+        const val SECURE_DOMAIN = "https://cloud.somewhere.com"
+        const val UNSECURE_DOMAIN = "http://somewhereelse.org"
     }
 }