dulvac commented on code in PR #3059: URL: https://github.com/apache/jackrabbit-oak/pull/3059#discussion_r3711334960
########## oak-core/src/main/java/org/apache/jackrabbit/oak/security/audit/AuditConfigurationImpl.java: ########## @@ -0,0 +1,493 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.security.audit; + +import java.util.Collections; +import java.util.List; +import java.util.Map; + +import org.apache.jackrabbit.oak.api.Root; +import org.apache.jackrabbit.oak.osgi.OsgiWhiteboard; +import org.apache.jackrabbit.oak.spi.audit.AuditBufferLifecycle; +import org.apache.jackrabbit.oak.spi.audit.AuditConfiguration; +import org.apache.jackrabbit.oak.spi.audit.AuditEvent; +import org.apache.jackrabbit.oak.spi.audit.AuditEventListener; +import org.apache.jackrabbit.oak.spi.audit.AuditEvents; +import org.apache.jackrabbit.oak.spi.commit.Observer; +import org.apache.jackrabbit.oak.spi.toggle.Feature; +import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard; +import org.jetbrains.annotations.NotNull; +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; +import org.osgi.service.component.annotations.Deactivate; +import org.osgi.service.metatype.annotations.Designate; +import org.osgi.service.metatype.annotations.ObjectClassDefinition; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Default {@link AuditConfiguration} implementation. Contributes the + * audit pipeline to Oak: + * <ul> + * <li>Registers a {@link Feature} toggle gating capture and dispatch.</li> + * <li>Installs a per-session buffer ({@link AuditBuffer}) into + * {@link AuditBufferLifecycle}.</li> + * <li>Installs the {@link AuditEvents} sink that routes capture-site + * calls into the buffer.</li> + * <li>Tracks {@code AuditEventListener} services on the Whiteboard + * via {@link WhiteboardAuditEventListenerRegistry}.</li> + * <li>Registers an {@link AuditDrainObserver} as an OSGi {@link Observer} + * service; Oak's {@code ObserverTracker} (in {@code oak-jcr}'s + * {@code RepositoryManager}) picks it up and subscribes it to the root + * NodeStore. The observer drains the buffer on commit success and + * dispatches events to listeners.</li> + * </ul> + * Registered as {@link AuditConfiguration} only — not a + * {@code SecurityConfiguration}, contributes no commit hooks, not reachable + * via {@code SecurityProvider.getConfiguration(AuditConfiguration.class)}. + * Embedded callers obtain the drain observer via {@link #getDrainObserver()}. + * <p> + * When the feature toggle is disabled, capture is a no-op and the observer + * short-circuits — see {@link AuditEvents#isEnabled()} and + * {@link AuditDrainObserver#contentChanged}. + */ +@Component(service = AuditConfiguration.class) +@Designate(ocd = AuditConfigurationImpl.Configuration.class) +public class AuditConfigurationImpl implements AuditConfiguration { + + /** + * Feature toggle name, following the {@code FT_OAK-<issue>} convention + * in {@code AGENTS.md}. Disabled by default: this is a new feature, + * not a bug fix. + * <p> + * <strong>Why not on the public SPI interface + * ({@link AuditConfiguration}):</strong> moving this constant to the + * SPI would commit the literal value to the public surface forever. + * It stays impl-local; promoting it later is a binary-additive change + * if a need arises. + */ + public static final String FEATURE_TOGGLE_NAME = "FT_OAK-12331"; + + @ObjectClassDefinition(name = "Apache Jackrabbit Oak AuditConfiguration", + description = "Audit event pipeline. Capture and dispatch are " + + "gated by the '" + FEATURE_TOGGLE_NAME + "' feature toggle " + + "(disabled by default).") + @interface Configuration { + // Configuration is currently empty by design: capture/dispatch behavior + // is controlled exclusively by the feature toggle. Listeners are + // contributed via OSGi services / the Whiteboard. + } + + private static final Logger log = LoggerFactory.getLogger(AuditConfigurationImpl.class); + + // Package-private (not private) on the internal-state fields so + // AuditConfigurationImplTest can mock-replace them to verify the + // dispose-order invariant via Mockito InOrder. Production callers MUST + // NOT touch these fields directly — go through initialize() / dispose(). + // + // JMM-safety. featureToggle, buffer, registry, and drainObserver are + // package-private and non-volatile by design. The invariant they rely on: + // they are mutated only by initialize(Whiteboard) and dispose(), which + // the contract specifies must each run exactly once and on the same + // thread; reads happen either + // (a) on that same thread — the static AuditEvents.record / dispatch + // facade reads through the AuditEvents.sink field (itself volatile, + // providing the publication barrier), and getDrainObserver() is + // called by activate() on the SCR thread AFTER initialize() on the + // SCR thread; or + // (b) on a commit thread that arrives via Observer.contentChanged, + // after the ServiceRegistration publication barrier established + // by BundleContext.registerService in activate(). + // Both paths satisfy the JMM happens-before contract without per-field + // volatile. If a future change adds a "share the singleton across + // pipelines" pattern OR cross-thread mutation of these fields, this + // invariant breaks — at that point the fields MUST be made volatile (or + // properly immutable via constructor injection). + Feature featureToggle; + AuditBuffer buffer; + WhiteboardAuditEventListenerRegistry registry; + + /** + * Singleton {@link AuditDrainObserver} instance constructed by + * {@link #initialize(Whiteboard)} and zeroed by {@link #dispose()}. + * Exposed via {@link #getDrainObserver()} as an {@link Observer}. + * <p> + * Singleton-not-factory by design: each {@code AuditConfigurationImpl} + * owns at most one Observer because (a) the {@link AuditBuffer} + * {@code ThreadLocal} is buffer-instance-scoped, so multiple Observer + * instances would compete for the same drain, and (b) the destructive + * {@code buffer.drain(sessionId)} contract is the cleanup mechanism — + * double-attach would mask future non-destructive-drain refactors. + */ + AuditDrainObserver drainObserver; + + /** + * OSGi service registration for the {@link AuditDrainObserver}. Held + * so {@link #deactivate} can unregister and let {@code ObserverTracker} + * close its subscription on the root NodeStore. {@code null} outside + * the OSGi-active window; embedded callers manage observer lifetime + * through their own {@code ((Observable) store).addObserver(...)} call + * (see class Javadoc). + */ + ServiceRegistration<?> observerRegistration; + + public AuditConfigurationImpl() { + super(); + } + + @SuppressWarnings("UnusedDeclaration") + @Activate + private void activate(@NotNull Configuration configuration, + @NotNull BundleContext bundleContext, + @NotNull Map<String, Object> properties) { + // Step 1-4: install sinks/registry/buffer/toggle. Capture-site + // record(...) calls reach the buffer as soon as initialize returns. + // The singleton AuditDrainObserver is also constructed inside + // initialize() (step 5 below) so the impl is fully wired before + // we publish anything externally. + initialize(new OsgiWhiteboard(bundleContext)); + // Step 6 LAST: publish the Observer service. ObserverTracker + // (oak-store-spi/.../spi/commit/ObserverTracker.java, instantiated + // per-NodeStoreService in DocumentNodeStoreService, SegmentNodeStoreRegistrar, + // CompositeNodeStoreService) subscribes it to the root NodeStore. + // Any commit thread racing with activation that reaches step 6 before + // ObserverTracker has noticed the service will simply miss the drain + // on this one commit — events stay in the per-thread buffer until the + // next commit on the same session. No correctness risk. + observerRegistration = bundleContext.registerService( + Observer.class.getName(), getDrainObserver(), null); + } + + /** + * Non-OSGi entry point for wiring up the audit pipeline. Called by + * {@link #activate} in OSGi deployments after the {@code BundleContext} + * has been unwrapped into an {@code OsgiWhiteboard}, and by embedded + * callers (tests, {@code OakFixture}) directly. + * <p> + * <strong>Embedded callers must follow up with + * {@link #getDrainObserver()}</strong> to obtain the Observer and attach + * it to the root NodeStore. See {@link #getDrainObserver()} Javadoc for + * the recommended attach pattern and the {@code Oak.with(Observer)} + * caveat. + * <p> + * <strong>Must be called exactly once per instance.</strong> Calling + * it more than once orphans the previous {@code Feature} toggle and + * registry tracker, and silently overwrites the static + * {@link AuditEvents} / {@link AuditBufferLifecycle} sinks. To rewire, + * call {@link #dispose()} first. + * <p> + * <strong>Activation ordering rationale.</strong> + * {@link AuditBufferLifecycle#install AuditBufferLifecycle.install(buffer)} + * runs before + * {@link AuditEvents#install AuditEvents.install(BufferSink)} so that any + * concurrent capture arriving in the install window goes through the + * NOOP sink (no buffer write) rather than through a live {@code BufferSink} + * with an orphaned lifecycle handle. The inverse ordering would minimize + * lifecycle bypass but maximize silent capture loss; we prefer the former. + * + * @param whiteboard the whiteboard to register the {@code Feature} + * toggle and {@code AuditEventListener} tracker on; + * non-null. + */ + public void initialize(@NotNull Whiteboard whiteboard) { + featureToggle = Feature.newFeature(FEATURE_TOGGLE_NAME, whiteboard); Review Comment: It's the `Feature default`, not set explicitly — `Feature.newFeature` creates an AtomicBoolean with no initial value, so it starts as `false`. Nothing in the audit code sets it. Will add a comment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
