From a4e539ba52835a80814f1bc6aadf51322d531eb6 Mon Sep 17 00:00:00 2001
From: Reema Bajwa <reemabajwa@google.com>
Date: Thu, 9 Mar 2023 19:29:00 +0000
Subject: [PATCH] Fix bug in getting enabled providers

Test: built & deployed locally
Bug: 272368541

Change-Id: Ic8c8cb2060aaca9f199e613d8b1a213006b6922e
---
 .../CredentialProviderInfoFactory.java        | 42 +++++--------------
 .../credentials/CredentialManagerService.java | 16 ++++---
 2 files changed, 22 insertions(+), 36 deletions(-)

diff --git a/core/java/android/service/credentials/CredentialProviderInfoFactory.java b/core/java/android/service/credentials/CredentialProviderInfoFactory.java
index 9cd5aa06463ac..3190c69abd928 100644
--- a/core/java/android/service/credentials/CredentialProviderInfoFactory.java
+++ b/core/java/android/service/credentials/CredentialProviderInfoFactory.java
@@ -328,7 +328,7 @@ public final class CredentialProviderInfoFactory {
             @NonNull Context context,
             @UserIdInt int userId,
             boolean disableSystemAppVerificationForTests,
-            Set<ServiceInfo> enabledServices) {
+            Set<ComponentName> enabledServices) {
         requireNonNull(context, "context must not be null");
 
         final List<CredentialProviderInfo> providerInfos = new ArrayList<>();
@@ -342,7 +342,7 @@ public final class CredentialProviderInfoFactory {
                                 si,
                                 /* isSystemProvider= */ true,
                                 disableSystemAppVerificationForTests,
-                                enabledServices.contains(si));
+                                enabledServices.contains(si.getComponentName()));
                 if (cpi.isSystemProvider()) {
                     providerInfos.add(cpi);
                 } else {
@@ -370,31 +370,6 @@ public final class CredentialProviderInfoFactory {
         return null;
     }
 
-    /**
-     * Returns a valid credential provider that has the given package name. Returns null if no
-     * match is found.
-     */
-    @Nullable
-    public static CredentialProviderInfo getCredentialProviderFromPackageName(
-            @NonNull Context context,
-            int userId,
-            @NonNull String packageName,
-            int providerFilter,
-            @NonNull Set<ServiceInfo> enabledServices) {
-        requireNonNull(context, "context must not be null");
-        requireNonNull(packageName, "package name must not be null");
-        requireNonNull(enabledServices, "enabledServices must not be null");
-
-        for (CredentialProviderInfo credentialProviderInfo : getCredentialProviderServices(context,
-                userId, providerFilter, enabledServices)) {
-            if (credentialProviderInfo.getServiceInfo()
-                    .packageName.equals(packageName)) {
-                return credentialProviderInfo;
-            }
-        }
-        return null;
-    }
-
     /**
      * Returns the valid credential provider services available for the user with the given {@code
      * userId}.
@@ -404,7 +379,7 @@ public final class CredentialProviderInfoFactory {
             @NonNull Context context,
             int userId,
             int providerFilter,
-            Set<ServiceInfo> enabledServices) {
+            Set<ComponentName> enabledServices) {
         requireNonNull(context, "context must not be null");
 
         // Get the device policy.
@@ -433,7 +408,7 @@ public final class CredentialProviderInfoFactory {
             @NonNull Context context,
             int userId,
             int providerFilter,
-            Set<ServiceInfo> enabledServices) {
+            Set<ComponentName> enabledServices) {
         requireNonNull(context, "context must not be null");
 
         // Get the device policy.
@@ -539,7 +514,7 @@ public final class CredentialProviderInfoFactory {
             @NonNull Context context,
             @UserIdInt int userId,
             boolean disableSystemAppVerificationForTests,
-            Set<ServiceInfo> enabledServices) {
+            Set<ComponentName> enabledServices) {
         final List<CredentialProviderInfo> services = new ArrayList<>();
         final List<ResolveInfo> resolveInfos =
                 context.getPackageManager()
@@ -549,6 +524,11 @@ public final class CredentialProviderInfoFactory {
                                 userId);
         for (ResolveInfo resolveInfo : resolveInfos) {
             final ServiceInfo serviceInfo = resolveInfo.serviceInfo;
+            if (serviceInfo == null) {
+                Log.i(TAG, "No serviceInfo found for resolveInfo so skipping this provider");
+                continue;
+            }
+
             try {
                 CredentialProviderInfo cpi =
                         CredentialProviderInfoFactory.create(
@@ -556,7 +536,7 @@ public final class CredentialProviderInfoFactory {
                                 serviceInfo,
                                 /* isSystemProvider= */ false,
                                 disableSystemAppVerificationForTests,
-                                enabledServices.contains(serviceInfo));
+                                enabledServices.contains(serviceInfo.getComponentName()));
                 if (!cpi.isSystemProvider()) {
                     services.add(cpi);
                 }
diff --git a/services/credentials/java/com/android/server/credentials/CredentialManagerService.java b/services/credentials/java/com/android/server/credentials/CredentialManagerService.java
index 53c88d3cd339e..a58dbe5814376 100644
--- a/services/credentials/java/com/android/server/credentials/CredentialManagerService.java
+++ b/services/credentials/java/com/android/server/credentials/CredentialManagerService.java
@@ -29,7 +29,6 @@ import android.content.ComponentName;
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.content.pm.PackageManager;
-import android.content.pm.ServiceInfo;
 import android.credentials.ClearCredentialStateRequest;
 import android.credentials.CreateCredentialException;
 import android.credentials.CreateCredentialRequest;
@@ -696,13 +695,20 @@ public final class CredentialManagerService
 
         @SuppressWarnings("GuardedBy") // ErrorProne requires service.mLock which is the same
         // this.mLock
-        private Set<ServiceInfo> getEnabledProviders() {
-            Set<ServiceInfo> enabledProviders = new HashSet<>();
+        private Set<ComponentName> getEnabledProviders() {
+            Set<ComponentName> enabledProviders = new HashSet<>();
             synchronized (mLock) {
                 runForUser(
                         (service) -> {
-                            enabledProviders.add(
-                                    service.getCredentialProviderInfo().getServiceInfo());
+                            try {
+                                enabledProviders.add(
+                                        service.getCredentialProviderInfo()
+                                                .getServiceInfo().getComponentName());
+                            } catch (NullPointerException e) {
+                                // Safe check
+                                Log.i(TAG, "Skipping provider as either the providerInfo"
+                                        + "or serviceInfo is null - weird");
+                            }
                         });
             }
             return enabledProviders;