Update after review

Author: joemahady-comm

server/src/main/java/org/cloudfoundry/identity/uaa/account/event/PasswordChangeEvent.java

@@ -31,8 +31,8 @@ public PasswordChangeEvent(String message, UaaUser user, Authentication principa
 
     @Override
     public AuditEvent getAuditEvent() {
-        return createAuditRecord(getUser().getId(), getUser().getUsername(), AuditEventType.PasswordChangeSuccess,
-                getOrigin(getPrincipal()), getMessage());
+        return createAuditRecord(getUser().getId(), AuditEventType.PasswordChangeSuccess,
+                getOrigin(getPrincipal()), getMessage(), getUser().getUsername());
     }
 
 }

server/src/main/java/org/cloudfoundry/identity/uaa/account/event/PasswordChangeFailureEvent.java

@@ -33,11 +33,11 @@ public PasswordChangeFailureEvent(String message, UaaUser user, Authentication p
     public AuditEvent getAuditEvent() {
         UaaUser user = getUser();
         if (user == null) {
-            return createAuditRecord(getPrincipal().getName(), getPrincipal().getName(), AuditEventType.PasswordChangeFailure,
-                    getOrigin(getPrincipal()), getMessage());
+            return createAuditRecord(getPrincipal().getName(), AuditEventType.PasswordChangeFailure,
+                    getOrigin(getPrincipal()), getMessage(), getPrincipal().getName());
         } else {
-            return createAuditRecord(user.getId(), user.getUsername(), AuditEventType.PasswordChangeFailure,
-                    getOrigin(getPrincipal()), getMessage());
+            return createAuditRecord(user.getId(), AuditEventType.PasswordChangeFailure,
+                    getOrigin(getPrincipal()), getMessage(), user.getUsername());
         }
     }
 

server/src/main/java/org/cloudfoundry/identity/uaa/audit/AuditEvent.java

@@ -29,10 +29,10 @@ public class AuditEvent {
     private final String authenticationType;
 
     public AuditEvent(AuditEventType type, String principalId, String origin, String data, long time, String identityZoneId, String authenticationType, String description) {
-        this(type, principalId, null, origin, data, time, identityZoneId, authenticationType, description);
+        this(type, principalId, origin, data, time, identityZoneId, authenticationType, description, null);
     }
 
-    public AuditEvent(AuditEventType type, String principalId, String principalName, String origin, String data, long time, String identityZoneId, String authenticationType, String description) {
+    public AuditEvent(AuditEventType type, String principalId, String origin, String data, long time, String identityZoneId, String authenticationType, String description, String principalName) {
         this.type = type;
         this.data = data;
         this.origin = origin;

server/src/main/java/org/cloudfoundry/identity/uaa/audit/event/AbstractUaaEvent.java

@@ -79,8 +79,8 @@ protected AuditEvent createAuditRecord(String principalId, AuditEventType type,
         return new AuditEvent(type, principalId, origin, data, System.currentTimeMillis(), zoneId, null, null);
     }
 
-    protected AuditEvent createAuditRecord(String principalId, String principalName, AuditEventType type, String origin, String data) {
-        return new AuditEvent(type, principalId, principalName, origin, data, System.currentTimeMillis(), zoneId, null, null);
+    protected AuditEvent createAuditRecord(String principalId, AuditEventType type, String origin, String data, String principalName) {
+        return new AuditEvent(type, principalId, origin, data, System.currentTimeMillis(), zoneId, null, null, principalName);
     }
 
     protected AuditEvent createAuditRecord(String principalId, AuditEventType type, String origin, String data, String authenticationType, String message) {

server/src/test/java/org/cloudfoundry/identity/uaa/audit/LoggingAuditServiceTest.java

@@ -51,7 +51,7 @@ void log_format_whenAuthTypeIsNull() {
 
     @Test
     void log_format_whenPrincipalNameIsPresent() {
-        AuditEvent auditEvent = new AuditEvent(PasswordChangeSuccess, "thePrincipalId", "thePrincipalName", "theOrigin", "theData", 42L, "theZoneId", null, null);
+        AuditEvent auditEvent = new AuditEvent(PasswordChangeSuccess, "thePrincipalId", "theOrigin", "theData", 42L, "theZoneId", null, null, "thePrincipalName");
 
         loggingAuditService.log(auditEvent, "not-used");
 
@@ -63,7 +63,7 @@ void log_format_whenPrincipalNameIsPresent() {
 
     @Test
     void log_format_whenPrincipalNameAndAuthTypeArePresent() {
-        AuditEvent auditEvent = new AuditEvent(PasswordChangeFailure, "thePrincipalId", "thePrincipalName", "theOrigin", "theData", 42L, "theZoneId", "theAuthType", "theDescription");
+        AuditEvent auditEvent = new AuditEvent(PasswordChangeFailure, "thePrincipalId", "theOrigin", "theData", 42L, "theZoneId", "theAuthType", "theDescription", "thePrincipalName");
 
         loggingAuditService.log(auditEvent, "not-used");
 
@@ -87,6 +87,20 @@ void log_sanitizesMaliciousInput() {
                 .contains(LogSanitizerUtil.SANITIZED_FLAG);
     }
 
+    @Test
+    void log_sanitizesMaliciousPrincipalName() {
+        AuditEvent auditEvent = new AuditEvent(PasswordChangeSuccess, "principalId", "origin", "data", 100L, "safe-zone", null, null, "malicious\r\n\tname");
+
+        loggingAuditService.log(auditEvent, "not-used");
+
+        ArgumentCaptor<String> stringCaptor = ArgumentCaptor.forClass(String.class);
+        verify(mockLogger).info(stringCaptor.capture());
+        assertThat(stringCaptor.getValue()).doesNotContain("\r")
+                .doesNotContain("\n")
+                .doesNotContain("\t")
+                .contains(LogSanitizerUtil.SANITIZED_FLAG);
+    }
+
     @Test
     void log_doesNotModifyNonMaliciousInput() {
         AuditEvent auditEvent = new AuditEvent(UserAuthenticationSuccess, "principalId", "origin", "data", 100L, "safe-zone", null, null);