diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java index 502ddd345..b3b41f14c 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListener.java @@ -20,7 +20,8 @@ import java.util.List; import java.util.Optional; import javax.annotation.Nullable; import javax.ws.rs.container.ContainerRequestContext; -import javax.ws.rs.container.ContainerRequestFilter; +import javax.ws.rs.container.ContainerResponseContext; +import javax.ws.rs.container.ContainerResponseFilter; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; @@ -34,8 +35,8 @@ import org.whispersystems.textsecuregcm.util.logging.UriInfoUtil; /** * Gathers and reports HTTP request metrics at the Jetty container level, which sits above Jersey. In order to get - * templated Jersey request paths, it implements {@link javax.ws.rs.container.ContainerRequestFilter}, in order to - * give itself access to the template. It is limited to {@link TrafficSource#HTTP} requests. + * templated Jersey request paths, it implements {@link javax.ws.rs.container.ContainerResponseFilter}, in order to give + * itself access to the template. It is limited to {@link TrafficSource#HTTP} requests. *

* It implements {@link LifeCycle.Listener} without overriding methods, so that it can be an event listener that * Dropwizard will attach to the container—the {@link Container.Listener} implementation is where it attaches @@ -44,7 +45,7 @@ import org.whispersystems.textsecuregcm.util.logging.UriInfoUtil; * @see MetricsRequestEventListener */ public class MetricsHttpChannelListener implements HttpChannel.Listener, Container.Listener, LifeCycle.Listener, - ContainerRequestFilter { + ContainerResponseFilter { private static final Logger logger = LoggerFactory.getLogger(MetricsHttpChannelListener.class); @@ -86,7 +87,7 @@ public class MetricsHttpChannelListener implements HttpChannel.Listener, Contain } public void configure(final Environment environment) { - // register as ContainerRequestFilter + // register as ContainerResponseFilter environment.jersey().register(this); // hook into lifecycle events, to react to the Connector being added @@ -138,7 +139,8 @@ public class MetricsHttpChannelListener implements HttpChannel.Listener, Contain } @Override - public void filter(final ContainerRequestContext requestContext) throws IOException { + public void filter(final ContainerRequestContext requestContext, final ContainerResponseContext responseContext) + throws IOException { requestContext.setProperty(URI_INFO_PROPERTY_NAME, requestContext.getUriInfo()); } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java index daf38d1e6..b2dcbf2c1 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/metrics/MetricsHttpChannelListenerIntegrationTest.java @@ -6,7 +6,9 @@ package org.whispersystems.textsecuregcm.metrics; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -24,6 +26,7 @@ import io.dropwizard.testing.junit5.DropwizardExtensionsSupport; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Tag; +import java.io.IOException; import java.security.Principal; import java.time.Duration; import java.util.HashSet; @@ -31,14 +34,22 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import java.util.stream.Stream; +import javax.annotation.Priority; import javax.security.auth.Subject; import javax.ws.rs.GET; +import javax.ws.rs.InternalServerErrorException; +import javax.ws.rs.NotAuthorizedException; import javax.ws.rs.Path; import javax.ws.rs.PathParam; +import javax.ws.rs.Priorities; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.client.Client; import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.container.ContainerRequestFilter; import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; @@ -76,7 +87,8 @@ class MetricsHttpChannelListenerIntegrationTest { @ParameterizedTest @MethodSource @SuppressWarnings("unchecked") - void testSimplePath(String requestPath, String expectedTagPath, String expectedResponse) throws Exception { + void testSimplePath(String requestPath, String expectedTagPath, String expectedResponse, int expectedStatus) + throws Exception { final CompletableFuture listenerCompleteFuture = new CompletableFuture<>(); LISTENER_FUTURE_REFERENCE.set(listenerCompleteFuture); @@ -90,13 +102,30 @@ class MetricsHttpChannelListenerIntegrationTest { Client client = EXTENSION.client(); - final String response = client.target( + final Supplier request = () -> client.target( String.format("http://localhost:%d%s", EXTENSION.getLocalPort(), requestPath)) .request() .header(HttpHeaders.USER_AGENT, "Signal-Android/4.53.7 (Android 8.1)") .get(String.class); - assertEquals(expectedResponse, response); + switch (expectedStatus) { + case 200: { + final String response = request.get(); + assertEquals(expectedResponse, response); + break; + } + case 401: { + assertThrows(NotAuthorizedException.class, request::get); + break; + } + case 500: { + assertThrows(InternalServerErrorException.class, request::get); + break; + } + default: { + fail("unexpected status"); + } + } listenerCompleteFuture.get(1000, TimeUnit.MILLISECONDS); @@ -113,7 +142,7 @@ class MetricsHttpChannelListenerIntegrationTest { assertEquals(5, tags.size()); assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.PATH_TAG, expectedTagPath))); assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.METHOD_TAG, "GET"))); - assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, "200"))); + assertTrue(tags.contains(Tag.of(MetricsHttpChannelListener.STATUS_CODE_TAG, String.valueOf(expectedStatus)))); assertTrue( tags.contains(Tag.of(MetricsHttpChannelListener.TRAFFIC_SOURCE_TAG, TRAFFIC_SOURCE.name().toLowerCase()))); assertTrue(tags.contains(Tag.of(UserAgentTagUtil.PLATFORM_TAG, "android"))); @@ -121,9 +150,11 @@ class MetricsHttpChannelListenerIntegrationTest { static Stream testSimplePath() { return Stream.of( - Arguments.of("/v1/test/hello", "/v1/test/hello", "Hello!"), + Arguments.of("/v1/test/hello", "/v1/test/hello", "Hello!", 200), Arguments.of("/v1/test/greet/friend", "/v1/test/greet/{name}", - String.format(TestResource.GREET_FORMAT, "friend")) + String.format(TestResource.GREET_FORMAT, "friend"), 200), + Arguments.of("/v1/test/greet/unauthorized", "/v1/test/greet/{name}", null, 401), + Arguments.of("/v1/test/greet/exception", "/v1/test/greet/{name}", null, 500) ); } @@ -141,6 +172,7 @@ class MetricsHttpChannelListenerIntegrationTest { environment.lifecycle().addEventListener(new TestListener(LISTENER_FUTURE_REFERENCE)); environment.jersey().register(new TestResource()); + environment.jersey().register(new TestAuthFilter()); // WebSocket set up final WebSocketConfiguration webSocketConfiguration = new WebSocketConfiguration(); @@ -159,6 +191,17 @@ class MetricsHttpChannelListenerIntegrationTest { } } + @Priority(Priorities.AUTHENTICATION) + static class TestAuthFilter implements ContainerRequestFilter { + + @Override + public void filter(final ContainerRequestContext requestContext) throws IOException { + if (requestContext.getUriInfo().getPath().contains("unauthorized")) { + throw new WebApplicationException(Response.Status.UNAUTHORIZED); + } + } + } + /** * A simple listener to signal that {@link HttpChannel.Listener} has completed its work, since its onComplete() is on * a different thread from the one that sends the response, creating a race condition between the listener and the @@ -208,7 +251,9 @@ class MetricsHttpChannelListenerIntegrationTest { @Path("/greet/{name}") public String testGreetByName(@PathParam("name") String name, @Context ContainerRequestContext context) { - context.setProperty("uriInfo", context.getUriInfo()); + if ("exception".equals(name)) { + throw new InternalServerErrorException(); + } return String.format(GREET_FORMAT, name); }