Merge "Review logging in CredManService & CredManServiceImpl" into udc-dev

This commit is contained in:
TreeHugger Robot
2023-03-29 07:11:46 +00:00
committed by Android (Google) Code Review
2 changed files with 44 additions and 53 deletions

View File

@@ -160,12 +160,10 @@ public final class CredentialManagerService
int resolvedUserId, boolean disabled, String[] serviceNames) { int resolvedUserId, boolean disabled, String[] serviceNames) {
getOrConstructSystemServiceListLock(resolvedUserId); getOrConstructSystemServiceListLock(resolvedUserId);
if (serviceNames == null || serviceNames.length == 0) { if (serviceNames == null || serviceNames.length == 0) {
Slog.i(TAG, "serviceNames sent in newServiceListLocked is null, or empty");
return new ArrayList<>(); return new ArrayList<>();
} }
List<CredentialManagerServiceImpl> serviceList = new ArrayList<>(serviceNames.length); List<CredentialManagerServiceImpl> serviceList = new ArrayList<>(serviceNames.length);
for (String serviceName : serviceNames) { for (String serviceName : serviceNames) {
Log.i(TAG, "in newServiceListLocked, service: " + serviceName);
if (TextUtils.isEmpty(serviceName)) { if (TextUtils.isEmpty(serviceName)) {
continue; continue;
} }
@@ -173,7 +171,7 @@ public final class CredentialManagerService
serviceList.add( serviceList.add(
new CredentialManagerServiceImpl(this, mLock, resolvedUserId, serviceName)); new CredentialManagerServiceImpl(this, mLock, resolvedUserId, serviceName));
} catch (PackageManager.NameNotFoundException | SecurityException e) { } catch (PackageManager.NameNotFoundException | SecurityException e) {
Log.i(TAG, "Unable to add serviceInfo : " + e.getMessage()); Slog.e(TAG, "Unable to add serviceInfo : ", e);
} }
} }
return serviceList; return serviceList;
@@ -423,7 +421,7 @@ public final class CredentialManagerService
userId); userId);
callingAppInfo = new CallingAppInfo(realPackageName, packageInfo.signingInfo, origin); callingAppInfo = new CallingAppInfo(realPackageName, packageInfo.signingInfo, origin);
} catch (PackageManager.NameNotFoundException e) { } catch (PackageManager.NameNotFoundException e) {
Log.i(TAG, "Issue while retrieving signatureInfo : " + e.getMessage()); Slog.e(TAG, "Issue while retrieving signatureInfo : ", e);
callingAppInfo = new CallingAppInfo(realPackageName, null, origin); callingAppInfo = new CallingAppInfo(realPackageName, null, origin);
} }
return callingAppInfo; return callingAppInfo;
@@ -436,7 +434,8 @@ public final class CredentialManagerService
IGetCredentialCallback callback, IGetCredentialCallback callback,
final String callingPackage) { final String callingPackage) {
final long timestampBegan = System.nanoTime(); final long timestampBegan = System.nanoTime();
Log.i(TAG, "starting executeGetCredential with callingPackage: " + callingPackage); Slog.d(TAG, "starting executeGetCredential with callingPackage: "
+ callingPackage);
ICancellationSignal cancelTransport = CancellationSignal.createTransport(); ICancellationSignal cancelTransport = CancellationSignal.createTransport();
if (request.getOrigin() != null) { if (request.getOrigin() != null) {
@@ -630,11 +629,10 @@ public final class CredentialManagerService
GetCredentialException.TYPE_NO_CREDENTIAL, GetCredentialException.TYPE_NO_CREDENTIAL,
"No credentials available on this device."); "No credentials available on this device.");
} catch (RemoteException e) { } catch (RemoteException e) {
Log.i( Slog.e(
TAG, TAG,
"Issue invoking onError on IGetCredentialCallback " "Issue invoking onError on IGetCredentialCallback "
+ "callback: " + "callback: ", e);
+ e.getMessage());
} }
} }
@@ -649,7 +647,7 @@ public final class CredentialManagerService
ICreateCredentialCallback callback, ICreateCredentialCallback callback,
String callingPackage) { String callingPackage) {
final long timestampBegan = System.nanoTime(); final long timestampBegan = System.nanoTime();
Log.i(TAG, "starting executeCreateCredential with callingPackage: " Slog.d(TAG, "starting executeCreateCredential with callingPackage: "
+ callingPackage); + callingPackage);
ICancellationSignal cancelTransport = CancellationSignal.createTransport(); ICancellationSignal cancelTransport = CancellationSignal.createTransport();
@@ -692,11 +690,10 @@ public final class CredentialManagerService
CreateCredentialException.TYPE_NO_CREATE_OPTIONS, CreateCredentialException.TYPE_NO_CREATE_OPTIONS,
"No create options available."); "No create options available.");
} catch (RemoteException e) { } catch (RemoteException e) {
Log.i( Slog.e(
TAG, TAG,
"Issue invoking onError on ICreateCredentialCallback " "Issue invoking onError on ICreateCredentialCallback "
+ "callback: " + "callback: ", e);
+ e.getMessage());
} }
} }
@@ -712,21 +709,19 @@ public final class CredentialManagerService
MetricUtilities.logApiCalledInitialPhase(initMetric, MetricUtilities.logApiCalledInitialPhase(initMetric,
session.mRequestSessionMetric.returnIncrementSequence()); session.mRequestSessionMetric.returnIncrementSequence());
} catch (Exception e) { } catch (Exception e) {
Log.w(TAG, "Unexpected error during metric logging: " + e); Log.w(TAG, "Unexpected error during metric logging: ", e);
} }
} }
@Override @Override
public void setEnabledProviders( public void setEnabledProviders(
List<String> providers, int userId, ISetEnabledProvidersCallback callback) { List<String> providers, int userId, ISetEnabledProvidersCallback callback) {
Log.i(TAG, "setEnabledProviders");
if (!hasWriteSecureSettingsPermission()) { if (!hasWriteSecureSettingsPermission()) {
try { try {
callback.onError( callback.onError(
PERMISSION_DENIED_ERROR, PERMISSION_DENIED_WRITE_SECURE_SETTINGS_ERROR); PERMISSION_DENIED_ERROR, PERMISSION_DENIED_WRITE_SECURE_SETTINGS_ERROR);
} catch (RemoteException e) { } catch (RemoteException e) {
Log.e(TAG, "Issue with invoking response: " + e.getMessage()); Slog.e(TAG, "Issue with invoking response: ", e);
} }
return; return;
} }
@@ -753,7 +748,7 @@ public final class CredentialManagerService
"failed_setting_store", "failed_setting_store",
"Failed to store setting containing enabled providers"); "Failed to store setting containing enabled providers");
} catch (RemoteException e) { } catch (RemoteException e) {
Log.i(TAG, "Issue with invoking error response: " + e.getMessage()); Slog.e(TAG, "Issue with invoking error response: ", e);
return; return;
} }
} }
@@ -762,7 +757,7 @@ public final class CredentialManagerService
try { try {
callback.onResponse(); callback.onResponse();
} catch (RemoteException e) { } catch (RemoteException e) {
Log.i(TAG, "Issue with invoking response: " + e.getMessage()); Slog.e(TAG, "Issue with invoking response: ", e);
// TODO: Propagate failure // TODO: Propagate failure
} }
@@ -774,7 +769,8 @@ public final class CredentialManagerService
@Override @Override
public boolean isEnabledCredentialProviderService( public boolean isEnabledCredentialProviderService(
ComponentName componentName, String callingPackage) { ComponentName componentName, String callingPackage) {
Log.i(TAG, "isEnabledCredentialProviderService"); Slog.d(TAG, "isEnabledCredentialProviderService with componentName: "
+ componentName.flattenToString());
// TODO(253157366): Check additional set of services. // TODO(253157366): Check additional set of services.
final int userId = UserHandle.getCallingUserId(); final int userId = UserHandle.getCallingUserId();
@@ -792,10 +788,10 @@ public final class CredentialManagerService
MetricUtilities.logApiCalledSimpleV1( MetricUtilities.logApiCalledSimpleV1(
ApiName.IS_ENABLED_CREDENTIAL_PROVIDER_SERVICE, ApiName.IS_ENABLED_CREDENTIAL_PROVIDER_SERVICE,
ApiStatus.FAILURE, callingUid); ApiStatus.FAILURE, callingUid);
Log.w( Slog.w(
TAG, TAG,
"isEnabledCredentialProviderService: Component name does not" "isEnabledCredentialProviderService: Component name does "
+ " match package name."); + "not match package name.");
return false; return false;
} }
MetricUtilities.logApiCalledSimpleV1( MetricUtilities.logApiCalledSimpleV1(
@@ -813,7 +809,6 @@ public final class CredentialManagerService
@Override @Override
public List<CredentialProviderInfo> getCredentialProviderServices( public List<CredentialProviderInfo> getCredentialProviderServices(
int userId, int providerFilter) { int userId, int providerFilter) {
Log.i(TAG, "getCredentialProviderServices");
verifyGetProvidersPermission(); verifyGetProvidersPermission();
return CredentialProviderInfoFactory.getCredentialProviderServices( return CredentialProviderInfoFactory.getCredentialProviderServices(
@@ -823,7 +818,6 @@ public final class CredentialManagerService
@Override @Override
public List<CredentialProviderInfo> getCredentialProviderServicesForTesting( public List<CredentialProviderInfo> getCredentialProviderServicesForTesting(
int providerFilter) { int providerFilter) {
Log.i(TAG, "getCredentialProviderServicesForTesting");
verifyGetProvidersPermission(); verifyGetProvidersPermission();
final int userId = UserHandle.getCallingUserId(); final int userId = UserHandle.getCallingUserId();
@@ -844,8 +838,8 @@ public final class CredentialManagerService
.getServiceInfo().getComponentName()); .getServiceInfo().getComponentName());
} catch (NullPointerException e) { } catch (NullPointerException e) {
// Safe check // Safe check
Log.i(TAG, "Skipping provider as either the providerInfo" Slog.e(TAG, "Skipping provider as either the providerInfo"
+ "or serviceInfo is null - weird"); + " or serviceInfo is null - weird");
} }
}); });
} }
@@ -858,7 +852,8 @@ public final class CredentialManagerService
IClearCredentialStateCallback callback, IClearCredentialStateCallback callback,
String callingPackage) { String callingPackage) {
final long timestampBegan = System.nanoTime(); final long timestampBegan = System.nanoTime();
Log.i(TAG, "starting clearCredentialState with callingPackage: " + callingPackage); Slog.d(TAG, "starting clearCredentialState with callingPackage: "
+ callingPackage);
final int userId = UserHandle.getCallingUserId(); final int userId = UserHandle.getCallingUserId();
int callingUid = Binder.getCallingUid(); int callingUid = Binder.getCallingUid();
enforceCallingPackage(callingPackage, callingUid); enforceCallingPackage(callingPackage, callingUid);
@@ -885,13 +880,13 @@ public final class CredentialManagerService
if (providerSessions.isEmpty()) { if (providerSessions.isEmpty()) {
try { try {
// TODO("Replace with properly defined error type") // TODO("Replace with properly defined error type")
callback.onError("UNKNOWN", "No crdentials available on this " + "device"); callback.onError("UNKNOWN", "No credentials available on "
+ "this device");
} catch (RemoteException e) { } catch (RemoteException e) {
Log.i( Slog.e(
TAG, TAG,
"Issue invoking onError on IClearCredentialStateCallback " "Issue invoking onError on IClearCredentialStateCallback "
+ "callback: " + "callback: ", e);
+ e.getMessage());
} }
} }
@@ -906,7 +901,7 @@ public final class CredentialManagerService
public void registerCredentialDescription( public void registerCredentialDescription(
RegisterCredentialDescriptionRequest request, String callingPackage) RegisterCredentialDescriptionRequest request, String callingPackage)
throws IllegalArgumentException, NonCredentialProviderCallerException { throws IllegalArgumentException, NonCredentialProviderCallerException {
Log.i(TAG, "registerCredentialDescription"); Slog.d(TAG, "registerCredentialDescription with callingPackage: " + callingPackage);
if (!isCredentialDescriptionApiEnabled()) { if (!isCredentialDescriptionApiEnabled()) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
@@ -924,7 +919,9 @@ public final class CredentialManagerService
public void unregisterCredentialDescription( public void unregisterCredentialDescription(
UnregisterCredentialDescriptionRequest request, String callingPackage) UnregisterCredentialDescriptionRequest request, String callingPackage)
throws IllegalArgumentException { throws IllegalArgumentException {
Log.i(TAG, "registerCredentialDescription"); Slog.d(TAG, "unregisterCredentialDescription with callingPackage: "
+ callingPackage);
if (!isCredentialDescriptionApiEnabled()) { if (!isCredentialDescriptionApiEnabled()) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();

View File

@@ -23,7 +23,6 @@ import android.content.pm.PackageManager;
import android.content.pm.ServiceInfo; import android.content.pm.ServiceInfo;
import android.credentials.CredentialProviderInfo; import android.credentials.CredentialProviderInfo;
import android.service.credentials.CredentialProviderInfoFactory; import android.service.credentials.CredentialProviderInfoFactory;
import android.util.Log;
import android.util.Slog; import android.util.Slog;
import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.GuardedBy;
@@ -41,14 +40,15 @@ public final class CredentialManagerServiceImpl extends
// TODO(b/210531) : Make final when update flow is fixed // TODO(b/210531) : Make final when update flow is fixed
@GuardedBy("mLock") @GuardedBy("mLock")
@NonNull private CredentialProviderInfo mInfo; @NonNull
private CredentialProviderInfo mInfo;
CredentialManagerServiceImpl( CredentialManagerServiceImpl(
@NonNull CredentialManagerService master, @NonNull CredentialManagerService master,
@NonNull Object lock, int userId, String serviceName) @NonNull Object lock, int userId, String serviceName)
throws PackageManager.NameNotFoundException { throws PackageManager.NameNotFoundException {
super(master, lock, userId); super(master, lock, userId);
Log.i(TAG, "in CredentialManagerServiceImpl constructed with: " + serviceName); Slog.d(TAG, "CredentialManagerServiceImpl constructed for: " + serviceName);
synchronized (mLock) { synchronized (mLock) {
newServiceInfoLocked(ComponentName.unflattenFromString(serviceName)); newServiceInfoLocked(ComponentName.unflattenFromString(serviceName));
} }
@@ -63,10 +63,8 @@ public final class CredentialManagerServiceImpl extends
@NonNull CredentialManagerService master, @NonNull CredentialManagerService master,
@NonNull Object lock, int userId, CredentialProviderInfo providerInfo) { @NonNull Object lock, int userId, CredentialProviderInfo providerInfo) {
super(master, lock, userId); super(master, lock, userId);
Log.i(TAG, "in CredentialManagerServiceImpl constructed with system constructor: " Slog.d(TAG, "CredentialManagerServiceImpl constructed for: "
+ providerInfo.isSystemProvider() + providerInfo.getServiceInfo().getComponentName().flattenToString());
+ " , " + providerInfo.getServiceInfo() == null ? "" :
providerInfo.getServiceInfo().getComponentName().flattenToString());
mInfo = providerInfo; mInfo = providerInfo;
} }
@@ -76,12 +74,12 @@ public final class CredentialManagerServiceImpl extends
throws PackageManager.NameNotFoundException { throws PackageManager.NameNotFoundException {
// TODO : Test update flows with multiple providers // TODO : Test update flows with multiple providers
if (mInfo != null) { if (mInfo != null) {
Log.i(TAG, "newServiceInfoLocked with : " Slog.d(TAG, "newServiceInfoLocked, mInfo not null : "
+ mInfo.getServiceInfo().getComponentName().flattenToString() + " , " + mInfo.getServiceInfo().getComponentName().flattenToString() + " , "
+ serviceComponent.getPackageName()); + serviceComponent.flattenToString());
} else { } else {
Log.i(TAG, "newServiceInfoLocked with null mInfo , " Slog.d(TAG, "newServiceInfoLocked, mInfo null, "
+ serviceComponent.getPackageName()); + serviceComponent.flattenToString());
} }
mInfo = CredentialProviderInfoFactory.create( mInfo = CredentialProviderInfoFactory.create(
getContext(), serviceComponent, getContext(), serviceComponent,
@@ -90,18 +88,18 @@ public final class CredentialManagerServiceImpl extends
} }
/** /**
* Starts a provider session and associates it with the given request session. */ * Starts a provider session and associates it with the given request session.
*/
@Nullable @Nullable
@GuardedBy("mLock") @GuardedBy("mLock")
public ProviderSession initiateProviderSessionForRequestLocked( public ProviderSession initiateProviderSessionForRequestLocked(
RequestSession requestSession, List<String> requestOptions) { RequestSession requestSession, List<String> requestOptions) {
if (!requestOptions.isEmpty() && !isServiceCapableLocked(requestOptions)) { if (!requestOptions.isEmpty() && !isServiceCapableLocked(requestOptions)) {
Log.i(TAG, "Service is not capable"); Slog.d(TAG, "Service does not have the required capabilities");
return null; return null;
} }
Slog.i(TAG, "in initiateProviderSessionForRequest in CredManServiceImpl");
if (mInfo == null) { if (mInfo == null) {
Slog.i(TAG, "in initiateProviderSessionForRequest in CredManServiceImpl, " Slog.w(TAG, "in initiateProviderSessionForRequest in CredManServiceImpl, "
+ "but mInfo is null. This shouldn't happen"); + "but mInfo is null. This shouldn't happen");
return null; return null;
} }
@@ -114,15 +112,11 @@ public final class CredentialManagerServiceImpl extends
@GuardedBy("mLock") @GuardedBy("mLock")
boolean isServiceCapableLocked(List<String> requestedOptions) { boolean isServiceCapableLocked(List<String> requestedOptions) {
if (mInfo == null) { if (mInfo == null) {
Slog.i(TAG, "in isServiceCapable, mInfo is null");
return false; return false;
} }
for (String capability : requestedOptions) { for (String capability : requestedOptions) {
if (mInfo.hasCapability(capability)) { if (mInfo.hasCapability(capability)) {
Slog.i(TAG, "Provider can handle: " + capability);
return true; return true;
} else {
Slog.i(TAG, "Provider cannot handle: " + capability);
} }
} }
return false; return false;
@@ -146,7 +140,7 @@ public final class CredentialManagerServiceImpl extends
try { try {
newServiceInfoLocked(mInfo.getServiceInfo().getComponentName()); newServiceInfoLocked(mInfo.getServiceInfo().getComponentName());
} catch (PackageManager.NameNotFoundException e) { } catch (PackageManager.NameNotFoundException e) {
Log.i(TAG, "Issue while updating serviceInfo: " + e.getMessage()); Slog.e(TAG, "Issue while updating serviceInfo: " + e.getMessage());
} }
} }
} }