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 · 3acafb8 · 3acafb8
1 parent f9bc103
commit 3acafb8
Show file tree

Hide file tree

Showing 14 changed files with 368 additions and 61 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/...mmon-impl/src/main/java/net/trajano/ms/vertx/beans/DefaultAssertionRequiredPredicate.java b/...mmon-impl/src/main/java/net/trajano/ms/vertx/beans/DefaultAssertionRequiredPredicate.java
@@ -27,6 +27,44 @@ public class DefaultAssertionRequiredPredicate implements
 
     private static final Logger LOG = LoggerFactory.getLogger(DefaultAssertionRequiredPredicate.class);
 
+    /**
+     * <p>
+     * The key logic for the test is as follows:
+     * </p>
+     * <p>
+     * Let:
+     * </p>
+     * <table>
+     * <tr>
+     * <td>a =</td>
+     * <td>resourceMethodHasRolesAllowed</td>
+     * </tr>
+     * <tr>
+     * <td>b =</td>
+     * <td>resourceClassHasRolesAllowed</td>
+     * </tr>
+     * <tr>
+     * <td>c =</td>
+     * <td>resourceMethodHasPermitAll</td>
+     * </tr>
+     * <tr>
+     * <td>d =</td>
+     * <td>resourceClassHasPermitAll</td>
+     * </tr>
+     * </table>
+     * <p>
+     * The rules that need to be applied translate to:
+     * </p>
+     *
+     * <pre>
+     * = a || ( b && !c ) || ( !a && !b && !c && !d )
+     * = (a || b || !d) && ( a || !c )
+     * </pre>
+     *
+     * @param resourceInfo
+     *            resource info
+     * @return <code>true</code> if the resource is protected
+     */
     @Override
     public boolean test(final ResourceInfo resourceInfo) {
 
@@ -49,13 +87,6 @@ public boolean test(final ResourceInfo resourceInfo) {
         } else if (resourceClassHasRolesAllowed && resourceClassHasPermitAll) {
             throw new IllegalArgumentException("The resource class " + resourceClass + " may not have both @RolesAllowed and @PermitAll annotations.");
         } else {
-            // resourceMethodHasRolesAllowed OR
-            // resourceClassHasRolesAllowed && !resourceMethodHasPermitAll OR
-            // !resourceMethodHasRolesAllowed && !resourceClassHasRolesAllowed && !resourceMethodHasPermitAll && !resourceClassHasPermitAll
-
-            // a || ( b && !c ) || ( !a && !b && !c && !d)
-
-            // (a || b || !d) && ( a || !c)
             return (resourceMethodHasRolesAllowed || resourceClassHasRolesAllowed || !resourceClassHasPermitAll) && (resourceMethodHasRolesAllowed || !resourceMethodHasPermitAll);
         }
 

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());