extend self serve functionality (AthenZ#919)

Co-authored-by: Henry Avetisyan <[email protected]>
j3lli21 · Mar 27, 2020 · 9d1f0f0 · 9d1f0f0
1 parent 72772d3
commit 9d1f0f0
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 52 deletions.
diff --git a/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java b/servers/zms/src/main/java/com/yahoo/athenz/zms/ZMSImpl.java
@@ -7470,15 +7470,6 @@ boolean isAllowedPutMembershipWithoutApproval(Principal principal, final AthenzD
         return isAllowedPutMembershipAccess(principal, reqDomain, role);
     }
 
-    boolean isAllowedPutMembershipSelfServe(final Principal principal, final Role role, final RoleMember member) {
-
-        if (role.getSelfServe() != Boolean.TRUE) {
-            return false;
-        }
-
-        return principal.getFullName().equals(member.getMemberName());
-    }
-
     boolean isAllowedPutMembership(Principal principal, final AthenzDomain domain, final Role role,
             final RoleMember member) {
 
@@ -7495,10 +7486,11 @@ boolean isAllowedPutMembership(Principal principal, final AthenzDomain domain, f
             member.setApproved(!auditEnabled);
             return true;
 
-        } else if (isAllowedPutMembershipSelfServe(principal, role, member)) {
+        } else if (role.getSelfServe() == Boolean.TRUE) {
 
-            // if the role is self-serve, and users are trying to add themselves, allow it
-            // but with member status set to inactive. It has to be approved by domain admins.
+            // if the role is self-serve then users are allowed to add anyone
+            // since the request must be approved by someone else so we'll allow it
+            // but with member status set to inactive.
 
             member.setActive(false);
             member.setApproved(false);

diff --git a/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSImplTest.java b/servers/zms/src/test/java/com/yahoo/athenz/zms/ZMSImplTest.java
@@ -16593,45 +16593,6 @@ public void testIsAllowedPutMembershipAccess(){
         zms.deleteTopLevelDomain(mockDomRsrcCtx, "testdomain1", auditRef);
     }
 
-    @Test
-    public void testIsAllowedPutMembershipSelfserve(){
-        TopLevelDomain dom1 = createTopLevelDomainObject("testdomain1",
-                "Role Test Domain1", "testOrg", adminUser);
-        zms.postTopLevelDomain(mockDomRsrcCtx, auditRef, dom1);
-
-        Role role1 = createRoleObject("testdomain1", "testrole1", null,"user.john", "user.jane");
-        role1.setSelfServe(true);
-        zms.putRole(mockDomRsrcCtx, "testdomain1", "testrole1", auditRef, role1);
-
-        RoleMeta rm = createRoleMetaObject(true);
-        zms.putRoleMeta(mockDomRsrcCtx, "testdomain1", "testrole1",  auditRef, rm);
-
-        AthenzDomain domain = zms.getAthenzDomain("testdomain1", false);
-        Role role = zms.getRoleFromDomain("testrole1", domain);
-
-        RoleMember roleMember = new RoleMember().setMemberName("user.user1");
-
-        assertFalse(zms.isAllowedPutMembershipAccess(mockDomRestRsrcCtx.principal(), domain, role)); // user.user1 does not have role access
-        assertTrue(zms.isAllowedPutMembership(mockDomRestRsrcCtx.principal(),domain, role, roleMember));// putmembership is allowed for user.user1
-
-        // match role and member
-        assertTrue(zms.isAllowedPutMembershipSelfServe(mockDomRestRsrcCtx.principal(), role, roleMember));
-
-        // role is not self serve
-        Role role2 = new Role().setName("testdomain1:role.role1").setSelfServe(false);
-        assertFalse(zms.isAllowedPutMembershipSelfServe(mockDomRestRsrcCtx.principal(), role2, roleMember));
-
-        // role is not self serve - null attribute
-        Role role3 = new Role().setName("testdomain1:role.role1").setSelfServe(null);
-        assertFalse(zms.isAllowedPutMembershipSelfServe(mockDomRestRsrcCtx.principal(), role3, roleMember));
-
-        // role member does not match
-        RoleMember roleMember1 = new RoleMember().setMemberName("user.user2");
-        assertFalse(zms.isAllowedPutMembershipSelfServe(mockDomRestRsrcCtx.principal(), role, roleMember1));
-
-        zms.deleteTopLevelDomain(mockDomRsrcCtx, "testdomain1", auditRef);
-    }
-
     @Test
     public void testIsAllowedPutMembershipWithoutApproval() {
 
@@ -16701,6 +16662,7 @@ public void testIsAllowedPutMembership() {
         roleMember = new RoleMember().setMemberName("user.bob");
         assertFalse(zms.isAllowedPutMembership(rsrcPrince, domain, role, roleMember));//bob trying to add himself
 
+        // without self-serve bob is not allowed to add dave
         roleMember = new RoleMember().setMemberName("user.dave");
         assertFalse(zms.isAllowedPutMembership(rsrcPrince, domain, role, roleMember));//bob trying to add dave
 
@@ -16717,8 +16679,10 @@ public void testIsAllowedPutMembership() {
         assertTrue(zms.isAllowedPutMembership(rsrcPrince, domain, role, roleMember));//bob trying to add himself
         assertFalse(roleMember.getApproved());
 
+        // with self-serve bob is now allowed to add dave
         roleMember.setMemberName("user.dave");
-        assertFalse(zms.isAllowedPutMembership(rsrcPrince, domain, role, roleMember));//bob trying to add dave
+        assertTrue(zms.isAllowedPutMembership(rsrcPrince, domain, role, roleMember));//bob trying to add dave
+        assertFalse(roleMember.getApproved());
 
         DomainMeta meta = createDomainMetaObject("Domain Meta for Role Meta test", "testOrg",
                 true, true, "12345", 1001);