summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoracleung@chromium.org <acleung@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-25 03:09:04 +0000
committeracleung@chromium.org <acleung@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-04-25 03:09:04 +0000
commit26f9ed0135aca70e48ea8107310a144af0541898 (patch)
tree61d5d198bf0d2688450b304aec2b35ec86f1926a
parent0ea3c436c962779674fb0627879cd21addd03113 (diff)
downloadchromium_src-26f9ed0135aca70e48ea8107310a144af0541898.zip
chromium_src-26f9ed0135aca70e48ea8107310a144af0541898.tar.gz
chromium_src-26f9ed0135aca70e48ea8107310a144af0541898.tar.bz2
Calls FireRefreshTokenRevoked if an account is removed.
We cannot reply on periodic reconcile actions to remove accounts from cookies. The reason is that we actually *don't* do it at that stage for Android. Instead, we are going to keep a list of accounts last seen by Chrome and store that in a shared pref. We will come to the old list to the new list and call FireRefreshTokenRevoked accordingingly. BUG=305086 Review URL: https://codereview.chromium.org/213823004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266108 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java45
-rw-r--r--chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java116
-rw-r--r--chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java18
-rw-r--r--chrome/browser/android/signin/signin_manager_android.cc3
-rw-r--r--chrome/browser/signin/android_profile_oauth2_token_service.cc95
-rw-r--r--chrome/browser/signin/android_profile_oauth2_token_service.h15
-rw-r--r--sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java6
7 files changed, 259 insertions, 39 deletions
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java b/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
index 2e5daa8..9adf0cc 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/signin/OAuth2TokenService.java
@@ -7,8 +7,11 @@ package org.chromium.chrome.browser.signin;
import android.accounts.Account;
import android.app.Activity;
import android.content.Context;
+import android.preference.PreferenceManager;
import android.util.Log;
+import com.google.common.annotations.VisibleForTesting;
+
import org.chromium.base.CalledByNative;
import org.chromium.base.ObserverList;
import org.chromium.base.ThreadUtils;
@@ -16,9 +19,12 @@ import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.sync.signin.AccountManagerHelper;
import org.chromium.sync.signin.ChromeSigninController;
+import java.util.Arrays;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.HashSet;
+import java.util.Set;
import javax.annotation.Nullable;
@@ -33,6 +39,9 @@ public final class OAuth2TokenService {
private static final String TAG = "OAuth2TokenService";
+ @VisibleForTesting
+ public static final String STORED_ACCOUNTS_KEY = "google.services.stored_accounts";
+
public interface OAuth2TokenServiceObserver {
void onRefreshTokenAvailable(Account account);
void onRefreshTokenRevoked(Account account);
@@ -86,16 +95,28 @@ public final class OAuth2TokenService {
}
/**
- * Called by native to list the accounts with OAuth2 refresh tokens.
+ * Called by native to list the activite accounts in the OS.
*/
+ @VisibleForTesting
@CalledByNative
- public static String[] getAccounts(Context context) {
+ public static String[] getSystemAccounts(Context context) {
AccountManagerHelper accountManagerHelper = AccountManagerHelper.get(context);
java.util.List<String> accountNames = accountManagerHelper.getGoogleAccountNames();
return accountNames.toArray(new String[accountNames.size()]);
}
/**
+ * Called by native to list the accounts with OAuth2 refresh tokens.
+ * This can differ from getSystemAccounts as the user add/remove accounts
+ * from the OS. validateAccounts should be called to keep these two
+ * in sync.
+ */
+ @CalledByNative
+ public static String[] getAccounts(Context context) {
+ return getStoredAccounts(context);
+ }
+
+ /**
* Called by native to retrieve OAuth2 tokens.
*
* @param username The native username (full address).
@@ -202,9 +223,7 @@ public final class OAuth2TokenService {
ThreadUtils.assertOnUiThread();
String currentlySignedInAccount =
ChromeSigninController.get(context).getSignedInAccountName();
- String[] accounts = getAccounts(context);
- nativeValidateAccounts(
- mNativeProfileOAuth2TokenService, accounts, currentlySignedInAccount);
+ nativeValidateAccounts(mNativeProfileOAuth2TokenService, currentlySignedInAccount);
}
/**
@@ -262,12 +281,26 @@ public final class OAuth2TokenService {
}
}
+ private static String[] getStoredAccounts(Context context) {
+ Set<String> accounts =
+ PreferenceManager.getDefaultSharedPreferences(context)
+ .getStringSet(STORED_ACCOUNTS_KEY, null);
+ return accounts == null ? new String[]{} : accounts.toArray(new String[accounts.size()]);
+ }
+
+ @CalledByNative
+ private static void saveStoredAccounts(Context context, String[] accounts) {
+ Set<String> set = new HashSet<String>(Arrays.asList(accounts));
+ PreferenceManager.getDefaultSharedPreferences(context).edit().
+ putStringSet(STORED_ACCOUNTS_KEY, set).apply();
+ }
+
private static native Object nativeGetForProfile(Profile profile);
private static native void nativeOAuth2TokenFetched(
String authToken, boolean result, long nativeCallback);
private native void nativeValidateAccounts(
long nativeAndroidProfileOAuth2TokenService,
- String[] accounts, String currentlySignedInAccount);
+ String currentlySignedInAccount);
private native void nativeFireRefreshTokenAvailableFromJava(
long nativeAndroidProfileOAuth2TokenService, String accountName);
private native void nativeFireRefreshTokenRevokedFromJava(
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
index c4067ce..1ba655e 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceIntegrationTest.java
@@ -201,6 +201,122 @@ public class OAuth2TokenServiceIntegrationTest extends ChromeShellTestBase {
@MediumTest
@UiThreadTest
+ public void testValidateAccountsSingleAccountWithoutChanges() {
+ // Add account.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+
+ // Mark user as signed in.
+ mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
+
+ // Run one validation.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(1, mObserver.getAvailableCallCount());
+ assertEquals(0, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+
+ // Re-run validation.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getAvailableCallCount());
+ assertEquals(0, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+ }
+
+ @MediumTest
+ @UiThreadTest
+ public void testValidateAccountsSingleAccountThenAddOne() {
+ // Add account.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+
+ // Mark user as signed in.
+ mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
+
+ // Run one validation.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(1, mObserver.getAvailableCallCount());
+ assertEquals(0, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+
+ // Add another account.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Re-run validation.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(3, mObserver.getAvailableCallCount());
+ assertEquals(0, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+ }
+
+ @MediumTest
+ @UiThreadTest
+ public void testValidateAccountsTwoAccountsThenRemoveOne() {
+ // Add accounts.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Mark user as signed in.
+ mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
+
+ // Run one validation.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getAvailableCallCount());
+
+ mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+ mOAuth2TokenService.validateAccounts(mContext);
+
+ assertEquals(3, mObserver.getAvailableCallCount());
+ assertEquals(1, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+ }
+
+ @MediumTest
+ @UiThreadTest
+ public void testValidateAccountsTwoAccountsThenRemoveAll() {
+ // Add accounts.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Mark user as signed in.
+ mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
+
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getAvailableCallCount());
+
+ // Remove all.
+ mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+ mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Re-validate and run checks.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+ }
+
+ @MediumTest
+ @UiThreadTest
+ public void testValidateAccountsTwoAccountsThenRemoveAllSignOut() {
+ // Add accounts.
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+ mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Mark user as signed in.
+ mChromeSigninController.setSignedInAccountName(TEST_ACCOUNT1.name);
+
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getAvailableCallCount());
+
+ // Remove all.
+ mChromeSigninController.clearSignedInUser();
+ mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
+ mAccountManager.removeAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_2);
+
+ // Re-validate and run checks.
+ mOAuth2TokenService.validateAccounts(mContext);
+ assertEquals(2, mObserver.getRevokedCallCount());
+ assertEquals(0, mObserver.getLoadedCallCount());
+ }
+
+ @MediumTest
+ @UiThreadTest
public void testValidateAccountsTwoAccountsRegisteredAndOneSignedIn() {
// Add accounts.
mAccountManager.addAccountHolderExplicitly(TEST_ACCOUNT_HOLDER_1);
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java
index 78baa84..c80ffd7 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/signin/OAuth2TokenServiceTest.java
@@ -46,9 +46,12 @@ public class OAuth2TokenServiceTest extends InstrumentationTestCase {
AccountHolder accountHolder1 = AccountHolder.create().account(account1).build();
mAccountManager.addAccountHolderExplicitly(accountHolder1);
+ String[] sysAccounts = OAuth2TokenService.getSystemAccounts(mContext);
+ assertEquals("There should be one registered account", 1, sysAccounts.length);
+ assertEquals("The account should be " + account1, account1.name, sysAccounts[0]);
+
String[] accounts = OAuth2TokenService.getAccounts(mContext);
- assertEquals("There should be one registered account", 1, accounts.length);
- assertEquals("The account should be " + account1, account1.name, accounts[0]);
+ assertEquals("There should be zero registered account", 0, accounts.length);
}
@SmallTest
@@ -61,12 +64,15 @@ public class OAuth2TokenServiceTest extends InstrumentationTestCase {
AccountHolder accountHolder2 = AccountHolder.create().account(account2).build();
mAccountManager.addAccountHolderExplicitly(accountHolder2);
- String[] accounts = OAuth2TokenService.getAccounts(mContext);
- assertEquals("There should be one registered account", 2, accounts.length);
+ String[] sysAccounts = OAuth2TokenService.getSystemAccounts(mContext);
+ assertEquals("There should be one registered account", 2, sysAccounts.length);
assertTrue("The list should contain " + account1,
- Arrays.asList(accounts).contains(account1.name));
+ Arrays.asList(sysAccounts).contains(account1.name));
assertTrue("The list should contain " + account2,
- Arrays.asList(accounts).contains(account2.name));
+ Arrays.asList(sysAccounts).contains(account2.name));
+
+ String[] accounts = OAuth2TokenService.getAccounts(mContext);
+ assertEquals("There should be zero registered account", 0, accounts.length);
}
@SmallTest
diff --git a/chrome/browser/android/signin/signin_manager_android.cc b/chrome/browser/android/signin/signin_manager_android.cc
index 43af5fa..153934a 100644
--- a/chrome/browser/android/signin/signin_manager_android.cc
+++ b/chrome/browser/android/signin/signin_manager_android.cc
@@ -225,8 +225,7 @@ void SigninManagerAndroid::LogInSignedInUser(JNIEnv* env, jobject obj) {
profile_);
const std::string& primary_acct =
signin_manager->GetAuthenticatedAccountId();
- const std::vector<std::string>& ids = token_service->GetAccounts();
- token_service->ValidateAccounts(primary_acct, ids);
+ token_service->ValidateAccounts(primary_acct);
} else {
DVLOG(1) << "SigninManagerAndroid::LogInSignedInUser "
diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.cc b/chrome/browser/signin/android_profile_oauth2_token_service.cc
index 71fe1c3..c88af81 100644
--- a/chrome/browser/signin/android_profile_oauth2_token_service.cc
+++ b/chrome/browser/signin/android_profile_oauth2_token_service.cc
@@ -98,6 +98,18 @@ std::vector<std::string> AndroidProfileOAuth2TokenService::GetAccounts() {
return accounts;
}
+std::vector<std::string> AndroidProfileOAuth2TokenService::GetSystemAccounts() {
+ std::vector<std::string> accounts;
+ JNIEnv* env = AttachCurrentThread();
+ ScopedJavaLocalRef<jobjectArray> j_accounts =
+ Java_OAuth2TokenService_getSystemAccounts(
+ env, base::android::GetApplicationContext());
+ base::android::AppendJavaStringArrayToStringVector(env,
+ j_accounts.obj(),
+ &accounts);
+ return accounts;
+}
+
void AndroidProfileOAuth2TokenService::FetchOAuth2Token(
RequestImpl* request,
const std::string& account_id,
@@ -157,43 +169,74 @@ void AndroidProfileOAuth2TokenService::InvalidateOAuth2Token(
j_access_token.obj());
}
-void AndroidProfileOAuth2TokenService::ValidateAccounts(JNIEnv* env,
- jobject obj,
- jobjectArray accounts,
- jstring j_current_acc) {
- std::vector<std::string> account_ids;
- base::android::AppendJavaStringArrayToStringVector(env,
- accounts,
- &account_ids);
+void AndroidProfileOAuth2TokenService::ValidateAccounts(
+ JNIEnv* env,
+ jobject obj,
+ jstring j_current_acc) {
std::string signed_in_account = ConvertJavaStringToUTF8(env, j_current_acc);
- ValidateAccounts(signed_in_account, account_ids);
+ ValidateAccounts(signed_in_account);
}
void AndroidProfileOAuth2TokenService::ValidateAccounts(
- const std::string& signed_in_account,
- const std::vector<std::string>& account_ids) {
- if (signed_in_account.empty())
- return;
+ const std::string& signed_in_account) {
+ std::vector<std::string> prev_ids = GetAccounts();
+ std::vector<std::string> curr_ids = GetSystemAccounts();
+
+ if (!ValidateAccounts(signed_in_account, prev_ids, curr_ids)) {
+ curr_ids.clear();
+ }
- if (std::find(account_ids.begin(),
- account_ids.end(),
- signed_in_account) != account_ids.end()) {
- // Currently signed in account still exists among accounts on system.
- std::vector<std::string> ids = GetAccounts();
+ JNIEnv* env = AttachCurrentThread();
+ ScopedJavaLocalRef<jobjectArray> java_accounts(
+ base::android::ToJavaArrayOfStrings(env, curr_ids));
+ Java_OAuth2TokenService_saveStoredAccounts(
+ env, base::android::GetApplicationContext(), java_accounts.obj());
+}
+
+bool AndroidProfileOAuth2TokenService::ValidateAccounts(
+ const std::string& signed_in_account,
+ const std::vector<std::string>& prev_account_ids,
+ const std::vector<std::string>& curr_account_ids) {
+ if (std::find(curr_account_ids.begin(),
+ curr_account_ids.end(),
+ signed_in_account) != curr_account_ids.end()) {
+ // Test to see if an account is removed from the Android AccountManager.
+ // If so, invoke FireRefreshTokenRevoked to notify the reconcilor.
+ for (std::vector<std::string>::const_iterator it = prev_account_ids.begin();
+ it != prev_account_ids.end(); it++) {
+ if (*it == signed_in_account)
+ continue;
+
+ if (std::find(curr_account_ids.begin(),
+ curr_account_ids.end(),
+ *it) == curr_account_ids.end()) {
+ FireRefreshTokenRevoked(*it);
+ }
+ }
// Always fire the primary signed in account first.
FireRefreshTokenAvailable(signed_in_account);
- for (std::vector<std::string>::iterator it = ids.begin();
- it != ids.end(); it++) {
+ for (std::vector<std::string>::const_iterator it = curr_account_ids.begin();
+ it != curr_account_ids.end(); it++) {
if (*it != signed_in_account) {
FireRefreshTokenAvailable(*it);
}
}
+ return true;
} else {
// Currently signed in account does not any longer exist among accounts on
- // system.
- FireRefreshTokenRevoked(signed_in_account);
+ // system together with all other accounts.
+ if (!signed_in_account.empty()) {
+ FireRefreshTokenRevoked(signed_in_account);
+ }
+ for (std::vector<std::string>::const_iterator it = prev_account_ids.begin();
+ it != prev_account_ids.end(); it++) {
+ if (*it == signed_in_account)
+ continue;
+ FireRefreshTokenRevoked(*it);
+ }
+ return false;
}
}
@@ -252,6 +295,14 @@ void AndroidProfileOAuth2TokenService::FireRefreshTokensLoaded() {
env, java_ref_.obj());
}
+void AndroidProfileOAuth2TokenService::RevokeAllCredentials() {
+ std::vector<std::string> accounts = GetAccounts();
+ for (std::vector<std::string>::iterator it = accounts.begin();
+ it != accounts.end(); it++) {
+ FireRefreshTokenRevoked(*it);
+ }
+}
+
// Called from Java when fetching of an OAuth2 token is finished. The
// |authToken| param is only valid when |result| is true.
void OAuth2TokenFetched(JNIEnv* env, jclass clazz,
diff --git a/chrome/browser/signin/android_profile_oauth2_token_service.h b/chrome/browser/signin/android_profile_oauth2_token_service.h
index a641e3e..d1de5a6 100644
--- a/chrome/browser/signin/android_profile_oauth2_token_service.h
+++ b/chrome/browser/signin/android_profile_oauth2_token_service.h
@@ -42,15 +42,20 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService {
// Lists account IDs of all accounts with a refresh token.
virtual std::vector<std::string> GetAccounts() OVERRIDE;
+ // Lists account at the OS level.
+ std::vector<std::string> GetSystemAccounts();
+
void ValidateAccounts(JNIEnv* env,
jobject obj,
- jobjectArray accounts,
jstring current_account);
// Takes a the signed in sync account as well as all the other
// android account ids and check the token status of each.
- void ValidateAccounts(const std::string& signed_in_account,
- const std::vector<std::string>& account_ids);
+ void ValidateAccounts(const std::string& signed_in_account);
+
+ bool ValidateAccounts(const std::string& signed_in_account,
+ const std::vector<std::string>& prev_account_ids,
+ const std::vector<std::string>& curr_account_ids);
// Triggers a notification to all observers of the OAuth2TokenService that a
// refresh token is now available. This may cause observers to retry
@@ -67,6 +72,10 @@ class AndroidProfileOAuth2TokenService : public ProfileOAuth2TokenService {
// refresh tokens have now been loaded.
virtual void FireRefreshTokensLoadedFromJava(JNIEnv* env, jobject obj);
+ // Overridden from OAuth2TokenService to complete signout of all
+ // OA2TService aware accounts.
+ virtual void RevokeAllCredentials() OVERRIDE;
+
protected:
friend class ProfileOAuth2TokenServiceFactory;
AndroidProfileOAuth2TokenService();
diff --git a/sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java b/sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java
index 6f575e2..89410cf 100644
--- a/sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java
+++ b/sync/test/android/javatests/src/org/chromium/sync/test/util/MockAccountManager.java
@@ -141,6 +141,12 @@ public class MockAccountManager implements AccountManagerDelegate {
return result;
}
+ public boolean removeAccountHolderExplicitly(AccountHolder accountHolder) {
+ boolean result = mAccounts.remove(accountHolder);
+ postAsyncAccountChangedEvent();
+ return result;
+ }
+
@Override
public AccountManagerFuture<Boolean> removeAccount(Account account,
AccountManagerCallback<Boolean> callback, Handler handler) {