Used MDC

Instead of calculating values from the header and having longer parameter lists, MDC is used to store more context information for the error responses. In addition the host name and port are loggable and exposed to the MicroserviceEngine API
trajano · Dec 12, 2017 · 4973865 · 4973865
1 parent f9bc103
commit 4973865
Show file tree

Hide file tree

Showing 13 changed files with 330 additions and 54 deletions.
diff --git a/ms-common-impl/src/main/java/net/trajano/ms/vertx/VertxMicroserviceEngine.java b/ms-common-impl/src/main/java/net/trajano/ms/vertx/VertxMicroserviceEngine.java
@@ -1,6 +1,8 @@
 package net.trajano.ms.vertx;
 
 import java.io.File;
+import java.net.InetAddress;
+import java.net.UnknownHostException;
 import java.util.Deque;
 import java.util.LinkedList;
 
@@ -48,6 +50,10 @@ public class VertxMicroserviceEngine implements
     @Autowired
     private HttpServerOptions httpServerOptions;
 
+    private String theHostname;
+
+    private int thePort = -1;
+
     private Vertx vertx;
 
     @Autowired
@@ -73,6 +79,18 @@ public Object[] bootstrap() {
         };
     }
 
+    @Override
+    public String hostname() {
+
+        return theHostname;
+    }
+
+    @Override
+    public int port() {
+
+        return thePort;
+    }
+
     @PostConstruct
     public void start() {
 
@@ -113,8 +131,14 @@ public void start() {
                 SpringApplication.exit(applicationContext, () -> -1);
             } else {
                 LOG.info("Listening on port {}", http.actualPort());
+                thePort = http.actualPort();
             }
         });
+        try {
+            theHostname = InetAddress.getLocalHost().getHostName();
+        } catch (final UnknownHostException e) {
+            throw new ExceptionInInitializerError(e);
+        }
     }
 
     @PreDestroy

diff --git a/ms-common-impl/src/main/java/net/trajano/ms/vertx/beans/CachedDataProvider.java b/ms-common-impl/src/main/java/net/trajano/ms/vertx/beans/CachedDataProvider.java
@@ -69,6 +69,8 @@ public JwtConsumer buildConsumer(final HttpsJwks jwks,
         if (jwks != null) {
             builder
                 .setVerificationKeyResolver(new HttpsJwksVerificationKeyResolver(jwks));
+        } else {
+            builder.setSkipSignatureVerification();
         }
         if (audience != null) {
             builder

diff --git a/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/JsonExceptionMapper.java b/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/JsonExceptionMapper.java
@@ -54,9 +54,6 @@ public class JsonExceptionMapper implements
     @Context
     private HttpHeaders headers;
 
-    @Value("${microservice.show_request_uri:#{null}}")
-    private Boolean showRequestUri;
-
     @Value("${microservice.show_stack_trace:#{null}}")
     private Boolean showStackTrace;
 
@@ -105,12 +102,10 @@ private void log(final Throwable exception) {
      */
     public void setContextData(final HttpHeaders headers,
         final UriInfo uriInfo,
-        final boolean showRequestUri,
         final boolean showStackTrace) {
 
         this.headers = headers;
         this.uriInfo = uriInfo;
-        this.showRequestUri = showRequestUri;
         this.showStackTrace = showStackTrace;
 
     }
@@ -122,11 +117,9 @@ public void setContextData(final HttpHeaders headers,
     @PostConstruct
     public void setDebugFlags() {
 
-        if (showRequestUri == null) {
-            showRequestUri = LOG.isDebugEnabled();
-        }
         if (showStackTrace == null) {
             showStackTrace = LOG.isDebugEnabled();
+            LOG.debug("stack trace enabled if this is shown");
         }
     }
 
@@ -152,7 +145,7 @@ public Response toResponse(final Throwable exception) {
                 .build();
         } else {
             return Response.status(status)
-                .entity(new ErrorResponse(exception, headers, uriInfo, showStackTrace, showRequestUri))
+                .entity(new ErrorResponse(exception, uriInfo, showStackTrace))
                 .type(mediaType)
                 .build();
         }

diff --git a/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/JwtAssertionInterceptor.java b/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/JwtAssertionInterceptor.java
@@ -2,7 +2,6 @@
 
 import static net.trajano.ms.core.ErrorCodes.FORBIDDEN;
 import static net.trajano.ms.core.ErrorCodes.UNAUTHORIZED_CLIENT;
-import static net.trajano.ms.core.Qualifiers.REQUEST_ID;
 
 import java.net.URI;
 import java.util.Arrays;
@@ -22,17 +21,20 @@
 import javax.ws.rs.core.Response.Status;
 import javax.ws.rs.ext.Provider;
 
-import net.trajano.ms.vertx.beans.CachedDataProvider;
 import org.jose4j.jwk.HttpsJwks;
 import org.jose4j.jwt.JwtClaims;
+import org.jose4j.jwt.MalformedClaimException;
 import org.jose4j.jwt.consumer.InvalidJwtException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.stereotype.Component;
 
 import net.trajano.ms.core.ErrorResponse;
+import net.trajano.ms.spi.MDCKeys;
+import net.trajano.ms.vertx.beans.CachedDataProvider;
 import net.trajano.ms.vertx.beans.DefaultAssertionRequiredPredicate;
 import net.trajano.ms.vertx.beans.JwtAssertionRequiredPredicate;
 import net.trajano.ms.vertx.beans.JwtClaimsProcessor;
@@ -45,7 +47,7 @@
  */
 @Component
 @Provider
-@Priority(Priorities.AUTHORIZATION)
+@Priority(Priorities.AUTHORIZATION + 1)
 public class JwtAssertionInterceptor implements
     ContainerRequestFilter {
 
@@ -59,6 +61,8 @@ public class JwtAssertionInterceptor implements
 
     private JwtAssertionRequiredPredicate assertionRequiredPredicate;
 
+    private CachedDataProvider cachedDataProvider;
+
     private JwtClaimsProcessor claimsProcessor;
 
     @Autowired(required = false)
@@ -70,8 +74,6 @@ public class JwtAssertionInterceptor implements
      */
     private final ConcurrentMap<String, HttpsJwks> jwks = new ConcurrentHashMap<>();
 
-    private CachedDataProvider cachedDataProvider;
-
     @Context
     private ResourceInfo resourceInfo;
 
@@ -86,7 +88,7 @@ public void filter(final ContainerRequestContext requestContext) {
             LOG.warn("Missing assertion on request for {}", requestContext.getUriInfo());
             requestContext.abortWith(Response.status(Status.UNAUTHORIZED)
                 .header(HttpHeaders.WWW_AUTHENTICATE, X_JWT_ASSERTION)
-                .entity(new ErrorResponse(UNAUTHORIZED_CLIENT, "Missing assertion", requestContext.getHeaderString(REQUEST_ID)))
+                .entity(new ErrorResponse(UNAUTHORIZED_CLIENT, "Missing assertion"))
                 .build());
             return;
         }
@@ -106,15 +108,17 @@ public void filter(final ContainerRequestContext requestContext) {
             }
             final List<String> audience = Arrays.asList(requestContext.getHeaderString(X_JWT_AUDIENCE).split(", "));
             claims = cachedDataProvider.buildConsumer(httpsJwks, audience).processToClaims(assertion);
-        } catch (final InvalidJwtException e) {
+            MDC.put(MDCKeys.JWT_ID, claims.getJwtId());
+        } catch (final InvalidJwtException
+            | MalformedClaimException e) {
             if (LOG.isDebugEnabled()) {
                 LOG.debug("JWT invalid", e);
             } else {
                 LOG.error("JWT Invalid");
             }
             requestContext.abortWith(Response.status(Status.UNAUTHORIZED)
                 .header(HttpHeaders.WWW_AUTHENTICATE, X_JWT_ASSERTION)
-                .entity(new ErrorResponse(UNAUTHORIZED_CLIENT, "JWT was not valid", requestContext.getHeaderString(REQUEST_ID)))
+                .entity(new ErrorResponse(UNAUTHORIZED_CLIENT, "JWT was not valid"))
                 .build());
             return;
         }
@@ -126,7 +130,7 @@ public void filter(final ContainerRequestContext requestContext) {
             if (!validateClaims) {
                 LOG.warn("Validation of claims failed on request for {}", requestContext.getUriInfo());
                 requestContext.abortWith(Response.status(Status.FORBIDDEN)
-                    .entity(new ErrorResponse(FORBIDDEN, "Claims validation failed", requestContext.getHeaderString(REQUEST_ID)))
+                    .entity(new ErrorResponse(FORBIDDEN, "Claims validation failed"))
                     .build());
             }
         }
@@ -154,16 +158,21 @@ public void setAssertionRequiredFunction(final JwtAssertionRequiredPredicate pre
         assertionRequiredPredicate = predicate;
     }
 
+    @Autowired
+    public void setCachedDataProvider(final CachedDataProvider cachedDataProvider) {
+
+        this.cachedDataProvider = cachedDataProvider;
+    }
+
     @Autowired(required = false)
     public void setClaimsProcessor(final JwtClaimsProcessor claimsProcessor) {
 
         this.claimsProcessor = claimsProcessor;
     }
 
-    @Autowired
-    public void setCachedDataProvider(final CachedDataProvider cachedDataProvider) {
+    public void setResourceInfo(final ResourceInfo resourceInfo) {
 
-        this.cachedDataProvider = cachedDataProvider;
+        this.resourceInfo = resourceInfo;
     }
 
 }
diff --git a/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/MDCInterceptor.java b/ms-common-impl/src/main/java/net/trajano/ms/vertx/jaxrs/MDCInterceptor.java
@@ -0,0 +1,47 @@
+package net.trajano.ms.vertx.jaxrs;
+
+import javax.annotation.Priority;
+import javax.ws.rs.Priorities;
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerRequestFilter;
+import javax.ws.rs.ext.Provider;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+import net.trajano.ms.spi.MDCKeys;
+import net.trajano.ms.spi.MicroserviceEngine;
+
+/**
+ * This populates the MDC based on the data available on the request. This will
+ * skip the method and request URI if debug is not enabled.
+ *
+ * @author Archimedes Trajano
+ */
+@Component
+@Provider
+@Priority(Priorities.AUTHORIZATION)
+public class MDCInterceptor implements
+    ContainerRequestFilter {
+
+    private static final Logger LOG = LoggerFactory.getLogger(MDCInterceptor.class);
+
+    @Autowired
+    private MicroserviceEngine engine;
+
+    @Override
+    public void filter(final ContainerRequestContext requestContext) {
+
+        MDC.put(MDCKeys.REQUEST_ID, requestContext.getHeaderString(MDCKeys.REQUEST_ID));
+        if (LOG.isDebugEnabled()) {
+            MDC.put(MDCKeys.REQUEST_METHOD, requestContext.getMethod());
+            MDC.put(MDCKeys.REQUEST_URI, requestContext.getUriInfo().getRequestUri().toASCIIString());
+            MDC.put(MDCKeys.HOST, engine.hostname() + ":" + engine.port());
+        }
+
+    }
+
+}
diff --git a/ms-common-impl/src/test/java/net/trajano/ms/vertx/test/ExceptionMapperTest.java b/ms-common-impl/src/test/java/net/trajano/ms/vertx/test/ExceptionMapperTest.java
@@ -47,7 +47,7 @@ public void setupMapper() {
         mapper.setDebugFlags();
         final HttpHeaders headers = Mockito.mock(HttpHeaders.class);
         Mockito.when(headers.getAcceptableMediaTypes()).thenReturn(Arrays.asList(MediaType.WILDCARD_TYPE));
-        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true, true);
+        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true);
     }
 
     @Test
@@ -77,7 +77,7 @@ public void testCheckedHtml() {
 
         final HttpHeaders headers = Mockito.mock(HttpHeaders.class);
         Mockito.when(headers.getAcceptableMediaTypes()).thenReturn(Arrays.asList(MediaType.TEXT_HTML_TYPE));
-        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true, true);
+        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true);
 
         final Response response = mapper.toResponse(new IOException("ahem"));
         Assert.assertEquals(500, response.getStatus());
@@ -97,7 +97,7 @@ public void testCheckedPlainText() {
 
         final HttpHeaders headers = Mockito.mock(HttpHeaders.class);
         Mockito.when(headers.getAcceptableMediaTypes()).thenReturn(Arrays.asList(MediaType.TEXT_PLAIN_TYPE));
-        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true, true);
+        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true);
 
         final Response response = mapper.toResponse(new IOException("ahem"));
         Assert.assertEquals(500, response.getStatus());
@@ -110,7 +110,7 @@ public void testCheckedUnsupportedType() {
 
         final HttpHeaders headers = Mockito.mock(HttpHeaders.class);
         Mockito.when(headers.getAcceptableMediaTypes()).thenReturn(Arrays.asList(MediaType.APPLICATION_OCTET_STREAM_TYPE));
-        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true, true);
+        mapper.setContextData(headers, Mockito.mock(UriInfo.class), true);
 
         final Response response = mapper.toResponse(new IOException("ahem"));
         Assert.assertEquals(500, response.getStatus());

diff --git a/ms-common-impl/src/test/java/net/trajano/ms/vertx/test/InterceptorTest.java b/ms-common-impl/src/test/java/net/trajano/ms/vertx/test/InterceptorTest.java
@@ -1,17 +1,26 @@
 package net.trajano.ms.vertx.test;
 
+import java.net.URI;
+
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ResourceInfo;
+import javax.ws.rs.core.UriInfo;
+
 import org.jose4j.jwt.JwtClaims;
 import org.jose4j.jwt.MalformedClaimException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Mockito;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.junit4.SpringRunner;
 
 import net.trajano.ms.core.CryptoOps;
+import net.trajano.ms.sample.Hello;
 import net.trajano.ms.vertx.VertxConfig;
 import net.trajano.ms.vertx.beans.JwtClaimsProcessor;
 import net.trajano.ms.vertx.jaxrs.JwtAssertionInterceptor;
+import net.trajano.ms.vertx.jaxrs.MDCInterceptor;
 
 /**
  * Tests are hanging on Travis for some odd reason.
@@ -53,16 +62,37 @@ public Boolean apply(final JwtClaims claims) {
     private CryptoOps cryptoOps;
 
     @Autowired
-    private JwtAssertionInterceptor interceptor;
+    private JwtAssertionInterceptor jwtInterceptor;
+
+    @Autowired
+    private MDCInterceptor mdcInterceptor;
 
+    @SuppressWarnings({
+        "unchecked",
+        "rawtypes"
+    })
     @Test
     public void testInterceptor() throws Exception {
 
-        interceptor.setClaimsProcessor(new ValidatingProcessor("typ", "https://example.com/register"));
+        jwtInterceptor.setClaimsProcessor(new ValidatingProcessor("typ", "https://example.com/register"));
 
-        final JwtClaims jwtClaims = JwtClaims.parse("{\"typ\":\"https://example.com/register\"}");
+        final JwtClaims jwtClaims = JwtClaims.parse("{\"typ\":\"https://example.com/register\", \"aud\":\"sample\", \"jti\": \"abc\", \"iss\":\"http://accounts.trajano.net\"}");
         final String jwt = cryptoOps.sign(jwtClaims);
         System.out.println(jwt);
+
+        final ResourceInfo resourceInfo = Mockito.mock(ResourceInfo.class);
+        Mockito.when(resourceInfo.getResourceMethod()).thenReturn(Hello.class.getMethod("hello2B"));
+        Mockito.when(resourceInfo.getResourceClass()).thenReturn((Class) Hello.class);
+        jwtInterceptor.setResourceInfo(resourceInfo);
+
+        final ContainerRequestContext containerRequestContext = Mockito.mock(ContainerRequestContext.class);
+        final UriInfo uriInfo = Mockito.mock(UriInfo.class);
+        Mockito.when(uriInfo.getRequestUri()).thenReturn(URI.create("http://trajano.net/sample"));
+        Mockito.when(containerRequestContext.getUriInfo()).thenReturn(uriInfo);
+        Mockito.when(containerRequestContext.getHeaderString("X-JWT-Assertion")).thenReturn(jwt);
+        Mockito.when(containerRequestContext.getHeaderString("X-JWT-Audience")).thenReturn("sample");
+        mdcInterceptor.filter(containerRequestContext);
+        jwtInterceptor.filter(containerRequestContext);
         //        final Request request = mock(Request.class);
         //        when(request.getHeader("X-JWT-Assertion")).thenReturn(jwt);
         //        final Response responder = mock(Response.class);

diff --git a/ms-common/pom.xml b/ms-common/pom.xml
@@ -116,6 +116,11 @@
       <version>${resteasy.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-jdk14</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.springframework.boot</groupId>
       <artifactId>spring-boot-starter-test</artifactId>