Merge pull request #9059 from vespa-engine/jvenstad/user-management

Jvenstad/user management

6 files changed, 118 insertions, 24 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
index 7419466cffc..7c182a3c0b4 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
@@ -29,17 +29,17 @@ public class UserRoles {
 
     /** Returns the list of {@link TenantRole}s a {@link UserId} may be a member of. */
     public List<TenantRole> tenantRoles(TenantName tenant) {
-        return List.of(roles.tenantOperator(tenant),
+        return List.of(roles.tenantOwner(tenant),
                        roles.tenantAdmin(tenant),
-                       roles.tenantOwner(tenant));
+                       roles.tenantOperator(tenant));
     }
 
     /** Returns the list of {@link ApplicationRole}s a {@link UserId} may be a member of. */
     public List<ApplicationRole> applicationRoles(TenantName tenant, ApplicationName application) {
-        return List.of(roles.applicationReader(tenant, application),
-                       roles.applicationDeveloper(tenant, application),
+        return List.of(roles.applicationAdmin(tenant, application),
                        roles.applicationOperator(tenant, application),
-                       roles.applicationAdmin(tenant, application));
+                       roles.applicationDeveloper(tenant, application),
+                       roles.applicationReader(tenant, application));
     }
 
     public List<UnboundRole> hostedOperator() {
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
index edf3f4e8711..797ca10ed3d 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/PathGroup.java
@@ -27,9 +27,6 @@ public enum PathGroup {
              "/provision/v2/{*}",
              "/zone/v2/{*}"),
 
-    /** Paths used for user management. */
-    userManagement("/user/v1/{*}"), // TODO probably add tenant and application levels.
-
     /** Paths used for creating user tenants. */
     user("/application/v4/user"),
 
@@ -37,6 +34,10 @@ public enum PathGroup {
     tenant(Matcher.tenant,
            "/application/v4/tenant/{tenant}"),
 
+    /** Paths used for user management on the tenant level. */
+    tenantUsers(Matcher.tenant,
+                "/user/v1/tenant/{tenant}"),
+
     /** Paths used by tenant administrators. */
     tenantInfo(Matcher.tenant,
                "/application/v4/tenant/{tenant}/application/"),
@@ -46,6 +47,11 @@ public enum PathGroup {
                 Matcher.application,
                 "/application/v4/tenant/{tenant}/application/{application}"),
 
+    /** Paths used for user management on the application level. */
+    applicationUsers(Matcher.tenant,
+                     Matcher.application,
+                     "/user/v1/tenant/{tenant}/application/{application}"),
+
     /** Paths used by application administrators. */
     applicationInfo(Matcher.tenant,
                     Matcher.application,
@@ -98,8 +104,7 @@ public enum PathGroup {
                    "/application/v4/tenant/",
                    "/",
                    "/d/{*}",
-                   "/statuspage/v1/{*}"
-    ),
+                   "/statuspage/v1/{*}"),
 
     /** Paths providing public information. */
     publicInfo("/badge/v1/{*}",
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
index 970717b14a3..db4cca20b9a 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java
@@ -23,10 +23,15 @@ public enum Policy {
                       .on(PathGroup.all())
                       .in(SystemName.all())),
 
-    /** Full access to user management in select systems. */
-    manager(Privilege.grant(Action.all())
-                     .on(PathGroup.userManagement)
-                     .in(SystemName.Public)),
+    /** Full access to user management for a tenant in select systems. */
+    tenantManager(Privilege.grant(Action.all())
+                           .on(PathGroup.tenantUsers)
+                           .in(SystemName.all())),
+
+    /** Full access to user management for an application in select systems. */
+    applicationManager(Privilege.grant(Action.all())
+                                .on(PathGroup.applicationUsers)
+                                .in(SystemName.all())),
 
     /** Access to create a user tenant in select systems. */
     userCreate(Privilege.grant(Action.update)
@@ -50,7 +55,7 @@ public enum Policy {
 
     /** Read access to tenant information and settings. */
     tenantRead(Privilege.grant(Action.read)
-                        .on(PathGroup.tenant, PathGroup.tenantInfo)
+                        .on(PathGroup.tenant, PathGroup.tenantInfo, PathGroup.tenantUsers, PathGroup.applicationUsers)
                         .in(SystemName.all())),
 
     /** Access to create application under a certain tenant. */
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
index 86d59b4bbb6..ff535e92033 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Role.java
@@ -30,6 +30,16 @@ public abstract class Role {
         return roleDefinition.policies().stream().anyMatch(policy -> policy.evaluate(action, uri, context));
     }
 
+    /** Returns whether the other role is a parent of this, and has a context included in this role's context. */
+    public boolean implies(Role other) {
+        if ( ! context.system().equals(other.context.system()))
+            throw new IllegalStateException("Coexisting roles should always be in the same system.");
+
+        return    (context.tenant().isEmpty() || context.tenant().equals(other.context.tenant()))
+               && (context.application().isEmpty() || context.application().equals(other.context.application()))
+               && roleDefinition.inherited().contains(other.roleDefinition);
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
index ac9fdaaa339..af068decc83 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/RoleDefinition.java
@@ -1,6 +1,7 @@
 package com.yahoo.vespa.hosted.controller.api.role;
 
 import java.util.EnumSet;
+import java.util.HashSet;
 import java.util.Set;
 
 /**
@@ -40,23 +41,28 @@ public enum RoleDefinition {
                          Policy.developmentDeployment),
 
     /** Application operator with access to normal, operational tasks of an application. */
-    applicationOperator(applicationDeveloper,
+    applicationOperator(applicationReader,
                         Policy.applicationOperations),
 
     /** Application administrator with full access to an already existing application, including emergency operations. */
-    applicationAdmin(applicationOperator,
+    applicationAdmin(applicationDeveloper,
+                     applicationOperator,
                      Policy.applicationUpdate,
+                     Policy.applicationDelete,
+                     Policy.applicationManager,
                      Policy.productionDeployment,
                      Policy.submission),
 
-    /** Tenant operator with admin access to all applications under the tenant, as well as the ability to create applications. */
-    tenantOperator(applicationAdmin,
+    /** Tenant operator with access to create application under a tenant, and to read the tenant's and public data. */
+    tenantOperator(everyone,
+                   Policy.tenantRead,
                    Policy.applicationCreate),
 
     /** Tenant admin with full access to all tenant resources, except deleting the tenant. */
     tenantAdmin(tenantOperator,
+                applicationAdmin,
                 Policy.applicationDelete,
-                Policy.manager,
+                Policy.tenantManager,
                 Policy.tenantUpdate),
 
     /** Tenant admin with full access to all tenant resources. */
@@ -80,17 +86,36 @@ public enum RoleDefinition {
                       Policy.applicationOperations,
                       Policy.developmentDeployment);
 
+    private final Set<RoleDefinition> parents;
     private final Set<Policy> policies;
 
     RoleDefinition(Policy... policies) {
-        this.policies = EnumSet.copyOf(Set.of(policies));
+        this(Set.of(), policies);
+    }
+
+    RoleDefinition(RoleDefinition first, Policy... policies) {
+        this(Set.of(first), policies);
+    }
+
+    RoleDefinition(RoleDefinition first, RoleDefinition second, Policy... policies) {
+        this(Set.of(first, second), policies);
     }
 
-    RoleDefinition(RoleDefinition inherited, Policy... policies) {
+    RoleDefinition(Set<RoleDefinition> parents, Policy... policies) {
+        this.parents = new HashSet<>(parents);
         this.policies = EnumSet.copyOf(Set.of(policies));
-        this.policies.addAll(inherited.policies);
+        for (RoleDefinition parent : parents) {
+            this.parents.addAll(parent.parents);
+            this.policies.addAll(parent.policies);
+        }
+    }
+
+    Set<Policy> policies() {
+        return policies;
     }
 
-    Set<Policy> policies() { return policies; }
+    Set<RoleDefinition> inherited() {
+        return parents;
+    }
 
 }
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
index 1badd157b1b..6cfe01cfb77 100644
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/role/RoleTest.java
@@ -51,4 +51,53 @@ public class RoleTest {
         assertFalse("No global read access", role.allows(Action.read, URI.create("/controller/v1/foo")));
     }
 
+    @Test
+    public void implications() {
+        Roles roles = new Roles(SystemName.main);
+        TenantName tenant1 = TenantName.from("t1");
+        ApplicationName application1 = ApplicationName.from("a1");
+        TenantName tenant2 = TenantName.from("t2");
+        ApplicationName application2 = ApplicationName.from("a2");
+
+        Role tenantOwner1 = roles.tenantOwner(tenant1);
+        Role tenantAdmin1 = roles.tenantAdmin(tenant1);
+        Role tenantAdmin2 = roles.tenantAdmin(tenant2);
+        Role tenantOperator1 = roles.tenantOperator(tenant1);
+        Role applicationAdmin11 = roles.applicationAdmin(tenant1, application1);
+        Role applicationOperator11 = roles.applicationOperator(tenant1, application1);
+        Role applicationDeveloper11 = roles.applicationDeveloper(tenant1, application1);
+        Role applicationReader11 = roles.applicationReader(tenant1, application1);
+        Role applicationReader12 = roles.applicationReader(tenant1, application2);
+        Role applicationReader22 = roles.applicationReader(tenant2, application2);
+
+        assertFalse(tenantOwner1.implies(tenantOwner1));
+        assertTrue(tenantOwner1.implies(tenantAdmin1));
+        assertFalse(tenantOwner1.implies(tenantAdmin2));
+        assertTrue(tenantOwner1.implies(tenantOperator1));
+        assertTrue(tenantOwner1.implies(applicationAdmin11));
+        assertTrue(tenantOwner1.implies(applicationReader11));
+        assertTrue(tenantOwner1.implies(applicationReader12));
+        assertFalse(tenantOwner1.implies(applicationReader22));
+
+        assertFalse(tenantAdmin1.implies(tenantOwner1));
+        assertFalse(tenantAdmin1.implies(tenantAdmin2));
+        assertTrue(tenantAdmin1.implies(applicationDeveloper11));
+
+        assertFalse(tenantOperator1.implies(applicationReader11));
+
+        assertFalse(applicationAdmin11.implies(tenantAdmin1));
+        assertFalse(applicationAdmin11.implies(tenantOperator1));
+        assertTrue(applicationAdmin11.implies(applicationOperator11));
+        assertTrue(applicationAdmin11.implies(applicationDeveloper11));
+        assertTrue(applicationAdmin11.implies(applicationReader11));
+        assertFalse(applicationAdmin11.implies(applicationReader12));
+        assertFalse(applicationAdmin11.implies(applicationReader22));
+
+        assertFalse(applicationOperator11.implies(applicationDeveloper11));
+        assertTrue(applicationOperator11.implies(applicationReader11));
+
+        assertFalse(applicationDeveloper11.implies(applicationOperator11));
+        assertTrue(applicationDeveloper11.implies(applicationReader11));
+    }
+
 }