Skip to content

Commit

Permalink
improve interoperability of TF with audit/review-enabled roles/groups (…
Browse files Browse the repository at this point in the history 
…#2805)

Signed-off-by: Henry Avetisyan <[email protected]>
  • Loading branch information
@havetisyan
havetisyan authored Nov 26, 2024
1 parent 4924f9a commit 8161f55
Show file tree
Hide file tree
Showing 12 changed files with 1,045 additions and 305 deletions.
35 changes: 24 additions & 11 deletions ...server_common/src/main/java/com/yahoo/athenz/common/server/store/impl/JDBCConnection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ public class JDBCConnection implements ObjectStoreConnection {
private static final String MYSQL_EXC_STATE_COMM_ERROR = "08S01";

private static final String AUDIT_OPERATION_APPROVE = "APPROVE";
private static final String AUDIT_OPERATION_REJECT = "REJECT";
private static final String AUDIT_OPERATION_ADD = "ADD";
private static final String AUDIT_OPERATION_UPDATE = "UPDATE";
private static final String AUDIT_OPERATION_REQUEST = "REQUEST";
private static final String AUDIT_OPERATION_ENABLE = "ENABLE";
private static final String AUDIT_OPERATION_DISABLE = "DISABLE";
private static final String AUDIT_OPERATION_DELETE = "DELETE";

private static final String SQL_TABLE_DOMAIN = "domain";
private static final String SQL_TABLE_ROLE = "role";
Expand Down Expand Up @@ -2837,7 +2841,7 @@ public boolean updateRoleMemberDisabledState(String domainName, String roleName,
// add return the result of the audit log insert operation

if (result) {
final String operation = disabledState == 0 ? "ENABLE" : "DISABLE";
final String operation = disabledState == 0 ? AUDIT_OPERATION_ENABLE : AUDIT_OPERATION_DISABLE;
result = insertRoleAuditLog(roleId, admin, principal, operation, auditRef);
}

Expand Down Expand Up @@ -2877,7 +2881,7 @@ public boolean deleteRoleMember(String domainName, String roleName, String princ
// add return the result of the audit log insert operation

if (result) {
result = insertRoleAuditLog(roleId, admin, principal, "DELETE", auditRef);
result = insertRoleAuditLog(roleId, admin, principal, AUDIT_OPERATION_DELETE, auditRef);
}

return result;
Expand Down Expand Up @@ -2919,7 +2923,7 @@ public boolean deleteExpiredRoleMember(String domainName, String roleName, Strin
// add return the result of the audit log insert operation

if (result) {
result = insertRoleAuditLog(roleId, admin, principal, "DELETE", auditRef);
result = insertRoleAuditLog(roleId, admin, principal, AUDIT_OPERATION_DELETE, auditRef);
}

return result;
Expand Down Expand Up @@ -5437,7 +5441,7 @@ public boolean executeDeletePendingRoleMember(int roleId, int principalId, final
}
boolean result = (affectedRows > 0);
if (result && auditLog) {
result = insertRoleAuditLog(roleId, admin, principal, "REJECT", auditRef);
result = insertRoleAuditLog(roleId, admin, principal, AUDIT_OPERATION_REJECT, auditRef);
}
return result;
}
Expand Down Expand Up @@ -6576,7 +6580,8 @@ boolean insertGroupAuditLog(int groupId, String admin, String member,
}

boolean insertPendingGroupMember(int groupId, int principalId, GroupMember groupMember,
final String admin, final String auditRef, boolean groupMemberExists, final String caller) throws ServerResourceException {
final String admin, final String principal, final String auditRef, boolean groupMemberExists,
final String caller) throws ServerResourceException {

java.sql.Timestamp expiration = groupMember.getExpiration() == null ? null
: new java.sql.Timestamp(groupMember.getExpiration().millis());
Expand Down Expand Up @@ -6610,7 +6615,15 @@ boolean insertPendingGroupMember(int groupId, int principalId, GroupMember group
}
}

return (affectedRows > 0);
// add audit log entry for this change if the operation was successful
// add return the result of the audit log insert operation

boolean result = affectedRows > 0;
if (result) {
result = insertGroupAuditLog(groupId, admin, principal, AUDIT_OPERATION_REQUEST, auditRef);
}

return result;
}

boolean insertStandardGroupMember(int groupId, int principalId, GroupMember groupMember,
Expand Down Expand Up @@ -6704,7 +6717,7 @@ public boolean insertGroupMember(String domainName, String groupName, GroupMembe
boolean result;
if (pendingRequest) {
result = insertPendingGroupMember(groupId, principalId, groupMember, admin,
auditRef, groupMemberExists, caller);
principal, auditRef, groupMemberExists, caller);
} else {
result = insertStandardGroupMember(groupId, principalId, groupMember, admin,
principal, auditRef, groupMemberExists, false, caller);
Expand Down Expand Up @@ -6744,7 +6757,7 @@ public boolean deleteGroupMember(String domainName, String groupName, String pri
// add return the result of the audit log insert operation

if (result) {
result = insertGroupAuditLog(groupId, admin, principal, "DELETE", auditRef);
result = insertGroupAuditLog(groupId, admin, principal, AUDIT_OPERATION_DELETE, auditRef);
}

return result;
Expand Down Expand Up @@ -6784,7 +6797,7 @@ public boolean deleteExpiredGroupMember(String domainName, String groupName, Str
// add return the result of the audit log insert operation

if (result) {
result = insertGroupAuditLog(groupId, admin, principal, "DELETE", auditRef);
result = insertGroupAuditLog(groupId, admin, principal, AUDIT_OPERATION_DELETE, auditRef);
}

return result; }
Expand Down Expand Up @@ -6826,7 +6839,7 @@ public boolean updateGroupMemberDisabledState(String domainName, String groupNam
// add return the result of the audit log insert operation

if (result) {
final String operation = disabledState == 0 ? "ENABLE" : "DISABLE";
final String operation = disabledState == 0 ? AUDIT_OPERATION_ENABLE : AUDIT_OPERATION_DISABLE;
result = insertGroupAuditLog(groupId, admin, principal, operation, auditRef);
}

Expand Down Expand Up @@ -6867,7 +6880,7 @@ public boolean executeDeletePendingGroupMember(int groupId, int principalId, fin
}
boolean result = (affectedRows > 0);
if (result && auditLog) {
result = insertGroupAuditLog(groupId, admin, principal, "REJECT", auditRef);
result = insertGroupAuditLog(groupId, admin, principal, AUDIT_OPERATION_REJECT, auditRef);
}
return result;
}
Expand Down
20 changes: 16 additions & 4 deletions ...er_common/src/test/java/com/yahoo/athenz/common/server/store/impl/JDBCConnectionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11903,10 +11903,16 @@ public void testInsertPendingGroupMember() throws Exception {

Mockito.verify(mockPrepStmt, times(1)).setString(1, "user.user1");

// additional operation to check for groupMember exist using groupID and principal ID.
Mockito.verify(mockPrepStmt, times(2)).setInt(1, 7);
// additional operation to check for groupMember exist using groupID and principal ID
// and audit log entry
Mockito.verify(mockPrepStmt, times(3)).setInt(1, 7);
Mockito.verify(mockPrepStmt, times(2)).setInt(2, 9);

Mockito.verify(mockPrepStmt, times(1)).setString(2, "user.admin");
Mockito.verify(mockPrepStmt, times(1)).setString(3, "user.user1");
Mockito.verify(mockPrepStmt, times(1)).setString(4, "REQUEST");
Mockito.verify(mockPrepStmt, times(1)).setString(5, "audit-ref");

assertTrue(requestSuccess);
jdbcConn.close();
}
Expand Down Expand Up @@ -11951,10 +11957,16 @@ public void testInsertPendingGroupMemberUpdate() throws Exception {
Mockito.verify(mockPrepStmt, times(1)).setInt(4, 7);
Mockito.verify(mockPrepStmt, times(1)).setInt(5, 9);

// operation to check for groupMember exist using groupID and principal ID.
Mockito.verify(mockPrepStmt, times(1)).setInt(1, 7);
// operation to check for groupMember exist using groupID and principal ID
// and audit log entry
Mockito.verify(mockPrepStmt, times(2)).setInt(1, 7);
Mockito.verify(mockPrepStmt, times(1)).setInt(2, 9);

Mockito.verify(mockPrepStmt, times(1)).setString(2, "user.admin");
Mockito.verify(mockPrepStmt, times(1)).setString(3, "user.user1");
Mockito.verify(mockPrepStmt, times(1)).setString(4, "REQUEST");
Mockito.verify(mockPrepStmt, times(1)).setString(5, "audit-ref");

assertTrue(requestSuccess);
jdbcConn.close();
}
Expand Down
2 changes: 1 addition & 1 deletion servers/zms/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<packaging>war</packaging>

<properties>
<code.coverage.min>0.9730</code.coverage.min>
<code.coverage.min>0.9731</code.coverage.min>
</properties>

<dependencies>
Expand Down
Loading

0 comments on commit 8161f55

Please sign in to comment.