Skip to content

Commit

Permalink
Merge pull request #3074 from owncloud/fix/cookie_handling
Browse files Browse the repository at this point in the history
[Fix] Cookie handling
  • Loading branch information
abelgardep authored Mar 8, 2021
2 parents c676f28 + 2140f2c commit 91ead87
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 57 deletions.
2 changes: 1 addition & 1 deletion owncloud-android-library
Submodule owncloud-android-library updated 16 files
+8 −1 owncloudComLibrary/build.gradle
+10 −1 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/OwnCloudClientFactory.java
+6 −30 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/SingleSessionManager.java
+0 −68 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/accounts/AccountUtils.java
+63 −0 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/CookieJarImpl.kt
+56 −66 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/http/HttpClient.java
+0 −5 owncloudComLibrary/src/main/java/com/owncloud/android/lib/common/operations/RemoteOperation.java
+31 −106 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/GetRemoteStatusOperation.kt
+32 −0 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/HttpScheme.kt
+30 −0 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/RemoteServerInfo.kt
+156 −0 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/StatusRequester.kt
+2 −2 owncloudComLibrary/src/main/java/com/owncloud/android/lib/resources/status/services/ServerInfoService.kt
+2 −2 ...rary/src/main/java/com/owncloud/android/lib/resources/status/services/implementation/OCServerInfoService.kt
+84 −0 owncloudComLibrary/src/test/java/com/owncloud/android/lib/CookieJarImplTest.kt
+145 −0 owncloudComLibrary/src/test/java/com/owncloud/android/lib/GetRemoteStatusOperationTest.kt
+104 −0 owncloudComLibrary/src/test/java/com/owncloud/android/lib/StatusRequesterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,6 @@ public static void updateAccountVersion(Context context) {
accountMgr.getUserData(account, Constants.KEY_OC_VERSION)
);

// copy cookies
accountMgr.setUserData(
newAccount,
Constants.KEY_COOKIES,
accountMgr.getUserData(account, Constants.KEY_COOKIES)
);

String isOauthStr = accountMgr.getUserData(account, Constants.KEY_SUPPORTS_OAUTH2);
boolean isOAuth = OAUTH_SUPPORTED_TRUE.equals(isOauthStr);
if (isOAuth) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.owncloud.android.lib.resources.status.GetRemoteStatusOperation;
import com.owncloud.android.lib.resources.status.RemoteCapability;
import com.owncloud.android.lib.resources.status.OwnCloudVersion;
import com.owncloud.android.lib.resources.status.RemoteServerInfo;
import com.owncloud.android.operations.common.SyncOperation;
import timber.log.Timber;

Expand Down Expand Up @@ -56,9 +57,9 @@ protected RemoteOperationResult<RemoteCapability> run(OwnCloudClient client) {
// server version is important; this fallback will try to get it from status.php
// if capabilities API is not available.
GetRemoteStatusOperation getStatus = new GetRemoteStatusOperation();
RemoteOperationResult<OwnCloudVersion> statusResult = getStatus.execute(client);
RemoteOperationResult<RemoteServerInfo> statusResult = getStatus.execute(client);
if (statusResult.isSuccess()) {
serverVersion = statusResult.getData();
serverVersion = statusResult.getData().getOwnCloudVersion();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,6 @@ public int onStartCommand(Intent intent, int flags, int startId) {
@Override
public void onDestroy() {
Timber.v("Destroying service");
// Saving cookies
SingleSessionManager.getDefaultSingleton().saveAllClients(this, MainApp.Companion.getAccountType());

mUndispatchedFinishedOperations.clear();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

package com.owncloud.android.data.server.datasources.implementation

import androidx.annotation.VisibleForTesting
import androidx.annotation.VisibleForTesting.PRIVATE
import com.owncloud.android.data.ClientManager
import com.owncloud.android.data.executeRemoteOperation
import com.owncloud.android.data.server.datasources.RemoteServerInfoDataSource
Expand All @@ -30,8 +28,7 @@ import com.owncloud.android.domain.server.model.AuthenticationMethod
import com.owncloud.android.domain.server.model.ServerInfo
import com.owncloud.android.lib.common.http.HttpConstants
import com.owncloud.android.lib.common.network.WebdavUtils.normalizeProtocolPrefix
import com.owncloud.android.lib.common.operations.RemoteOperationResult
import com.owncloud.android.lib.resources.status.OwnCloudVersion
import com.owncloud.android.lib.resources.status.RemoteServerInfo
import com.owncloud.android.lib.resources.status.services.ServerInfoService

class OCRemoteServerInfoDataSource(
Expand All @@ -40,16 +37,14 @@ class OCRemoteServerInfoDataSource(
) : RemoteServerInfoDataSource {

/* Basically, tries to access to the root folder without authorization and analyzes the response.*/
@VisibleForTesting(otherwise = PRIVATE)
fun getAuthenticationMethod(path: String): AuthenticationMethod {
val owncloudClient = clientManager.getClientForUnExistingAccount(path, false)
val owncloudClient = clientManager.getClientForUnExistingAccount(path, true)

// Step 1: check whether the root folder exists, following redirections
var checkPathExistenceResult =
serverInfoService.checkPathExistence(path, isUserLogged = false, client = owncloudClient)
var redirectionLocation = checkPathExistenceResult.redirectedLocation
while (!redirectionLocation.isNullOrEmpty()) {
owncloudClient.setFollowRedirects(true)
checkPathExistenceResult =
serverInfoService.checkPathExistence(redirectionLocation, isUserLogged = false, client = owncloudClient)
redirectionLocation = checkPathExistenceResult.redirectedLocation
Expand Down Expand Up @@ -81,36 +76,36 @@ class OCRemoteServerInfoDataSource(
return authenticationMethod
}

@VisibleForTesting(otherwise = PRIVATE)
fun getRemoteStatus(path: String): Pair<OwnCloudVersion, Boolean> {
fun getRemoteStatus(path: String): RemoteServerInfo {
val ownCloudClient = clientManager.getClientForUnExistingAccount(path, true)

val remoteStatusResult = serverInfoService.getRemoteStatus(path, ownCloudClient)

val ownCloudVersion = executeRemoteOperation {
val remoteServerInfo = executeRemoteOperation {
remoteStatusResult
}

if (!ownCloudVersion.isServerVersionSupported && !ownCloudVersion.isVersionHidden) {
if (!remoteServerInfo.ownCloudVersion.isServerVersionSupported && !remoteServerInfo.ownCloudVersion.isVersionHidden) {
throw OwncloudVersionNotSupportedException()
}

return Pair(ownCloudVersion, remoteStatusResult.code == RemoteOperationResult.ResultCode.OK_SSL)
return remoteServerInfo
}

override fun getServerInfo(path: String): ServerInfo {
// First step: check the status of the server (including its version)
val pairVersionAndSecure = getRemoteStatus(path)
val normalizedProtocolPrefix = normalizeProtocolPrefix(path, pairVersionAndSecure.second)
val remoteServerInfo = getRemoteStatus(path)
val normalizedProtocolPrefix =
normalizeProtocolPrefix(remoteServerInfo.baseUrl, remoteServerInfo.isSecureConnection)

// Second step: get authentication method required by the server
val authenticationMethod = getAuthenticationMethod(normalizedProtocolPrefix)

return ServerInfo(
ownCloudVersion = pairVersionAndSecure.first.version,
ownCloudVersion = remoteServerInfo.ownCloudVersion.version,
baseUrl = normalizedProtocolPrefix,
authenticationMethod = authenticationMethod,
isSecureConnection = pairVersionAndSecure.second
isSecureConnection = remoteServerInfo.isSecureConnection
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.owncloud.android.lib.common.operations.RemoteOperationResult
import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.OK_NO_SSL
import com.owncloud.android.lib.common.operations.RemoteOperationResult.ResultCode.OK_SSL
import com.owncloud.android.lib.resources.status.OwnCloudVersion
import com.owncloud.android.lib.resources.status.RemoteServerInfo
import com.owncloud.android.lib.resources.status.services.implementation.OCServerInfoService
import com.owncloud.android.testutil.OC_SERVER_INFO
import com.owncloud.android.utils.createRemoteOperationResultMock
Expand All @@ -39,6 +40,7 @@ import io.mockk.mockk
import io.mockk.verify
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test

Expand All @@ -49,7 +51,11 @@ class OCRemoteServerInfoDataSourceTest {
private val clientManager: ClientManager = mockk(relaxed = true)
private val ocClientMocked: OwnCloudClient = mockk(relaxed = true)

private val ocOwncloudVersion = OwnCloudVersion("10.3.2")
private val OC_REMOTE_SERVER_INFO = RemoteServerInfo(
ownCloudVersion = OwnCloudVersion(OC_SERVER_INFO.ownCloudVersion),
baseUrl = OC_SERVER_INFO.baseUrl,
isSecureConnection = OC_SERVER_INFO.isSecureConnection
)
private val basicAuthHeader = "basic realm=\"owncloud\", charset=\"utf-8\""
private val bearerHeader = "bearer realm=\"owncloud\""
private val authHeadersBasic = listOf(basicAuthHeader)
Expand Down Expand Up @@ -155,7 +161,7 @@ class OCRemoteServerInfoDataSourceTest {

@Test
fun getRemoteStatusIsSecureConnection() {
val expectedValue = Pair(ocOwncloudVersion, true)
val expectedValue = OC_REMOTE_SERVER_INFO.copy(isSecureConnection = true)
prepareRemoteStatusToBeRetrieved(expectedValue)

val currentValue = ocRemoteServerInfoDatasource.getRemoteStatus(OC_SERVER_INFO.baseUrl)
Expand All @@ -168,7 +174,7 @@ class OCRemoteServerInfoDataSourceTest {

@Test
fun getRemoteStatusIsNotSecureConnection() {
val expectedValue = Pair(ocOwncloudVersion, false)
val expectedValue = OC_REMOTE_SERVER_INFO.copy(isSecureConnection = false)
prepareRemoteStatusToBeRetrieved(expectedValue)

val currentValue = ocRemoteServerInfoDatasource.getRemoteStatus(OC_SERVER_INFO.baseUrl)
Expand All @@ -181,22 +187,22 @@ class OCRemoteServerInfoDataSourceTest {

@Test(expected = OwncloudVersionNotSupportedException::class)
fun getRemoteStatusOwncloudVersionNotSupported() {
val expectedValue = Pair(OwnCloudVersion("9.0.0"), false)
val expectedValue = OC_REMOTE_SERVER_INFO.copy(ownCloudVersion = OwnCloudVersion("9.0.0"))
prepareRemoteStatusToBeRetrieved(expectedValue)

ocRemoteServerInfoDatasource.getRemoteStatus(OC_SERVER_INFO.baseUrl)
}

@Test
fun getRemoteStatusOwncloudVersionHidden() {
val expectedValue = Pair(OwnCloudVersion(""), false)
val expectedValue = OC_REMOTE_SERVER_INFO.copy(ownCloudVersion = OwnCloudVersion(""))
prepareRemoteStatusToBeRetrieved(expectedValue)

ocRemoteServerInfoDatasource.getRemoteStatus(OC_SERVER_INFO.baseUrl)

val remoteStatus = ocRemoteServerInfoDatasource.getRemoteStatus(OC_SERVER_INFO.baseUrl)

assertEquals(true, remoteStatus.first.isVersionHidden)
assertTrue(remoteStatus.ownCloudVersion.isVersionHidden)
verify { ocServerInfoService.getRemoteStatus(OC_SERVER_INFO.baseUrl, ocClientMocked) }
}

Expand All @@ -214,9 +220,7 @@ class OCRemoteServerInfoDataSourceTest {
val expectedValue =
OC_SERVER_INFO.copy(authenticationMethod = AuthenticationMethod.BASIC_HTTP_AUTH, isSecureConnection = true)

prepareRemoteStatusToBeRetrieved(
Pair(OwnCloudVersion(expectedValue.ownCloudVersion), expectedValue.isSecureConnection)
)
prepareRemoteStatusToBeRetrieved(OC_REMOTE_SERVER_INFO.copy(isSecureConnection = true))
prepareAuthorizationMethodToBeRetrieved(expectedValue.authenticationMethod, true)

val currentValue = ocRemoteServerInfoDatasource.getServerInfo(OC_SERVER_INFO.baseUrl)
Expand All @@ -231,9 +235,7 @@ class OCRemoteServerInfoDataSourceTest {
val expectedValue =
OC_SERVER_INFO.copy(authenticationMethod = AuthenticationMethod.BASIC_HTTP_AUTH, isSecureConnection = false)

prepareRemoteStatusToBeRetrieved(
Pair(OwnCloudVersion(expectedValue.ownCloudVersion), expectedValue.isSecureConnection)
)
prepareRemoteStatusToBeRetrieved(OC_REMOTE_SERVER_INFO.copy(isSecureConnection = false))
prepareAuthorizationMethodToBeRetrieved(expectedValue.authenticationMethod, true)

val currentValue = ocRemoteServerInfoDatasource.getServerInfo(expectedValue.baseUrl)
Expand All @@ -248,9 +250,7 @@ class OCRemoteServerInfoDataSourceTest {
val expectedValue =
OC_SERVER_INFO.copy(authenticationMethod = AuthenticationMethod.BEARER_TOKEN, isSecureConnection = true)

prepareRemoteStatusToBeRetrieved(
Pair(OwnCloudVersion(expectedValue.ownCloudVersion), expectedValue.isSecureConnection)
)
prepareRemoteStatusToBeRetrieved(OC_REMOTE_SERVER_INFO.copy(isSecureConnection = true))
prepareAuthorizationMethodToBeRetrieved(expectedValue.authenticationMethod, true)

val currentValue = ocRemoteServerInfoDatasource.getServerInfo(OC_SERVER_INFO.baseUrl)
Expand All @@ -265,9 +265,7 @@ class OCRemoteServerInfoDataSourceTest {
val expectedValue =
OC_SERVER_INFO.copy(authenticationMethod = AuthenticationMethod.BASIC_HTTP_AUTH, isSecureConnection = true)

prepareRemoteStatusToBeRetrieved(
Pair(OwnCloudVersion(expectedValue.ownCloudVersion), expectedValue.isSecureConnection)
)
prepareRemoteStatusToBeRetrieved(OC_REMOTE_SERVER_INFO.copy(isSecureConnection = true))
prepareAuthorizationMethodToBeRetrieved(expectedValue.authenticationMethod, true)

val currentValue = ocRemoteServerInfoDatasource.getServerInfo(OC_SERVER_INFO.baseUrl)
Expand All @@ -279,13 +277,7 @@ class OCRemoteServerInfoDataSourceTest {

@Test(expected = NoConnectionWithServerException::class)
fun getServerInfoNoConnection() {
val expectedValue =
OC_SERVER_INFO.copy(authenticationMethod = AuthenticationMethod.BASIC_HTTP_AUTH, isSecureConnection = true)

prepareRemoteStatusToBeRetrieved(
Pair(OwnCloudVersion(expectedValue.ownCloudVersion), expectedValue.isSecureConnection),
NoConnectionWithServerException()
)
prepareRemoteStatusToBeRetrieved(OC_REMOTE_SERVER_INFO, NoConnectionWithServerException())

ocRemoteServerInfoDatasource.getServerInfo(OC_SERVER_INFO.baseUrl)

Expand Down Expand Up @@ -318,17 +310,17 @@ class OCRemoteServerInfoDataSourceTest {
}

private fun prepareRemoteStatusToBeRetrieved(
expectedPair: Pair<OwnCloudVersion, Boolean>,
expectedConfig: RemoteServerInfo,
exception: Exception? = null
) {
val expectedResultCode = when (expectedPair.second) {
val expectedResultCode = when (expectedConfig.isSecureConnection) {
true -> OK_SSL
false -> OK_NO_SSL
}

val remoteStatusResultMocked: RemoteOperationResult<OwnCloudVersion> =
val remoteStatusResultMocked: RemoteOperationResult<RemoteServerInfo> =
createRemoteOperationResultMock(
data = expectedPair.first,
data = expectedConfig,
isSuccess = true,
resultCode = expectedResultCode,
exception = exception
Expand Down

0 comments on commit 91ead87

Please sign in to comment.