From cbea8cd9dcc406a938e8c106be90da7f92d822b8 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Tue, 26 Mar 2019 11:19:53 +0100 Subject: Move Principal to RoleResolver.membership signature --- .../filter/ControllerAuthorizationFilter.java | 30 +++++++++++----------- .../hosted/controller/role/RoleMembership.java | 14 +++++++++- .../hosted/controller/role/PathGroupTest.java | 12 ++++----- 3 files changed, 34 insertions(+), 22 deletions(-) (limited to 'controller-server/src') 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 fcd5ec9a64e..982442c028e 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 @@ -34,6 +34,7 @@ import com.yahoo.yolean.chain.Provides; import javax.ws.rs.ForbiddenException; import javax.ws.rs.InternalServerErrorException; import javax.ws.rs.WebApplicationException; +import java.security.Principal; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -74,10 +75,10 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { public Optional filterRequest(DiscFilterRequest request) { try { Path path = new Path(request.getRequestURI()); - Optional principal = principalFrom(request); Action action = Action.from(HttpRequest.Method.valueOf(request.getMethod())); - AthenzRoleResolver resolver = new AthenzRoleResolver(principal, athenz, controller, path); - RoleMembership roles = resolver.membership(); + RoleMembership roles = Optional.ofNullable(request.getUserPrincipal()) + .map(new AthenzRoleResolver(athenz, controller, path)::membership) + .orElse(RoleMembership.everyone()); if (!roles.allows(action, request.getRequestURI())) { throw new ForbiddenException("Access denied"); } @@ -93,14 +94,12 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { // TODO: Pull class up and resolve roles from roles defined in Athenz private static class AthenzRoleResolver implements RoleMembership.Resolver { - private final Optional principal; private final AthenzFacade athenz; private final TenantController tenants; private final Path path; private final SystemName system; - public AthenzRoleResolver(Optional principal, AthenzFacade athenz, Controller controller, Path path) { - this.principal = principal; + public AthenzRoleResolver(AthenzFacade athenz, Controller controller, Path path) { this.athenz = athenz; this.tenants = controller.tenants(); this.path = path; @@ -136,25 +135,26 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { } @Override - public RoleMembership membership() { + public RoleMembership membership(Principal principal) { + if ( ! (principal instanceof AthenzPrincipal)) + throw new IllegalStateException("Expected an AthenzPrincipal to be set on the request."); + + AthenzIdentity identity = ((AthenzPrincipal) principal).getIdentity(); Optional tenant = tenant(path); Context context = context(tenant); Set contexts = Set.of(context); - if (principal.isEmpty()) { - return new RoleMembership(Map.of(Role.everyone, contexts)); - } - if (isHostedOperator(principal.get().getIdentity())) { + if (isHostedOperator(identity)) { return new RoleMembership(Map.of(Role.hostedOperator, contexts)); } - if (tenant.isPresent() && isTenantAdmin(principal.get().getIdentity(), tenant.get())) { + if (tenant.isPresent() && isTenantAdmin(identity, tenant.get())) { return new RoleMembership(Map.of(Role.tenantAdmin, contexts)); } - AthenzDomain principalDomain = principal.get().getIdentity().getDomain(); + 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(principal.get().getIdentity(), tenantDomain, context.application().get())) { + if (hasDeployerAccess(identity, tenantDomain, context.application().get())) { return new RoleMembership(Map.of(Role.tenantPipelineOperator, contexts)); } } else { @@ -173,7 +173,7 @@ public class ControllerAuthorizationFilter extends CorsRequestFilterBase { // TODO: Currently there's only one context for each role, but this will change private Context context(Optional tenant) { - if (principal.isEmpty() || tenant.isEmpty()) { + if (tenant.isEmpty()) { return Context.unlimitedIn(system); } // TODO: Remove this. Current behaviour always allows tenant full access to all its applications, but with 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 index 588aac4e3ad..d3940ac631f 100644 --- 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 @@ -1,8 +1,13 @@ // 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 @@ -12,12 +17,19 @@ import java.util.Set; */ 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> roles; public RoleMembership(Map> 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 -> { @@ -37,7 +49,7 @@ public class RoleMembership { * membership to a {@link RoleMembership}. */ public interface Resolver { - RoleMembership membership(); + RoleMembership membership(Principal user); } } 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 index a4d685dfb1e..b7c751638c8 100644 --- 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 @@ -5,6 +5,8 @@ import org.junit.Test; import java.util.LinkedHashSet; import java.util.Set; +import static org.junit.Assert.fail; + /** * @author jonmv * @author mpolden @@ -19,8 +21,7 @@ public class PathGroupTest { Set overlapping = new LinkedHashSet<>(pg.pathSpecs); overlapping.retainAll(pg2.pathSpecs); if (!overlapping.isEmpty()) { - throw new AssertionError("The following path specs overlap in " + pg + " and " + pg2 + - ": " + overlapping); + fail("The following path specs overlap in " + pg + " and " + pg2 + ": " + overlapping); } } } @@ -29,7 +30,7 @@ public class PathGroupTest { @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 (PathGroup group : PathGroup.values()) for (String path1 : group.pathSpecs) for (String path2 : group.pathSpecs) { if (path1 == path2) continue; @@ -43,13 +44,12 @@ public class PathGroupTest { int i; for (i = 0; i < end; i++) - if ( ! parts1[i].equals(parts2[i]) + if ( ! parts1[i].equals(parts2[i]) && ! (parts1[i].startsWith("{") && parts1[i].endsWith("}")) && ! (parts2[i].startsWith("{") && parts2[i].endsWith("}"))) break; - if (i == end) throw new AssertionError("Paths '" + path1 + "' and '" + path2 +"' overlap."); + if (i == end) fail("Paths '" + path1 + "' and '" + path2 + "' overlap."); } - } } } -- cgit v1.2.3