Enforce Mandatory Client ID (Github Issue #704)

aggregation/simple/src/main/java/com/spotify/heroic/aggregation/simple/Module.java

@@ -35,10 +35,9 @@
 import com.spotify.heroic.grammar.IntegerExpression;
 import dagger.Component;
 import eu.toolchain.serializer.SerializerFramework;
-
+import java.util.Optional;
 import javax.inject.Inject;
 import javax.inject.Named;
-import java.util.Optional;
 
 public class Module implements HeroicModule {
     @Override

aggregation/simple/src/test/java/com/spotify/heroic/aggregation/simple/FilterAggregationTest.java

@@ -1,15 +1,14 @@
 package com.spotify.heroic.aggregation.simple;
 
+import static org.junit.Assert.assertEquals;
+
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.jsontype.NamedType;
 import com.spotify.heroic.aggregation.AggregationInstance;
 import com.spotify.heroic.test.FakeModuleLoader;
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
-
 public class FilterAggregationTest {
     private ObjectMapper mapper;
 

aggregation/simple/src/test/java/com/spotify/heroic/aggregation/simple/FilterKAreaAggregationTest.java

@@ -1,5 +1,7 @@
 package com.spotify.heroic.aggregation.simple;
 
+import static org.junit.Assert.assertEquals;
+
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.spotify.heroic.aggregation.AggregationInstance;
@@ -12,17 +14,14 @@
 import com.spotify.heroic.common.DateRange;
 import com.spotify.heroic.common.Series;
 import com.spotify.heroic.metric.Point;
-import org.junit.Assert;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.runners.MockitoJUnitRunner;
-
 import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-
-import static org.junit.Assert.assertEquals;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
 public class FilterKAreaAggregationTest {

aggregation/simple/src/test/java/com/spotify/heroic/aggregation/simple/TdigestAggregationTest.java

@@ -1,9 +1,7 @@
 package com.spotify.heroic.aggregation.simple;
 
-
 import static org.junit.Assert.assertEquals;
 
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.protobuf.ByteString;

aggregation/simple/src/test/java/com/spotify/heroic/aggregation/simple/ValueBucketIntegrationTest.java

@@ -6,11 +6,8 @@
 import com.spotify.heroic.aggregation.DoubleBucket;
 import com.spotify.heroic.aggregation.TDigestBucket;
 import com.spotify.heroic.metric.DistributionPoint;
-import com.spotify.heroic.metric.HeroicDistribution;
 import com.spotify.heroic.metric.Point;
-import com.tdunning.math.stats.MergingDigest;
 import com.tdunning.math.stats.TDigest;
-import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -25,7 +22,6 @@
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
-import com.google.protobuf.ByteString;
 
 public abstract class ValueBucketIntegrationTest {
     private static final int NCPU = Runtime.getRuntime().availableProcessors();

consumer/pubsub/src/main/java/com/spotify/heroic/consumer/pubsub/PubSubConsumer.java

@@ -21,11 +21,11 @@
 
 package com.spotify.heroic.consumer.pubsub;
 
+import com.google.common.collect.ImmutableMap;
 import com.spotify.heroic.common.Statistics;
 import com.spotify.heroic.consumer.Consumer;
 import com.spotify.heroic.lifecycle.LifeCycleRegistry;
 import com.spotify.heroic.lifecycle.LifeCycles;
-import com.google.common.collect.ImmutableMap;
 import eu.toolchain.async.AsyncFramework;
 import eu.toolchain.async.AsyncFuture;
 import eu.toolchain.async.Managed;

docs/_layouts/api-endpoint.html

@@ -44,7 +44,9 @@ <h4>Example Request</h4>
       <h4>Example Curl</h4>
       <pre><code class="language-bash">
           {%- capture headers %}
-          {%- unless page.empty %}-H "Content-Type: application/json" {% endunless %}
+          {%- unless page.empty %}-H "X-Client-Id: my_app_name" -H "Content-Type: application/json"
+        {%
+        endunless %}
           {%- endcapture %}
           $ curl -X{{ page.method }} {{ headers -}}
           http://localhost:8080{{ page.endpoint }}

heroic-component/src/main/java/com/spotify/heroic/aggregation/AbstractBucket.java

@@ -22,11 +22,10 @@
 package com.spotify.heroic.aggregation;
 
 import com.spotify.heroic.metric.DistributionPoint;
-import com.spotify.heroic.metric.Payload;
 import com.spotify.heroic.metric.MetricGroup;
+import com.spotify.heroic.metric.Payload;
 import com.spotify.heroic.metric.Point;
 import com.spotify.heroic.metric.Spread;
-
 import com.spotify.heroic.metric.TdigestPoint;
 import java.util.Map;
 

heroic-component/src/main/java/com/spotify/heroic/aggregation/AnyBucket.java

@@ -27,7 +27,6 @@
 import com.spotify.heroic.metric.Payload;
 import com.spotify.heroic.metric.Point;
 import com.spotify.heroic.metric.Spread;
-
 import com.spotify.heroic.metric.TdigestPoint;
 import java.util.Map;
 

heroic-component/src/main/java/com/spotify/heroic/metric/DistributionPointDeserialize.java

@@ -25,9 +25,7 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.DeserializationContext;
 import com.fasterxml.jackson.databind.JsonNode;
-
 import com.fasterxml.jackson.databind.deser.std.StdDeserializer;
-
 import com.google.protobuf.ByteString;
 import java.io.IOException;
 

heroic-component/src/main/java/com/spotify/heroic/metric/HeroicDistribution.java

@@ -21,9 +21,9 @@
 
 package com.spotify.heroic.metric;
 
-    import com.google.auto.value.AutoValue;
-    import com.google.protobuf.ByteString;
-    import java.nio.ByteBuffer;
+import com.google.auto.value.AutoValue;
+import com.google.protobuf.ByteString;
+import java.nio.ByteBuffer;
 
 @AutoValue
 public abstract class HeroicDistribution implements Distribution {

heroic-component/src/main/java/com/spotify/heroic/metric/MetricType.java

@@ -22,7 +22,6 @@
 package com.spotify.heroic.metric;
 
 import com.google.common.collect.ImmutableMap;
-
 import java.util.Arrays;
 import java.util.Map;
 import java.util.Optional;

heroic-core/src/main/java/com/spotify/heroic/CoreQueryManager.java

@@ -25,7 +25,6 @@
 import static io.opencensus.trace.AttributeValue.longAttributeValue;
 import static io.opencensus.trace.AttributeValue.stringAttributeValue;
 
-
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.ImmutableSortedSet;
 import com.spotify.heroic.aggregation.Aggregation;

heroic-core/src/main/java/com/spotify/heroic/http/HttpServer.java

@@ -32,6 +32,7 @@
 import com.spotify.heroic.jetty.JettyServerConnector;
 import com.spotify.heroic.lifecycle.LifeCycleRegistry;
 import com.spotify.heroic.lifecycle.LifeCycles;
+import com.spotify.heroic.servlet.MandatoryClientIdFilter;
 import com.spotify.heroic.servlet.ShutdownFilter;
 import com.spotify.heroic.tracing.TracingConfig;
 import eu.toolchain.async.AsyncFramework;
@@ -212,6 +213,7 @@ private HandlerCollection setupHandler() throws Exception {
 
         context.addServlet(jerseyServlet, "/*");
         context.addFilter(new FilterHolder(new ShutdownFilter(stopping, mapper)), "/*", null);
+        context.addFilter(new FilterHolder(new MandatoryClientIdFilter()), "/*", null);
         context.setErrorHandler(new JettyJSONErrorHandler(mapper));
 
         final RequestLogHandler requestLogHandler = new RequestLogHandler();
@@ -221,6 +223,7 @@ private HandlerCollection setupHandler() throws Exception {
         final RewriteHandler rewrite = new RewriteHandler();
         makeRewriteRules(rewrite);
 
+
         final HandlerCollection handlers = new HandlerCollection();
         handlers.setHandlers(new Handler[]{rewrite, context, requestLogHandler});
 

heroic-core/src/main/java/com/spotify/heroic/servlet/MandatoryClientIdFilter.java

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2015 Spotify AB.
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.spotify.heroic.servlet;
+
+import com.google.common.base.Strings;
+import java.io.IOException;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.core.Response.Status;
+
+/**
+ * Rejects anonymous requests. That is, requests to any API endpoint that are
+ * missing a non-null X-Client-Id HTTP header.
+ */
+// Note that this @Suppress has to go here even though it's just for the doFilter
+// method, else the JavaDoc for it doesn't render. Weird.
+@SuppressWarnings("checkstyle:LineLength")
+public class MandatoryClientIdFilter implements Filter {
+
+    public static final String X_CLIENT_ID_HEADER_NAME = "X-Client-Id";
+
+    @Override
+    public void init(FilterConfig filterConfig) throws ServletException { /* intentionally empty
+    */ }
+
+    /**
+     * Reject (with a 400) the request, if the X-Client-Id HTTP header is not present
+     * or is non-null/empty.<p>
+     * Calling {@link javax.servlet.FilterChain#doFilter}
+     * effectively "passes" this filter and the next
+     * filter gets a stab at it. <p>
+     * Conversely, not calling doFilter halts "happy path" processing altogether
+     * and that's the mechanism with which we stop anonymous requests.<p>
+     * Finally, using
+     * {@link javax.servlet.http.HttpServletResponse#sendError(int, java.lang.String)} to
+     * return a status message didn't work and instead sent text of "internal error" back
+     * to the client.
+     */
+    @Override
+    public void doFilter(
+        ServletRequest request, ServletResponse response, FilterChain chain
+    ) throws IOException, ServletException {
+        if (passesFilter(request)) {
+            chain.doFilter(request, response);
+        } else {
+            var httpResponse = HttpServletResponse.class.cast(response);
+            httpResponse.setStatus(Status.BAD_REQUEST.getStatusCode());
+        }
+    }
+
+    /**
+     * Returns true if the HTTP header X-Client-Id is present and non-null and not empty.
+     * @param request request to pluck X-Client-Id's value from
+     * @return see above
+     */
+    public static boolean passesFilter(ServletRequest request) {
+        var req = HttpServletRequest.class.cast(request);
+        return !Strings.isNullOrEmpty(req.getHeader(X_CLIENT_ID_HEADER_NAME));
+    }
+
+    @Override
+    public void destroy() { /* intentionally empty */ }
+};

heroic-core/src/test/java/com/spotify/heroic/metric/LocalMetricManagerTest.java

@@ -66,9 +66,19 @@ public void setup() {
         final QueryLoggerFactory queryLoggerFactory = mock(QueryLoggerFactory.class);
         when(queryLoggerFactory.create(any())).thenReturn(queryLogger);
 
-        manager = new LocalMetricManager(groupLimit, seriesLimit, aggregationLimit, dataLimit,
-            concurrentQueriesBackoff, fetchParallelism, failOnLimits, async, groupSet, metadata,
-            reporter, queryLoggerFactory);
+        manager = new LocalMetricManager(
+            groupLimit,
+            seriesLimit,
+            aggregationLimit,
+            dataLimit,
+            concurrentQueriesBackoff,
+            fetchParallelism,
+            failOnLimits,
+            async,
+            groupSet,
+            metadata,
+            reporter,
+            queryLoggerFactory);
     }
 
     @Test