diff --git a/commons/audit/core/src/main/java/org/forgerock/audit/events/handlers/writers/RotatableWriter.java b/commons/audit/core/src/main/java/org/forgerock/audit/events/handlers/writers/RotatableWriter.java index 95345de08..4f12350b3 100644 --- a/commons/audit/core/src/main/java/org/forgerock/audit/events/handlers/writers/RotatableWriter.java +++ b/commons/audit/core/src/main/java/org/forgerock/audit/events/handlers/writers/RotatableWriter.java @@ -73,6 +73,7 @@ public class RotatableWriter implements TextWriter, RotatableObject { private BufferedWriter writer; private final ReentrantReadWriteLock readWriteLock = new ReentrantReadWriteLock(); private final RolloverLifecycleHook rolloverLifecycleHook; + private final Duration rotationCheckInterval; /** * Constructs a {@link RotatableWriter} given an initial file to manage rotation/retention, and @@ -99,8 +100,29 @@ public RotatableWriter(final File file, final FileBasedEventHandlerConfiguration */ public RotatableWriter(final File file, final FileBasedEventHandlerConfiguration configuration, final boolean append, final RolloverLifecycleHook rolloverLifecycleHook) throws IOException { + this(file, configuration, append, rolloverLifecycleHook, true); + } + + /** + * Constructs a {@link RotatableWriter} given an initial file to manage rotation/retention, a + * {@link FileBasedEventHandlerConfiguration} and a {@link RolloverLifecycleHook}, and starts the rotation and + * retention checks, or leaves them to {@link #startRotationAndRetentionChecks()}. + *
+ * A rotation renames the file: a writer that reads the file before it writes to it starts the checks once it has + * read it. + * + * @param file The initial file to manage rotation/retention. + * @param configuration The configuration of the rotation and retention policies. + * @param append Whether to append to the rotatable file or not. + * @param rolloverLifecycleHook Hook to use before and after rotation/retention checks. + * @param startChecks Whether to start the rotation and retention checks now. + * @throws IOException If a problem occurs. + */ + public RotatableWriter(final File file, final FileBasedEventHandlerConfiguration configuration, + final boolean append, final RolloverLifecycleHook rolloverLifecycleHook, final boolean startChecks) + throws IOException { this(file, configuration, append, configuration.getFileRotation().buildTimeStampFileNamingPolicy(file), - rolloverLifecycleHook); + rolloverLifecycleHook, startChecks); } /** @@ -110,13 +132,13 @@ public RotatableWriter(final File file, final FileBasedEventHandlerConfiguration @VisibleForTesting RotatableWriter(final File file, final FileBasedEventHandlerConfiguration configuration, final boolean append, final FileNamingPolicy fileNamingPolicy) throws IOException { - this(file, configuration, append, fileNamingPolicy, NOOP_ROLLOVER_LIFECYCLE_HOOK); + this(file, configuration, append, fileNamingPolicy, NOOP_ROLLOVER_LIFECYCLE_HOOK, true); } /** Constructor with all possible parameters. */ private RotatableWriter(final File file, final FileBasedEventHandlerConfiguration configuration, final boolean append, final FileNamingPolicy fileNamingPolicy, - final RolloverLifecycleHook rolloverLifecycleHook) throws IOException { + final RolloverLifecycleHook rolloverLifecycleHook, final boolean startChecks) throws IOException { this.file = file; this.fileNamingPolicy = fileNamingPolicy; this.rotationEnabled = configuration.getFileRotation().isRotationEnabled(); @@ -128,7 +150,10 @@ private RotatableWriter(final File file, final FileBasedEventHandlerConfiguratio this.writer = constructWriter(file, append); retentionPolicies = configuration.getFileRetention().buildRetentionPolicies(); rotationPolicies = configuration.getFileRotation().buildRotationPolicies(); - scheduleRotationAndRetentionChecks(configuration); + rotationCheckInterval = parseRotationCheckInterval(configuration); + if (startChecks) { + startRotationAndRetentionChecks(); + } } /** @@ -355,21 +380,30 @@ private BufferedWriter constructWriter(File csvFile, boolean append) } /** - * Schedule checks for rotations and retention policies. + * Parses the interval of the checks for rotations and retention policies. *
* The check interval is provided by the RotationRetentionCheckInterval property, which must have
* a non-zero value if at least one policy is enabled.
*/
- private void scheduleRotationAndRetentionChecks(FileBasedEventHandlerConfiguration configuration)
+ private Duration parseRotationCheckInterval(FileBasedEventHandlerConfiguration configuration)
throws IOException {
final Duration rotationCheckInterval = parseDuration("rotation and retention check interval",
configuration.getRotationRetentionCheckInterval(), FIVE_SECONDS);
- if (!rotationPolicies.isEmpty() || !retentionPolicies.isEmpty()) {
- if (rotationCheckInterval.isUnlimited() || rotationCheckInterval.isZero()) {
- throw new IOException("Rotation and retention check interval set to an invalid value: "
- + rotationCheckInterval);
- }
+ if ((!rotationPolicies.isEmpty() || !retentionPolicies.isEmpty())
+ && (rotationCheckInterval.isUnlimited() || rotationCheckInterval.isZero())) {
+ throw new IOException("Rotation and retention check interval set to an invalid value: "
+ + rotationCheckInterval);
+ }
+ return rotationCheckInterval;
+ }
+
+ /**
+ * Schedules the checks for rotations and retention policies, if at least one policy is enabled, at the check
+ * interval. Does nothing if they are scheduled already.
+ */
+ public void startRotationAndRetentionChecks() {
+ if (rotator == null && (!rotationPolicies.isEmpty() || !retentionPolicies.isEmpty())) {
rotator = Executors.newScheduledThreadPool(1);
rotator.scheduleAtFixedRate(
new Runnable() {
diff --git a/commons/audit/core/src/test/java/org/forgerock/audit/events/handlers/writers/RotatableWriterTest.java b/commons/audit/core/src/test/java/org/forgerock/audit/events/handlers/writers/RotatableWriterTest.java
index d2845fb30..bbc528671 100644
--- a/commons/audit/core/src/test/java/org/forgerock/audit/events/handlers/writers/RotatableWriterTest.java
+++ b/commons/audit/core/src/test/java/org/forgerock/audit/events/handlers/writers/RotatableWriterTest.java
@@ -442,6 +442,34 @@ public void testAutomaticallyEvaluatesPolicesPeriodicallyIfRotationIntervalSpeci
assertThat(rotatableWriter.getBytesWritten()).isEqualTo(0L);
}
+ @Test
+ public void testEvaluatesPoliciesPeriodicallyOnlyOnceTheChecksAreStarted() throws Exception {
+ // given
+ final File file = getTempFile();
+ final String prefix = "testEvaluatesPoliciesPeriodicallyOnlyOnceTheChecksAreStarted";
+ assertThat(file.setLastModified(System.currentTimeMillis() - HOURS.toMillis(2))).isTrue();
+ final FileBasedEventHandlerConfiguration configuration = new DefaultFileBasedAuditEventHandlerConfiguration();
+ configuration.setRotationRetentionCheckInterval("10 ms");
+ configuration.getFileRotation().setRotationEnabled(true);
+ configuration.getFileRotation().setRotationFilePrefix(prefix);
+ configuration.getFileRotation().setRotationFileSuffix(ROTATION_FILE_SUFFIX.replace(".gz", ""));
+ configuration.getFileRotation().setRotationInterval("1 hour");
+ rotatableWriter = new RotatableWriter(
+ file, configuration, true, RotatableWriter.NOOP_ROLLOVER_LIFECYCLE_HOOK, false);
+
+ // when
+ Thread.sleep(500);
+
+ // then
+ assertThat(getRetainedHistoricalFiles(file, prefix)).isEmpty();
+
+ // when
+ rotatableWriter.startRotationAndRetentionChecks();
+
+ // then
+ assertRetainedHistoricalFiles(file, prefix, 1);
+ }
+
private void writeThenFlushBytes(final RotatableWriter writer, final int bytesToWrite) throws IOException {
writer.write(new String(new byte[bytesToWrite]));
writer.flush();
diff --git a/commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/SecureCsvWriter.java b/commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/SecureCsvWriter.java
index 1109e708d..3ad652b56 100644
--- a/commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/SecureCsvWriter.java
+++ b/commons/audit/handler-csv/src/main/java/org/forgerock/audit/handlers/csv/SecureCsvWriter.java
@@ -176,6 +176,11 @@ public void run() {
} catch (Exception e) {
throw new RuntimeException("Error when initializing a secure CSV writer", e);
}
+ if (rotatableWriter != null) {
+ // Only once the keys are read and the file is verified: a rotation renames the file and its keystore,
+ // and writes new keys into the secure storage that the verifier reads.
+ rotatableWriter.startRotationAndRetentionChecks();
+ }
}
@Override
@@ -208,7 +213,8 @@ private Writer constructWriter(File csvFile, boolean append, CsvAuditEventHandle
throws IOException {
TextWriter textWriter;
if (config.getFileRotation().isRotationEnabled()) {
- rotatableWriter = new RotatableWriter(csvFile, config, append, this);
+ // the checks are started at the end of the constructor
+ rotatableWriter = new RotatableWriter(csvFile, config, append, this, false);
rotatableWriter.registerRotationHooks(new SecureCsvWriterRotationHooks());
textWriter = rotatableWriter;
} else {
diff --git a/commons/audit/handler-csv/src/test/java/org/forgerock/audit/handlers/csv/SecureCsvWriterTest.java b/commons/audit/handler-csv/src/test/java/org/forgerock/audit/handlers/csv/SecureCsvWriterTest.java
index 58274d13c..783a8cfd9 100644
--- a/commons/audit/handler-csv/src/test/java/org/forgerock/audit/handlers/csv/SecureCsvWriterTest.java
+++ b/commons/audit/handler-csv/src/test/java/org/forgerock/audit/handlers/csv/SecureCsvWriterTest.java
@@ -269,6 +269,59 @@ static void assertThreadsTerminate(ThreadGroup group) throws InterruptedExceptio
}
}
+ @Test(timeOut = 60_000)
+ public void shouldRotateResumedFileOnlyOnceItIsVerified() throws Exception {
+ final Path logDirectory = Files.createTempDirectory("SecureCsvWriterTest");
+ final File actual =
+ new File(logDirectory.toFile(), CsvAuditEventHandler.SECURE_CSV_FILENAME_PREFIX + "access.csv");
+ final String header = "FOO";
+ final String rotationFileSuffix = "-yyyy.MM.dd-HH.mm.ss.SSS";
+ try (SecureCsvWriter secureCsvWriter = new SecureCsvWriter(actual, new String[]{header},
+ CsvPreference.EXCEL_PREFERENCE, createBasicSecureConfig("5 minutes"), keyStoreHandler, random)) {
+ secureCsvWriter.writeEvent(singletonMap(header, "one"));
+ }
+ // the time limit policy rotates the file at the first check after it is resumed
+ assertThat(actual.setLastModified(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(2))).isTrue();
+
+ CsvAuditEventHandlerConfiguration config = createBasicSecureConfig("5 minutes");
+ config.getFileRotation().setRotationEnabled(true);
+ config.getFileRotation().setRotationFileSuffix(rotationFileSuffix);
+ config.getFileRotation().setRotationInterval("1 hour");
+ config.setRotationRetentionCheckInterval("10 milliseconds");
+ // a keystore slow to read, so that the construction outlasts many check intervals
+ final KeyStoreHandlerDecorator slowKeyStoreHandler = new KeyStoreHandlerDecorator(keyStoreHandler) {
+ @Override
+ public KeyStore getStore() {
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ return super.getStore();
+ }
+ };
+ final FileNamingPolicy fileNamingPolicy = new TimeStampFileNamingPolicy(actual, rotationFileSuffix, null);
+ try (SecureCsvWriter secureCsvWriter = new SecureCsvWriter(actual, new String[]{header},
+ CsvPreference.EXCEL_PREFERENCE, config, slowKeyStoreHandler, random)) {
+ // close() waits for the rotation that renamed the file to end
+ final long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(10);
+ while (fileNamingPolicy.listFiles().isEmpty() && System.nanoTime() < deadline) {
+ Thread.sleep(10);
+ }
+ }
+
+ // the resumed file is archived with its own keystore
+ final String keystorePassword =
+ Base64.encode(keyStoreHandler.readSecretKeyFromKeyStore(ENTRY_PASSWORD).getEncoded());
+ final PublicKey publicKey = keyStoreHandler.readPublicKeyFromKeyStore(KeyStoreSecureStorage.ENTRY_SIGNATURE);
+ final List