[DO NOT MERGE] Use tokens instead of account access trackers

We keep track which process saw and account to whitelist
the app for future access as an optimization to avoid
prompting the user for account access approval. Some apps
use SefeParcelable where the parcels are marshalled
which does not allow the parcel to contain IBinders.
To avoid this we are switching from account tracker remote
objects to unforgeable tokens.

bug:31162498

Change-Id: I3b52bff720655f695ad0c58d420eb35ef93161b9

diff --git a/Android.mk b/Android.mk
index d765e9c..6f3eb21 100644 (file)
--- a/Android.mk
+++ b/Android.mk
@@ -63,7 +63,6 @@ LOCAL_SRC_FILES += \
        core/java/android/accessibilityservice/IAccessibilityServiceClient.aidl \
        core/java/android/accounts/IAccountManager.aidl \
        core/java/android/accounts/IAccountManagerResponse.aidl \
-       core/java/android/accounts/IAccountAccessTracker.aidl \
        core/java/android/accounts/IAccountAuthenticator.aidl \
        core/java/android/accounts/IAccountAuthenticatorResponse.aidl \
        core/java/android/app/IActivityContainer.aidl \

diff --git a/core/java/android/accounts/Account.java b/core/java/android/accounts/Account.java
index 6c16e32..b6e85f1 100644 (file)
--- a/core/java/android/accounts/Account.java
+++ b/core/java/android/accounts/Account.java
@@ -18,9 +18,11 @@ package android.accounts;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
+import android.content.Context;
 import android.os.Parcelable;
 import android.os.Parcel;
 import android.os.RemoteException;
+import android.os.ServiceManager;
 import android.text.TextUtils;
 import android.util.ArraySet;
 import android.util.Log;
@@ -41,7 +43,7 @@ public class Account implements Parcelable {
 
     public final String name;
     public final String type;
-    private final @Nullable IAccountAccessTracker mAccessTracker;
+    private final @Nullable String accessId;
 
     public boolean equals(Object o) {
         if (o == this) return true;
@@ -64,14 +66,14 @@ public class Account implements Parcelable {
     /**
      * @hide
      */
-    public Account(@NonNull Account other, @Nullable IAccountAccessTracker accessTracker) {
-        this(other.name, other.type, accessTracker);
+    public Account(@NonNull Account other, @NonNull String accessId) {
+        this(other.name, other.type, accessId);
     }
 
     /**
      * @hide
      */
-    public Account(String name, String type, IAccountAccessTracker accessTracker) {
+    public Account(String name, String type, String accessId) {
         if (TextUtils.isEmpty(name)) {
             throw new IllegalArgumentException("the name must not be empty: " + name);
         }
@@ -80,18 +82,20 @@ public class Account implements Parcelable {
         }
         this.name = name;
         this.type = type;
-        this.mAccessTracker = accessTracker;
+        this.accessId = accessId;
     }
 
     public Account(Parcel in) {
         this.name = in.readString();
         this.type = in.readString();
-        this.mAccessTracker = IAccountAccessTracker.Stub.asInterface(in.readStrongBinder());
-        if (mAccessTracker != null) {
+        this.accessId = in.readString();
+        if (accessId != null) {
             synchronized (sAccessedAccounts) {
                 if (sAccessedAccounts.add(this)) {
                     try {
-                        mAccessTracker.onAccountAccessed();
+                        IAccountManager accountManager = IAccountManager.Stub.asInterface(
+                                ServiceManager.getService(Context.ACCOUNT_SERVICE));
+                        accountManager.onAccountAccessed(accessId);
                     } catch (RemoteException e) {
                         Log.e(TAG, "Error noting account access", e);
                     }
@@ -101,8 +105,8 @@ public class Account implements Parcelable {
     }
 
     /** @hide */
-    public IAccountAccessTracker getAccessTracker() {
-        return mAccessTracker;
+    public String getAccessId() {
+        return accessId;
     }
 
     public int describeContents() {
@@ -112,7 +116,7 @@ public class Account implements Parcelable {
     public void writeToParcel(Parcel dest, int flags) {
         dest.writeString(name);
         dest.writeString(type);
-        dest.writeStrongInterface(mAccessTracker);
+        dest.writeString(accessId);
     }
 
     public static final Creator<Account> CREATOR = new Creator<Account>() {

diff --git a/core/java/android/accounts/AccountManager.java b/core/java/android/accounts/AccountManager.java
index 0a9dc3a..72707b7 100644 (file)
--- a/core/java/android/accounts/AccountManager.java
+++ b/core/java/android/accounts/AccountManager.java
@@ -179,12 +179,12 @@ public class AccountManager {
     public static final String KEY_ACCOUNT_TYPE = "accountType";
 
     /**
-     * Bundle key used for the {@link IAccountAccessTracker} account access tracker
-     * used for noting the account was accessed when unmarshalled from a parcel.
+     * Bundle key used for the account access id used for noting the
+     * account was accessed when unmarshalled from a parcel.
      *
      * @hide
      */
-    public static final String KEY_ACCOUNT_ACCESS_TRACKER = "accountAccessTracker";
+    public static final String KEY_ACCOUNT_ACCESS_ID = "accountAccessId";
 
     /**
      * Bundle key used for the auth token value in results
@@ -821,9 +821,8 @@ public class AccountManager {
             public Account bundleToResult(Bundle bundle) throws AuthenticatorException {
                 String name = bundle.getString(KEY_ACCOUNT_NAME);
                 String type = bundle.getString(KEY_ACCOUNT_TYPE);
-                IAccountAccessTracker tracker = IAccountAccessTracker.Stub.asInterface(
-                        bundle.getBinder(KEY_ACCOUNT_ACCESS_TRACKER));
-                return new Account(name, type, tracker);
+                String accessId = bundle.getString(KEY_ACCOUNT_ACCESS_ID);
+                return new Account(name, type, accessId);
             }
         }.start();
     }
@@ -2279,7 +2278,7 @@ public class AccountManager {
                                     result.putString(KEY_ACCOUNT_NAME, null);
                                     result.putString(KEY_ACCOUNT_TYPE, null);
                                     result.putString(KEY_AUTHTOKEN, null);
-                                    result.putBinder(KEY_ACCOUNT_ACCESS_TRACKER, null);
+                                    result.putBinder(KEY_ACCOUNT_ACCESS_ID, null);
                                     try {
                                         mResponse.onResult(result);
                                     } catch (RemoteException e) {
@@ -2306,9 +2305,7 @@ public class AccountManager {
                                             Account account = new Account(
                                                     value.getString(KEY_ACCOUNT_NAME),
                                                     value.getString(KEY_ACCOUNT_TYPE),
-                                                    IAccountAccessTracker.Stub.asInterface(
-                                                            value.getBinder(
-                                                                    KEY_ACCOUNT_ACCESS_TRACKER)));
+                                                    value.getString(KEY_ACCOUNT_ACCESS_ID));
                                             mFuture = getAuthToken(account, mAuthTokenType,
                                                     mLoginOptions,  mActivity, mMyCallback,
                                                     mHandler);
@@ -2358,9 +2355,8 @@ public class AccountManager {
                         setException(new AuthenticatorException("account not in result"));
                         return;
                     }
-                    final IAccountAccessTracker tracker = IAccountAccessTracker.Stub.asInterface(
-                            result.getBinder(KEY_ACCOUNT_ACCESS_TRACKER));
-                    final Account account = new Account(accountName, accountType, tracker);
+                    final String accessId = result.getString(KEY_ACCOUNT_ACCESS_ID);
+                    final Account account = new Account(accountName, accountType, accessId);
                     mNumAccounts = 1;
                     getAuthToken(account, mAuthTokenType, null /* options */, mActivity,
                             mMyCallback, mHandler);

diff --git a/core/java/android/accounts/IAccountAccessTracker.aidl b/core/java/android/accounts/IAccountAccessTracker.aidl
deleted file mode 100644 (file)
index e12b3d1..0000000
--- a/core/java/android/accounts/IAccountAccessTracker.aidl
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package android.accounts;
-
-/**
- * Interface to track which apps accessed an account
- *
- * @hide
- */
-oneway interface IAccountAccessTracker {
-    void onAccountAccessed();
-}

diff --git a/core/java/android/accounts/IAccountManager.aidl b/core/java/android/accounts/IAccountManager.aidl
index 56a6488..c271e7e 100644 (file)
--- a/core/java/android/accounts/IAccountManager.aidl
+++ b/core/java/android/accounts/IAccountManager.aidl
@@ -110,4 +110,6 @@ interface IAccountManager {
     /* Crate an intent to request account access for package and a given user id */
     IntentSender createRequestAccountAccessIntentSenderAsUser(in Account account,
         String packageName, in UserHandle userHandle);
+
+    void onAccountAccessed(String token);
 }

diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index 82605c6..a7a79cd 100644 (file)
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -26,7 +26,6 @@ import android.accounts.AccountManagerInternal;
 import android.accounts.AuthenticatorDescription;
 import android.accounts.CantAddAccountActivity;
 import android.accounts.GrantCredentialsPermissionActivity;
-import android.accounts.IAccountAccessTracker;
 import android.accounts.IAccountAuthenticator;
 import android.accounts.IAccountAuthenticatorResponse;
 import android.accounts.IAccountManager;
@@ -89,7 +88,6 @@ import android.os.UserManager;
 import android.os.storage.StorageManager;
 import android.text.TextUtils;
 import android.util.Log;
-import android.util.PackageUtils;
 import android.util.Pair;
 import android.util.Slog;
 import android.util.SparseArray;
@@ -128,6 +126,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.UUID;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
@@ -692,7 +692,7 @@ public class AccountManagerService
                     final Account[] accountsForType = new Account[accountNames.size()];
                     for (int i = 0; i < accountsForType.length; i++) {
                         accountsForType[i] = new Account(accountNames.get(i), accountType,
-                                new AccountAccessTracker());
+                                UUID.randomUUID().toString());
                     }
                     accounts.accountCache.put(accountType, accountsForType);
                 }
@@ -1502,8 +1502,8 @@ public class AccountManagerService
             Bundle result = new Bundle();
             result.putString(AccountManager.KEY_ACCOUNT_NAME, resultingAccount.name);
             result.putString(AccountManager.KEY_ACCOUNT_TYPE, resultingAccount.type);
-            result.putBinder(AccountManager.KEY_ACCOUNT_ACCESS_TRACKER,
-                    resultingAccount.getAccessTracker().asBinder());
+            result.putString(AccountManager.KEY_ACCOUNT_ACCESS_ID,
+                    resultingAccount.getAccessId());
             try {
                 response.onResult(result);
             } catch (RemoteException e) {
@@ -4079,6 +4079,30 @@ public class AccountManagerService
         }
     }
 
+    @Override
+    public void onAccountAccessed(String token) throws RemoteException {
+        final int uid = Binder.getCallingUid();
+        if (UserHandle.getAppId(uid) == Process.SYSTEM_UID) {
+            return;
+        }
+        final int userId = UserHandle.getCallingUserId();
+        final long identity = Binder.clearCallingIdentity();
+        try {
+            for (Account account : getAccounts(userId, mContext.getOpPackageName())) {
+                if (Objects.equals(account.getAccessId(), token)) {
+                    // An app just accessed the account. At this point it knows about
+                    // it and there is not need to hide this account from the app.
+                    if (!hasAccountAccess(account, null, uid)) {
+                        updateAppPermission(account, AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE,
+                                uid, true);
+                    }
+                }
+            }
+        } finally {
+            Binder.restoreCallingIdentity(identity);
+        }
+    }
+
     private abstract class Session extends IAccountAuthenticatorResponse.Stub
             implements IBinder.DeathRecipient, ServiceConnection {
         IAccountManagerResponse mResponse;
@@ -5714,9 +5738,9 @@ public class AccountManagerService
         if (accountsForType != null) {
             System.arraycopy(accountsForType, 0, newAccountsForType, 0, oldLength);
         }
-        IAccountAccessTracker accessTracker = account.getAccessTracker() != null
-                ? account.getAccessTracker() : new AccountAccessTracker();
-        newAccountsForType[oldLength] = new Account(account, accessTracker);
+        String token = account.getAccessId() != null ? account.getAccessId()
+                : UUID.randomUUID().toString();
+        newAccountsForType[oldLength] = new Account(account, token);
         accounts.accountCache.put(account.type, newAccountsForType);
         return newAccountsForType[oldLength];
     }
@@ -5961,33 +5985,6 @@ public class AccountManagerService
         }
     }
 
-    private final class AccountAccessTracker extends IAccountAccessTracker.Stub {
-        @Override
-        public void onAccountAccessed() throws RemoteException {
-            final int uid = Binder.getCallingUid();
-            if (UserHandle.getAppId(uid) == Process.SYSTEM_UID) {
-                return;
-            }
-            final int userId = UserHandle.getCallingUserId();
-            final long identity = Binder.clearCallingIdentity();
-            try {
-                for (Account account : getAccounts(userId, mContext.getOpPackageName())) {
-                    IAccountAccessTracker accountTracker = account.getAccessTracker();
-                    if (accountTracker != null && asBinder() == accountTracker.asBinder()) {
-                        // An app just accessed the account. At this point it knows about
-                        // it and there is not need to hide this account from the app.
-                        if (!hasAccountAccess(account, null, uid)) {
-                            updateAppPermission(account, AccountManager.ACCOUNT_ACCESS_TOKEN_TYPE,
-                                    uid, true);
-                        }
-                    }
-                }
-            } finally {
-                Binder.restoreCallingIdentity(identity);
-            }
-        }
-    }
-
     private final class AccountManagerInternalImpl extends AccountManagerInternal {
         private final Object mLock = new Object();