Skip to content

core: add CallCredentials2 and deprecate CallCredentials' old interface #4902

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 11, 2018
Merged

core: add CallCredentials2 and deprecate CallCredentials' old interface #4902

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
082aca6
core: add CallCredentials2 and deprecate CallCredentials.
zhangkun83 Oct 2, 2018
b044cb0
Fix warnings
zhangkun83 Oct 3, 2018
5bdeeed
Add more notes and annotate CallCredentials2 with `@Internal`
zhangkun83 Oct 3, 2018
2ff6c07
ExperimentalApi
zhangkun83 Oct 3, 2018
9dc5f4c
Move RequestInfo to CallCredentials
zhangkun83 Oct 11, 2018
a5c8261
Merge branch 'master' into call_credentials2
zhangkun83 Oct 11, 2018
8ae4bf2
static MetadataApplier
zhangkun83 Oct 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions auth/src/main/java/io/grpc/auth/GoogleAuthLibraryCallCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
import com.google.auth.RequestMetadataCallback;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.io.BaseEncoding;
import io.grpc.Attributes;
import io.grpc.CallCredentials;
import io.grpc.CallCredentials2;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.SecurityLevel;
Expand All @@ -47,7 +46,7 @@
/**
* Wraps {@link Credentials} as a {@link CallCredentials}.
*/
final class GoogleAuthLibraryCallCredentials implements CallCredentials {
final class GoogleAuthLibraryCallCredentials extends CallCredentials2 {
private static final Logger log
= Logger.getLogger(GoogleAuthLibraryCallCredentials.class.getName());
private static final JwtHelper jwtHelper
Expand Down Expand Up @@ -88,26 +87,20 @@ public GoogleAuthLibraryCallCredentials(Credentials creds) {
public void thisUsesUnstableApi() {}

@Override
public void applyRequestMetadata(MethodDescriptor<?, ?> method, Attributes attrs,
Executor appExecutor, final MetadataApplier applier) {
SecurityLevel security = attrs.get(ATTR_SECURITY_LEVEL);
if (security == null) {
// Although the API says ATTR_SECURITY_LEVEL is required, no one was really looking at it thus
// there may be transports that got away without setting it. Now we start to check it, it'd
// be less disruptive to tolerate nulls.
security = SecurityLevel.NONE;
}
public void applyRequestMetadata(
RequestInfo info, Executor appExecutor, final MetadataApplier applier) {
SecurityLevel security = info.getSecurityLevel();
if (requirePrivacy && security != SecurityLevel.PRIVACY_AND_INTEGRITY) {
applier.fail(Status.UNAUTHENTICATED
.withDescription("Credentials require channel with PRIVACY_AND_INTEGRITY security level. "
+ "Observed security level: " + security));
return;
}

String authority = checkNotNull(attrs.get(ATTR_AUTHORITY), "authority");
String authority = checkNotNull(info.getAuthority(), "authority");
final URI uri;
try {
uri = serviceUri(authority, method);
uri = serviceUri(authority, info.getMethodDescriptor());
} catch (StatusException e) {
applier.fail(e.getStatus());
return;
Expand Down
122 changes: 63 additions & 59 deletions auth/src/test/java/io/grpc/auth/GoogleAuthLibraryCallCredentialsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Multimaps;
import io.grpc.Attributes;
import io.grpc.CallCredentials;
import io.grpc.CallCredentials.MetadataApplier;
import io.grpc.CallCredentials2;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.SecurityLevel;
Expand Down Expand Up @@ -105,11 +105,8 @@ public class GoogleAuthLibraryCallCredentialsTest {
.build();
private URI expectedUri = URI.create("https://testauthority/a.service");

private final String authority = "testauthority";
private final Attributes attrs = Attributes.newBuilder()
.set(CallCredentials.ATTR_AUTHORITY, authority)
.set(CallCredentials.ATTR_SECURITY_LEVEL, SecurityLevel.PRIVACY_AND_INTEGRITY)
.build();
private static final String AUTHORITY = "testauthority";
private static final SecurityLevel SECURITY_LEVEL = SecurityLevel.PRIVACY_AND_INTEGRITY;

private ArrayList<Runnable> pendingRunnables = new ArrayList<>();

Expand Down Expand Up @@ -155,7 +152,7 @@ public void copyCredentialsToHeaders() throws Exception {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);

verify(credentials).getRequestMetadata(eq(expectedUri));
verify(applier).apply(headersCaptor.capture());
Expand All @@ -177,7 +174,7 @@ public void invalidBase64() throws Exception {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);

verify(credentials).getRequestMetadata(eq(expectedUri));
verify(applier).fail(statusCaptor.capture());
Expand All @@ -193,7 +190,7 @@ public void credentialsFailsWithIoException() throws Exception {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);

verify(credentials).getRequestMetadata(eq(expectedUri));
verify(applier).fail(statusCaptor.capture());
Expand All @@ -209,7 +206,7 @@ public void credentialsFailsWithRuntimeException() throws Exception {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);

verify(credentials).getRequestMetadata(eq(expectedUri));
verify(applier).fail(statusCaptor.capture());
Expand All @@ -229,7 +226,7 @@ public void credentialsReturnNullMetadata() throws Exception {
GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
for (int i = 0; i < 3; i++) {
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);
}

verify(credentials, times(3)).getRequestMetadata(eq(expectedUri));
Expand All @@ -255,14 +252,11 @@ public AccessToken refreshAccessToken() throws IOException {
return token;
}
};
// Security level should not impact non-GoogleCredentials
Attributes securityNone = attrs.toBuilder()
.set(CallCredentials.ATTR_SECURITY_LEVEL, SecurityLevel.NONE)
.build();

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, securityNone, executor, applier);
callCredentials.applyRequestMetadata(
new RequestInfoImpl(SecurityLevel.NONE), executor, applier);
assertEquals(1, runPendingRunnables());

verify(applier).apply(headersCaptor.capture());
Expand All @@ -276,13 +270,11 @@ public AccessToken refreshAccessToken() throws IOException {
public void googleCredential_privacyAndIntegrityAllowed() {
final AccessToken token = new AccessToken("allyourbase", new Date(Long.MAX_VALUE));
final Credentials credentials = GoogleCredentials.create(token);
Attributes privacy = attrs.toBuilder()
.set(CallCredentials.ATTR_SECURITY_LEVEL, SecurityLevel.PRIVACY_AND_INTEGRITY)
.build();

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, privacy, executor, applier);
callCredentials.applyRequestMetadata(
new RequestInfoImpl(SecurityLevel.PRIVACY_AND_INTEGRITY), executor, applier);
runPendingRunnables();

verify(applier).apply(headersCaptor.capture());
Expand All @@ -297,33 +289,11 @@ public void googleCredential_integrityDenied() {
final AccessToken token = new AccessToken("allyourbase", new Date(Long.MAX_VALUE));
final Credentials credentials = GoogleCredentials.create(token);
// Anything less than PRIVACY_AND_INTEGRITY should fail
Attributes integrity = attrs.toBuilder()
.set(CallCredentials.ATTR_SECURITY_LEVEL, SecurityLevel.INTEGRITY)
.build();

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, integrity, executor, applier);
runPendingRunnables();

verify(applier).fail(statusCaptor.capture());
Status status = statusCaptor.getValue();
assertEquals(Status.Code.UNAUTHENTICATED, status.getCode());
}

@Test
public void googleCredential_nullSecurityDenied() {
final AccessToken token = new AccessToken("allyourbase", new Date(Long.MAX_VALUE));
final Credentials credentials = GoogleCredentials.create(token);
// Null should not (for the moment) crash in horrible ways. In the future this could be changed,
// since it technically isn't allowed per the API.
Attributes integrity = attrs.toBuilder()
.set(CallCredentials.ATTR_SECURITY_LEVEL, null)
.build();

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, integrity, executor, applier);
callCredentials.applyRequestMetadata(
new RequestInfoImpl(SecurityLevel.INTEGRITY), executor, applier);
runPendingRunnables();

verify(applier).fail(statusCaptor.capture());
Expand All @@ -335,20 +305,12 @@ public void googleCredential_nullSecurityDenied() {
public void serviceUri() throws Exception {
GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method,
Attributes.newBuilder()
.setAll(attrs)
.set(CallCredentials.ATTR_AUTHORITY, "example.com:443")
.build(),
executor, applier);
callCredentials.applyRequestMetadata(
new RequestInfoImpl("example.com:443"), executor, applier);
verify(credentials).getRequestMetadata(eq(new URI("https://example.com/a.service")));

callCredentials.applyRequestMetadata(method,
Attributes.newBuilder()
.setAll(attrs)
.set(CallCredentials.ATTR_AUTHORITY, "example.com:123")
.build(),
executor, applier);
callCredentials.applyRequestMetadata(
new RequestInfoImpl("example.com:123"), executor, applier);
verify(credentials).getRequestMetadata(eq(new URI("https://example.com:123/a.service")));
}

Expand All @@ -366,7 +328,7 @@ public AccessToken refreshAccessToken() {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);
assertEquals(0, runPendingRunnables());

verify(applier).apply(headersCaptor.capture());
Expand All @@ -393,7 +355,7 @@ public AccessToken refreshAccessToken() {

GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);
assertEquals(1, runPendingRunnables());

verify(applier).apply(headersCaptor.capture());
Expand All @@ -412,7 +374,7 @@ public void oauthClassesNotInClassPath() throws Exception {
assertNull(GoogleAuthLibraryCallCredentials.createJwtHelperOrNull(null));
GoogleAuthLibraryCallCredentials callCredentials =
new GoogleAuthLibraryCallCredentials(credentials, null);
callCredentials.applyRequestMetadata(method, attrs, executor, applier);
callCredentials.applyRequestMetadata(new RequestInfoImpl(), executor, applier);

verify(credentials).getRequestMetadata(eq(expectedUri));
verify(applier).apply(headersCaptor.capture());
Expand All @@ -430,4 +392,46 @@ private int runPendingRunnables() {
}
return savedPendingRunnables.size();
}

private final class RequestInfoImpl extends CallCredentials2.RequestInfo {
final String authority;
final SecurityLevel securityLevel;

RequestInfoImpl() {
this(AUTHORITY, SECURITY_LEVEL);
}

RequestInfoImpl(SecurityLevel securityLevel) {
this(AUTHORITY, securityLevel);
}

RequestInfoImpl(String authority) {
this(authority, SECURITY_LEVEL);
}

RequestInfoImpl(String authority, SecurityLevel securityLevel) {
this.authority = authority;
this.securityLevel = securityLevel;
}

@Override
public MethodDescriptor<?, ?> getMethodDescriptor() {
return method;
}

@Override
public SecurityLevel getSecurityLevel() {
return securityLevel;
}

@Override
public String getAuthority() {
return authority;
}

@Override
public Attributes getTransportAttrs() {
return Attributes.EMPTY;
}
}
}
16 changes: 12 additions & 4 deletions core/src/main/java/io/grpc/CallCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@ public interface CallCredentials {
* The security level of the transport. It is guaranteed to be present in the {@code attrs} passed
* to {@link #applyRequestMetadata}. It is by default {@link SecurityLevel#NONE} but can be
* overridden by the transport.
*
* @deprecated transport implementations should use {@code
* io.grpc.internal.GrpcAttributes.ATTR_SECURITY_LEVEL} instead.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1914")
@Grpc.TransportAttr
@Deprecated
public static final Key<SecurityLevel> ATTR_SECURITY_LEVEL =
Key.create("io.grpc.CallCredentials.securityLevel");
Key.create("io.grpc.internal.GrpcAttributes.securityLevel");

/**
* The authority string used to authenticate the server. Usually it's the server's host name. It
Expand All @@ -54,6 +58,7 @@ public interface CallCredentials {
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1914")
@Grpc.TransportAttr
@Deprecated
public static final Key<String> ATTR_AUTHORITY = Key.create("io.grpc.CallCredentials.authority");

/**
Expand All @@ -73,8 +78,11 @@ public interface CallCredentials {
* needs to perform blocking operations.
* @param applier The outlet of the produced headers. It can be called either before or after this
* method returns.
*
* @deprecated implement {@link CallCredentials2} instead.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1914")
@Deprecated
void applyRequestMetadata(
MethodDescriptor<?, ?> method, Attributes attrs,
Executor appExecutor, MetadataApplier applier);
Expand All @@ -92,16 +100,16 @@ void applyRequestMetadata(
* <p>Exactly one of its methods must be called to make the RPC proceed.
*/
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1914")
public interface MetadataApplier {
public abstract class MetadataApplier {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An class under an interface is implicitly static. But it's no harm to add it (and it will be needed once CallCredentials becomes an abstract class).

/**
* Called when headers are successfully generated. They will be merged into the original
* headers.
*/
void apply(Metadata headers);
public abstract void apply(Metadata headers);

/**
* Called when there has been an error when preparing the headers. This will fail the RPC.
*/
void fail(Status status);
public abstract void fail(Status status);
}
}
Loading