diff options
18 files changed, 791 insertions, 188 deletions
diff --git a/config-provisioning/abi-spec.json b/config-provisioning/abi-spec.json index 80680a4d095..6e110b708cf 100644 --- a/config-provisioning/abi-spec.json +++ b/config-provisioning/abi-spec.json @@ -707,7 +707,8 @@ "public static com.yahoo.config.provision.SystemName[] values()", "public static com.yahoo.config.provision.SystemName valueOf(java.lang.String)", "public static com.yahoo.config.provision.SystemName defaultSystem()", - "public static com.yahoo.config.provision.SystemName from(java.lang.String)" + "public static com.yahoo.config.provision.SystemName from(java.lang.String)", + "public static java.util.Set all()" ], "fields": [ "public static final enum com.yahoo.config.provision.SystemName dev", diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java index 891c213e9aa..2de11be08f2 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/SystemName.java @@ -1,6 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.provision; +import java.util.EnumSet; +import java.util.Set; + /** * Systems in hosted Vespa * @@ -39,4 +42,8 @@ public enum SystemName { } } + public static Set<SystemName> all() { + return EnumSet.allOf(SystemName.class); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java index 309c37af341..7298a0a91d0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilter.java @@ -3,8 +3,10 @@ package com.yahoo.vespa.hosted.controller.restapi.filter; import com.google.inject.Inject; import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.SystemName; import com.yahoo.config.provision.TenantName; -import com.yahoo.jdisc.http.HttpRequest.Method; +import com.yahoo.jdisc.Response; +import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.security.cors.CorsFilterConfig; import com.yahoo.jdisc.http.filter.security.cors.CorsRequestFilterBase; @@ -20,6 +22,10 @@ import com.yahoo.vespa.hosted.controller.TenantController; import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory; import com.yahoo.vespa.hosted.controller.athenz.ApplicationAction; import com.yahoo.vespa.hosted.controller.athenz.impl.AthenzFacade; +import com.yahoo.vespa.hosted.controller.role.Action; +import com.yahoo.vespa.hosted.controller.role.Context; +import com.yahoo.vespa.hosted.controller.role.Role; +import com.yahoo.vespa.hosted.controller.role.RoleMembership; import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant; import com.yahoo.vespa.hosted.controller.tenant.Tenant; import com.yahoo.vespa.hosted.controller.tenant.UserTenant; @@ -28,18 +34,14 @@ import com.yahoo.yolean.chain.Provides; import javax.ws.rs.ForbiddenException; import javax.ws.rs.InternalServerErrorException; -import javax.ws.rs.NotAuthorizedException; import javax.ws.rs.WebApplicationException; -import java.util.List; +import java.security.Principal; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.logging.Logger; -import static com.yahoo.jdisc.http.HttpRequest.Method.GET; -import static com.yahoo.jdisc.http.HttpRequest.Method.HEAD; -import static com.yahoo.jdisc.http.HttpRequest.Method.OPTIONS; -import static com.yahoo.jdisc.http.HttpRequest.Method.POST; -import static com.yahoo.jdisc.http.HttpRequest.Method.PUT; import static com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities.SCREWDRIVER_DOMAIN; /** @@ -51,53 +53,47 @@ import static com.yahoo.vespa.hosted.controller.athenz.HostedAthenzIdentities.SC @Provides("ControllerAuthorizationFilter") public class ControllerAuthorizationFilter extends CorsRequestFilterBase { - private static final List<Method> WHITELISTED_METHODS = List.of(GET, OPTIONS, HEAD); - private static final Logger log = Logger.getLogger(ControllerAuthorizationFilter.class.getName()); private final AthenzFacade athenz; - private final TenantController tenantController; + private final Controller controller; @Inject public ControllerAuthorizationFilter(AthenzClientFactory clientFactory, Controller controller, CorsFilterConfig corsConfig) { - super(corsConfig); - this.athenz = new AthenzFacade(clientFactory); - this.tenantController = controller.tenants(); + this(clientFactory, controller, Set.copyOf(corsConfig.allowedUrls())); } ControllerAuthorizationFilter(AthenzClientFactory clientFactory, - TenantController tenantController, + Controller controller, Set<String> allowedUrls) { super(allowedUrls); this.athenz = new AthenzFacade(clientFactory);; - this.tenantController = tenantController; + this.controller = controller; } - // NOTE: Be aware of the ordering of the path pattern matching. Semantics may change if the patterns are evaluated - // in different order. @Override public Optional<ErrorResponse> filterRequest(DiscFilterRequest request) { - Method method = getMethod(request); - if (isWhiteListedMethod(method)) return Optional.empty(); - try { + Principal principal = request.getUserPrincipal(); + if (principal == null) + return Optional.of(new ErrorResponse(Response.Status.FORBIDDEN, "Access denied")); + Path path = new Path(request.getRequestURI()); - AthenzPrincipal principal = getPrincipalOrThrow(request); - if (isWhiteListedOperation(path, method)) { - // no authz check - } else if (isHostedOperatorOperation(path, method)) { - verifyIsHostedOperator(principal); - } else if (isTenantAdminOperation(path, method)) { - verifyIsTenantAdmin(principal, getTenantName(path)); - } else if (isTenantPipelineOperation(path, method)) { - verifyIsTenantPipelineOperator(principal, getTenantName(path), getApplicationName(path)); - } else { - throw new ForbiddenException("No access control is declared for path: '" + path.asString() + "'"); - } - return Optional.empty(); - } catch (WebApplicationException e) { + Action action = Action.from(HttpRequest.Method.valueOf(request.getMethod())); + + // Avoid expensive lookups when request is always legal. + if (RoleMembership.everyone().allows(action, request.getRequestURI())) + return Optional.empty(); + + RoleMembership roles = new AthenzRoleResolver(athenz, controller, path).membership(principal); + if (roles.allows(action, request.getRequestURI())) + return Optional.empty(); + + return Optional.of(new ErrorResponse(Response.Status.FORBIDDEN, "Access denied")); + } + catch (WebApplicationException e) { int statusCode = e.getResponse().getStatus(); String errorMessage = e.getMessage(); log.log(LogLevel.WARNING, String.format("Access denied (%d): %s", statusCode, errorMessage)); @@ -105,146 +101,106 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { } } - private static boolean isWhiteListedMethod(Method method) { - return WHITELISTED_METHODS.contains(method); - } - - private static boolean isWhiteListedOperation(Path path, Method method) { - return path.matches("/application/v4/user") && method == PUT || // Create user tenant - path.matches("/application/v4/tenant/{tenant}") && method == POST; // Create tenant - } - - private static boolean isHostedOperatorOperation(Path path, Method method) { - if (isWhiteListedOperation(path, method)) return false; - return path.matches("/controller/v1/{*}") || - path.matches("/provision/v2/{*}") || - path.matches("/flags/v1/{*}") || - path.matches("/os/v1/{*}") || - path.matches("/zone/v2/{*}") || - path.matches("/nodes/v2/{*}") || - path.matches("/orchestrator/v1/{*}"); - } - - private static boolean isTenantAdminOperation(Path path, Method method) { - if (isHostedOperatorOperation(path, method)) return false; - return path.matches("/application/v4/tenant/{tenant}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/deploying/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{job}/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/dev/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/perf/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/{environment}/region/{region}/instance/{instance}/global-rotation/override"); - } + // TODO: Pull class up and resolve roles from roles defined in Athenz + private static class AthenzRoleResolver implements RoleMembership.Resolver { - private static boolean isTenantPipelineOperation(Path path, Method method) { - if (isTenantAdminOperation(path, method)) return false; - return path.matches("/application/v4/tenant/{tenant}/application/{application}/jobreport") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/submit") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/promote") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/prod/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/test/{*}") || - path.matches("/application/v4/tenant/{tenant}/application/{application}/environment/staging/{*}"); - } + private final AthenzFacade athenz; + private final TenantController tenants; + private final Path path; + private final SystemName system; - private void verifyIsHostedOperator(AthenzPrincipal principal) { - if (!isHostedOperator(principal.getIdentity())) { - throw new ForbiddenException("Vespa operator role required"); + public AthenzRoleResolver(AthenzFacade athenz, Controller controller, Path path) { + this.athenz = athenz; + this.tenants = controller.tenants(); + this.path = path; + this.system = controller.system(); } - } - - private boolean isHostedOperator(AthenzIdentity identity) { - return athenz.hasHostedOperatorAccess(identity); - } - - private void verifyIsTenantAdmin(AthenzPrincipal principal, TenantName name) { - tenantController.get(name) - .ifPresent(tenant -> { - if (!isTenantAdmin(principal.getIdentity(), tenant)) { - throw new ForbiddenException("Tenant admin or Vespa operator role required"); - } - }); - } - private boolean isTenantAdmin(AthenzIdentity identity, Tenant tenant) { - if (tenant instanceof AthenzTenant) { - return athenz.hasTenantAdminAccess(identity, ((AthenzTenant) tenant).domain()); - } else if (tenant instanceof UserTenant) { - if (!(identity instanceof AthenzUser)) { - return false; + private boolean isTenantAdmin(AthenzIdentity identity, Tenant tenant) { + if (tenant instanceof AthenzTenant) { + return athenz.hasTenantAdminAccess(identity, ((AthenzTenant) tenant).domain()); + } else if (tenant instanceof UserTenant) { + if (!(identity instanceof AthenzUser)) { + return false; + } + AthenzUser user = (AthenzUser) identity; + return ((UserTenant) tenant).is(user.getName()) || isHostedOperator(identity); } - AthenzUser user = (AthenzUser) identity; - return ((UserTenant) tenant).is(user.getName()) || isHostedOperator(identity); + throw new InternalServerErrorException("Unknown tenant type: " + tenant.getClass().getSimpleName()); } - throw new InternalServerErrorException("Unknown tenant type: " + tenant.getClass().getSimpleName()); - } - - private void verifyIsTenantPipelineOperator(AthenzPrincipal principal, - TenantName name, - ApplicationName application) { - tenantController.get(name) - .ifPresent(tenant -> verifyIsTenantPipelineOperator(principal.getIdentity(), tenant, application)); - } - private void verifyIsTenantPipelineOperator(AthenzIdentity identity, Tenant tenant, ApplicationName application) { - if (isHostedOperator(identity)) return; + private boolean hasDeployerAccess(AthenzIdentity identity, AthenzDomain tenantDomain, ApplicationName application) { + try { + return athenz.hasApplicationAccess(identity, + ApplicationAction.deploy, + tenantDomain, + application); + } catch (ZmsClientException e) { + throw new InternalServerErrorException("Failed to authorize operation: (" + e.getMessage() + ")", e); + } + } - AthenzDomain principalDomain = identity.getDomain(); - if (!principalDomain.equals(SCREWDRIVER_DOMAIN)) { - throw new ForbiddenException(String.format( - "'%s' is not a Screwdriver identity. Only Screwdriver is allowed to deploy to this environment.", - identity.getFullName())); + private boolean isHostedOperator(AthenzIdentity identity) { + return athenz.hasHostedOperatorAccess(identity); } - // NOTE: no fine-grained deploy authorization for non-Athenz tenants - if (tenant instanceof AthenzTenant) { - AthenzDomain tenantDomain = ((AthenzTenant) tenant).domain(); - if (!hasDeployerAccess(identity, tenantDomain, application)) { - throw new ForbiddenException(String.format( - "'%1$s' does not have access to '%2$s'. " + - "Either the application has not been created at Vespa dashboard or " + - "'%1$s' is not added to the application's deployer role in Athenz domain '%3$s'.", - identity.getFullName(), application.value(), tenantDomain.getName())); + @Override + public RoleMembership membership(Principal principal) { + if ( ! (principal instanceof AthenzPrincipal)) + throw new IllegalStateException("Expected an AthenzPrincipal to be set on the request."); + + Map<Role, Set<Context>> memberships = new HashMap<>(); + AthenzIdentity identity = ((AthenzPrincipal) principal).getIdentity(); + Optional<Tenant> tenant = tenant(); + Context context = context(tenant); // TODO this way of computing a context is wrong, but we must + // do it until we ask properly for roles based on token. + Set<Context> contexts = Set.of(context); + if (isHostedOperator(identity)) { + memberships.put(Role.hostedOperator, contexts); + } + if (tenant.isPresent() && isTenantAdmin(identity, tenant.get())) { + memberships.put(Role.tenantAdmin, contexts); } + AthenzDomain principalDomain = identity.getDomain(); + if (principalDomain.equals(SCREWDRIVER_DOMAIN)) { + // NOTE: Only fine-grained deploy authorization for Athenz tenants + if (context.application().isPresent() && tenant.isPresent() && tenant.get() instanceof AthenzTenant) { + AthenzDomain tenantDomain = ((AthenzTenant) tenant.get()).domain(); + if (hasDeployerAccess(identity, tenantDomain, context.application().get())) { + memberships.put(Role.tenantPipelineOperator, contexts); + } + } else { + memberships.put(Role.tenantPipelineOperator, contexts); + } + } + memberships.put(Role.everyone, contexts); + return new RoleMembership(memberships); } - } - private boolean hasDeployerAccess(AthenzIdentity identity, AthenzDomain tenantDomain, ApplicationName application) { - try { - return athenz - .hasApplicationAccess( - identity, - ApplicationAction.deploy, - tenantDomain, - application); - } catch (ZmsClientException e) { - throw new InternalServerErrorException("Failed to authorize operation: (" + e.getMessage() + ")", e); + private Optional<Tenant> tenant() { + if (!path.matches("/application/v4/tenant/{tenant}/{*}")) { + return Optional.empty(); + } + return tenants.get(TenantName.from(path.get("tenant"))); } - } - - private static TenantName getTenantName(Path path) { - if (!path.matches("/application/v4/tenant/{tenant}/{*}")) - throw new InternalServerErrorException("Unable to handle path: " + path.asString()); - return TenantName.from(path.get("tenant")); - } - - private static ApplicationName getApplicationName(Path path) { - if (!path.matches("/application/v4/tenant/{tenant}/application/{application}/{*}")) - throw new InternalServerErrorException("Unable to handle path: " + path.asString()); - return ApplicationName.from(path.get("application")); - } - private static Method getMethod(DiscFilterRequest request) { - return Method.valueOf(request.getMethod().toUpperCase()); - } - - private static AthenzPrincipal getPrincipalOrThrow(DiscFilterRequest request) { - return getPrincipal(request) - .orElseThrow(() -> new NotAuthorizedException("User not authenticated")); + // TODO: Currently there's only one context for each role, but this will change + private Context context(Optional<Tenant> tenant) { + if (tenant.isEmpty()) { + return Context.unlimitedIn(system); + } + // TODO: Remove this. Current behaviour always allows tenant full access to all its applications, but with + // the new role setup, each role will include a complete context (tenant + app) + if (path.matches("/application/v4/tenant/{tenant}/application/{application}/{*}")) { + return Context.limitedTo(tenant.get().name(), ApplicationName.from(path.get("application")), system); + } + return Context.limitedTo(tenant.get().name(), system); + } } - private static Optional<AthenzPrincipal> getPrincipal(DiscFilterRequest request) { + private static Optional<AthenzPrincipal> principalFrom(DiscFilterRequest request) { return Optional.ofNullable(request.getUserPrincipal()) - .map(AthenzPrincipal.class::cast); + .map(AthenzPrincipal.class::cast); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Action.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Action.java new file mode 100644 index 00000000000..533c28905a9 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Action.java @@ -0,0 +1,43 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.jdisc.http.HttpRequest; + +import java.util.EnumSet; + +/** + * Action defines an operation, typically a HTTP method, that may be performed on an entity in the controller + * (e.g. tenant or application). + * + * @author mpolden + */ +public enum Action { + + create, + read, + update, + delete; + + public static EnumSet<Action> all() { + return EnumSet.allOf(Action.class); + } + + public static EnumSet<Action> write() { + return EnumSet.of(create, update, delete); + } + + /** Returns the appropriate action for given HTTP method */ + public static Action from(HttpRequest.Method method) { + switch (method) { + case POST: return Action.create; + case GET: + case OPTIONS: + case HEAD: return Action.read; + case PUT: + case PATCH: return Action.update; + case DELETE: return Action.delete; + default: throw new IllegalArgumentException("No action defined for method " + method); + } + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Context.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Context.java new file mode 100644 index 00000000000..71452a3ef20 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Context.java @@ -0,0 +1,84 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; + +import java.util.Objects; +import java.util.Optional; + +/** + * The context in which a role is valid. + * + * @author mpolden + */ +public class Context { + + private final Optional<TenantName> tenant; + private final Optional<ApplicationName> application; + private final SystemName system; + + private Context(Optional<TenantName> tenant, Optional<ApplicationName> application, SystemName system) { + this.tenant = Objects.requireNonNull(tenant, "tenant must be non-null"); + this.application = Objects.requireNonNull(application, "application must be non-null"); + this.system = Objects.requireNonNull(system, "system must be non-null"); + } + + /** A specific tenant this is valid for, if any */ + public Optional<TenantName> tenant() { + return tenant; + } + + /** A specific application this is valid for, if any */ + public Optional<ApplicationName> application() { + return application; + } + + /** System in which this is valid */ + public SystemName system() { + return system; + } + + /** Returns whether this context is considered limited */ + public boolean limited() { + return tenant.isPresent() || application.isPresent(); + } + + /** Returns a context that has no restrictions on tenant or application in given system */ + public static Context unlimitedIn(SystemName system) { + return new Context(Optional.empty(), Optional.empty(), system); + } + + /** Returns a context that is limited to given tenant and system */ + public static Context limitedTo(TenantName tenant, SystemName system) { + return new Context(Optional.of(tenant), Optional.empty(), system); + } + + /** Returns a context that is limited to given tenant, application and system */ + public static Context limitedTo(TenantName tenant, ApplicationName application, SystemName system) { + return new Context(Optional.of(tenant), Optional.of(application), system); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Context context = (Context) o; + return tenant.equals(context.tenant) && + application.equals(context.application) && + system == context.system; + } + + @Override + public int hashCode() { + return Objects.hash(tenant, application, system); + } + + @Override + public String toString() { + return "tenant " + tenant.map(TenantName::value).orElse("[none]") + ", application " + + application.map(ApplicationName::value).orElse("[none]") + ", system " + system; + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java new file mode 100644 index 00000000000..653c1d40684 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/PathGroup.java @@ -0,0 +1,102 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.restapi.Path; + +import java.util.EnumSet; +import java.util.Optional; +import java.util.Set; + +/** + * This declares and groups all known REST API paths in the controller. + * + * When creating a new API, its paths must be added here and a policy must be declared in {@link Policy}. + * + * @author mpolden + */ +public enum PathGroup { + + /** Paths used for system management by operators */ + operator("/controller/v1/{*}", + "/provision/v2/{*}", + "/flags/v1/{*}", + "/os/v1/{*}", + "/cost/v1/{*}", + "/zone/v2/{*}", + "/nodes/v2/{*}", + "/orchestrator/v1/{*}"), + + /** Paths used when onboarding and creating a new tenants */ + onboardingUser("/application/v4/user"), + + // Tenant parameter is ignored here as context for the role is not defined until after a tenant has been created + onboardingTenant("/application/v4/tenant/{ignored}"), + + /** Read-only paths used when onboarding tenants */ + onboardingTenantInformation("/athenz/v1/", + "/athenz/v1/domains"), + + + /** Paths used by tenant/application administrators */ + tenant("/application/v4/", + "/application/v4/property/", + "/application/v4/tenant/", + "/application/v4/tenant-pipeline/", + "/application/v4/tenant/{tenant}", + "/application/v4/tenant/{tenant}/application/", + "/application/v4/tenant/{tenant}/application/{application}", + "/application/v4/tenant/{tenant}/application/{application}/deploying/{*}", + "/application/v4/tenant/{tenant}/application/{application}/instance/{instance}/job/{job}/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/dev/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/perf/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/region/{region}/instance/{instance}/global-rotation/override"), + + /** Paths used for deployments by build service(s) */ + buildService("/application/v4/tenant/{tenant}/application/{application}/jobreport", + "/application/v4/tenant/{tenant}/application/{application}/submit", + "/application/v4/tenant/{tenant}/application/{application}/promote", + "/application/v4/tenant/{tenant}/application/{application}/environment/prod/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/test/{*}", + "/application/v4/tenant/{tenant}/application/{application}/environment/staging/{*}"), + + /** Read-only paths providing information related to deployments */ + deploymentStatus("/badge/v1/{*}", + "/deployment/v1/{*}", + "/zone/v1/{*}"); + + final Set<String> pathSpecs; + + PathGroup(String... pathSpecs) { + this.pathSpecs = Set.of(pathSpecs); + } + + /** Returns path if it matches any spec in this group, with match groups set by the match. */ + private Optional<Path> get(String path) { + Path matcher = new Path(path); + for (String spec : pathSpecs) // Iterate to be sure the Path's state is that of the match. + if (matcher.matches(spec)) return Optional.of(matcher); + return Optional.empty(); + } + + /** All known path groups */ + public static Set<PathGroup> all() { + return EnumSet.allOf(PathGroup.class); + } + + /** Returns whether this group matches path in given context */ + public boolean matches(String path, Context context) { + return get(path).map(p -> { + boolean match = true; + String tenant = p.get("tenant"); + if (tenant != null && context.tenant().isPresent()) { + match = context.tenant().get().value().equals(tenant); + } + String application = p.get("application"); + if (application != null && context.application().isPresent()) { + match &= context.application().get().value().equals(application); + } + return match; + }).orElse(false); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java new file mode 100644 index 00000000000..05c9871dd66 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Policy.java @@ -0,0 +1,64 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.config.provision.SystemName; + +import java.util.Set; + +/** + * Policies for REST APIs in the controller. A policy is only considered when defined in a {@link Role}. + * + * @author mpolden + */ +public enum Policy { + + /** Operator policy allows access to everything in all systems */ + operator(Privilege.grant(Action.all()) + .on(PathGroup.all()) + .in(SystemName.all())), + + /** + * Tenant policy allows tenants to access their own tenant, in all systems, and allows global read access in + * selected systems + */ + tenant(Privilege.grant(Action.all()) + .on(PathGroup.tenant) + .in(SystemName.all())), + + /** Build service policy only allows access relevant for build service(s) */ + buildService(Privilege.grant(Action.all()) + .on(PathGroup.buildService) + .in(SystemName.all())), + + /** Unauthorized policy allows creation of tenants and read of everything in selected systems */ + unauthorized(Privilege.grant(Action.update) + .on(PathGroup.onboardingUser) + .in(SystemName.main, SystemName.cd, SystemName.dev), + Privilege.grant(Action.create) + .on(PathGroup.onboardingTenant) + .in(SystemName.main, SystemName.cd, SystemName.dev), + Privilege.grant(Action.read) + .on(PathGroup.onboardingTenantInformation) + .in(SystemName.main, SystemName.cd, SystemName.dev), + Privilege.grant(Action.read) + .on(PathGroup.all()) + .in(SystemName.main, SystemName.cd, SystemName.dev), + Privilege.grant(Action.read) + .on(PathGroup.deploymentStatus) + .in(SystemName.all())); + + private final Set<Privilege> privileges; + + Policy(Privilege... privileges) { + this.privileges = Set.of(privileges); + } + + /** Returns whether action is allowed on path in given context */ + public boolean evaluate(Action action, String path, Context context) { + return privileges.stream().anyMatch(privilege -> privilege.actions().contains(action) && + privilege.systems().contains(context.system()) && + privilege.pathGroups().stream() + .anyMatch(pg -> pg.matches(path, context))); + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Privilege.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Privilege.java new file mode 100644 index 00000000000..4c5ad136f56 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Privilege.java @@ -0,0 +1,99 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.config.provision.SystemName; + +import java.util.EnumSet; +import java.util.LinkedHashSet; +import java.util.Objects; +import java.util.Set; + +/** + * This describes a privilege in the controller. A privilege expresses the actions (e.g. create or read) granted + * for a particular group of REST API paths. A privilege is valid in one or more systems. + * + * @author mpolden + */ +public class Privilege { + + private final Set<SystemName> systems; + private final Set<Action> actions; + private final Set<PathGroup> pathGroups; + + private Privilege(Set<SystemName> systems, Set<Action> actions, Set<PathGroup> pathGroups) { + this.systems = EnumSet.copyOf(Objects.requireNonNull(systems, "system must be non-null")); + this.actions = EnumSet.copyOf(Objects.requireNonNull(actions, "actions must be non-null")); + this.pathGroups = EnumSet.copyOf(Objects.requireNonNull(pathGroups, "pathGroups must be non-null")); + if (systems.isEmpty()) { + throw new IllegalArgumentException("systems must be non-empty"); + } + } + + /** Systems where this applies */ + public Set<SystemName> systems() { + return systems; + } + + /** Actions allowed by this */ + public Set<Action> actions() { + return actions; + } + + /** Path groups where this applies */ + public Set<PathGroup> pathGroups() { + return pathGroups; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Privilege privilege = (Privilege) o; + return systems.equals(privilege.systems) && + actions.equals(privilege.actions) && + pathGroups.equals(privilege.pathGroups); + } + + @Override + public int hashCode() { + return Objects.hash(systems, actions, pathGroups); + } + + public static PrivilegeBuilder grant(Action... actions) { + return grant(Set.of(actions)); + } + + public static PrivilegeBuilder grant(Set<Action> actions) { + return new PrivilegeBuilder(actions); + } + + public static class PrivilegeBuilder { + + private Set<Action> actions; + private Set<PathGroup> pathGroups; + + private PrivilegeBuilder(Set<Action> actions) { + this.actions = EnumSet.copyOf(actions); + this.pathGroups = new LinkedHashSet<>(); + } + + public PrivilegeBuilder on(PathGroup... pathGroups) { + return on(Set.of(pathGroups)); + } + + public PrivilegeBuilder on(Set<PathGroup> pathGroups) { + this.pathGroups.addAll(pathGroups); + return this; + } + + public Privilege in(SystemName... systems) { + return in(Set.of(systems)); + } + + public Privilege in(Set<SystemName> systems) { + return new Privilege(systems, actions, pathGroups); + } + + } + +} diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java new file mode 100644 index 00000000000..01d1c7e0eb8 --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/Role.java @@ -0,0 +1,35 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import java.util.EnumSet; +import java.util.Set; + +/** + * This declares all tenant roles known to the controller. A role contains one or more {@link Policy}'s which decide + * what actions a member of a role can perform. + * + * @author mpolden + */ +public enum Role { + + hostedOperator(Policy.operator), + tenantAdmin(Policy.tenant), + tenantPipelineOperator(Policy.buildService), + everyone(Policy.unauthorized); + + private final Set<Policy> policies; + + Role(Policy... policies) { + this.policies = EnumSet.copyOf(Set.of(policies)); + } + + /** + * Returns whether this role is allowed to perform action in given role context. Action is allowed if at least one + * policy evaluates to true. + */ + public boolean allows(Action action, String path, Context context) { + return policies.stream().anyMatch(policy -> policy.evaluate(action, path, context)); + } + +} + diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java new file mode 100644 index 00000000000..d3940ac631f --- /dev/null +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/role/RoleMembership.java @@ -0,0 +1,55 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.config.provision.SystemName; + +import java.security.Principal; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * A list of roles and their associated contexts. This defines the role membership of a tenant, and in which contexts + * (see {@link Context}) those roles apply. + * + * @author mpolden + */ +public class RoleMembership { + + private static final RoleMembership everyone = new RoleMembership(Map.of(Role.everyone, + Stream.of(SystemName.values()) + .map(Context::unlimitedIn) + .collect(Collectors.toUnmodifiableSet()))); + + private final Map<Role, Set<Context>> roles; + + public RoleMembership(Map<Role, Set<Context>> roles) { + this.roles = Map.copyOf(roles); + } + + public static RoleMembership everyone() { return everyone; } + + /** Returns whether any role in this allows action to take place in path */ + public boolean allows(Action action, String path) { + return roles.entrySet().stream().anyMatch(kv -> { + Role role = kv.getKey(); + Set<Context> contexts = kv.getValue(); + return contexts.stream().anyMatch(context -> role.allows(action, path, context)); + }); + } + + @Override + public String toString() { + return "roles " + roles; + } + + /** + * A role resolver. Identity providers can implement this to translate their internal representation of role + * membership to a {@link RoleMembership}. + */ + public interface Resolver { + RoleMembership membership(Principal user); + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/AthenzFilterMock.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/AthenzFilterMock.java index 09515b8905e..8eb64a00d40 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/AthenzFilterMock.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/integration/AthenzFilterMock.java @@ -31,7 +31,6 @@ public class AthenzFilterMock implements SecurityRequestFilter { @Override public void filter(DiscFilterRequest request, ResponseHandler handler) { - if (request.getMethod().equalsIgnoreCase("OPTIONS")) return; String identityName = request.getHeader(IDENTITY_HEADER_NAME); if (identityName == null) { Response response = new Response(HttpResponse.Status.UNAUTHORIZED); 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 a6573f61c29..2ae17a5624e 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 @@ -321,6 +321,12 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), new File("application2.json")); + // GET application having both change and outstanding change + tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", GET) + .screwdriverIdentity(SCREWDRIVER_ID), + new File("application2.json")); + + // PATCH in a major version override tester.assertResponse(request("/application/v4/tenant/tenant2/application/application2", PATCH) .userIdentity(USER_ID) @@ -382,10 +388,10 @@ public class ApplicationApiTest extends ControllerContainerTest { new File("application1-recursive.json")); // GET logs - tester.assertResponse(request("/application/v4/tenant/tenant2/application//application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214", GET) + tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214", GET) .userIdentity(USER_ID), new File("logs.json")); - tester.assertResponse(request("/application/v4/tenant/tenant2/application//application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214&streaming", GET) + tester.assertResponse(request("/application/v4/tenant/tenant2/application/application1/environment/prod/region/us-central-1/instance/default/logs?from=1233&to=3214&streaming", GET) .userIdentity(USER_ID), "INFO - All good"); @@ -570,7 +576,8 @@ public class ApplicationApiTest extends ControllerContainerTest { "{\"user\":\"other_user\",\"tenants\":[],\"tenantExists\":false}"); // OPTIONS return 200 OK - tester.assertResponse(request("/application/v4/", Request.Method.OPTIONS), + tester.assertResponse(request("/application/v4/", Request.Method.OPTIONS) + .userIdentity(USER_ID), ""); // Promote from pipeline @@ -674,6 +681,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Setup tester.computeVersionStatus(); createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); + addUserToHostedOperatorRole(HostedAthenzIdentities.from(HOSTED_VESPA_OPERATOR)); // Create tenant tester.assertResponse(request("/application/v4/tenant/tenant1", POST).userIdentity(USER_ID) @@ -703,19 +711,19 @@ public class ApplicationApiTest extends ControllerContainerTest { HttpEntity noAppEntity = createApplicationDeployData(Optional.empty(), true); tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST) .data(noAppEntity) - .userIdentity(USER_ID), + .userIdentity(HOSTED_VESPA_OPERATOR), "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Deployment of system applications during a system upgrade is not allowed\"}", 400); tester.upgradeSystem(tester.controller().versionStatus().controllerVersion().get().versionNumber()); tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/routing/environment/prod/region/us-central-1/instance/default/deploy", POST) .data(noAppEntity) - .userIdentity(USER_ID), + .userIdentity(HOSTED_VESPA_OPERATOR), new File("deploy-result.json")); // POST (deploy) a system application without an application package tester.assertResponse(request("/application/v4/tenant/hosted-vespa/application/proxy-host/environment/prod/region/us-central-1/instance/default/deploy", POST) .data(noAppEntity) - .userIdentity(USER_ID), + .userIdentity(HOSTED_VESPA_OPERATOR), new File("deploy-no-deployment.json"), 400); } @@ -809,13 +817,13 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.computeVersionStatus(); createAthenzDomainWithAdmin(ATHENZ_TENANT_DOMAIN, USER_ID); - // PUT (update) non-existing tenant + // PUT (update) non-existing tenant returns 403 as tenant access cannot be determined when the tenant does not exist tester.assertResponse(request("/application/v4/tenant/tenant1", PUT) .userIdentity(USER_ID) .oktaAccessToken(OKTA_AT) .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}"), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"Tenant 'tenant1' does not exist\"}", - 404); + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", + 403); // GET non-existing tenant tester.assertResponse(request("/application/v4/tenant/tenant1", GET) @@ -936,20 +944,21 @@ public class ApplicationApiTest extends ControllerContainerTest { .userIdentity(USER_ID), "{\"error-code\":\"NOT_FOUND\",\"message\":\"Could not delete application 'tenant1.application1': Application not found\"}", 404); + // DELETE tenant tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE) .userIdentity(USER_ID) .oktaAccessToken(OKTA_AT), new File("tenant-without-applications.json")); - // DELETE tenant again - should produce 404 + // DELETE tenant again returns 403 as tenant access cannot be determined when the tenant does not exist tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE) .userIdentity(USER_ID), - "{\"error-code\":\"NOT_FOUND\",\"message\":\"Could not delete tenant 'tenant1': Tenant not found\"}", - 404); + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", + 403); // Promote application chef env for nonexistent tenant/application tester.assertResponse(request("/application/v4/tenant/dontexist/application/dontexist/environment/prod/region/us-west-1/instance/default/promote", POST) - .userIdentity(USER_ID), + .screwdriverIdentity(SCREWDRIVER_ID), "{\"error-code\":\"INTERNAL_SERVER_ERROR\",\"message\":\"Unable to promote Chef environments for application\"}", 500); @@ -1005,7 +1014,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", POST) .userIdentity(unauthorizedUser) .oktaAccessToken(OKTA_AT), - "{\n \"code\" : 403,\n \"message\" : \"Tenant admin or Vespa operator role required\"\n}", + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); // (Create it with the right tenant id) @@ -1020,13 +1029,13 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1/environment/prod/region/us-west-1/instance/default/deploy", POST) .data(entity) .userIdentity(USER_ID), - "{\n \"code\" : 403,\n \"message\" : \"'user.myuser' is not a Screwdriver identity. Only Screwdriver is allowed to deploy to this environment.\"\n}", + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); // Deleting an application for an Athens domain the user is not admin for is disallowed tester.assertResponse(request("/application/v4/tenant/tenant1/application/application1", DELETE) .userIdentity(unauthorizedUser), - "{\n \"code\" : 403,\n \"message\" : \"Tenant admin or Vespa operator role required\"\n}", + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); // (Deleting it with the right tenant id) @@ -1040,7 +1049,7 @@ public class ApplicationApiTest extends ControllerContainerTest { tester.assertResponse(request("/application/v4/tenant/tenant1", PUT) .data("{\"athensDomain\":\"domain1\", \"property\":\"property1\"}") .userIdentity(unauthorizedUser), - "{\n \"code\" : 403,\n \"message\" : \"Tenant admin or Vespa operator role required\"\n}", + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); // Change Athens domain @@ -1055,7 +1064,7 @@ public class ApplicationApiTest extends ControllerContainerTest { // Deleting a tenant for an Athens domain the user is not admin for is disallowed tester.assertResponse(request("/application/v4/tenant/tenant1", DELETE) .userIdentity(unauthorizedUser), - "{\n \"code\" : 403,\n \"message\" : \"Tenant admin or Vespa operator role required\"\n}", + "{\n \"code\" : 403,\n \"message\" : \"Access denied\"\n}", 403); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java index 3b47ba7bc71..fc73590e448 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/AthenzApiTest.java @@ -10,6 +10,9 @@ import org.junit.Test; import java.io.File; +/** + * @author jonmv + */ public class AthenzApiTest extends ControllerContainerTest { private static final String responseFiles = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/responses/"; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java index 60ba8235b39..6eb7663e4ab 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/BadgeApiTest.java @@ -3,8 +3,6 @@ package com.yahoo.vespa.hosted.controller.restapi.deployment; import com.yahoo.config.provision.AthenzService; import com.yahoo.config.provision.Environment; import com.yahoo.vespa.hosted.controller.Application; -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.deployment.SourceRevision; import com.yahoo.vespa.hosted.controller.application.ApplicationPackage; import com.yahoo.vespa.hosted.controller.deployment.ApplicationPackageBuilder; diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java index 1dc0e14d447..e94526c7dd9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/filter/ControllerAuthorizationFilterTest.java @@ -23,15 +23,16 @@ import org.junit.Test; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.Set; import static com.yahoo.container.jdisc.RequestHandlerTestDriver.MockResponseHandler; import static com.yahoo.jdisc.http.HttpRequest.Method.DELETE; import static com.yahoo.jdisc.http.HttpRequest.Method.POST; import static com.yahoo.jdisc.http.HttpRequest.Method.PUT; import static com.yahoo.jdisc.http.HttpResponse.Status.FORBIDDEN; -import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -47,9 +48,11 @@ public class ControllerAuthorizationFilterTest { private static final AthenzUser USER = user("john"); private static final AthenzUser HOSTED_OPERATOR = user("hosted-operator"); private static final AthenzDomain TENANT_DOMAIN = new AthenzDomain("tenantdomain"); + private static final AthenzDomain TENANT_DOMAIN2 = new AthenzDomain("tenantdomain2"); private static final AthenzService TENANT_ADMIN = new AthenzService(TENANT_DOMAIN, "adminservice"); private static final AthenzService TENANT_PIPELINE = HostedAthenzIdentities.from(new ScrewdriverId("12345")); private static final TenantName TENANT = TenantName.from("mytenant"); + private static final TenantName TENANT2 = TenantName.from("othertenant"); private static final ApplicationId APPLICATION = new ApplicationId("myapp"); @Test @@ -101,6 +104,8 @@ public class ControllerAuthorizationFilterTest { controllerTester.athenzDb().hostedOperators.add(HOSTED_OPERATOR); controllerTester.createTenant(TENANT.value(), TENANT_DOMAIN.getName(), null); controllerTester.createApplication(TENANT, APPLICATION.id(), "default", 12345); + controllerTester.createTenant(TENANT2.value(), TENANT_DOMAIN2.getName(), null); + controllerTester.createApplication(TENANT2, APPLICATION.id(), "default", 42); AthenzDbMock.Domain domainMock = controllerTester.athenzDb().domains.get(TENANT_DOMAIN); domainMock.admins.add(TENANT_ADMIN); domainMock.applications.get(APPLICATION).addRoleMember(ApplicationAction.deploy, TENANT_PIPELINE); @@ -122,6 +127,12 @@ public class ControllerAuthorizationFilterTest { testApiAccess(POST, "/application/v4/tenant/mytenant/application/myapp/submit", allowed, forbidden, filter); + // Pipeline cannot access other app + List<AthenzIdentity> forbiddenIncludingPipeline = new ArrayList<>(forbidden); + forbiddenIncludingPipeline.add(TENANT_PIPELINE); + testApiAccess(POST, "/application/v4/tenant/othertenant/application/myapp/environment/prod/region/myregion/instance/default/deploy", + List.of(HOSTED_OPERATOR), forbiddenIncludingPipeline, filter); + } @Test @@ -155,13 +166,13 @@ public class ControllerAuthorizationFilterTest { private static void assertIsForbidden(Optional<AuthorizationResponse> response) { assertTrue("Expected a response from filter", response.isPresent()); - assertEquals("Invalid status code", response.get().statusCode, FORBIDDEN); + assertEquals("Invalid status code", FORBIDDEN, response.get().statusCode); } private static ControllerAuthorizationFilter createFilter(ControllerTester controllerTester) { return new ControllerAuthorizationFilter(new AthenzClientFactoryMock(controllerTester.athenzDb()), - controllerTester.controller().tenants(), - singleton("http://localhost")); + controllerTester.controller(), + Set.of("http://localhost")); } private static Optional<AuthorizationResponse> invokeFilter(ControllerAuthorizationFilter filter, diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java new file mode 100644 index 00000000000..b7c751638c8 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/PathGroupTest.java @@ -0,0 +1,55 @@ +package com.yahoo.vespa.hosted.controller.role; + +import org.junit.Test; + +import java.util.LinkedHashSet; +import java.util.Set; + +import static org.junit.Assert.fail; + +/** + * @author jonmv + * @author mpolden + */ +public class PathGroupTest { + + @Test + public void nonOverlappingGroups() { + for (PathGroup pg : PathGroup.all()) { + for (PathGroup pg2 : PathGroup.all()) { + if (pg == pg2) continue; + Set<String> overlapping = new LinkedHashSet<>(pg.pathSpecs); + overlapping.retainAll(pg2.pathSpecs); + if (!overlapping.isEmpty()) { + fail("The following path specs overlap in " + pg + " and " + pg2 + ": " + overlapping); + } + } + } + } + + @Test + public void uniqueMatches() { + // Ensure that each path group contains at most one match for any given path, to avoid undefined context extraction. + for (PathGroup group : PathGroup.values()) + for (String path1 : group.pathSpecs) + for (String path2 : group.pathSpecs) { + if (path1 == path2) continue; + + String[] parts1 = path1.split("/"); + String[] parts2 = path2.split("/"); + + int end = Math.min(parts1.length, parts2.length); + if (end < parts1.length && ! parts2[end - 1].equals("{*}") && ! parts1[end].equals("{*}")) continue; + if (end < parts2.length && ! parts1[end - 1].equals("{*}") && ! parts2[end].equals("{*}")) continue; + + int i; + for (i = 0; i < end; i++) + if ( ! parts1[i].equals(parts2[i]) + && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}")) + && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; + + if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap."); + } + } + +} diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java new file mode 100644 index 00000000000..bc810fdb5c5 --- /dev/null +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/role/RoleMembershipTest.java @@ -0,0 +1,87 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.hosted.controller.role; + +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.SystemName; +import com.yahoo.config.provision.TenantName; +import org.junit.Test; + +import java.util.Map; +import java.util.Set; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +/** + * @author mpolden + */ +public class RoleMembershipTest { + + @Test + public void operator_membership() { + RoleMembership roles = new RoleMembership(Map.of(Role.hostedOperator, Set.of(Context.unlimitedIn(SystemName.main)))); + + // Operator actions + assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); + assertTrue(roles.allows(Action.create, "/controller/v1/foo")); + assertTrue(roles.allows(Action.update, "/os/v1/bar")); + assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertTrue(roles.allows(Action.update, "/application/v4/tenant/t2/application/a2")); + } + + @Test + public void tenant_membership() { + RoleMembership roles = new RoleMembership(Map.of(Role.tenantAdmin, + Set.of(Context.limitedTo(TenantName.from("t1"), + ApplicationName.from("a1"), + SystemName.main)))); + + assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); + assertFalse("Deny access to operator API", roles.allows(Action.create, "/controller/v1/foo")); + assertFalse("Deny access to other tenant and app", roles.allows(Action.update, "/application/v4/tenant/t2/application/a2")); + assertFalse("Deny access to other app", roles.allows(Action.update, "/application/v4/tenant/t1/application/a2")); + assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + + RoleMembership multiContext = new RoleMembership(Map.of(Role.tenantAdmin, + Set.of(Context.limitedTo(TenantName.from("t1"), + ApplicationName.from("a1"), + SystemName.main), + Context.limitedTo(TenantName.from("t2"), + ApplicationName.from("a2"), + SystemName.main)))); + assertFalse("Deny access to other tenant and app", multiContext.allows(Action.update, "/application/v4/tenant/t3/application/a3")); + assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t2/application/a2")); + assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + + RoleMembership publicSystem = new RoleMembership(Map.of(Role.tenantAdmin, + Set.of(Context.limitedTo(TenantName.from("t1"), + ApplicationName.from("a1"), + SystemName.vaas)))); + assertFalse(publicSystem.allows(Action.read, "/controller/v1/foo")); + assertTrue(multiContext.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + } + + @Test + public void build_service_membership() { + RoleMembership roles = new RoleMembership(Map.of(Role.tenantPipelineOperator, Set.of(Context.unlimitedIn(SystemName.main)))); + assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); + assertFalse(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertTrue(roles.allows(Action.create, "/application/v4/tenant/t1/application/a1/jobreport")); + assertFalse("No global read access", roles.allows(Action.read, "/controller/v1/foo")); + } + + @Test + public void multi_role_membership() { + RoleMembership roles = new RoleMembership(Map.of(Role.tenantAdmin, Set.of(Context.limitedTo(TenantName.from("t1"), + ApplicationName.from("a1"), + SystemName.main)), + Role.tenantPipelineOperator, Set.of(Context.unlimitedIn(SystemName.main)), + Role.everyone, Set.of(Context.unlimitedIn(SystemName.main)))); + assertFalse(roles.allows(Action.create, "/not/explicitly/defined")); + assertFalse(roles.allows(Action.create, "/controller/v1/foo")); + assertTrue(roles.allows(Action.create, "/application/v4/tenant/t1/application/a1/jobreport")); + assertTrue(roles.allows(Action.update, "/application/v4/tenant/t1/application/a1")); + assertTrue("Global read access", roles.allows(Action.read, "/controller/v1/foo")); + } + +} diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java index eafe17153ad..b565ad374ed 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/cors/CorsRequestFilterBase.java @@ -5,7 +5,6 @@ import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.security.base.JsonSecurityRequestFilterBase; -import java.util.HashSet; import java.util.Optional; import java.util.Set; @@ -21,10 +20,6 @@ public abstract class CorsRequestFilterBase extends JsonSecurityRequestFilterBas private final Set<String> allowedUrls; - protected CorsRequestFilterBase(CorsFilterConfig config) { - this(new HashSet<>(config.allowedUrls())); - } - protected CorsRequestFilterBase(Set<String> allowedUrls) { this.allowedUrls = allowedUrls; } |