From 1535be387609481a589487cd9efcc7d0cb8be79c Mon Sep 17 00:00:00 2001 From: "David A. Velasco" Date: Thu, 2 Feb 2017 13:32:42 +0100 Subject: [PATCH] Prevent access to accounts of other apps with the same name as an OC account --- AndroidManifest.xml | 5 + .../lib/common/SingleSessionManager.java | 223 +++++++++--------- .../lib/common/accounts/AccountUtils.java | 75 ++---- 3 files changed, 140 insertions(+), 163 deletions(-) diff --git a/AndroidManifest.xml b/AndroidManifest.xml index fe7d2ac0..adb2be3f 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -28,6 +28,11 @@ android:versionCode="1" android:versionName="1.0" > + + + + diff --git a/src/com/owncloud/android/lib/common/SingleSessionManager.java b/src/com/owncloud/android/lib/common/SingleSessionManager.java index f2c1d756..589b33fd 100644 --- a/src/com/owncloud/android/lib/common/SingleSessionManager.java +++ b/src/com/owncloud/android/lib/common/SingleSessionManager.java @@ -25,9 +25,7 @@ package com.owncloud.android.lib.common; import java.io.IOException; -import java.util.HashMap; import java.util.Iterator; -import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -46,193 +44,192 @@ import com.owncloud.android.lib.common.utils.Log_OC; /** * Implementation of {@link OwnCloudClientManager} - * + *

* TODO check multithreading safety - * + * * @author David A. Velasco * @author masensio */ public class SingleSessionManager implements OwnCloudClientManager { - - private static final String TAG = SingleSessionManager.class.getSimpleName(); + + private static final String TAG = SingleSessionManager.class.getSimpleName(); private ConcurrentMap mClientsWithKnownUsername = - new ConcurrentHashMap(); - + new ConcurrentHashMap(); + private ConcurrentMap mClientsWithUnknownUsername = - new ConcurrentHashMap(); - - + new ConcurrentHashMap(); + + @Override public OwnCloudClient getClientFor(OwnCloudAccount account, Context context) - throws AccountNotFoundException, OperationCanceledException, AuthenticatorException, - IOException { + throws AccountNotFoundException, OperationCanceledException, AuthenticatorException, + IOException { if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "getClientFor starting "); } - if (account == null) { - throw new IllegalArgumentException("Cannot get an OwnCloudClient for a null account"); - } + if (account == null) { + throw new IllegalArgumentException("Cannot get an OwnCloudClient for a null account"); + } - OwnCloudClient client = null; - String accountName = account.getName(); - String sessionName = account.getCredentials() == null ? "" : - AccountUtils.buildAccountName ( + OwnCloudClient client = null; + String accountName = account.getName(); + String sessionName = account.getCredentials() == null ? "" : + AccountUtils.buildAccountName( account.getBaseUri(), account.getCredentials().getAuthToken() - ) - ; + ); - if (accountName != null) { - client = mClientsWithKnownUsername.get(accountName); - } - boolean reusingKnown = false; // just for logs - if (client == null) { - if (accountName != null) { - client = mClientsWithUnknownUsername.remove(sessionName); - if (client != null) { + if (accountName != null) { + client = mClientsWithKnownUsername.get(accountName); + } + boolean reusingKnown = false; // just for logs + if (client == null) { + if (accountName != null) { + client = mClientsWithUnknownUsername.remove(sessionName); + if (client != null) { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "reusing client for session " + sessionName); } - mClientsWithKnownUsername.put(accountName, client); + mClientsWithKnownUsername.put(accountName, client); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "moved client to account " + accountName); } - } - } else { - client = mClientsWithUnknownUsername.get(sessionName); - } - } else { + } + } else { + client = mClientsWithUnknownUsername.get(sessionName); + } + } else { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "reusing client for account " + accountName); } - reusingKnown = true; - } - - if (client == null) { - // no client to reuse - create a new one - client = OwnCloudClientFactory.createOwnCloudClient( - account.getBaseUri(), - context.getApplicationContext(), - true); // TODO remove dependency on OwnCloudClientFactory + reusingKnown = true; + } + + if (client == null) { + // no client to reuse - create a new one + client = OwnCloudClientFactory.createOwnCloudClient( + account.getBaseUri(), + context.getApplicationContext(), + true); // TODO remove dependency on OwnCloudClientFactory client.getParams().setCookiePolicy(CookiePolicy.BROWSER_COMPATIBILITY); - // enable cookie tracking - - AccountUtils.restoreCookies(accountName, client, context); + // enable cookie tracking + + AccountUtils.restoreCookies(account.getSavedAccount(), client, context); account.loadCredentials(context); - client.setCredentials(account.getCredentials()); - if (accountName != null) { - mClientsWithKnownUsername.put(accountName, client); + client.setCredentials(account.getCredentials()); + if (accountName != null) { + mClientsWithKnownUsername.put(accountName, client); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "new client for account " + accountName); } - } else { - mClientsWithUnknownUsername.put(sessionName, client); + } else { + mClientsWithUnknownUsername.put(sessionName, client); if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "new client for session " + sessionName); } - } - } else { - if (!reusingKnown && Log.isLoggable(TAG, Log.VERBOSE)) { - Log_OC.v(TAG, "reusing client for session " + sessionName); - } - keepCredentialsUpdated(account, client); - keepUriUpdated(account, client); - } + } + } else { + if (!reusingKnown && Log.isLoggable(TAG, Log.VERBOSE)) { + Log_OC.v(TAG, "reusing client for session " + sessionName); + } + keepCredentialsUpdated(account, client); + keepUriUpdated(account, client); + } if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "getClientFor finishing "); } - return client; + return client; } - - - @Override - public OwnCloudClient removeClientFor(OwnCloudAccount account) { + + + @Override + public OwnCloudClient removeClientFor(OwnCloudAccount account) { if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "removeClientFor starting "); } - if (account == null) { - return null; - } + if (account == null) { + return null; + } - OwnCloudClient client = null; - String accountName = account.getName(); - if (accountName != null) { - client = mClientsWithKnownUsername.remove(accountName); - if (client != null) { + OwnCloudClient client; + String accountName = account.getName(); + if (accountName != null) { + client = mClientsWithKnownUsername.remove(accountName); + if (client != null) { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "Removed client for account " + accountName); } - return client; - } else { + return client; + } else { if (Log.isLoggable(TAG, Log.VERBOSE)) { Log_OC.v(TAG, "No client tracked for account " + accountName); } - } - } + } + } mClientsWithUnknownUsername.clear(); if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "removeClientFor finishing "); } - return null; - - } + return null; + + } + - @Override public void saveAllClients(Context context, String accountType) - throws AccountNotFoundException, AuthenticatorException, IOException, - OperationCanceledException { + throws AccountNotFoundException, AuthenticatorException, IOException, + OperationCanceledException { if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "Saving sessions... "); } - Iterator accountNames = mClientsWithKnownUsername.keySet().iterator(); - String accountName = null; - Account account = null; - while (accountNames.hasNext()) { - accountName = accountNames.next(); - account = new Account(accountName, accountType); - AccountUtils.saveClient( - mClientsWithKnownUsername.get(accountName), - account, - context); - } + Iterator accountNames = mClientsWithKnownUsername.keySet().iterator(); + String accountName; + Account account; + while (accountNames.hasNext()) { + accountName = accountNames.next(); + account = new Account(accountName, accountType); + AccountUtils.saveClient( + mClientsWithKnownUsername.get(accountName), + account, + context); + } if (Log.isLoggable(TAG, Log.DEBUG)) { Log_OC.d(TAG, "All sessions saved"); } } - - private void keepCredentialsUpdated(OwnCloudAccount account, OwnCloudClient reusedClient) { - OwnCloudCredentials recentCredentials = account.getCredentials(); - if (recentCredentials != null && !recentCredentials.getAuthToken().equals( - reusedClient.getCredentials().getAuthToken())) { - reusedClient.setCredentials(recentCredentials); - } - - } - // this method is just a patch; we need to distinguish accounts in the same host but - // different paths; but that requires updating the accountNames for apps upgrading - private void keepUriUpdated(OwnCloudAccount account, OwnCloudClient reusedClient) { - Uri recentUri = account.getBaseUri(); - if (!recentUri.equals(reusedClient.getBaseUri())) { - reusedClient.setBaseUri(recentUri); - } - - } + private void keepCredentialsUpdated(OwnCloudAccount account, OwnCloudClient reusedClient) { + OwnCloudCredentials recentCredentials = account.getCredentials(); + if (recentCredentials != null && !recentCredentials.getAuthToken().equals( + reusedClient.getCredentials().getAuthToken())) { + reusedClient.setCredentials(recentCredentials); + } + + } + + // this method is just a patch; we need to distinguish accounts in the same host but + // different paths; but that requires updating the accountNames for apps upgrading + private void keepUriUpdated(OwnCloudAccount account, OwnCloudClient reusedClient) { + Uri recentUri = account.getBaseUri(); + if (!recentUri.equals(reusedClient.getBaseUri())) { + reusedClient.setBaseUri(recentUri); + } + + } } diff --git a/src/com/owncloud/android/lib/common/accounts/AccountUtils.java b/src/com/owncloud/android/lib/common/accounts/AccountUtils.java index 7ebf3d11..8cb1f115 100644 --- a/src/com/owncloud/android/lib/common/accounts/AccountUtils.java +++ b/src/com/owncloud/android/lib/common/accounts/AccountUtils.java @@ -282,68 +282,43 @@ public class AccountUtils { /** - * Restore the client cookies + * Restore the client cookies persisted in an account stored in the system AccountManager. * - * @param account - * @param client - * @param context + * @param account Stored account. + * @param client Client to restore cookies in. + * @param context Android context used to access the system AccountManager. */ public static void restoreCookies(Account account, OwnCloudClient client, Context context) { + if (account == null) { + Log_OC.d(TAG, "Cannot restore cookie for null account"); - Log_OC.d(TAG, "Restoring cookies for " + account.name); + } else { + Log_OC.d(TAG, "Restoring cookies for " + account.name); - // Account Manager - AccountManager am = AccountManager.get(context.getApplicationContext()); + // Account Manager + AccountManager am = AccountManager.get(context.getApplicationContext()); - Uri serverUri = (client.getBaseUri() != null) ? client.getBaseUri() : client.getWebdavUri(); + Uri serverUri = (client.getBaseUri() != null) ? client.getBaseUri() : client.getWebdavUri(); - String cookiesString = am.getUserData(account, Constants.KEY_COOKIES); - if (cookiesString != null) { - String[] cookies = cookiesString.split(";"); - if (cookies.length > 0) { - for (int i = 0; i < cookies.length; i++) { - Cookie cookie = new Cookie(); - int equalPos = cookies[i].indexOf('='); - cookie.setName(cookies[i].substring(0, equalPos)); - cookie.setValue(cookies[i].substring(equalPos + 1)); - cookie.setDomain(serverUri.getHost()); // VERY IMPORTANT - cookie.setPath(serverUri.getPath()); // VERY IMPORTANT + String cookiesString = am.getUserData(account, Constants.KEY_COOKIES); + if (cookiesString != null) { + String[] cookies = cookiesString.split(";"); + if (cookies.length > 0) { + for (int i = 0; i < cookies.length; i++) { + Cookie cookie = new Cookie(); + int equalPos = cookies[i].indexOf('='); + cookie.setName(cookies[i].substring(0, equalPos)); + cookie.setValue(cookies[i].substring(equalPos + 1)); + cookie.setDomain(serverUri.getHost()); // VERY IMPORTANT + cookie.setPath(serverUri.getPath()); // VERY IMPORTANT - client.getState().addCookie(cookie); + client.getState().addCookie(cookie); + } } } } } - /** - * Restore the client cookies from accountName - * - * @param accountName - * @param client - * @param context - */ - public static void restoreCookies(String accountName, OwnCloudClient client, Context context) { - Log_OC.d(TAG, "Restoring cookies for " + accountName); - - // Account Manager - AccountManager am = AccountManager.get(context.getApplicationContext()); - - // Get account - Account account = null; - Account accounts[] = am.getAccounts(); - for (Account a : accounts) { - if (a.name.equals(accountName)) { - account = a; - break; - } - } - - // Restoring cookies - if (account != null) { - restoreCookies(account, client, context); - } - } - public static class AccountNotFoundException extends AccountsException { /** @@ -368,7 +343,7 @@ public class AccountUtils { /** * Value under this key should handle path to webdav php script. Will be * removed and usage should be replaced by combining - * {@link com.owncloud.android.authentication.AuthenticatorActivity.KEY_OC_BASE_URL} and + * {@link #KEY_OC_BASE_URL } and * {@link com.owncloud.android.lib.resources.status.OwnCloudVersion} * * @deprecated