Remove audit feature

[coudot]

Audit feature seems broken since many versions, should be removed for 2.2

[coudot]

Before removing it, check if something work in master?
Could the feature be replaced only by logback?

[davidcoutadeur]

Here is the status of audit feature. I have compiled LSC from master branch (1b800cc), with openjdk 17.
I have tested on a simple use case OpenLDAP -> OpenLDAP with openjdk 17.

Globally, the audit feature is working.

CSV audit log

Example of corresponding CSV:

test1,uid=test,ou=people2,dc=my-domain,dc=com
,uid=test2,ou=people2,dc=my-domain,dc=com

First line is a creation with cn,dn
Second line is a deletion

Most options displayed in the documentation are ok:

• file: ok (but the documentation name it filename)
• append mode: ok
• operations: as stated in the documentation, it is currently limited to create and delete operations. No space allowed as operation separator.
• attrs: list of attributes. Same separator as below. No extra space allowed as separator. special attribute dn is working. All attributes but dn will be empty in the file for delete operations.
• separator: ok
• outputHeader: does not seem to work

LDIF audit log

Example of corresponding LDIF:

# Thu May 16 19:30:55 CEST 2024
dn: uid=test,ou=people2,dc=my-domain,dc=com
changetype: add
uid: test
userPassword: secret
objectClass: inetOrgPerson
objectClass: organizationalPerson
objectClass: person
objectClass: top
cn: test
sn: test

# Thu May 16 19:35:42 CEST 2024
dn: uid=test2,ou=people2,dc=my-domain,dc=com
changetype: delete

First section is an add operation, second section is a delete operation.

Most options displayed in the documentation are ok:

• file: ok (but the documentation name it filename)
• append mode: ok
• operations: as stated in the documentation, it is currently limited to create and delete operations. No space allowed as operation separator.
• logOnlyLdif: no idea what this option does. The output seems identical for true or false values.

Next step is to check if we can do (at least) the same with logback.xml configuration.

[davidcoutadeur]

The logback configuration seems more complete than audit.

Here is the status of my tests about the logback configuration. I have compiled LSC from master branch (1b800cc), with openjdk 17.
I have tested on a simple use case OpenLDAP -> OpenLDAP with openjdk 17.

CSV layout

Most of the options described in the documentation are accurate:

• File: ok
• Append: ok
• logOperations: works with create and update, as stated in the documentation. Also works with delete operation. (and probable changeId too) If an operation is not present here, it is not logged. We should improve the doc to specify all working operations. Note that nothing distinguish a create / update / delete operation. For example, here are the logs for a creation, update of sn, and delete operations:

uid=test,ou=people2,dc=my-domain,dc=com;test;test;;test1;;inetOrgPerson
uid=test,ou=people2,dc=my-domain,dc=com;;test;;;;
uid=test2,ou=people2,dc=my-domain,dc=com;test2;;;;;

• attrs: ok, special attribute dn is working. when an attribute value is not present, it is empty in the line. Limitation: only the first value is displayed (see unique value inetOrgPerson above whereas 4 objectClass values were present)
• separator: ok
• outputHeader: does not work. A simple fix would be:

diff --git a/src/main/java/org/lsc/utils/output/CsvLayout.java b/src/main/java/org/lsc/utils/output/CsvLayout.java
index fcfc8952..226b1b25 100644
--- a/src/main/java/org/lsc/utils/output/CsvLayout.java
+++ b/src/main/java/org/lsc/utils/output/CsvLayout.java
@@ -123,6 +123,7 @@ public class CsvLayout extends LayoutBase<ILoggingEvent> {
                                                        ( taskNamesList.size() == 0 ||
                                                          taskNamesList.contains(lm.getTaskName().toLowerCase()))) {
                                StringBuilder sb = new StringBuilder(1024);
+                               sb.append(this.getHeader());
 
                                Map<String, List<Object>> modifications = lm.getModificationsItemsByHash();
 
diff --git a/src/test/java/org/lsc/utils/output/CsvLayoutTest.java b/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
index 65c27bbd..a9f81c35 100644
--- a/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
+++ b/src/test/java/org/lsc/utils/output/CsvLayoutTest.java
@@ -178,10 +178,10 @@ public class CsvLayoutTest {
                // log one line to check that the outputHeader is prepended
                ILoggingEvent event = makeLoggingEvent(jm.toString(), jm);
                assertEquals("givenName%sn%dn%%cn\n", layout.getHeader());
-               assertEquals("Jon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
+               assertEquals("givenName%sn%dn%%cn\nJon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
 
                // log the same line again to check that the outputHeader is not logged again
                event = makeLoggingEvent(jm.toString(), jm);
-               assertEquals("Jon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
+               assertEquals("givenName%sn%dn%%cn\nJon%%cn=test,o=testing%%Tester CN\n", layout.doLayout(event));
        }
 }

Note: there is actually a test /src/test/java/org/lsc/utils/output/CsvLayoutTest.java, but it just verifies the method getHeader() is returning the corresponding header. But the getHeader() method does not seem to be called anywhere in our code. If we apply the fix above, we must also fix this test.

• taskNames: does not seem to work. Setting a dummy task name here still produces a CSV output. The taskNames parameter is empty in CsvLayout.java.

LDIF layout

Globally, the ldif output is working.

• File: ok
• Append: ok
• logOperations: whatever operation is set up, all operations are logged. (create, update, delete) We need to fix this.
• onlyLdif: no idea what this option does. The output seems identical for true or false values.

[davidcoutadeur]

Given the analysis above, I think the audit is redundant, and can be replaced by logback configuration.

I am going to remove it.

[davidcoutadeur]

• Proposition of removal of audit logs is done in remove audit feature (#107) #288
• removal of audit logs in documentation is done in PR: Remove audit feature documentation#7
• Need to fix the issues discovered in logback ldif and csv logging (see above): better documentation of CSV parameters: logOperations and attrs, fix outputHeader and taskNames CSV parameters, fix logOperations ldif parameter, and find the utility of onlyLdif option. (may be interresting to do this in dedicated issues)

[davidcoutadeur]

Last point in the todo-list will be treated in a dedicated issue: #294

So current issue is to be closed.