email notifications improvements with notify roles and groups

core/zms/src/main/java/com/yahoo/athenz/zms/GroupMember.java

@@ -54,6 +54,9 @@ public class GroupMember {
     @RdlOptional
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
     public String pendingState;
+    @RdlOptional
+    @JsonInclude(JsonInclude.Include.NON_EMPTY)
+    public String notifyRoles;
 
     public GroupMember setMemberName(String memberName) {
         this.memberName = memberName;
@@ -153,6 +156,13 @@ public GroupMember setPendingState(String pendingState) {
     public String getPendingState() {
         return pendingState;
     }
+    public GroupMember setNotifyRoles(String notifyRoles) {
+        this.notifyRoles = notifyRoles;
+        return this;
+    }
+    public String getNotifyRoles() {
+        return notifyRoles;
+    }
 
     @Override
     public boolean equals(Object another) {
@@ -203,6 +213,9 @@ public boolean equals(Object another) {
             if (pendingState == null ? a.pendingState != null : !pendingState.equals(a.pendingState)) {
                 return false;
             }
+            if (notifyRoles == null ? a.notifyRoles != null : !notifyRoles.equals(a.notifyRoles)) {
+                return false;
+            }
         }
         return true;
     }

core/zms/src/main/java/com/yahoo/athenz/zms/MemberRole.java

@@ -46,6 +46,9 @@ public class MemberRole {
     @RdlOptional
     @JsonInclude(JsonInclude.Include.NON_EMPTY)
     public String trustRoleName;
+    @RdlOptional
+    @JsonInclude(JsonInclude.Include.NON_EMPTY)
+    public String notifyRoles;
 
     public MemberRole setRoleName(String roleName) {
         this.roleName = roleName;
@@ -131,6 +134,13 @@ public MemberRole setTrustRoleName(String trustRoleName) {
     public String getTrustRoleName() {
         return trustRoleName;
     }
+    public MemberRole setNotifyRoles(String notifyRoles) {
+        this.notifyRoles = notifyRoles;
+        return this;
+    }
+    public String getNotifyRoles() {
+        return notifyRoles;
+    }
 
     @Override
     public boolean equals(Object another) {
@@ -175,6 +185,9 @@ public boolean equals(Object another) {
             if (trustRoleName == null ? a.trustRoleName != null : !trustRoleName.equals(a.trustRoleName)) {
                 return false;
             }
+            if (notifyRoles == null ? a.notifyRoles != null : !notifyRoles.equals(a.notifyRoles)) {
+                return false;
+            }
         }
         return true;
     }

core/zms/src/main/java/com/yahoo/athenz/zms/ZMSSchema.java

@@ -284,7 +284,8 @@ private static Schema build() {
             .field("requestTime", "Timestamp", true, "for pending membership requests, the request time")
             .field("systemDisabled", "Int32", true, "user disabled by system based on configured role setting")
             .field("pendingState", "String", true, "for pending membership requests, the request state - e.g. add, delete")
-            .field("trustRoleName", "ResourceName", true, "name of the role that handles the membership delegation for the role specified in roleName");
+            .field("trustRoleName", "ResourceName", true, "name of the role that handles the membership delegation for the role specified in roleName")
+            .field("notifyRoles", "String", true, "list of roles whose members should be notified for member review/approval/expiry");
 
         sb.structType("DomainRoleMember")
             .field("memberName", "MemberName", false, "name of the member")
@@ -521,7 +522,8 @@ private static Schema build() {
             .field("reviewLastNotifiedTime", "Timestamp", true, "for pending membership requests, time when last notification was sent (for file store)")
             .field("systemDisabled", "Int32", true, "user disabled by system based on configured group setting")
             .field("principalType", "Int32", true, "server use only - principal type: unknown(0), user(1) or service(2)")
-            .field("pendingState", "String", true, "for pending membership requests, the request state - e.g. add, delete");
+            .field("pendingState", "String", true, "for pending membership requests, the request state - e.g. add, delete")
+            .field("notifyRoles", "String", true, "list of roles whose members should be notified for member review/approval/expiry");
 
         sb.structType("GroupMembership")
             .comment("The representation for a group membership.")

core/zms/src/main/rdl/Group.tdl

@@ -27,6 +27,7 @@ type GroupMember Struct {
     Int32 systemDisabled (optional); //user disabled by system based on configured group setting
     Int32 principalType (optional); //server use only - principal type: unknown(0), user(1) or service(2)
     String pendingState (optional); //for pending membership requests, the request state - e.g. add, delete
+    String notifyRoles (optional); //list of roles whose members should be notified for member review/approval/expiry
 }
 
 //The representation for a group membership.

core/zms/src/main/rdl/Role.tdl

@@ -123,6 +123,7 @@ type MemberRole Struct {
     Int32 systemDisabled (optional); //user disabled by system based on configured role setting
     String pendingState (optional); //for pending membership requests, the request state - e.g. add, delete
     ResourceName trustRoleName (optional); //name of the role that handles the membership delegation for the role specified in roleName
+    String notifyRoles (optional); //list of roles whose members should be notified for member review/approval/expiry
 }
 
 type DomainRoleMember Struct {

core/zms/src/test/java/com/yahoo/athenz/zms/GroupTest.java

@@ -275,7 +275,8 @@ public void testGroupMember() {
                 .setReviewLastNotifiedTime(Timestamp.fromMillis(123456789127L))
                 .setSystemDisabled(1)
                 .setPrincipalType(1)
-                .setPendingState("ADD");
+                .setPendingState("ADD")
+                .setNotifyRoles("role1,role2");
 
         assertEquals(rm, rm);
         assertNotEquals("data", rm);
@@ -298,6 +299,7 @@ public void testGroupMember() {
         assertEquals(rm.getSystemDisabled(), Integer.valueOf(1));
         assertEquals(rm.getPrincipalType(), Integer.valueOf(1));
         assertEquals(rm.getPendingState(), "ADD");
+        assertEquals(rm.getNotifyRoles(), "role1,role2");
 
         GroupMember rm2 = new GroupMember()
                 .setGroupName("group1")
@@ -313,7 +315,8 @@ public void testGroupMember() {
                 .setReviewLastNotifiedTime(Timestamp.fromMillis(123456789127L))
                 .setSystemDisabled(1)
                 .setPrincipalType(1)
-                .setPendingState("ADD");
+                .setPendingState("ADD")
+                .setNotifyRoles("role1,role2");
         assertEquals(rm, rm2);
 
         rm2.setRequestPrincipal("user.test2");
@@ -414,6 +417,13 @@ public void testGroupMember() {
         rm2.setPrincipalType(1);
         assertEquals(rm, rm2);
 
+        rm2.setNotifyRoles("role2,role3");
+        assertNotEquals(rm, rm2);
+        rm2.setNotifyRoles(null);
+        assertNotEquals(rm, rm2);
+        rm2.setNotifyRoles("role1,role2");
+        assertEquals(rm, rm2);
+
         assertNotEquals(rm2, null);
 
         GroupMember rm3 = new GroupMember();

core/zms/src/test/java/com/yahoo/athenz/zms/MemberRoleTest.java

@@ -39,6 +39,7 @@ public void testMemberRole() {
         mbr1.setReviewReminder(Timestamp.fromMillis(100));
         mbr1.setPendingState("ADD");
         mbr1.setTrustRoleName("domain:role.trust");
+        mbr1.setNotifyRoles("role1,role2");
 
         assertEquals("role1", mbr1.getRoleName());
         assertEquals(Timestamp.fromMillis(100), mbr1.getExpiration());
@@ -52,6 +53,7 @@ public void testMemberRole() {
         assertEquals(Timestamp.fromMillis(100), mbr1.getReviewReminder());
         assertEquals(mbr1.getPendingState(), "ADD");
         assertEquals(mbr1.getTrustRoleName(), "domain:role.trust");
+        assertEquals(mbr1.getNotifyRoles(), "role1,role2");
 
         assertEquals(mbr1, mbr1);
         assertNotEquals(null, mbr1);
@@ -69,7 +71,8 @@ public void testMemberRole() {
                 .setSystemDisabled(1)
                 .setReviewReminder(Timestamp.fromMillis(100))
                 .setPendingState("ADD")
-                .setTrustRoleName("domain:role.trust");
+                .setTrustRoleName("domain:role.trust")
+                .setNotifyRoles("role1,role2");
 
         assertEquals(mbr1, mbr2);
 
@@ -156,6 +159,13 @@ public void testMemberRole() {
         assertNotEquals(mbr1, mbr2);
         mbr2.setTrustRoleName("domain:role.trust");
         assertEquals(mbr1, mbr2);
+
+        mbr2.setNotifyRoles("role2,role3");
+        assertNotEquals(mbr1, mbr2);
+        mbr2.setNotifyRoles(null);
+        assertNotEquals(mbr1, mbr2);
+        mbr2.setNotifyRoles("role1,role2");
+        assertEquals(mbr1, mbr2);
     }
 
 }

libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/notification/DomainRoleMembersFetcher.java

@@ -19,6 +19,7 @@
 import com.yahoo.athenz.auth.AuthorityConsts;
 import com.yahoo.athenz.common.server.db.RolesProvider;
 
+import com.yahoo.athenz.common.server.util.ResourceUtils;
 import com.yahoo.athenz.zms.Role;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -49,21 +50,33 @@ public Set<String> getDomainRoleMembers(String domainName, String roleName) {
         // provider does not support this interface then we're going
         // fall back to the old method of getting the role members
 
-        try {
-            // our given role name is the full arn, so first we need to
-            // extract the local role component from the role name
+        // if our given role name is the full arn, so first we need to
+        // extract the local role component from the role name
+
+        String roleLocalName;
+        int idx = roleName.indexOf(AuthorityConsts.ROLE_SEP);
+        if (idx == -1) {
+            roleLocalName = roleName;
+        } else {
+            roleLocalName = roleName.substring(idx + AuthorityConsts.ROLE_SEP.length());
+        }
 
-            int idx = roleName.indexOf(AuthorityConsts.ROLE_SEP);
-            Role role = rolesProvider.getRole(domainName, roleName.substring(idx + AuthorityConsts.ROLE_SEP.length()),
-                    Boolean.FALSE, Boolean.TRUE, Boolean.FALSE);
+        try {
+            Role role = rolesProvider.getRole(domainName, roleLocalName, Boolean.FALSE, Boolean.TRUE, Boolean.FALSE);
             return domainRoleMembersFetcherCommon.getDomainRoleMembers(role);
         } catch (Exception ex) {
             if (ex instanceof UnsupportedOperationException) {
-                return domainRoleMembersFetcherCommon.getDomainRoleMembers(roleName,
+                String roleFullName;
+                if (idx == -1) {
+                    roleFullName = ResourceUtils.roleResourceName(domainName, roleName);
+                } else {
+                    roleFullName = roleName;
+                }
+                return domainRoleMembersFetcherCommon.getDomainRoleMembers(roleFullName,
                         rolesProvider.getRolesByDomain(domainName));
             }
             LOGGER.error("unable to fetch members for role: {} in domain: {} error: {}",
-                    roleName, domainName, ex.getMessage());
+                    roleName, domainName, ex.getMessage(), ex);
             return new HashSet<>();
         }
     }

libs/java/server_common/src/main/java/com/yahoo/athenz/common/server/notification/NotificationCommon.java

@@ -19,7 +19,6 @@
 import com.yahoo.athenz.auth.AuthorityConsts;
 import com.yahoo.athenz.auth.util.AthenzUtils;
 import com.yahoo.athenz.common.ServerCommonConsts;
-import com.yahoo.athenz.common.server.util.ResourceUtils;
 import com.yahoo.athenz.zms.ResourceException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -114,15 +113,16 @@ void addNotificationRecipient(Notification notification, final String recipient,
 
         int roleDomainIndex = recipient.indexOf(AuthorityConsts.ROLE_SEP);
         if (roleDomainIndex != -1) {
-            addDomainRoleRecipients(notification, recipient.substring(0, roleDomainIndex), recipient);
+            addDomainRoleRecipients(notification, recipient.substring(0, roleDomainIndex),
+                    recipient.substring(roleDomainIndex + AuthorityConsts.ROLE_SEP.length()));
         } else if (recipient.contains(AuthorityConsts.GROUP_SEP)) {
             // Do nothing. Group members will not get individual notifications.
         } else if (recipient.startsWith(userDomainPrefix)) {
             notification.addRecipient(recipient);
         } else if (!ignoreService) {
             final String domainName = AthenzUtils.extractPrincipalDomainName(recipient);
             if (domainName != null) {
-                addDomainRoleRecipients(notification, domainName, ResourceUtils.roleResourceName(domainName, ServerCommonConsts.ADMIN_ROLE_NAME));
+                addDomainRoleRecipients(notification, domainName, ServerCommonConsts.ADMIN_ROLE_NAME);
             }
         }
     }

libs/java/server_common/src/test/java/com/yahoo/athenz/common/server/notification/DomainRoleMembersFetcherCommonTest.java

@@ -132,7 +132,7 @@ public Role getRole(String domainName, String roleName, Boolean auditLog, Boolea
     public void testDomainRoleMembersFetcherNotImpl() {
 
         Role role1 = new Role();
-        role1.setName("role1");
+        role1.setName("domain1:role.role1");
         List<RoleMember> role1MemberList = Collections.singletonList(new RoleMember().setMemberName("user.user1"));
         role1.setRoleMembers(role1MemberList);
 

servers/zms/src/main/java/com/yahoo/athenz/zms/notification/GroupMemberExpiryNotificationTask.java

@@ -22,6 +22,7 @@
 import com.yahoo.athenz.zms.*;
 import com.yahoo.athenz.zms.utils.ZMSUtils;
 import com.yahoo.rdl.Timestamp;
+import org.eclipse.jetty.util.StringUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -365,8 +366,7 @@ Map<String, DomainGroupMember> consolidateGroupMembers(Map<String, DomainGroupMe
             } else {
                 // domain role fetcher only returns the human users
 
-                Set<String> domainAdminMembers = domainRoleMembersFetcher.getDomainRoleMembers(domainName,
-                        ResourceUtils.roleResourceName(domainName, ADMIN_ROLE_NAME));
+                Set<String> domainAdminMembers = domainRoleMembersFetcher.getDomainRoleMembers(domainName, ADMIN_ROLE_NAME);
                 if (ZMSUtils.isCollectionEmpty(domainAdminMembers)) {
                     continue;
                 }
@@ -383,22 +383,42 @@ Map<String, DomainGroupMember> consolidateDomainAdmins(Map<String, List<GroupMem
 
         Map<String, DomainGroupMember> consolidatedDomainAdmins = new HashMap<>();
 
-        // iterate through each principal. if the principal is:
-        // user -> as the roles to the list
-        // service -> lookup domain admins for the service and add to the individual human users only
-        // group -> skip
+        // iterate through each domain and the groups within each domain.
+        // if the group does not have the notify roles setup, then we'll
+        // add the notifications to the domain admins otherwise we'll
+        // add it to the configured notify roles members only
 
         for (String domainName : domainGroupMembers.keySet()) {
 
-            // domain role fetcher only returns the human users
-
-            Set<String> domainAdminMembers = domainRoleMembersFetcher.getDomainRoleMembers(domainName,
-                    ResourceUtils.roleResourceName(domainName, ADMIN_ROLE_NAME));
-            if (ZMSUtils.isCollectionEmpty(domainAdminMembers)) {
+            List<GroupMember> groupMemberList = domainGroupMembers.get(domainName);
+            if (ZMSUtils.isCollectionEmpty(groupMemberList)) {
                 continue;
             }
-            for (String domainAdminMember : domainAdminMembers) {
-                addGroupMembers(domainAdminMember, consolidatedDomainAdmins, domainGroupMembers.get(domainName));
+
+            // domain role fetcher only returns the human users
+
+            Set<String> domainAdminMembers = domainRoleMembersFetcher.getDomainRoleMembers(domainName, ADMIN_ROLE_NAME);
+
+            for (GroupMember groupMember : groupMemberList) {
+
+                // if we have a notify-roles configured then we're going to
+                // extract the list of members from those roles, otherwise
+                // we're going to use the domain admin members
+
+                Set<String> groupAdminMembers;
+                if (!StringUtil.isEmpty(groupMember.getNotifyRoles())) {
+                    groupAdminMembers = NotificationUtils.extractNotifyRoleMembers(domainRoleMembersFetcher,
+                            groupMember.getDomainName(), groupMember.getNotifyRoles());
+                } else {
+                    groupAdminMembers = domainAdminMembers;
+                }
+
+                if (ZMSUtils.isCollectionEmpty(groupAdminMembers)) {
+                    continue;
+                }
+                for (String groupAdminMember : groupAdminMembers) {
+                    addGroupMembers(groupAdminMember, consolidatedDomainAdmins, Collections.singletonList(groupMember));
+                }
             }
         }