diff --git a/binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java b/binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java index 6c1026e212..c4c17bb2ce 100644 --- a/binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java +++ b/binder/src/main/java/io/grpc/binder/AndroidComponentAddress.java @@ -18,10 +18,14 @@ package io.grpc.binder; 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 @@ import javax.annotation.Nullable; * fields, namely, an action of {@link ApiConstants#ACTION_BIND}, an empty category set and null * type and data URI. * - *

The semantics of {@link #equals(Object)} are the same as {@link Intent#filterEquals(Intent)}. + *

Optionally contains a {@link UserHandle} that must be considered wherever the {@link Intent} + * is evaluated. + * + *

{@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 final class AndroidComponentAddress extends SocketAddress { * @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 final class AndroidComponentAddress extends SocketAddress { */ 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 final class AndroidComponentAddress extends SocketAddress { /** * Returns this address as an explicit {@link Intent} suitable for passing to {@link * Context#bindService}. + * + *

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 final class AndroidComponentAddress extends SocketAddress { 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. + * + *

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'. + * + *

'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); + } } } diff --git a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java index d054c8d8ba..0b8f0bb4b3 100644 --- a/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java +++ b/binder/src/main/java/io/grpc/binder/BinderChannelBuilder.java @@ -236,20 +236,28 @@ public final class BinderChannelBuilder extends ForwardingChannelBuilderWhen 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. + *

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. * + *

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; } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java index 1e2b80b2fd..3852b21d5c 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderClientTransportFactory.java @@ -50,7 +50,7 @@ public final class BinderClientTransportFactory implements ClientTransportFactor final ObjectPool scheduledExecutorPool; final ObjectPool 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 @@ public final class BinderClientTransportFactory implements ClientTransportFactor 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 final class BinderClientTransportFactory implements ClientTransportFactor ObjectPool 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 final class BinderClientTransportFactory implements ClientTransportFactor return this; } - public Builder setTargetUserHandle(@Nullable UserHandle targetUserHandle) { - this.targetUserHandle = targetUserHandle; + public Builder setDefaultTargetUserHandle(@Nullable UserHandle defaultTargetUserHandle) { + this.defaultTargetUserHandle = defaultTargetUserHandle; return this; } diff --git a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java index 8c428fac98..254ad5bb40 100644 --- a/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java +++ b/binder/src/main/java/io/grpc/binder/internal/BinderTransport.java @@ -611,7 +611,9 @@ public abstract class BinderTransport factory.sourceContext, factory.channelCredentials, targetAddress.asBindIntent(), - factory.targetUserHandle, + targetAddress.getTargetUser() != null + ? targetAddress.getTargetUser() + : factory.defaultTargetUserHandle, factory.bindServiceFlags.toInteger(), this); } diff --git a/binder/src/test/java/io/grpc/binder/AndroidComponentAddressTest.java b/binder/src/test/java/io/grpc/binder/AndroidComponentAddressTest.java index 6d7e53e5a1..d7d77d7feb 100644 --- a/binder/src/test/java/io/grpc/binder/AndroidComponentAddressTest.java +++ b/binder/src/test/java/io/grpc/binder/AndroidComponentAddressTest.java @@ -18,11 +18,14 @@ package io.grpc.binder; 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 final class AndroidComponentAddressTest { 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 final class AndroidComponentAddressTest { 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 final class AndroidComponentAddressTest { .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 final class AndroidComponentAddressTest { .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(); + } + } }