Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 40bb46d7 authored by Svetoslav's avatar Svetoslav Committed by The Android Automerger
Browse files

Add get_accounts app op 

For each runtime permission we have an app op to toggle the
permission for legacy apps as they cannot handle permission
revocations. We were lacking an app op for get_accounts
which prevented the user from controlling access to accounts
regardelss that they change the state of the permission
toggle in the UI. Even worse the permission UI is written
with the assumption that every runtime permission has an
app op and as a result revoking the contacts group (if the
app requests the get_accounts permission) is reset back to
allowed in the UI.

bug:23854618

Change-Id: I9e3f9bfeb320bed561d718db99ee285915d5701b
parent 704a5a0c

core/java/android/accounts/AccountManager.java

+10 −7
Original line number Diff line number Diff line
@@ -426,7 +426,7 @@ public class AccountManager { 
    @RequiresPermission(GET_ACCOUNTS)

    public Account[] getAccounts() {

        try {

            return mService.getAccounts(null);

            return mService.getAccounts(null, mContext.getOpPackageName());

        } catch (RemoteException e) {

            // won't ever happen

            throw new RuntimeException(e);
@@ -451,7 +451,7 @@ public class AccountManager { 
    @RequiresPermission(GET_ACCOUNTS)

    public Account[] getAccountsAsUser(int userId) {

        try {

            return mService.getAccountsAsUser(null, userId);

            return mService.getAccountsAsUser(null, userId, mContext.getOpPackageName());

        } catch (RemoteException e) {

            // won't ever happen

            throw new RuntimeException(e);
@@ -468,7 +468,7 @@ public class AccountManager { 
     */

    public Account[] getAccountsForPackage(String packageName, int uid) {

        try {

            return mService.getAccountsForPackage(packageName, uid);

            return mService.getAccountsForPackage(packageName, uid, mContext.getOpPackageName());

        } catch (RemoteException re) {

            // won't ever happen

            throw new RuntimeException(re);
@@ -485,7 +485,8 @@ public class AccountManager { 
     */

    public Account[] getAccountsByTypeForPackage(String type, String packageName) {

        try {

            return mService.getAccountsByTypeForPackage(type, packageName);

            return mService.getAccountsByTypeForPackage(type, packageName,

                    mContext.getOpPackageName());

        } catch (RemoteException re) {

            // won't ever happen

            throw new RuntimeException(re);
@@ -522,7 +523,8 @@ public class AccountManager { 
    /** @hide Same as {@link #getAccountsByType(String)} but for a specific user. */

    public Account[] getAccountsByTypeAsUser(String type, UserHandle userHandle) {

        try {

            return mService.getAccountsAsUser(type, userHandle.getIdentifier());

            return mService.getAccountsAsUser(type, userHandle.getIdentifier(),

                    mContext.getOpPackageName());

        } catch (RemoteException e) {

            // won't ever happen

            throw new RuntimeException(e);
@@ -610,7 +612,7 @@ public class AccountManager { 
        if (features == null) throw new IllegalArgumentException("features is null");

        return new Future2Task<Boolean>(handler, callback) {

            public void doWork() throws RemoteException {

                mService.hasFeatures(mResponse, account, features);

                mService.hasFeatures(mResponse, account, features, mContext.getOpPackageName());

            }

            public Boolean bundleToResult(Bundle bundle) throws AuthenticatorException {

                if (!bundle.containsKey(KEY_BOOLEAN_RESULT)) {
@@ -662,7 +664,8 @@ public class AccountManager { 
        if (type == null) throw new IllegalArgumentException("type is null");

        return new Future2Task<Account[]>(handler, callback) {

            public void doWork() throws RemoteException {

                mService.getAccountsByFeatures(mResponse, type, features);

                mService.getAccountsByFeatures(mResponse, type, features,

                        mContext.getOpPackageName());

            }

            public Account[] bundleToResult(Bundle bundle) throws AuthenticatorException {

                if (!bundle.containsKey(KEY_ACCOUNTS)) {

core/java/android/accounts/IAccountManager.aidl

+8 −6
Original line number Diff line number Diff line
@@ -30,12 +30,14 @@ interface IAccountManager { 
    String getPassword(in Account account);

    String getUserData(in Account account, String key);

    AuthenticatorDescription[] getAuthenticatorTypes(int userId);

    Account[] getAccounts(String accountType);

    Account[] getAccountsForPackage(String packageName, int uid);

    Account[] getAccountsByTypeForPackage(String type, String packageName);

    Account[] getAccountsAsUser(String accountType, int userId);

    void hasFeatures(in IAccountManagerResponse response, in Account account, in String[] features);

    void getAccountsByFeatures(in IAccountManagerResponse response, String accountType, in String[] features);

    Account[] getAccounts(String accountType, String opPackageName);

    Account[] getAccountsForPackage(String packageName, int uid, String opPackageName);

    Account[] getAccountsByTypeForPackage(String type, String packageName, String opPackageName);

    Account[] getAccountsAsUser(String accountType, int userId, String opPackageName);

    void hasFeatures(in IAccountManagerResponse response, in Account account, in String[] features,

        String opPackageName);

    void getAccountsByFeatures(in IAccountManagerResponse response, String accountType,

        in String[] features, String opPackageName);

    boolean addAccountExplicitly(in Account account, String password, in Bundle extras);

    void removeAccount(in IAccountManagerResponse response, in Account account,

        boolean expectActivityLaunch);

core/java/android/app/AppOpsManager.java

+14 −1
Original line number Diff line number Diff line
@@ -235,8 +235,10 @@ public class AppOpsManager { 
    public static final int OP_WRITE_EXTERNAL_STORAGE = 60;

    /** @hide Turned on the screen. */

    public static final int OP_TURN_SCREEN_ON = 61;

    /** @hide Get device accounts. */

    public static final int OP_GET_ACCOUNTS = 62;

    /** @hide */

    public static final int _NUM_OP = 62;

    public static final int _NUM_OP = 63;



    /** Access to coarse location information. */

    public static final String OPSTR_COARSE_LOCATION = "android:coarse_location";
@@ -331,6 +333,9 @@ public class AppOpsManager { 
    /** Required to write/modify/update system settingss. */

    public static final String OPSTR_WRITE_SETTINGS

            = "android:write_settings";

    /** @hide Get device accounts. */

    public static final String OPSTR_GET_ACCOUNTS

            = "android:get_accounts";



    /**

     * This maps each operation to the operation that serves as the
@@ -403,6 +408,7 @@ public class AppOpsManager { 
            OP_READ_EXTERNAL_STORAGE,

            OP_WRITE_EXTERNAL_STORAGE,

            OP_TURN_SCREEN_ON,

            OP_GET_ACCOUNTS,

    };



    /**
@@ -472,6 +478,7 @@ public class AppOpsManager { 
            OPSTR_READ_EXTERNAL_STORAGE,

            OPSTR_WRITE_EXTERNAL_STORAGE,

            null,

            OPSTR_GET_ACCOUNTS

    };



    /**
@@ -541,6 +548,7 @@ public class AppOpsManager { 
            "READ_EXTERNAL_STORAGE",

            "WRITE_EXTERNAL_STORAGE",

            "TURN_ON_SCREEN",

            "GET_ACCOUNTS",

    };



    /**
@@ -610,6 +618,7 @@ public class AppOpsManager { 
            Manifest.permission.READ_EXTERNAL_STORAGE,

            Manifest.permission.WRITE_EXTERNAL_STORAGE,

            null, // no permission for turning the screen on

            Manifest.permission.GET_ACCOUNTS

    };



    /**
@@ -680,6 +689,7 @@ public class AppOpsManager { 
            null, // READ_EXTERNAL_STORAGE

            null, // WRITE_EXTERNAL_STORAGE

            null, // TURN_ON_SCREEN

            null, // GET_ACCOUNTS

    };



    /**
@@ -749,6 +759,7 @@ public class AppOpsManager { 
            false, // READ_EXTERNAL_STORAGE

            false, // WRITE_EXTERNAL_STORAGE

            false, // TURN_ON_SCREEN

            false, // GET_ACCOUNTS

    };



    /**
@@ -817,6 +828,7 @@ public class AppOpsManager { 
            AppOpsManager.MODE_ALLOWED,

            AppOpsManager.MODE_ALLOWED,

            AppOpsManager.MODE_ALLOWED,  // OP_TURN_ON_SCREEN

            AppOpsManager.MODE_ALLOWED,

    };



    /**
@@ -889,6 +901,7 @@ public class AppOpsManager { 
            false,

            false,

            false,

            false

    };



    /**

services/core/java/com/android/server/accounts/AccountManagerService.java

+47 −39
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ import android.accounts.IAccountManagerResponse; 
import android.app.ActivityManager;

import android.app.ActivityManagerNative;

import android.app.AppGlobals;

import android.app.AppOpsManager;

import android.app.Notification;

import android.app.NotificationManager;

import android.app.PendingIntent;
@@ -122,6 +123,7 @@ public class AccountManagerService 
    private final Context mContext;



    private final PackageManager mPackageManager;

    private final AppOpsManager mAppOpsManager;

    private UserManager mUserManager;



    private final MessageHandler mMessageHandler;
@@ -266,6 +268,7 @@ public class AccountManagerService 
            IAccountAuthenticatorCache authenticatorCache) {

        mContext = context;

        mPackageManager = packageManager;

        mAppOpsManager = mContext.getSystemService(AppOpsManager.class);



        mMessageHandler = new MessageHandler(FgThread.get().getLooper());
@@ -510,7 +513,7 @@ public class AccountManagerService 
        // Check if there's a shared account that needs to be created as an account

        Account[] sharedAccounts = getSharedAccountsAsUser(userId);

        if (sharedAccounts == null || sharedAccounts.length == 0) return;

        Account[] accounts = getAccountsAsUser(null, userId);

        Account[] accounts = getAccountsAsUser(null, userId, mContext.getOpPackageName());

        for (Account sa : sharedAccounts) {

            if (ArrayUtils.contains(accounts, sa)) continue;

            // Account doesn't exist. Copy it now.
@@ -868,7 +871,8 @@ public class AccountManagerService 
                    // Confirm that the owner's account still exists before this step.

                    UserAccounts owner = getUserAccounts(UserHandle.USER_OWNER);

                    synchronized (owner.cacheLock) {

                        for (Account acc : getAccounts(UserHandle.USER_OWNER)) {

                        for (Account acc : getAccounts(UserHandle.USER_OWNER,

                                mContext.getOpPackageName())) {

                            if (acc.equals(account)) {

                                mAuthenticator.addAccountFromCredentials(

                                        this, account, accountCredentials);
@@ -988,7 +992,7 @@ public class AccountManagerService 


    @Override

    public void hasFeatures(IAccountManagerResponse response,

            Account account, String[] features) {

            Account account, String[] features, String opPackageName) {

        int callingUid = Binder.getCallingUid();

        if (Log.isLoggable(TAG, Log.VERBOSE)) {

            Log.v(TAG, "hasFeatures: " + account
@@ -1001,7 +1005,8 @@ public class AccountManagerService 
        if (account == null) throw new IllegalArgumentException("account is null");

        if (features == null) throw new IllegalArgumentException("features is null");

        int userId = UserHandle.getCallingUserId();

        checkReadAccountsPermitted(callingUid, account.type, userId);

        checkReadAccountsPermitted(callingUid, account.type, userId,

                opPackageName);



        long identityToken = clearCallingIdentity();

        try {
@@ -2507,9 +2512,10 @@ public class AccountManagerService 
     * Returns the accounts visible to the client within the context of a specific user

     * @hide

     */

    public Account[] getAccounts(int userId) {

    public Account[] getAccounts(int userId, String opPackageName) {

        int callingUid = Binder.getCallingUid();

        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);

        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId,

                opPackageName);

        if (visibleAccountTypes.isEmpty()) {

            return new Account[0];

        }
@@ -2571,15 +2577,16 @@ public class AccountManagerService 
    }



    @Override

    public Account[] getAccountsAsUser(String type, int userId) {

        return getAccountsAsUser(type, userId, null, -1);

    public Account[] getAccountsAsUser(String type, int userId, String opPackageName) {

        return getAccountsAsUser(type, userId, null, -1, opPackageName);

    }



    private Account[] getAccountsAsUser(

            String type,

            int userId,

            String callingPackage,

            int packageUid) {

            int packageUid,

            String opPackageName) {

        int callingUid = Binder.getCallingUid();

        // Only allow the system process to read accounts of other users

        if (userId != UserHandle.getCallingUserId()
@@ -2602,7 +2609,8 @@ public class AccountManagerService 
            callingUid = packageUid;

        }



        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);

        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId,

                opPackageName);

        if (visibleAccountTypes.isEmpty()

                || (type != null && !visibleAccountTypes.contains(type))) {

            return new Account[0];
@@ -2741,22 +2749,24 @@ public class AccountManagerService 
    }



    @Override

    public Account[] getAccounts(String type) {

        return getAccountsAsUser(type, UserHandle.getCallingUserId());

    public Account[] getAccounts(String type, String opPackageName) {

        return getAccountsAsUser(type, UserHandle.getCallingUserId(), opPackageName);

    }



    @Override

    public Account[] getAccountsForPackage(String packageName, int uid) {

    public Account[] getAccountsForPackage(String packageName, int uid, String opPackageName) {

        int callingUid = Binder.getCallingUid();

        if (!UserHandle.isSameApp(callingUid, Process.myUid())) {

            throw new SecurityException("getAccountsForPackage() called from unauthorized uid "

                    + callingUid + " with uid=" + uid);

        }

        return getAccountsAsUser(null, UserHandle.getCallingUserId(), packageName, uid);

        return getAccountsAsUser(null, UserHandle.getCallingUserId(), packageName, uid,

                opPackageName);

    }



    @Override

    public Account[] getAccountsByTypeForPackage(String type, String packageName) {

    public Account[] getAccountsByTypeForPackage(String type, String packageName,

            String opPackageName) {

        int packageUid = -1;

        try {

            packageUid = AppGlobals.getPackageManager().getPackageUid(
@@ -2765,14 +2775,16 @@ public class AccountManagerService 
            Slog.e(TAG, "Couldn't determine the packageUid for " + packageName + re);

            return new Account[0];

        }

        return getAccountsAsUser(type, UserHandle.getCallingUserId(), packageName, packageUid);

        return getAccountsAsUser(type, UserHandle.getCallingUserId(), packageName,

                packageUid, opPackageName);

    }



    @Override

    public void getAccountsByFeatures(

            IAccountManagerResponse response,

            String type,

            String[] features) {

            String[] features,

            String opPackageName) {

        int callingUid = Binder.getCallingUid();

        if (Log.isLoggable(TAG, Log.VERBOSE)) {

            Log.v(TAG, "getAccounts: accountType " + type
@@ -2785,7 +2797,8 @@ public class AccountManagerService 
        if (type == null) throw new IllegalArgumentException("accountType is null");

        int userId = UserHandle.getCallingUserId();



        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId);

        List<String> visibleAccountTypes = getTypesVisibleToCaller(callingUid, userId,

                opPackageName);

        if (!visibleAccountTypes.contains(type)) {

            Bundle result = new Bundle();

            // Need to return just the accounts that are from matching signatures.
@@ -3685,29 +3698,20 @@ public class AccountManagerService 
        }

    }



    private boolean isPermitted(int callingUid, String... permissions) {

    private boolean isPermitted(String opPackageName, int callingUid, String... permissions) {

        for (String perm : permissions) {

            if (mContext.checkCallingOrSelfPermission(perm) == PackageManager.PERMISSION_GRANTED) {

                if (Log.isLoggable(TAG, Log.VERBOSE)) {

                    Log.v(TAG, "  caller uid " + callingUid + " has " + perm);

                }

                final int opCode = AppOpsManager.permissionToOpCode(perm);

                if (opCode == AppOpsManager.OP_NONE || mAppOpsManager.noteOp(

                        opCode, callingUid, opPackageName) == AppOpsManager.MODE_ALLOWED) {

                    return true;

                }

            }

        return false;

    }



    /** Succeeds if any of the specified permissions are granted. */

    private void checkBinderPermission(String... permissions) {

        final int callingUid = Binder.getCallingUid();

        if (isPermitted(callingUid, permissions)) {

            String msg = String.format(

                    "caller uid %s  lacks any of %s",

                    callingUid,

                    TextUtils.join(",", permissions));

            Log.w(TAG, "  " + msg);

            throw new SecurityException(msg);

        }

        return false;

    }



    private int handleIncomingUser(int userId) {
@@ -3763,11 +3767,13 @@ public class AccountManagerService 
        return fromAuthenticator || hasExplicitGrants || isPrivileged;

    }



    private boolean isAccountVisibleToCaller(String accountType, int callingUid, int userId) {

    private boolean isAccountVisibleToCaller(String accountType, int callingUid, int userId,

            String opPackageName) {

        if (accountType == null) {

            return false;

        } else {

            return getTypesVisibleToCaller(callingUid, userId).contains(accountType);

            return getTypesVisibleToCaller(callingUid, userId,

                    opPackageName).contains(accountType);

        }

    }
@@ -3779,9 +3785,10 @@ public class AccountManagerService 
        }

    }



    private List<String> getTypesVisibleToCaller(int callingUid, int userId) {

    private List<String> getTypesVisibleToCaller(int callingUid, int userId,

            String opPackageName) {

        boolean isPermitted =

                isPermitted(callingUid, Manifest.permission.GET_ACCOUNTS,

                isPermitted(opPackageName, callingUid, Manifest.permission.GET_ACCOUNTS,

                        Manifest.permission.GET_ACCOUNTS_PRIVILEGED);

        Log.i(TAG, String.format("getTypesVisibleToCaller: isPermitted? %s", isPermitted));

        return getTypesForCaller(callingUid, userId, isPermitted);
@@ -3877,8 +3884,9 @@ public class AccountManagerService 
    private void checkReadAccountsPermitted(

            int callingUid,

            String accountType,

            int userId) {

        if (!isAccountVisibleToCaller(accountType, callingUid, userId)) {

            int userId,

            String opPackageName) {

        if (!isAccountVisibleToCaller(accountType, callingUid, userId, opPackageName)) {

            String msg = String.format(

                    "caller uid %s cannot access %s accounts",

                    callingUid,

services/core/java/com/android/server/content/SyncManager.java

+4 −2
Original line number Diff line number Diff line
@@ -340,7 +340,8 @@ public class SyncManager { 
        for (UserInfo user : mUserManager.getUsers(true)) {

            // Skip any partially created/removed users

            if (user.partial) continue;

            Account[] accountsForUser = AccountManagerService.getSingleton().getAccounts(user.id);

            Account[] accountsForUser = AccountManagerService.getSingleton().getAccounts(

                    user.id, mContext.getOpPackageName());

            mSyncStorageEngine.doDatabaseCleanup(accountsForUser, user.id);

        }

    }
@@ -1232,7 +1233,8 @@ public class SyncManager { 
        }



        // Schedule sync for any accounts under started user

        final Account[] accounts = AccountManagerService.getSingleton().getAccounts(userId);

        final Account[] accounts = AccountManagerService.getSingleton().getAccounts(userId,

                mContext.getOpPackageName());

        for (Account account : accounts) {

            scheduleSync(account, userId, SyncOperation.REASON_USER_START, null, null,

                    0 /* no delay */, 0 /* No flex */,