Merge branch 'master' into jvenstad/zone-cleanup-4

7 files changed, 195 insertions, 68 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ClusterCost.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ClusterCost.java
index 1b2ad9f938a..fb675862320 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ClusterCost.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ClusterCost.java
@@ -24,6 +24,7 @@ import java.util.Objects;
  * @author smorgrav
  */
 public class ClusterCost {
+
     private final double tco;
     private final double waste;
     private final ClusterInfo clusterInfo;
@@ -32,8 +33,8 @@ public class ClusterCost {
     private final ClusterUtilization resultUtilization;
 
     /**
-     * @param clusterInfo       Value object with cluster info e.g. the TCO for the hardware used
-     * @param systemUtilization Utilization of system resources (as ratios)
+     * @param clusterInfo       value object with cluster info e.g. the TCO for the hardware used
+     * @param systemUtilization utilization of system resources (as ratios)
      */
     public ClusterCost(ClusterInfo clusterInfo,
                        ClusterUtilization systemUtilization) {
@@ -79,10 +80,10 @@ public class ClusterCost {
     }
 
     static ClusterUtilization calculateResultUtilization(ClusterUtilization system, ClusterUtilization target) {
-        double cpu = ratio(system.getCpu(),target.getCpu());
-        double mem = ratio(system.getMemory(),target.getMemory());
-        double disk = ratio(system.getDisk(),target.getDisk());
-        double diskbusy = ratio(system.getDiskBusy(),target.getDiskBusy());
+        double cpu = ratio(system.getCpu(), target.getCpu());
+        double mem = ratio(system.getMemory(), target.getMemory());
+        double disk = ratio(system.getDisk(), target.getDisk());
+        double diskbusy = ratio(system.getDiskBusy(), target.getDiskBusy());
 
         return new ClusterUtilization(mem, cpu, disk, diskbusy);
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentCost.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentCost.java
index 585690793bb..371e1c41e32 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentCost.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentCost.java
@@ -44,17 +44,17 @@ public class DeploymentCost {
         return clusters;
     }
 
-    /** @return Total cost of ownership for the deployment (sum of all clusters) */
+    /** Returns the total monthly cost of ownership for the deployment (sum of all clusters) */
     public double getTco() {
         return tco;
     }
 
-    /** @return The utilization of clusters that wastes most money in this deployment */
+    /** Returns the utilization of clusters that wastes most money in this deployment */
     public double getUtilization() {
         return utilization;
     }
 
-    /** @return The amount of dollars spent and not utilized */
+    /** Returns the amount of dollars spent and not utilized */
     public double getWaste() {
         return waste;
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
index ceb04d88026..a7940076277 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
@@ -11,14 +11,14 @@ import java.util.Optional;
 /**
  * The last known build status of a particular deployment job for a particular application.
  * This is immutable.
- * 
+ *
  * @author bratseth
  * @author mpolden
  */
 public class JobStatus {
-    
+
     private final DeploymentJobs.JobType type;
-    
+
     private final Optional<JobRun> lastTriggered;
     private final Optional<JobRun> lastCompleted;
     private final Optional<JobRun> firstFailing;
@@ -42,7 +42,7 @@ public class JobStatus {
 
         this.type = type;
         this.jobError = jobError;
-        
+
         // Never say we triggered component because we don't:
         this.lastTriggered = type == DeploymentJobs.JobType.component ? Optional.empty() : lastTriggered;
         this.lastCompleted = lastCompleted;
@@ -52,7 +52,7 @@ public class JobStatus {
 
     /** Returns an empty job status */
     public static JobStatus initial(DeploymentJobs.JobType type) {
-        return new JobStatus(type, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); 
+        return new JobStatus(type, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty());
     }
 
     public JobStatus withTriggering(Version version, Optional<ApplicationRevision> revision,
@@ -89,13 +89,13 @@ public class JobStatus {
         Optional<JobRun> firstFailing = this.firstFailing;
         if (jobError.isPresent() &&  ! this.firstFailing.isPresent())
             firstFailing = Optional.of(thisCompletion);
-            
+
         Optional<JobRun> lastSuccess = this.lastSuccess;
         if ( ! jobError.isPresent()) {
             lastSuccess = Optional.of(thisCompletion);
             firstFailing = Optional.empty();
         }
-        
+
         return new JobStatus(type, jobError, lastTriggered, Optional.of(thisCompletion), firstFailing, lastSuccess);
     }
 
@@ -105,7 +105,7 @@ public class JobStatus {
     public boolean isSuccess() {
         return lastCompleted().isPresent() && ! jobError.isPresent();
     }
-    
+
     /** Returns true if last triggered is newer than last completed and was started after timeoutLimit */
     public boolean isRunning(Instant timeoutLimit) {
         if ( ! lastTriggered.isPresent()) return false;
@@ -114,6 +114,11 @@ public class JobStatus {
         return ! lastTriggered.get().at().isBefore(lastCompleted.get().at());
     }
 
+    /** Returns true if this is running and has been so since before the given limit */
+    public boolean isHanging(Instant timeoutLimit) {
+        return isRunning(Instant.MIN) && lastTriggered.get().at().isBefore(timeoutLimit.plusMillis(1));
+    }
+
     /** The error of the last completion, or empty if the last run succeeded */
     public Optional<DeploymentJobs.JobError> jobError() { return jobError; }
 
@@ -140,10 +145,10 @@ public class JobStatus {
                ", first failing: " + firstFailing.map(JobRun::toString).orElse("(not failing)") +
                ", lastSuccess: " + lastSuccess.map(JobRun::toString).orElse("(never)") + "]";
     }
-    
+
     @Override
     public int hashCode() { return Objects.hash(type, jobError, lastTriggered, lastCompleted, firstFailing, lastSuccess); }
-    
+
     @Override
     public boolean equals(Object o) {
         if (o == this) return true;
@@ -159,15 +164,15 @@ public class JobStatus {
 
     /** Information about a particular triggering or completion of a run of a job. This is immutable. */
     public static class JobRun {
-        
+
         private final long id;
         private final Version version;
         private final Optional<ApplicationRevision> revision;
         private final boolean upgrade;
         private final String reason;
         private final Instant at;
-        
-        public JobRun(long id, Version version, Optional<ApplicationRevision> revision, 
+
+        public JobRun(long id, Version version, Optional<ApplicationRevision> revision,
                       boolean upgrade, String reason, Instant at) {
             Objects.requireNonNull(version, "version cannot be null");
             Objects.requireNonNull(revision, "revision cannot be null");
@@ -188,16 +193,16 @@ public class JobStatus {
         // TODO: Fix how this is set, and add an applicationChange() method as well, in the same vein.
         /** Returns whether this job run was a Vespa upgrade */
         public boolean upgrade() { return upgrade; }
-        
+
         /** Returns the Vespa version used on this run */
         public Version version() { return version; }
-        
+
         /** Returns the application revision used for this run, or empty when not known */
         public Optional<ApplicationRevision> revision() { return revision; }
-        
+
         /** Returns a human-readable reason for this particular job run */
         public String reason() { return reason; }
-        
+
         /** Returns the time if this triggering or completion */
         public Instant at() { return at; }
 
@@ -218,7 +223,7 @@ public class JobStatus {
         public int hashCode() {
             return Objects.hash(version, revision, upgrade, at);
         }
-        
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
@@ -234,7 +239,7 @@ public class JobStatus {
         @Override
         public String toString() { return "job run " + id + " of version " + (upgrade() ? "upgrade " : "") + version + " "
                                           + revision + " at " + at; }
-        
+
     }
 
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
index 328461355db..7aaaad534db 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
@@ -7,17 +7,24 @@ import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.InvalidTokenException;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUtils;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsKeystore;
 import com.yahoo.vespa.hosted.controller.athenz.config.AthenzConfig;
 
+import java.security.cert.X509Certificate;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
 import static com.yahoo.vespa.hosted.controller.athenz.filter.SecurityFilterUtils.sendErrorResponse;
 
 /**
- * Performs authentication by validating the principal token (NToken) header.
+ * Authenticates Athenz principal, either through:
+ *  1. TLS client authentication (based on Athenz x509 identity certficiate).
+ *  2. The principal token (NToken) header.
+ * The TLS authentication is based on the following assumptions:
+ *  - The underlying connector is configured with 'clientAuth' set to either WANT_AUTH or NEED_AUTH.
+ *  - The trust store is configured with the Athenz CA certificates only.
  *
  * @author bjorncs
  */
@@ -43,18 +50,45 @@ public class AthenzPrincipalFilter implements SecurityRequestFilter {
 
     @Override
     public void filter(DiscFilterRequest request, ResponseHandler responseHandler) {
-        String rawToken = request.getHeader(principalTokenHeader);
-        if (rawToken == null || rawToken.isEmpty()) {
-            sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, "NToken is missing");
-            return;
-        }
         try {
-            AthenzPrincipal principal = validator.validate(new NToken(rawToken));
+            Optional<AthenzPrincipal> certificatePrincipal = getClientCertificate(request)
+                    .map(AthenzUtils::createAthenzIdentity)
+                    .map(AthenzPrincipal::new);
+            Optional<AthenzPrincipal> nTokenPrincipal = getPrincipalToken(request, principalTokenHeader)
+                    .map(validator::validate);
+
+            if (!certificatePrincipal.isPresent() && !nTokenPrincipal.isPresent()) {
+                String errorMessage = "Unable to authenticate Athenz identity. " +
+                        "Both client certificate missing and principal token header are missing.";
+                sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, errorMessage);
+                return;
+            }
+            if (certificatePrincipal.isPresent() && nTokenPrincipal.isPresent()
+                    && !certificatePrincipal.get().getIdentity().equals(nTokenPrincipal.get().getIdentity())) {
+                String errorMessage = String.format(
+                        "Identity in principal token does not match x509 CN: token-identity=%s, cert-identity=%s",
+                        nTokenPrincipal.get().getIdentity().getFullName(),
+                        certificatePrincipal.get().getIdentity().getFullName());
+                sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, errorMessage);
+                return;
+            }
+            AthenzPrincipal principal = nTokenPrincipal.orElseGet(certificatePrincipal::get);
             request.setUserPrincipal(principal);
             request.setRemoteUser(principal.getName());
-        } catch (InvalidTokenException e) {
+        } catch (Exception e) {
             sendErrorResponse(responseHandler,Response.Status.UNAUTHORIZED, e.getMessage());
         }
     }
 
+    private static Optional<X509Certificate> getClientCertificate(DiscFilterRequest request) {
+        return Optional.ofNullable((X509Certificate[]) request.getAttribute("jdisc.request.X509Certificate"))
+                .map(chain -> chain[0]);
+    }
+
+    private static Optional<NToken> getPrincipalToken(DiscFilterRequest request, String principalTokenHeaderName) {
+        return Optional.ofNullable(request.getHeader(principalTokenHeaderName))
+                .filter(token -> !token.isEmpty())
+                .map(NToken::new);
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index 487cbc02acc..6a9db3ae917 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -32,9 +32,9 @@ import java.util.logging.Logger;
 /**
  * Responsible for scheduling deployment jobs in a build system and keeping
  * Application.deploying() in sync with what is scheduled.
- * 
+ *
  * This class is multithread safe.
- * 
+ *
  * @author bratseth
  * @author mpolden
  */
@@ -60,7 +60,7 @@ public class DeploymentTrigger {
         this.order = new DeploymentOrder(controller);
         this.jobTimeout = controller.system().equals(SystemName.main) ? Duration.ofHours(12) : Duration.ofHours(1);
     }
-    
+
     /** Returns the time in the past before which jobs are at this moment considered unresponsive */
     public Instant jobTimeoutLimit() { return clock.instant().minus(jobTimeout); }
 
@@ -70,10 +70,10 @@ public class DeploymentTrigger {
 
     //--- Start of methods which triggers deployment jobs -------------------------
 
-    /** 
+    /**
      * Called each time a job completes (successfully or not) to cause triggering of one or more follow-up jobs
      * (which may possibly the same job once over).
-     * 
+     *
      * @param report information about the job that just completed
      */
     public void triggerFromCompletion(JobReport report) {
@@ -143,10 +143,11 @@ public class DeploymentTrigger {
             JobStatus systemTestStatus = application.deploymentJobs().jobStatus().get(JobType.systemTest);
             if (application.deploying().get() instanceof Change.VersionChange) {
                 Version target = ((Change.VersionChange) application.deploying().get()).version();
-                if (systemTestStatus == null 
+                if (systemTestStatus == null
                     || ! systemTestStatus.lastTriggered().isPresent()
                     || ! systemTestStatus.isSuccess()
-                    || ! systemTestStatus.lastTriggered().get().version().equals(target)) {
+                    || ! systemTestStatus.lastTriggered().get().version().equals(target)
+                    || systemTestStatus.isHanging(jobTimeoutLimit())) {
                     application = trigger(JobType.systemTest, application, false, "Upgrade to " + target);
                     controller.applications().store(application);
                 }
@@ -170,7 +171,7 @@ public class DeploymentTrigger {
             List<JobType> nextToTrigger = new ArrayList<>();
             for (JobType nextJobType : order.nextAfter(jobType, application)) {
                 JobStatus nextStatus = application.deploymentJobs().jobStatus().get(nextJobType);
-                if (changesAvailable(application, jobStatus, nextStatus))
+                if (changesAvailable(application, jobStatus, nextStatus) || nextStatus.isHanging(jobTimeoutLimit()))
                     nextToTrigger.add(nextJobType);
             }
             // Trigger them in parallel
@@ -209,10 +210,10 @@ public class DeploymentTrigger {
             return true;
         return false;
     }
-    
+
     /**
      * Triggers a change of this application
-     * 
+     *
      * @param applicationId the application to trigger
      * @throws IllegalArgumentException if this application already have an ongoing change
      */
@@ -267,7 +268,7 @@ public class DeploymentTrigger {
     }
 
     /**
-     * Trigger a job for an application 
+     * Trigger a job for an application
      *
      * @param jobType the type of the job to trigger, or null to trigger nothing
      * @param application the application to trigger the job for
@@ -289,7 +290,7 @@ public class DeploymentTrigger {
 
     /**
      * Trigger a job for an application, if allowed
-     * 
+     *
      * @param jobType the type of the job to trigger, or null to trigger nothing
      * @param application the application to trigger the job for
      * @param first whether to trigger the job before other jobs
@@ -323,7 +324,7 @@ public class DeploymentTrigger {
 
     /** Returns true if the given proposed job triggering should be effected */
     private boolean allowedTriggering(JobType jobType, LockedApplication application) {
-        // Note: We could make a more fine-grained and more correct determination about whether to block 
+        // Note: We could make a more fine-grained and more correct determination about whether to block
         //       by instead basing the decision on what is currently deployed in the zone. However,
         //       this leads to some additional corner cases, and the possibility of blocking an application
         //       fix to a version upgrade, so not doing it now
@@ -341,7 +342,7 @@ public class DeploymentTrigger {
 
         return true;
     }
-    
+
     private boolean isRunningProductionJob(Application application) {
         return JobList.from(application)
                 .production()
@@ -364,7 +365,7 @@ public class DeploymentTrigger {
         if (existingDeployment == null) return false;
         return existingDeployment.version().isAfter(version);
     }
-    
+
     private boolean acceptNewRevisionNow(LockedApplication application) {
         if ( ! application.deploying().isPresent()) return true;
 
@@ -377,5 +378,5 @@ public class DeploymentTrigger {
         // Otherwise, the application is currently upgrading, without failures, and we should wait with the revision.
         return false;
     }
-    
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
index b7080a763f0..77ce49eaf47 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
@@ -9,14 +9,13 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
-import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzIdentity;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUser;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
+import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.common.ContextAttributes;
-import com.yahoo.vespa.hosted.controller.restapi.filter.NTokenRequestFilter;
 
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.HttpMethod;
@@ -78,8 +77,7 @@ public class Authorizer {
     }
 
     public Optional<NToken> getNToken(HttpRequest request) {
-        String nTokenHeader = (String)request.getJDiscRequest().context().get(NTokenRequestFilter.NTOKEN_HEADER);
-        return Optional.ofNullable(nTokenHeader).map(NToken::new);
+        return getPrincipalIfAny(request).flatMap(AthenzPrincipal::getNToken);
     }
 
     public boolean isSuperUser(HttpRequest request) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
index ffb78b7342a..c887fbfc1a8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
@@ -7,10 +7,19 @@ import com.yahoo.jdisc.handler.ReadableContentChannel;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzIdentity;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUser;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.InvalidTokenException;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
+import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.cert.X509v3CertificateBuilder;
+import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
+import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.ContentSigner;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -18,6 +27,15 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
+import java.math.BigInteger;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Date;
 import java.util.Objects;
 
 import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED;
@@ -37,21 +55,21 @@ public class AthenzPrincipalFilterTest {
 
     private static final NToken NTOKEN = new NToken("dummy");
     private static final String ATHENZ_PRINCIPAL_HEADER = "Athenz-Principal-Auth";
+    private static final AthenzIdentity IDENTITY = AthenzUser.fromUserId(new UserId("bob"));
+    private static final X509Certificate CERTIFICATE = createSelfSignedCertificate(IDENTITY);
 
     private NTokenValidator validator;
-    private AthenzPrincipal principal;
 
     @Before
     public void before() {
         validator = mock(NTokenValidator.class);
-        principal = new AthenzPrincipal(AthenzUser.fromUserId(new UserId("bob")), NTOKEN);
     }
 
     @Test
-    public void valid_ntoken_is_accepted() throws Exception {
+    public void valid_ntoken_is_accepted() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzPrincipal principal = new AthenzPrincipal(IDENTITY, NTOKEN);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
-
         when(validator.validate(NTOKEN)).thenReturn(principal);
 
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
@@ -61,7 +79,7 @@ public class AthenzPrincipalFilterTest {
     }
 
     @Test
-    public void missing_token_is_unauthorized() throws Exception {
+    public void missing_token_and_certificate_is_unauthorized() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(null);
 
@@ -70,26 +88,76 @@ public class AthenzPrincipalFilterTest {
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
         filter.filter(request, responseHandler);
 
-        assertThat(responseHandler.response, notNullValue());
-        assertThat(responseHandler.response.getStatus(), equalTo(UNAUTHORIZED));
-        assertThat(responseHandler.getResponseContent(), containsString("NToken is missing"));
+        assertUnauthorized(responseHandler, "Unable to authenticate Athenz identity");
+    }
+
+    @Test
+    public void invalid_token_is_unauthorized() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        String errorMessage = "Invalid token";
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(validator.validate(NTOKEN)).thenThrow(new InvalidTokenException(errorMessage));
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
+
+        assertUnauthorized(responseHandler, errorMessage);
+    }
+
+    @Test
+    public void certificate_is_accepted() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(null);
+        when(request.getAttribute("jdisc.request.X509Certificate")).thenReturn(new X509Certificate[]{CERTIFICATE});
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
+
+        AthenzPrincipal expectedPrincipal = new AthenzPrincipal(IDENTITY);
+        verify(request).setUserPrincipal(expectedPrincipal);
     }
 
     @Test
-    public void invalid_token_is_unauthorized() throws Exception {
+    public void both_ntoken_and_certificate_is_accepted() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzPrincipal principalWithToken = new AthenzPrincipal(IDENTITY, NTOKEN);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(request.getAttribute("jdisc.request.X509Certificate")).thenReturn(new X509Certificate[]{CERTIFICATE});
+        when(validator.validate(NTOKEN)).thenReturn(principalWithToken);
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
 
-        when(validator.validate(NTOKEN)).thenThrow(new InvalidTokenException("Invalid token"));
+        verify(request).setUserPrincipal(principalWithToken);
+    }
+
+    @Test
+    public void conflicting_ntoken_and_certificate_is_unauthorized() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzUser conflictingIdentity = AthenzUser.fromUserId(new UserId("mallory"));
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(request.getAttribute("jdisc.request.X509Certificate"))
+                .thenReturn(new X509Certificate[]{createSelfSignedCertificate(conflictingIdentity)});
+        when(validator.validate(NTOKEN)).thenReturn(new AthenzPrincipal(IDENTITY));
 
         ResponseHandlerMock responseHandler = new ResponseHandlerMock();
 
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
         filter.filter(request, responseHandler);
 
+        assertUnauthorized(responseHandler, "Identity in principal token does not match x509 CN");
+    }
+
+    private static void assertUnauthorized(ResponseHandlerMock responseHandler, String expectedMessageSubstring) {
         assertThat(responseHandler.response, notNullValue());
         assertThat(responseHandler.response.getStatus(), equalTo(UNAUTHORIZED));
-        assertThat(responseHandler.getResponseContent(), containsString("Invalid token"));
+        assertThat(responseHandler.getResponseContent(), containsString(expectedMessageSubstring));
     }
 
     private static class ResponseHandlerMock implements ResponseHandler {
@@ -114,4 +182,24 @@ public class AthenzPrincipalFilterTest {
 
     }
 
+    // TODO Move this to separate athenz module/bundle
+    private static X509Certificate createSelfSignedCertificate(AthenzIdentity identity) {
+        try {
+            KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+            keyGen.initialize(512);
+            KeyPair keyPair = keyGen.genKeyPair();
+            ContentSigner contentSigner = new JcaContentSignerBuilder("SHA256WithRSA").build(keyPair.getPrivate());
+            X500Name x500Name = new X500Name("CN="+ identity.getFullName());
+            X509v3CertificateBuilder certificateBuilder =
+                    new JcaX509v3CertificateBuilder(
+                            x500Name, BigInteger.ONE, new Date(), Date.from(Instant.now().plus(Duration.ofDays(30))),
+                            x500Name, keyPair.getPublic());
+            return new JcaX509CertificateConverter()
+                    .setProvider(new BouncyCastleProvider())
+                    .getCertificate(certificateBuilder.build(contentSigner));
+        } catch (CertificateException | NoSuchAlgorithmException | OperatorCreationException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
 }