diff --git a/build.gradle b/build.gradle index ee5fbb31..807441c0 100644 --- a/build.gradle +++ b/build.gradle @@ -86,7 +86,12 @@ tasks.sourcesJar { } // All it does is complain about generated code. -javadoc { options.addStringOption('Xdoclint:none', '-quiet') } +javadoc { + options.addStringOption('Xdoclint:none', '-quiet') + // Keep gradle from barking about an implicit dependency on the generated test + // sources, which live under the same build/generated directory. + dependsOn tasks.compileTestJava +} def grpcVersion = "1.78.0" def protocVersion = "4.33.5" @@ -179,6 +184,8 @@ configurations { // Test things dependencies { + testImplementation "junit:junit:4.13.2" + testImplementation "org.assertj:assertj-core:3.27.7" intTestImplementation "junit:junit:4.13.2" intTestImplementation "org.assertj:assertj-core:3.27.7" } diff --git a/src/main/java/com/authzed/grpcutil/BearerToken.java b/src/main/java/com/authzed/grpcutil/BearerToken.java index 0e0cb0ff..1f77c1c5 100644 --- a/src/main/java/com/authzed/grpcutil/BearerToken.java +++ b/src/main/java/com/authzed/grpcutil/BearerToken.java @@ -23,13 +23,18 @@ public BearerToken(String value) { @Override public void applyRequestMetadata(RequestInfo requestInfo, Executor executor, MetadataApplier applier) { executor.execute(() -> { + Metadata headers; try { - Metadata headers = new Metadata(); + headers = new Metadata(); headers.put(META_DATA_KEY, header); - applier.apply(headers); - } catch (Throwable e) { + } catch (RuntimeException e) { applier.fail(Status.UNAUTHENTICATED.withCause(e)); + return; } + // apply() must stay outside the try block: if it throws, the applier is + // already finalized and calling fail() would raise "apply() or fail() + // already called", masking the original exception. + applier.apply(headers); }); } } diff --git a/src/test/java/com/authzed/grpcutil/BearerTokenTest.java b/src/test/java/com/authzed/grpcutil/BearerTokenTest.java new file mode 100644 index 00000000..ffb86994 --- /dev/null +++ b/src/test/java/com/authzed/grpcutil/BearerTokenTest.java @@ -0,0 +1,60 @@ +package com.authzed.grpcutil; + +import io.grpc.CallCredentials; +import io.grpc.Metadata; +import io.grpc.Status; + +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class BearerTokenTest { + private static final Metadata.Key AUTHORIZATION_KEY = + Metadata.Key.of("authorization", Metadata.ASCII_STRING_MARSHALLER); + + @Test + public void appliesBearerHeader() { + BearerToken credentials = new BearerToken("sometoken"); + RecordingApplier applier = new RecordingApplier(); + + credentials.applyRequestMetadata(null, Runnable::run, applier); + + assertThat(applier.failure).isNull(); + assertThat(applier.headers).isNotNull(); + assertThat(applier.headers.get(AUTHORIZATION_KEY)).isEqualTo("Bearer sometoken"); + } + + @Test + public void doesNotCallFailAfterApplyThrows() { + BearerToken credentials = new BearerToken("sometoken"); + RuntimeException applyError = new RuntimeException("apply failed"); + RecordingApplier applier = new RecordingApplier() { + @Override + public void apply(Metadata headers) { + throw applyError; + } + }; + + // If apply() itself throws, calling fail() afterwards is illegal and would + // mask the original exception, so the exception should propagate as-is. + assertThatThrownBy(() -> credentials.applyRequestMetadata(null, Runnable::run, applier)) + .isSameAs(applyError); + assertThat(applier.failure).isNull(); + } + + private static class RecordingApplier extends CallCredentials.MetadataApplier { + Metadata headers; + Status failure; + + @Override + public void apply(Metadata headers) { + this.headers = headers; + } + + @Override + public void fail(Status status) { + this.failure = status; + } + } +}