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

Commit f98823a3 authored by Piyush Mehrotra's avatar Piyush Mehrotra Committed by Sarp Misoglu
Browse files

Improve `dumpsys backup` 

This is a re-submit of the previously reverted ag/22327091 by
piee@google.com.

Original description:
This change refactors `dumpsys backup` code and adds some improvements -
	- Removes `isUserReadyForBackup()` check on calling user. This check is not needed, as it prevents a User with INTERACT_ACROSS_USERS_FULL permission, but not Backup activated, to get info about Backup service on other users. This is particularly relevant in case of headless mode, as Backup service isn't running for SYSTEM_USER.
	- Adds `--user <userId>` optional parameter for `dumpsys backup`, which will return info about Backup service for the user passed in argument. This is also helpul in case of multiple users on the device.

Changes by sarpm@google.com over the original:
	- Some test improvements: remove @VisibleForTesting, increase
	  coverage slightly.
	- Remove redundant robo tests that are now covered by the unit
	  tests.

Bug: 267755698
Test: atest BackupManagerServiceTest
	Manual test by running dumpsys backup with optional arguments.
Change-Id: Id7790a501a8befb12a212f66adfc0cd8cc9f9e8b
parent efc35f89

services/backup/java/com/android/server/backup/BackupManagerService.java

+58 −30
Original line number Diff line number Diff line
@@ -1510,39 +1510,46 @@ public class BackupManagerService extends IBackupManager.Stub { 
        if (!DumpUtils.checkDumpAndUsageStatsPermission(mContext, TAG, pw)) {

            return;

        }

        dumpWithoutCheckingPermission(fd, pw, args);

    }



    @VisibleForTesting

    void dumpWithoutCheckingPermission(FileDescriptor fd, PrintWriter pw, String[] args) {

        int userId = binderGetCallingUserId();

        if (!isUserReadyForBackup(userId)) {

            pw.println("Inactive");

            return;

        }

        int argIndex = 0;



        if (args != null) {

            for (String arg : args) {

                if ("-h".equals(arg)) {

                    pw.println("'dumpsys backup' optional arguments:");

                    pw.println("  -h       : this help text");

                    pw.println("  a[gents] : dump information about defined backup agents");

                    pw.println("  transportclients : dump information about transport clients");

                    pw.println("  transportstats : dump transport statts");

                    pw.println("  users    : dump the list of users for which backup service "

                            + "is running");

        String op = nextArg(args, argIndex);

        argIndex++;



        if ("--help".equals(op)) {

            showDumpUsage(pw);

            return;

                } else if ("users".equals(arg.toLowerCase())) {

        }

        if ("users".equals(op)) {

            pw.print(DUMP_RUNNING_USERS_MESSAGE);

            for (int i = 0; i < mUserServices.size(); i++) {

                        pw.print(" " + mUserServices.keyAt(i));

                UserBackupManagerService userBackupManagerService =

                        getServiceForUserIfCallerHasPermission(mUserServices.keyAt(i),

                                "dump()");

                if (userBackupManagerService != null) {

                    pw.print(" " + userBackupManagerService.getUserId());

                }

            }

            pw.println();

            return;

        }

        if ("--user".equals(op)) {

            String userArg = nextArg(args, argIndex);

            argIndex++;

            if (userArg == null) {

                showDumpUsage(pw);

                return;

            }

            int userId = UserHandle.parseUserArg(userArg);

            UserBackupManagerService userBackupManagerService =

                    getServiceForUserIfCallerHasPermission(userId, "dump()");

            if (userBackupManagerService != null) {

                userBackupManagerService.dump(fd, pw, args);

            }



            return;

        }

        if (op == null || "agents".startsWith(op) || "transportclients".equals(op)

                || "transportstats".equals(op)) {

            for (int i = 0; i < mUserServices.size(); i++) {

                UserBackupManagerService userBackupManagerService =

                        getServiceForUserIfCallerHasPermission(mUserServices.keyAt(i), "dump()");
@@ -1550,6 +1557,27 @@ public class BackupManagerService extends IBackupManager.Stub { 
                    userBackupManagerService.dump(fd, pw, args);

                }

            }

            return;

        }



        showDumpUsage(pw);

    }



    private String nextArg(String[] args, int argIndex) {

        if (argIndex >= args.length) {

            return null;

        }

        return args[argIndex];

    }



    private static void showDumpUsage(PrintWriter pw) {

        pw.println("'dumpsys backup' optional arguments:");

        pw.println("  --help    : this help text");

        pw.println("  a[gents] : dump information about defined backup agents");

        pw.println("  transportclients : dump information about transport clients");

        pw.println("  transportstats : dump transport stats");

        pw.println("  users    : dump the list of users for which backup service is running");

        pw.println("  --user <userId> : dump information for user userId");

    }



    /**
@@ -1661,7 +1689,7 @@ public class BackupManagerService extends IBackupManager.Stub { 
     * @param message A message to include in the exception if it is thrown.

     */

    void enforceCallingPermissionOnUserId(@UserIdInt int userId, String message) {

        if (Binder.getCallingUserHandle().getIdentifier() != userId) {

        if (binderGetCallingUserId() != userId) {

            mContext.enforceCallingOrSelfPermission(

                    Manifest.permission.INTERACT_ACROSS_USERS_FULL, message);

        }

services/robotests/backup/src/com/android/server/backup/BackupManagerServiceRoboTest.java

+0 −60
Original line number Diff line number Diff line
@@ -17,9 +17,7 @@ 
package com.android.server.backup;



import static android.Manifest.permission.BACKUP;

import static android.Manifest.permission.DUMP;

import static android.Manifest.permission.INTERACT_ACROSS_USERS_FULL;

import static android.Manifest.permission.PACKAGE_USAGE_STATS;



import static com.android.server.backup.testing.TransportData.backupTransport;
@@ -73,10 +71,7 @@ import org.robolectric.shadow.api.Shadow; 
import org.robolectric.shadows.ShadowContextWrapper;



import java.io.File;

import java.io.FileDescriptor;

import java.io.IOException;

import java.io.PrintWriter;

import java.io.StringWriter;



/** Tests for {@link BackupManagerService}. */

@RunWith(RobolectricTestRunner.class)
@@ -1461,67 +1456,12 @@ public class BackupManagerServiceRoboTest { 
    //  Service tests

    // ---------------------------------------------



    /** Test that the backup service routes methods correctly to the user that requests it. */

    @Test

    public void testDump_onRegisteredUser_callsMethodForUser() throws Exception {

        grantDumpPermissions();

        BackupManagerService backupManagerService = createSystemRegisteredService();

        File testFile = createTestFile();

        FileDescriptor fileDescriptor = new FileDescriptor();

        PrintWriter printWriter = new PrintWriter(testFile);

        String[] args = {"1", "2"};

        ShadowBinder.setCallingUserHandle(UserHandle.of(UserHandle.USER_SYSTEM));



        backupManagerService.dump(fileDescriptor, printWriter, args);



        verify(mUserSystemService).dump(fileDescriptor, printWriter, args);

    }



    /** Test that the backup service does not route methods for non-registered users. */

    @Test

    public void testDump_onUnknownUser_doesNotPropagateCall() throws Exception {

        grantDumpPermissions();

        BackupManagerService backupManagerService = createService();

        File testFile = createTestFile();

        FileDescriptor fileDescriptor = new FileDescriptor();

        PrintWriter printWriter = new PrintWriter(testFile);

        String[] args = {"1", "2"};



        backupManagerService.dump(fileDescriptor, printWriter, args);



        verify(mUserOneService, never()).dump(fileDescriptor, printWriter, args);

    }



    /** Test that 'dumpsys backup users' dumps the list of users registered in backup service*/

    @Test

    public void testDump_users_dumpsListOfRegisteredUsers() {

        grantDumpPermissions();

        BackupManagerService backupManagerService = createSystemRegisteredService();

        registerUser(backupManagerService, mUserOneId, mUserOneService);

        StringWriter out = new StringWriter();

        PrintWriter writer = new PrintWriter(out);

        String[] args = {"users"};



        backupManagerService.dump(null, writer, args);



        writer.flush();

        assertEquals(

                String.format("%s %d %d\n", BackupManagerService.DUMP_RUNNING_USERS_MESSAGE,

                        UserHandle.USER_SYSTEM, mUserOneId),

                out.toString());

    }



    private File createTestFile() throws IOException {

        File testFile = new File(mContext.getFilesDir(), "test");

        testFile.createNewFile();

        return testFile;

    }



    private void grantDumpPermissions() {

        mShadowContext.grantPermissions(DUMP);

        mShadowContext.grantPermissions(PACKAGE_USAGE_STATS);

    }



    /**

     * Test that the backup services throws a {@link SecurityException} if the caller does not have

     * INTERACT_ACROSS_USERS_FULL permission and passes a different user id.

services/tests/mockingservicestests/src/com/android/server/backup/BackupManagerServiceTest.java

+88 −33
Original line number Diff line number Diff line
@@ -38,7 +38,6 @@ import static org.mockito.Mockito.verify; 
import static org.mockito.Mockito.when;



import android.Manifest;

import android.annotation.UserIdInt;

import android.app.backup.BackupManager;

import android.app.backup.ISelectBackupTransportCallback;

import android.app.job.JobScheduler;
@@ -59,10 +58,12 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; 
import androidx.test.filters.SmallTest;



import com.android.dx.mockito.inline.extended.ExtendedMockito;

import com.android.internal.util.DumpUtils;

import com.android.server.SystemService;

import com.android.server.backup.utils.RandomAccessFileUtils;



import org.junit.After;

import org.junit.Assert;

import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -74,6 +75,7 @@ import org.mockito.quality.Strictness; 
import java.io.File;

import java.io.FileDescriptor;

import java.io.PrintWriter;

import java.io.Writer;

import java.util.concurrent.CompletableFuture;

import java.util.concurrent.TimeUnit;
@@ -85,8 +87,6 @@ public class BackupManagerServiceTest { 
            "class");

    private static final int NON_SYSTEM_USER = UserHandle.USER_SYSTEM + 1;



    @UserIdInt

    private int mUserId;

    @Mock

    private UserBackupManagerService mSystemUserBackupManagerService;

    @Mock
@@ -94,8 +94,6 @@ public class BackupManagerServiceTest { 
    @Mock

    private Context mContextMock;

    @Mock

    private PrintWriter mPrintWriterMock;

    @Mock

    private UserManager mUserManagerMock;

    @Mock

    private UserInfo mUserInfoMock;
@@ -104,6 +102,7 @@ public class BackupManagerServiceTest { 


    private BackupManagerServiceTestable mService;

    private BackupManagerService.Lifecycle mServiceLifecycle;

    private FakePrintWriter mFakePrintWriter;

    private static File sTestDir;

    private MockitoSession mSession;
@@ -114,6 +113,7 @@ public class BackupManagerServiceTest { 
                                this)

                        .strictness(Strictness.LENIENT)

                        .spyStatic(UserBackupManagerService.class)

                        .spyStatic(DumpUtils.class)

                        .startMocking();

        doReturn(mSystemUserBackupManagerService).when(

                () -> UserBackupManagerService.createAndInitializeService(
@@ -122,8 +122,7 @@ public class BackupManagerServiceTest { 
                () -> UserBackupManagerService.createAndInitializeService(eq(NON_SYSTEM_USER),

                        any(), any(), any()));



        mUserId = UserHandle.USER_SYSTEM;



        when(mNonSystemUserBackupManagerService.getUserId()).thenReturn(NON_SYSTEM_USER);

        when(mUserManagerMock.getUserInfo(UserHandle.USER_SYSTEM)).thenReturn(mUserInfoMock);

        when(mUserManagerMock.getUserInfo(NON_SYSTEM_USER)).thenReturn(mUserInfoMock);

        // Null main user means there is no main user on the device.
@@ -139,6 +138,8 @@ public class BackupManagerServiceTest { 


        when(mContextMock.getSystemService(Context.JOB_SCHEDULER_SERVICE))

                .thenReturn(mock(JobScheduler.class));



        mFakePrintWriter = new FakePrintWriter();

    }



    @After
@@ -552,7 +553,8 @@ public class BackupManagerServiceTest { 
                    }

                };



        mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, listener);

        mService.selectBackupTransportAsyncForUser(

                UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, listener);



        assertEquals(BackupManager.ERROR_BACKUP_NOT_ALLOWED, (int) future.get(5, TimeUnit.SECONDS));

    }
@@ -560,9 +562,10 @@ public class BackupManagerServiceTest { 
    @Test

    public void selectBackupTransportAsyncForUser_beforeUserUnlockedWithNullListener_doesNotThrow()

            throws Exception {

        createBackupManagerServiceAndUnlockSystemUser();

        mService = new BackupManagerServiceTestable(mContextMock);



        mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, null);

        mService.selectBackupTransportAsyncForUser(

                UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, null);



        // No crash.

    }
@@ -570,13 +573,11 @@ public class BackupManagerServiceTest { 
    @Test

    public void selectBackupTransportAsyncForUser_beforeUserUnlockedListenerThrowing_doesNotThrow()

            throws Exception {

        createBackupManagerServiceAndUnlockSystemUser();



        mService = new BackupManagerServiceTestable(mContextMock);

        ISelectBackupTransportCallback.Stub listener =

                new ISelectBackupTransportCallback.Stub() {

                    @Override

                    public void onSuccess(String transportName) {

                    }

                    public void onSuccess(String transportName) {}



                    @Override

                    public void onFailure(int reason) throws RemoteException {
@@ -584,55 +585,91 @@ public class BackupManagerServiceTest { 
                    }

                };



        mService.selectBackupTransportAsyncForUser(mUserId, TRANSPORT_COMPONENT_NAME, listener);

        mService.selectBackupTransportAsyncForUser(

                UserHandle.USER_SYSTEM, TRANSPORT_COMPONENT_NAME, listener);



        // No crash.

    }



    @Test

    public void dump_callerDoesNotHaveDumpPermission_ignored() {

    public void dump_callerDoesNotHaveDumpOrUsageStatsPermission_ignored() {

        mockDumpPermissionsGranted(false);

        createBackupManagerServiceAndUnlockSystemUser();

        when(mContextMock.checkCallingOrSelfPermission(

                Manifest.permission.DUMP)).thenReturn(

                PackageManager.PERMISSION_DENIED);

        when(mContextMock.checkCallingOrSelfPermission(Manifest.permission.DUMP))

                .thenReturn(PackageManager.PERMISSION_DENIED);



        mService.dump(mFileDescriptorStub, mPrintWriterMock, new String[0]);

        mService.dump(mFileDescriptorStub, mFakePrintWriter, new String[0]);



        verify(mSystemUserBackupManagerService, never()).dump(any(), any(), any());

        verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any());

    }



    @Test

    public void dump_callerDoesNotHavePackageUsageStatsPermission_ignored() {

    public void dump_forOneUser_callerDoesNotHaveInteractAcrossUsersFullPermission_ignored() {

        mockDumpPermissionsGranted(true);

        createBackupManagerServiceAndUnlockSystemUser();

        when(mContextMock.checkCallingOrSelfPermission(

                Manifest.permission.PACKAGE_USAGE_STATS)).thenReturn(

                PackageManager.PERMISSION_DENIED);

        mService.setBackupServiceActive(NON_SYSTEM_USER, true);

        simulateUserUnlocked(NON_SYSTEM_USER);

        doThrow(new SecurityException())

                .when(mContextMock)

                .enforceCallingOrSelfPermission(

                        eq(Manifest.permission.INTERACT_ACROSS_USERS_FULL), anyString());



        mService.dump(mFileDescriptorStub, mPrintWriterMock, new String[0]);

        String[] args = new String[] {"--user", Integer.toString(NON_SYSTEM_USER)};

        Assert.assertThrows(

                SecurityException.class,

                () -> mService.dump(mFileDescriptorStub, mFakePrintWriter, args));



        verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any());

    }



    @Test

    public void dump_forOneUser_callerHasInteractAcrossUsersFullPermission_dumpsSpecifiedUser() {

        mockDumpPermissionsGranted(true);

        createBackupManagerServiceAndUnlockSystemUser();

        mService.setBackupServiceActive(NON_SYSTEM_USER, true);

        simulateUserUnlocked(NON_SYSTEM_USER);



        String[] args = new String[] {"--user", Integer.toString(UserHandle.USER_SYSTEM)};

        mService.dump(mFileDescriptorStub, mFakePrintWriter, args);



        verify(mSystemUserBackupManagerService).dump(any(), any(), any());

    }



    @Test

    public void dump_users_callerHasInteractAcrossUsersFullPermission_dumpsUsers() {

        mockDumpPermissionsGranted(true);

        createBackupManagerServiceAndUnlockSystemUser();

        mService.setBackupServiceActive(NON_SYSTEM_USER, true);

        simulateUserUnlocked(NON_SYSTEM_USER);



        String[] args = new String[] {"users"};

        mService.dump(mFileDescriptorStub, mFakePrintWriter, args);



        // Check that dump() invocations are not called on user's Backup service,

        // as 'dumpsys backup users' only list users for whom Backup service is running.

        verify(mSystemUserBackupManagerService, never()).dump(any(), any(), any());

        verify(mNonSystemUserBackupManagerService, never()).dump(any(), any(), any());

        assertThat(mFakePrintWriter.mPrintedSoFar)

                .isEqualTo("Backup Manager is running for users: 0 1");

    }



    /**

     * Test that {@link BackupManagerService#dump(FileDescriptor, PrintWriter, String[])} dumps

     * system user information before non-system user information.

     */

    @Test

    public void testDump_systemUserFirst() {

    public void dump_allUsers_dumpsSystemUserFirst() {

        mockDumpPermissionsGranted(true);

        createBackupManagerServiceAndUnlockSystemUser();

        mService.setBackupServiceActive(NON_SYSTEM_USER, true);

        simulateUserUnlocked(NON_SYSTEM_USER);



        String[] args = new String[0];

        mService.dumpWithoutCheckingPermission(mFileDescriptorStub, mPrintWriterMock, args);

        mService.dump(mFileDescriptorStub, mFakePrintWriter, args);



        InOrder inOrder =

                inOrder(mSystemUserBackupManagerService, mNonSystemUserBackupManagerService);

        inOrder.verify(mSystemUserBackupManagerService)

                .dump(mFileDescriptorStub, mPrintWriterMock, args);

                .dump(mFileDescriptorStub, mFakePrintWriter, args);

        inOrder.verify(mNonSystemUserBackupManagerService)

                .dump(mFileDescriptorStub, mPrintWriterMock, args);

                .dump(mFileDescriptorStub, mFakePrintWriter, args);

        inOrder.verifyNoMoreInteractions();

    }
@@ -753,6 +790,11 @@ public class BackupManagerServiceTest { 
        return new File(sTestDir, "rememberActivated-" + userId);

    }



    private static void mockDumpPermissionsGranted(boolean granted) {

        doReturn(granted)

                .when(() -> DumpUtils.checkDumpAndUsageStatsPermission(any(), any(), any()));

    }



    private static class BackupManagerServiceTestable extends BackupManagerService {

        static boolean sBackupDisabled = false;

        static int sCallingUserId = -1;
@@ -803,4 +845,17 @@ public class BackupManagerServiceTest { 
            runnable.run();

        }

    }



    private static class FakePrintWriter extends PrintWriter {

        String mPrintedSoFar = "";



        FakePrintWriter() {

            super(Writer.nullWriter());

        }



        @Override

        public void print(String s) {

            mPrintedSoFar = mPrintedSoFar + s;

        }

    }

}