diff options
author | Bjørn Christian Seime <bjorncs@oath.com> | 2017-10-27 16:04:03 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@oath.com> | 2017-10-27 16:04:03 +0200 |
commit | 0926ed6d8a73f5207f759b1b23c3d528a76cf886 (patch) | |
tree | 0911d87488d66275ac9fa36abfedbf4a88c74ac4 | |
parent | b75f4a64a832a405da5012de6233f43296393933 (diff) |
Cleanup deployment authorization
All Screwdriver deployments are now authenticated using Athenz.
The authorization rules can therefore be simplified.
4 files changed, 36 insertions, 87 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java index 6ae9761b305..fb3ebf18f01 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java @@ -67,7 +67,6 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics; import com.yahoo.vespa.hosted.controller.application.JobStatus; import com.yahoo.vespa.hosted.controller.application.SourceRevision; import com.yahoo.vespa.hosted.controller.athenz.AthenzClientFactory; -import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal; import com.yahoo.vespa.hosted.controller.athenz.NToken; import com.yahoo.vespa.hosted.controller.athenz.ZmsException; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; @@ -780,25 +779,8 @@ public class ApplicationApiHandler extends LoggingRequestHandler { DeployAuthorizer deployAuthorizer = new DeployAuthorizer(controller.zoneRegistry(), athenzClientFactory); Tenant tenant = controller.tenants().tenant(new TenantId(tenantName)).orElseThrow(() -> new NotExistsException(new TenantId(tenantName))); Principal principal = authorizer.getPrincipal(request); - if (principal instanceof AthenzPrincipal) { - deployAuthorizer.throwIfUnauthorizedForDeploy(principal, - Environment.from(environment), - tenant, - applicationId); - } else { // In case of host-based principal - // TODO What about other user type principals like Bouncer? - log.log(LogLevel.WARNING, - "Using deprecated DeployAuthorizer.throwIfUnauthorizedForDeploy. Principal=" + principal); - UserId userId = new UserId(principal.getName()); - deployAuthorizer.throwIfUnauthorizedForDeploy( - Environment.from(environment), - userId, - tenant, - applicationId, - optional("screwdriverBuildJob", deployOptions).map(ScrewdriverId::new)); - } + deployAuthorizer.throwIfUnauthorizedForDeploy(principal, Environment.from(environment), tenant, applicationId); - // TODO: get rid of the json object DeployOptions deployOptionsJsonClass = new DeployOptions(screwdriverBuildJobFromSlime(deployOptions.field("screwdriverBuildJob")), optional("vespaVersion", deployOptions).map(Version::new), 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 93dc2541385..0c808e30c2a 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 @@ -1,7 +1,6 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.restapi.application; -import com.google.common.collect.ImmutableSet; import com.yahoo.config.provision.Environment; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.vespa.hosted.controller.Controller; @@ -12,19 +11,16 @@ 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.athenz.AthenzClientFactory; -import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal; import com.yahoo.vespa.hosted.controller.athenz.AthenzUtils; import com.yahoo.vespa.hosted.controller.athenz.NToken; import com.yahoo.vespa.hosted.controller.common.ContextAttributes; import com.yahoo.vespa.hosted.controller.restapi.filter.NTokenRequestFilter; -import com.yahoo.vespa.hosted.controller.restapi.filter.UnauthenticatedUserPrincipal; import javax.ws.rs.ForbiddenException; import javax.ws.rs.HttpMethod; import javax.ws.rs.core.SecurityContext; import java.security.Principal; import java.util.Optional; -import java.util.Set; import java.util.logging.Logger; @@ -41,11 +37,6 @@ public class Authorizer { // Must be kept in sync with bouncer filter configuration. private static final String VESPA_HOSTED_ADMIN_ROLE = "10707.A"; - private static final Set<UserId> SCREWDRIVER_USERS = ImmutableSet.of(new UserId("screwdrv"), - new UserId("screwdriver"), - new UserId("sdrvtest"), - new UserId("screwdriver-test")); - private final Controller controller; private final AthenzClientFactory athenzClientFactory; private final EntityService entityService; @@ -92,17 +83,8 @@ public class Authorizer { } public boolean isSuperUser(HttpRequest request) { - // TODO Check membership of admin role in Vespa's Athenz domain - return isMemberOfVespaBouncerGroup(request) || isScrewdriverPrincipal(getPrincipal(request)); - } - - public static boolean isScrewdriverPrincipal(Principal principal) { - if (principal instanceof UnauthenticatedUserPrincipal) // Host-based authentication - return SCREWDRIVER_USERS.contains(new UserId(principal.getName())); - else if (principal instanceof AthenzPrincipal) - return ((AthenzPrincipal)principal).getDomain().equals(AthenzUtils.SCREWDRIVER_DOMAIN); - else - return false; + // TODO Replace check with membership of a dedicated 'hosted Vespa super-user' role in Vespa's Athenz domain + return isMemberOfVespaBouncerGroup(request); } private static ForbiddenException loggedForbiddenException(String message, Object... args) { @@ -149,6 +131,8 @@ public class Authorizer { return method.equals(HttpMethod.GET) || method.equals(HttpMethod.HEAD) || method.equals(HttpMethod.OPTIONS); } + @Deprecated + // TODO Remove this method. Stop using Bouncer for authorization and use Athenz instead private boolean isMemberOfVespaBouncerGroup(HttpRequest request) { Optional<SecurityContext> securityContext = securityContextOf(request); if ( ! securityContext.isPresent() ) throw Authorizer.loggedForbiddenException("User is not authenticated"); diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java index 7cf19629774..d6560af6ac3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/DeployAuthorizer.java @@ -5,23 +5,19 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.api.Tenant; import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain; -import com.yahoo.vespa.hosted.controller.api.identifiers.ScrewdriverId; -import com.yahoo.vespa.hosted.controller.api.identifiers.UserId; import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneRegistry; import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction; import com.yahoo.vespa.hosted.controller.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.athenz.AthenzPrincipal; import com.yahoo.vespa.hosted.controller.athenz.AthenzUtils; import com.yahoo.vespa.hosted.controller.athenz.ZmsException; -import com.yahoo.vespa.hosted.controller.restapi.filter.UnauthenticatedUserPrincipal; import javax.ws.rs.ForbiddenException; +import javax.ws.rs.NotAuthorizedException; import java.security.Principal; -import java.util.Optional; import java.util.logging.Logger; import static com.yahoo.vespa.hosted.controller.restapi.application.Authorizer.environmentRequiresAuthorization; -import static com.yahoo.vespa.hosted.controller.restapi.application.Authorizer.isScrewdriverPrincipal; /** * @author bjorncs @@ -43,36 +39,36 @@ public class DeployAuthorizer { Environment environment, Tenant tenant, ApplicationId applicationId) { - if (athenzCredentialsRequired(environment, tenant, applicationId, principal)) - checkAthenzCredentials(principal, tenant, applicationId); - } - - // TODO: inline when deployment via ssh is removed - private boolean athenzCredentialsRequired(Environment environment, Tenant tenant, ApplicationId applicationId, Principal principal) { - if (!environmentRequiresAuthorization(environment)) return false; - - if (! isScrewdriverPrincipal(principal)) - throw loggedForbiddenException( - "Principal '%s' is not a screwdriver principal, and does not have deploy access to application '%s'", - principal.getName(), applicationId.toShortString()); + if (!environmentRequiresAuthorization(environment)) return; - return tenant.isAthensTenant(); - } + if (principal == null) { + throw loggedUnauthorizedException("Principal '%s' is not authenticated.", principal.getName()); + } + if (!(principal instanceof AthenzPrincipal)) { + throw loggedUnauthorizedException( + "Principal '%s' of type '%s' is not an Athenz principal, which is required for production deployments.", + principal.getName(), principal.getClass().getSimpleName()); + } - // TODO: inline when deployment via ssh is removed - private void checkAthenzCredentials(Principal principal, Tenant tenant, ApplicationId applicationId) { - AthenzDomain domain = tenant.getAthensDomain().get(); - if (! (principal instanceof AthenzPrincipal)) - throw loggedForbiddenException("Principal '%s' is not authenticated.", principal.getName()); + AthenzPrincipal athenzPrincipal = (AthenzPrincipal) principal; + AthenzDomain principalDomain = athenzPrincipal.getDomain(); - AthenzPrincipal athensPrincipal = (AthenzPrincipal)principal; - if ( ! hasDeployAccessToAthenzApplication(athensPrincipal, domain, applicationId)) + if (!principalDomain.equals(AthenzUtils.SCREWDRIVER_DOMAIN)) throw loggedForbiddenException( - "Screwdriver principal '%1$s' does not have deploy access to '%2$s'. " + - "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " + - "'%1$s' is not added to the application's deployer role in Athens domain '%3$s'.", - athensPrincipal, applicationId, tenant.getAthensDomain().get()); + "Principal '%s' is not a Screwdriver principal. Excepted principal with Athenz domain '%s', got '%s'.", + principal.getName(), AthenzUtils.SCREWDRIVER_DOMAIN.id(), principalDomain.id()); + + // NOTE: no fine-grained deploy authorization for non-Athenz tenants + if (tenant.isAthensTenant()) { + AthenzDomain tenantDomain = tenant.getAthensDomain().get(); + if ( ! hasDeployAccessToAthenzApplication(athenzPrincipal, tenantDomain, applicationId)) + throw loggedForbiddenException( + "Screwdriver principal '%1$s' does not have deploy access to '%2$s'. " + + "Either the application has not been created at " + zoneRegistry.getDashboardUri() + " or " + + "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.", + athenzPrincipal.toYRN(), applicationId, tenantDomain.id()); + } } private static ForbiddenException loggedForbiddenException(String message, Object... args) { @@ -81,23 +77,10 @@ public class DeployAuthorizer { return new ForbiddenException(formattedMessage); } - /** - * @deprecated Only usable for ssh. Use the method that takes Principal instead of UserId and screwdriverId. - */ - @Deprecated - public void throwIfUnauthorizedForDeploy(Environment environment, - UserId userId, - Tenant tenant, - ApplicationId applicationId, - Optional<ScrewdriverId> optionalScrewdriverId) { - Principal principal = new UnauthenticatedUserPrincipal(userId.id()); - - if (athenzCredentialsRequired(environment, tenant, applicationId, principal)) { - ScrewdriverId screwdriverId = optionalScrewdriverId.orElseThrow( - () -> loggedForbiddenException("Screwdriver id must be provided when deploying from Screwdriver.")); - principal = AthenzUtils.createPrincipal(screwdriverId); - checkAthenzCredentials(principal, tenant, applicationId); - } + private static NotAuthorizedException loggedUnauthorizedException(String message, Object... args) { + String formattedMessage = String.format(message, args); + log.info(formattedMessage); + return new NotAuthorizedException(formattedMessage); } private boolean hasDeployAccessToAthenzApplication(AthenzPrincipal principal, AthenzDomain domain, ApplicationId applicationId) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java index b36a88ca1d4..eeeed08c2b8 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java @@ -577,7 +577,7 @@ public class ApplicationApiTest extends ControllerContainerTest { entity, Request.Method.POST, athenzUserDomain, "mytenant"), - "{\"error-code\":\"FORBIDDEN\",\"message\":\"Principal 'mytenant' is not a screwdriver principal, and does not have deploy access to application 'tenant1.application1'\"}", + "{\"error-code\":\"FORBIDDEN\",\"message\":\"Principal 'mytenant' is not a Screwdriver principal. Excepted principal with Athenz domain 'cd.screwdriver.project', got 'domain1'.\"}", 403); // Deleting an application for an Athens domain the user is not admin for is disallowed |