include configured role/group notify details in expiry notification e…

…mails (#2807) Signed-off-by: Henry Avetisyan <[email protected]>
AthenZ · Nov 26, 2024 · 4924f9a · 4924f9a
1 parent ef2a5b0
commit 4924f9a
Show file tree

Hide file tree

Showing 14 changed files with 103 additions and 68 deletions.
diff --git a/.../java/com/yahoo/athenz/common/server/notification/NotificationToEmailConverterCommon.java b/.../java/com/yahoo/athenz/common/server/notification/NotificationToEmailConverterCommon.java
@@ -24,6 +24,8 @@
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.URL;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.text.MessageFormat;
 import java.util.*;
 import java.util.stream.Collectors;
@@ -223,7 +225,8 @@ void processEntry(StringBuilder body, final String entryNames, final String entr
         }
         String[] entries = entryNames.split("\\|");
         for (String entry : entries) {
-            String[] comps = entry.split(";");
+            String[] comps = entry.split(";", -1);
+            decodeTableComponents(comps);
             if (comps.length != entryLength) {
                 continue;
             }
@@ -237,6 +240,14 @@ void processEntry(StringBuilder body, final String entryNames, final String entr
         }
     }
 
+    void decodeTableComponents(String[] comps) {
+        for (int i = 0; i < comps.length; i++) {
+            if (comps[i].contains("%") || comps[i].contains("+")) {
+                comps[i] = URLDecoder.decode(comps[i], StandardCharsets.UTF_8);
+            }
+        }
+    }
+
     public String getSubject(String propertyName) {
         return RB.getString(propertyName);
     }

diff --git a/...ms/src/main/java/com/yahoo/athenz/zms/notification/GroupMemberExpiryNotificationTask.java b/...ms/src/main/java/com/yahoo/athenz/zms/notification/GroupMemberExpiryNotificationTask.java
@@ -25,6 +25,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 import static com.yahoo.athenz.common.ServerCommonConsts.ADMIN_ROLE_NAME;
@@ -44,7 +46,7 @@ public class GroupMemberExpiryNotificationTask implements NotificationTask {
     private final GroupExpiryDomainNotificationToMetricConverter groupExpiryDomainNotificationToMetricConverter;
     private final GroupExpiryPrincipalNotificationToToMetricConverter groupExpiryPrincipalNotificationToToMetricConverter;
 
-    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "GROUP", "MEMBER", "EXPIRATION" };
+    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "GROUP", "MEMBER", "EXPIRATION", "NOTES" };
 
     public GroupMemberExpiryNotificationTask(DBService dbService, String userDomainPrefix,
             NotificationToEmailConverterCommon notificationToEmailConverterCommon) {
@@ -86,7 +88,9 @@ public StringBuilder getDetailString(GroupMember memberGroup) {
         detailsRow.append(memberGroup.getDomainName()).append(';');
         detailsRow.append(memberGroup.getGroupName()).append(';');
         detailsRow.append(memberGroup.getMemberName()).append(';');
-        detailsRow.append(memberGroup.getExpiration());
+        detailsRow.append(memberGroup.getExpiration()).append(';');
+        detailsRow.append(memberGroup.getNotifyDetails() == null ?
+                "" : URLEncoder.encode(memberGroup.getNotifyDetails(), StandardCharsets.UTF_8));
         return detailsRow;
     }
 

diff --git a/...zms/src/main/java/com/yahoo/athenz/zms/notification/RoleMemberExpiryNotificationTask.java b/...zms/src/main/java/com/yahoo/athenz/zms/notification/RoleMemberExpiryNotificationTask.java
@@ -22,6 +22,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 import static com.yahoo.athenz.common.server.notification.NotificationServiceConstants.*;
@@ -38,7 +40,7 @@ public class RoleMemberExpiryNotificationTask implements NotificationTask {
     private final RoleExpiryDomainNotificationToMetricConverter roleExpiryDomainNotificationToMetricConverter;
     private final RoleExpiryPrincipalNotificationToMetricConverter roleExpiryPrincipalNotificationToMetricConverter;
 
-    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "ROLE", "MEMBER", "EXPIRATION" };
+    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "ROLE", "MEMBER", "EXPIRATION", "NOTES" };
 
     public RoleMemberExpiryNotificationTask(DBService dbService, String userDomainPrefix,
             NotificationToEmailConverterCommon notificationToEmailConverterCommon) {
@@ -79,7 +81,9 @@ public StringBuilder getDetailString(MemberRole memberRole) {
             detailsRow.append(memberRole.getDomainName()).append(';');
             detailsRow.append(memberRole.getRoleName()).append(';');
             detailsRow.append(memberRole.getMemberName()).append(';');
-            detailsRow.append(memberRole.getExpiration());
+            detailsRow.append(memberRole.getExpiration()).append(';');
+            detailsRow.append(memberRole.getNotifyDetails() == null ?
+                    "" : URLEncoder.encode(memberRole.getNotifyDetails(), StandardCharsets.UTF_8));
             return detailsRow;
         }
 

diff --git a/...zms/src/main/java/com/yahoo/athenz/zms/notification/RoleMemberReviewNotificationTask.java b/...zms/src/main/java/com/yahoo/athenz/zms/notification/RoleMemberReviewNotificationTask.java
@@ -22,6 +22,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 import static com.yahoo.athenz.common.server.notification.NotificationServiceConstants.*;
@@ -37,7 +39,7 @@ public class RoleMemberReviewNotificationTask implements NotificationTask {
     private final RoleReviewPrincipalNotificationToMetricConverter roleReviewPrincipalNotificationToMetricConverter;
     private final RoleReviewDomainNotificationToMetricConverter roleReviewDomainNotificationToMetricConverter;
 
-    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "ROLE", "MEMBER", "REVIEW" };
+    private final static String[] TEMPLATE_COLUMN_NAMES = { "DOMAIN", "ROLE", "MEMBER", "REVIEW", "NOTES" };
 
     public RoleMemberReviewNotificationTask(DBService dbService, String userDomainPrefix,
             NotificationToEmailConverterCommon notificationToEmailConverterCommon) {
@@ -82,7 +84,9 @@ public StringBuilder getDetailString(MemberRole memberRole) {
             detailsRow.append(memberRole.getDomainName()).append(';');
             detailsRow.append(memberRole.getRoleName()).append(';');
             detailsRow.append(memberRole.getMemberName()).append(';');
-            detailsRow.append(memberRole.getReviewReminder());
+            detailsRow.append(memberRole.getReviewReminder()).append(';');
+            detailsRow.append(memberRole.getNotifyDetails() == null ?
+                    "" : URLEncoder.encode(memberRole.getNotifyDetails(), StandardCharsets.UTF_8));
             return detailsRow;
         }
 

diff --git a/servers/zms/src/main/resources/messages/domain-group-member-expiry.html b/servers/zms/src/main/resources/messages/domain-group-member-expiry.html
@@ -22,6 +22,7 @@
                         <th class="ch">GROUP</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">EXPIRATION</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/servers/zms/src/main/resources/messages/domain-role-member-expiry.html b/servers/zms/src/main/resources/messages/domain-role-member-expiry.html
@@ -22,6 +22,7 @@
                         <th class="ch">ROLE</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">EXPIRATION</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/servers/zms/src/main/resources/messages/domain-role-member-review.html b/servers/zms/src/main/resources/messages/domain-role-member-review.html
@@ -22,6 +22,7 @@
                         <th class="ch">ROLE</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">REVIEW</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/servers/zms/src/main/resources/messages/group-member-expiry.html b/servers/zms/src/main/resources/messages/group-member-expiry.html
@@ -22,6 +22,7 @@
                         <th class="ch">GROUP</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">EXPIRATION</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/servers/zms/src/main/resources/messages/role-member-expiry.html b/servers/zms/src/main/resources/messages/role-member-expiry.html
@@ -22,6 +22,7 @@
                         <th class="ch">ROLE</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">EXPIRATION</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/servers/zms/src/main/resources/messages/role-member-review.html b/servers/zms/src/main/resources/messages/role-member-review.html
@@ -22,6 +22,7 @@
                         <th class="ch">ROLE</th>
                         <th class="ch">MEMBER</th>
                         <th class="ch">REVIEW</th>
+                        <th class="ch">NOTES</th>
                     </tr>
                 </thead>
                 <tbody></tbody>

diff --git a/...rc/test/java/com/yahoo/athenz/zms/notification/GroupMemberExpiryNotificationTaskTest.java b/...rc/test/java/com/yahoo/athenz/zms/notification/GroupMemberExpiryNotificationTaskTest.java
@@ -142,7 +142,7 @@ public void testSendGroupMemberExpiryReminders() throws ServerResourceException
         // Verify contents of notifications is as expected
         Notification expectedFirstNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedFirstNotification.addRecipient("user.joe");
-        expectedFirstNotification.addDetails(NOTIFICATION_DETAILS_ROLES_LIST, "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z");
+        expectedFirstNotification.addDetails(NOTIFICATION_DETAILS_ROLES_LIST, "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z;");
         expectedFirstNotification.addDetails("member", "user.joe");
         expectedFirstNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryPrincipalNotificationToEmailConverter(
@@ -152,7 +152,7 @@ public void testSendGroupMemberExpiryReminders() throws ServerResourceException
 
         Notification expectedSecondNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedSecondNotification.addRecipient("user.jane");
-        expectedSecondNotification.addDetails(NOTIFICATION_DETAILS_MEMBERS_LIST, "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z");
+        expectedSecondNotification.addDetails(NOTIFICATION_DETAILS_MEMBERS_LIST, "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z;");
         expectedSecondNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryDomainNotificationToEmailConverter(
                         notificationToEmailConverterCommon));
@@ -239,7 +239,7 @@ void testSendGroupMemberExpiryRemindersDisabledOverOneWeekWithTag(final String t
         // Verify contents of notifications is as expected
         Notification expectedFirstNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedFirstNotification.addRecipient("user.joe");
-        expectedFirstNotification.addDetails(NOTIFICATION_DETAILS_ROLES_LIST, "athenz1;group2;user.joe;" + oneDayExpiry);
+        expectedFirstNotification.addDetails(NOTIFICATION_DETAILS_ROLES_LIST, "athenz1;group2;user.joe;" + oneDayExpiry + ";");
         expectedFirstNotification.addDetails("member", "user.joe");
         expectedFirstNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryPrincipalNotificationToEmailConverter(
@@ -249,7 +249,7 @@ void testSendGroupMemberExpiryRemindersDisabledOverOneWeekWithTag(final String t
 
         Notification expectedSecondNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedSecondNotification.addRecipient("user.jane");
-        expectedSecondNotification.addDetails(NOTIFICATION_DETAILS_MEMBERS_LIST, "athenz1;group2;user.joe;" + oneDayExpiry);
+        expectedSecondNotification.addDetails(NOTIFICATION_DETAILS_MEMBERS_LIST, "athenz1;group2;user.joe;" + oneDayExpiry + ";");
         expectedSecondNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryDomainNotificationToEmailConverter(
                         notificationToEmailConverterCommon));
@@ -332,7 +332,7 @@ public void testGetEmailBody() {
         // with one bad entry that should be skipped
 
         details.put(NOTIFICATION_DETAILS_MEMBERS_LIST,
-                "athenz;group1;user.joe;2020-12-01T12:00:00.000Z|athenz;group1;user.jane;2020-12-01T12:00:00.000Z|athenz;group3;user.bad");
+                "athenz;group1;user.joe;2020-12-01T12:00:00.000Z;notify+details|athenz;group1;user.jane;2020-12-01T12:00:00.000Z;|athenz;group3;user.bad");
 
         NotificationEmail notificationAsEmailWithMembers = converter.getNotificationAsEmail(notification);
         body = notificationAsEmailWithMembers.getBody();
@@ -341,6 +341,7 @@ public void testGetEmailBody() {
         assertTrue(body.contains("user.jane"));
         assertTrue(body.contains("group1"));
         assertTrue(body.contains("2020-12-01T12:00:00.000Z"));
+        assertTrue(body.contains("notify details"));
 
         // make sure the bad entries are not included
 
@@ -356,7 +357,7 @@ public void testGetEmailBody() {
         notification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         notification.setDetails(details);
         details.put(NOTIFICATION_DETAILS_ROLES_LIST,
-                "athenz1;group1;user.joe;2020-12-01T12:00:00.000Z|athenz2;group2;user.joe;2020-12-01T12:00:00.000Z");
+                "athenz1;group1;user.joe;2020-12-01T12:00:00.000Z;notify%20details|athenz2;group2;user.joe;2020-12-01T12:00:00.000Z;");
         GroupMemberExpiryNotificationTask.GroupExpiryPrincipalNotificationToEmailConverter principalConverter =
                 new GroupMemberExpiryNotificationTask.GroupExpiryPrincipalNotificationToEmailConverter(
                         notificationToEmailConverterCommon);
@@ -369,6 +370,7 @@ public void testGetEmailBody() {
         assertTrue(body.contains("group1"));
         assertTrue(body.contains("group2"));
         assertTrue(body.contains("2020-12-01T12:00:00.000Z"));
+        assertTrue(body.contains("notify details"));
 
         // Make sure support text and url do not appear
 
@@ -541,7 +543,7 @@ public void testSendConsolidatedGroupMemberExpiryReminders() throws ServerResour
         Notification expectedFirstNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedFirstNotification.addRecipient("user.joe");
         expectedFirstNotification.addDetails(NOTIFICATION_DETAILS_ROLES_LIST,
-                "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z|athenz1;group2;user.joe;1970-01-01T00:00:00.100Z");
+                "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z;|athenz1;group2;user.joe;1970-01-01T00:00:00.100Z;");
         expectedFirstNotification.addDetails("member", "user.joe");
         expectedFirstNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryPrincipalNotificationToEmailConverter(
@@ -552,7 +554,7 @@ public void testSendConsolidatedGroupMemberExpiryReminders() throws ServerResour
         Notification expectedSecondNotification = new Notification(Notification.Type.GROUP_MEMBER_EXPIRY);
         expectedSecondNotification.addRecipient("user.jane");
         expectedSecondNotification.addDetails(NOTIFICATION_DETAILS_MEMBERS_LIST,
-                "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z|athenz1;group2;user.joe;1970-01-01T00:00:00.100Z");
+                "athenz1;group1;user.joe;1970-01-01T00:00:00.100Z;|athenz1;group2;user.joe;1970-01-01T00:00:00.100Z;");
         expectedSecondNotification.setNotificationToEmailConverter(
                 new GroupMemberExpiryNotificationTask.GroupExpiryDomainNotificationToEmailConverter(
                         notificationToEmailConverterCommon));
@@ -853,7 +855,7 @@ public void testGetConsolidatedNotificationDetails() {
         domainGroupMember = new DomainGroupMember().setMemberName("athenz.api");
         memberGroups = new ArrayList<>();
         memberGroups.add(new GroupMember().setGroupName("deployment").setDomainName("home.joe")
-                .setMemberName("athenz.api").setExpiration(currentTime));
+                .setMemberName("athenz.api").setExpiration(currentTime).setNotifyDetails("notify details"));
         domainGroupMember.setMemberGroups(memberGroups);
         members.put("athenz.api", domainGroupMember);
 
@@ -902,9 +904,9 @@ public void testGetConsolidatedNotificationDetails() {
         assertEquals(1, notification.getRecipients().size());
         assertEquals(2, notification.getDetails().size());
         assertEquals("user.joe", notification.getDetails().get(NOTIFICATION_DETAILS_MEMBER));
-        assertEquals("home.joe;deployment;athenz.api;" + currentTime +
-                        "|home.joe;deployment;home.joe.openhouse;" + currentTime +
-                        "|home.joe;deployment;athenz.backend;" + currentTime,
+        assertEquals("home.joe;deployment;athenz.api;" + currentTime + ";notify+details" +
+                        "|home.joe;deployment;home.joe.openhouse;" + currentTime + ";" +
+                        "|home.joe;deployment;athenz.backend;" + currentTime + ";",
                 notification.getDetails().get(NOTIFICATION_DETAILS_ROLES_LIST));
 
         // get the notification for user.jane as the admin of the domains
@@ -915,8 +917,8 @@ public void testGetConsolidatedNotificationDetails() {
         assertEquals(1, notification.getRecipients().size());
         assertEquals(2, notification.getDetails().size());
         assertEquals("user.jane", notification.getDetails().get(NOTIFICATION_DETAILS_MEMBER));
-        assertEquals("home.joe;deployment;athenz.api;" + currentTime +
-                        "|home.joe;deployment;athenz.backend;" + currentTime,
+        assertEquals("home.joe;deployment;athenz.api;" + currentTime + ";notify+details" +
+                        "|home.joe;deployment;athenz.backend;" + currentTime + ";",
                 notification.getDetails().get(NOTIFICATION_DETAILS_ROLES_LIST));
 
         // get the notification for user.joe as the owner of the principals
@@ -926,9 +928,9 @@ public void testGetConsolidatedNotificationDetails() {
 
         assertEquals(1, notification.getRecipients().size());
         assertEquals(1, notification.getDetails().size());
-        assertEquals("home.joe;deployment;athenz.api;" + currentTime +
-                        "|home.joe;deployment;home.joe.openhouse;" + currentTime +
-                        "|home.joe;deployment;athenz.backend;" + currentTime,
+        assertEquals("home.joe;deployment;athenz.api;" + currentTime + ";notify+details" +
+                        "|home.joe;deployment;home.joe.openhouse;" + currentTime + ";" +
+                        "|home.joe;deployment;athenz.backend;" + currentTime + ";",
                 notification.getDetails().get(NOTIFICATION_DETAILS_MEMBERS_LIST));
     }