AndroidComponentAddress includes a target UserHandle (grpc#11670)

The target UserHandle is best modeled as part of the SocketAddress not the Channel since it's part of the server's location.

This change allows a NameResolver to select different target users over time within a single Channel.

Author: jdcormie

binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java

@@ -18,10 +18,14 @@
 
 import static android.content.Intent.URI_ANDROID_APP_SCHEME;
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
+import android.os.UserHandle;
+import com.google.common.base.Objects;
+import io.grpc.ExperimentalApi;
 import java.net.SocketAddress;
 import javax.annotation.Nullable;
 
@@ -41,18 +45,25 @@
  * fields, namely, an action of {@link ApiConstants#ACTION_BIND}, an empty category set and null
  * type and data URI.
  *
- * <p>The semantics of {@link #equals(Object)} are the same as {@link Intent#filterEquals(Intent)}.
+ * <p>Optionally contains a {@link UserHandle} that must be considered wherever the {@link Intent}
+ * is evaluated.
+ *
+ * <p>{@link #equals(Object)} uses {@link Intent#filterEquals(Intent)} semantics to compare Intents.
  */
 public final class AndroidComponentAddress extends SocketAddress {
   private static final long serialVersionUID = 0L;
 
   private final Intent bindIntent; // "Explicit", having either a component or package restriction.
 
-  protected AndroidComponentAddress(Intent bindIntent) {
+  @Nullable
+  private final UserHandle targetUser; // null means the same user that hosts this process.
+
+  protected AndroidComponentAddress(Intent bindIntent, @Nullable UserHandle targetUser) {
     checkArgument(
         bindIntent.getComponent() != null || bindIntent.getPackage() != null,
         "'bindIntent' must be explicit. Specify either a package or ComponentName.");
     this.bindIntent = bindIntent;
+    this.targetUser = targetUser;
   }
 
   /**
@@ -99,7 +110,7 @@ public static AndroidComponentAddress forRemoteComponent(
    * @throws IllegalArgumentException if 'intent' isn't "explicit"
    */
   public static AndroidComponentAddress forBindIntent(Intent intent) {
-    return new AndroidComponentAddress(intent.cloneFilter());
+    return new AndroidComponentAddress(intent.cloneFilter(), null);
   }
 
   /**
@@ -108,7 +119,7 @@ public static AndroidComponentAddress forBindIntent(Intent intent) {
    */
   public static AndroidComponentAddress forComponent(ComponentName component) {
     return new AndroidComponentAddress(
-        new Intent(ApiConstants.ACTION_BIND).setComponent(component));
+        new Intent(ApiConstants.ACTION_BIND).setComponent(component), null);
   }
 
   /**
@@ -141,6 +152,9 @@ public ComponentName getComponent() {
   /**
    * Returns this address as an explicit {@link Intent} suitable for passing to {@link
    * Context#bindService}.
+   *
+   * <p>NB: The returned Intent does not specify a target Android user. If {@link #getTargetUser()}
+   * is non-null, {@link Context#bindServiceAsUser} should be called instead.
    */
   public Intent asBindIntent() {
     return bindIntent.cloneFilter(); // Intent is mutable so return a copy.
@@ -177,13 +191,77 @@ public int hashCode() {
   public boolean equals(Object obj) {
     if (obj instanceof AndroidComponentAddress) {
       AndroidComponentAddress that = (AndroidComponentAddress) obj;
-      return bindIntent.filterEquals(that.bindIntent);
+      return bindIntent.filterEquals(that.bindIntent)
+          && Objects.equal(this.targetUser, that.targetUser);
     }
     return false;
   }
 
   @Override
   public String toString() {
-    return "AndroidComponentAddress[" + bindIntent + "]";
+    StringBuilder builder = new StringBuilder("AndroidComponentAddress[");
+    if (targetUser != null) {
+      builder.append(targetUser);
+      builder.append("@");
+    }
+    builder.append(bindIntent);
+    builder.append("]");
+    return builder.toString();
+  }
+
+  /**
+   * Identifies the Android user in which the bind Intent will be evaluated.
+   *
+   * <p>Returns the {@link UserHandle}, or null which means that the Android user hosting the
+   * current process will be used.
+   */
+  @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10173")
+  @Nullable
+  public UserHandle getTargetUser() {
+    return targetUser;
+  }
+
+  public static Builder newBuilder() {
+    return new Builder();
+  }
+
+  /** Fluently builds instances of {@link AndroidComponentAddress}. */
+  public static class Builder {
+    Intent bindIntent;
+    UserHandle targetUser;
+
+    /**
+     * Sets the binding {@link Intent} to one having the "filter matching" fields of 'intent'.
+     *
+     * <p>'intent' must be "explicit", i.e. having either a target component ({@link
+     * Intent#getComponent()}) or package restriction ({@link Intent#getPackage()}).
+     */
+    public Builder setBindIntent(Intent intent) {
+      this.bindIntent = intent.cloneFilter();
+      return this;
+    }
+
+    /**
+     * Sets the binding {@link Intent} to one with the specified 'component' and default values for
+     * all other fields, for convenience.
+     */
+    public Builder setBindIntentFromComponent(ComponentName component) {
+      this.bindIntent = new Intent(ApiConstants.ACTION_BIND).setComponent(component);
+      return this;
+    }
+
+    /** See {@link AndroidComponentAddress#getTargetUser()}. */
+    @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10173")
+    public Builder setTargetUser(@Nullable UserHandle targetUser) {
+      this.targetUser = targetUser;
+      return this;
+    }
+
+    public AndroidComponentAddress build() {
+      // We clone any incoming mutable intent in the setter, not here. AndroidComponentAddress
+      // itself is immutable so multiple instances built from here can safely share 'bindIntent'.
+      checkState(bindIntent != null, "Required property 'bindIntent' unset");
+      return new AndroidComponentAddress(bindIntent, targetUser);
+    }
   }
 }

binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java

@@ -236,20 +236,28 @@ public BinderChannelBuilder securityPolicy(SecurityPolicy securityPolicy) {
   }
 
   /**
-   * Provides the target {@UserHandle} of the remote Android service.
+   * Specifies the {@link UserHandle} to be searched for the remote Android Service by default.
    *
-   * <p>When targetUserHandle is set, Context.bindServiceAsUser will used and additional Android
-   * permissions will be required. If your usage does not require cross-user communications, please
-   * do not set this field. It is the caller's responsibility to make sure that it holds the
-   * corresponding permissions.
+   * <p>Used only as a fallback if the direct or resolved {@link AndroidComponentAddress} doesn't
+   * specify a {@link UserHandle}. If neither the Channel nor the {@link AndroidComponentAddress}
+   * specifies a target user, the {@link UserHandle} of the current process will be used.
    *
+   * <p>Targeting a Service in a different Android user is uncommon and requires special permissions
+   * normally reserved for system apps. See {@link android.content.Context#bindServiceAsUser} for
+   * details.
+   *
+   * @deprecated This method's name is misleading because it implies an impersonated client identity
+   *     when it's actually specifying part of the server's location. It's also no longer necessary
+   *     since the target user is part of {@link AndroidComponentAddress}. Prefer to specify target
+   *     user in the address instead, either directly or via a {@link io.grpc.NameResolverProvider}.
    * @param targetUserHandle the target user to bind into.
    * @return this
    */
   @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10173")
   @RequiresApi(30)
+  @Deprecated
   public BinderChannelBuilder bindAsUser(UserHandle targetUserHandle) {
-    transportFactoryBuilder.setTargetUserHandle(targetUserHandle);
+    transportFactoryBuilder.setDefaultTargetUserHandle(targetUserHandle);
     return this;
   }
 

binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java

@@ -50,7 +50,7 @@ public final class BinderClientTransportFactory implements ClientTransportFactor
   final ObjectPool<ScheduledExecutorService> scheduledExecutorPool;
   final ObjectPool<? extends Executor> offloadExecutorPool;
   final SecurityPolicy securityPolicy;
-  @Nullable final UserHandle targetUserHandle;
+  @Nullable final UserHandle defaultTargetUserHandle;
   final BindServiceFlags bindServiceFlags;
   final InboundParcelablePolicy inboundParcelablePolicy;
   final OneWayBinderProxy.Decorator binderDecorator;
@@ -70,7 +70,7 @@ private BinderClientTransportFactory(Builder builder) {
     scheduledExecutorPool = checkNotNull(builder.scheduledExecutorPool);
     offloadExecutorPool = checkNotNull(builder.offloadExecutorPool);
     securityPolicy = checkNotNull(builder.securityPolicy);
-    targetUserHandle = builder.targetUserHandle;
+    defaultTargetUserHandle = builder.defaultTargetUserHandle;
     bindServiceFlags = checkNotNull(builder.bindServiceFlags);
     inboundParcelablePolicy = checkNotNull(builder.inboundParcelablePolicy);
     binderDecorator = checkNotNull(builder.binderDecorator);
@@ -123,7 +123,7 @@ public static final class Builder implements ClientTransportFactoryBuilder {
     ObjectPool<ScheduledExecutorService> scheduledExecutorPool =
         SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE);
     SecurityPolicy securityPolicy = SecurityPolicies.internalOnly();
-    @Nullable UserHandle targetUserHandle;
+    @Nullable UserHandle defaultTargetUserHandle;
     BindServiceFlags bindServiceFlags = BindServiceFlags.DEFAULTS;
     InboundParcelablePolicy inboundParcelablePolicy = InboundParcelablePolicy.DEFAULT;
     OneWayBinderProxy.Decorator binderDecorator = OneWayBinderProxy.IDENTITY_DECORATOR;
@@ -165,8 +165,8 @@ public Builder setSecurityPolicy(SecurityPolicy securityPolicy) {
       return this;
     }
 
-    public Builder setTargetUserHandle(@Nullable UserHandle targetUserHandle) {
-      this.targetUserHandle = targetUserHandle;
+    public Builder setDefaultTargetUserHandle(@Nullable UserHandle defaultTargetUserHandle) {
+      this.defaultTargetUserHandle = defaultTargetUserHandle;
       return this;
     }
 

binder/src/main/java/io/grpc/binder/internal/BinderTransport.java

@@ -611,7 +611,9 @@ public BinderClientTransport(
               factory.sourceContext,
               factory.channelCredentials,
               targetAddress.asBindIntent(),
-              factory.targetUserHandle,
+              targetAddress.getTargetUser() != null
+                  ? targetAddress.getTargetUser()
+                  : factory.defaultTargetUserHandle,
               factory.bindServiceFlags.toInteger(),
               this);
     }

binder/src/test/java/io/grpc/binder/AndroidComponentAddressTest.java

@@ -18,11 +18,14 @@
 
 import static android.content.Intent.URI_ANDROID_APP_SCHEME;
 import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
 
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.net.Uri;
+import android.os.Parcel;
+import android.os.UserHandle;
 import androidx.test.core.app.ApplicationProvider;
 import com.google.common.testing.EqualsTester;
 import java.net.URISyntaxException;
@@ -83,6 +86,32 @@ public void testAsBindIntent() {
     assertThat(addr.asBindIntent().filterEquals(bindIntent)).isTrue();
   }
 
+  @Test
+  public void testPostCreateIntentMutation() {
+    Intent bindIntent = new Intent().setAction("foo-action").setComponent(hostComponent);
+    AndroidComponentAddress addr = AndroidComponentAddress.forBindIntent(bindIntent);
+    bindIntent.setAction("bar-action");
+    assertThat(addr.asBindIntent().getAction()).isEqualTo("foo-action");
+  }
+
+  @Test
+  public void testPostBuildIntentMutation() {
+    Intent bindIntent = new Intent().setAction("foo-action").setComponent(hostComponent);
+    AndroidComponentAddress addr =
+        AndroidComponentAddress.newBuilder().setBindIntent(bindIntent).build();
+    bindIntent.setAction("bar-action");
+    assertThat(addr.asBindIntent().getAction()).isEqualTo("foo-action");
+  }
+
+  @Test
+  public void testBuilderMissingRequired() {
+    IllegalStateException ise =
+        assertThrows(
+            IllegalStateException.class,
+            () -> AndroidComponentAddress.newBuilder().setTargetUser(newUserHandle(123)).build());
+    assertThat(ise.getMessage()).contains("bindIntent");
+  }
+
   @Test
   @Config(sdk = 30)
   public void testAsAndroidAppUriSdk30() throws URISyntaxException {
@@ -117,13 +146,21 @@ public void testEquality() {
             AndroidComponentAddress.forContext(appContext),
             AndroidComponentAddress.forLocalComponent(appContext, appContext.getClass()),
             AndroidComponentAddress.forRemoteComponent(
-                appContext.getPackageName(), appContext.getClass().getName()))
+                appContext.getPackageName(), appContext.getClass().getName()),
+            AndroidComponentAddress.newBuilder()
+                .setBindIntentFromComponent(hostComponent)
+                .setTargetUser(null)
+                .build())
         .addEqualityGroup(
             AndroidComponentAddress.forRemoteComponent("appy.mcappface", ".McActivity"))
         .addEqualityGroup(AndroidComponentAddress.forLocalComponent(appContext, getClass()))
         .addEqualityGroup(
             AndroidComponentAddress.forBindIntent(
-                new Intent().setAction("custom-action").setComponent(hostComponent)))
+                new Intent().setAction("custom-action").setComponent(hostComponent)),
+            AndroidComponentAddress.newBuilder()
+                .setBindIntent(new Intent().setAction("custom-action").setComponent(hostComponent))
+                .setTargetUser(null)
+                .build())
         .addEqualityGroup(
             AndroidComponentAddress.forBindIntent(
                 new Intent()
@@ -133,6 +170,31 @@ public void testEquality() {
         .testEquals();
   }
 
+  @Test
+  public void testUnequalTargetUsers() {
+    new EqualsTester()
+        .addEqualityGroup(
+            AndroidComponentAddress.newBuilder()
+                .setBindIntentFromComponent(hostComponent)
+                .setTargetUser(newUserHandle(10))
+                .build(),
+            AndroidComponentAddress.newBuilder()
+                .setBindIntentFromComponent(hostComponent)
+                .setTargetUser(newUserHandle(10))
+                .build())
+        .addEqualityGroup(
+            AndroidComponentAddress.newBuilder()
+                .setBindIntentFromComponent(hostComponent)
+                .setTargetUser(newUserHandle(11))
+                .build())
+        .addEqualityGroup(
+            AndroidComponentAddress.newBuilder()
+                .setBindIntentFromComponent(hostComponent)
+                .setTargetUser(null)
+                .build())
+        .testEquals();
+  }
+
   @Test
   @Config(sdk = 30)
   public void testPackageFilterEquality30AndUp() {
@@ -163,4 +225,15 @@ public void testPackageFilterEqualityPre30() {
                     .setComponent(new ComponentName("pkg", "cls"))))
         .testEquals();
   }
+
+  private static UserHandle newUserHandle(int userId) {
+    Parcel parcel = Parcel.obtain();
+    try {
+      parcel.writeInt(userId);
+      parcel.setDataPosition(0);
+      return new UserHandle(parcel);
+    } finally {
+      parcel.recycle();
+    }
+  }
 }