Add @AuditRequestResponse which will send request, response,

and metadata to the bound `AuditClient`. This involved a refactor of the
existing RequestLoggingInterceptor to provide for a more easily
extensible hook to support logging, sending to audit client, or other
future use cases.

GitOrigin-RevId: 2bce3195b1e703101597dca876ba269eb3cb1991

Author: adrw

misk-audit-client/api/misk-audit-client.api

@@ -17,11 +17,25 @@ public final class misk/audit/AuditClientConfig {
 	public fun toString ()Ljava/lang/String;
 }
 
+public abstract interface annotation class misk/audit/AuditRequestResponse : java/lang/annotation/Annotation {
+	public abstract fun applicationName ()Ljava/lang/String;
+	public abstract fun automatedChange ()Z
+	public abstract fun description ()Ljava/lang/String;
+	public abstract fun detailURL ()Ljava/lang/String;
+	public abstract fun includeRequest ()Z
+	public abstract fun includeRequestHeaders ()Z
+	public abstract fun includeReseponseHeaders ()Z
+	public abstract fun includeResponse ()Z
+	public abstract fun richDescription ()Ljava/lang/String;
+	public abstract fun target ()Ljava/lang/String;
+}
+
 public final class misk/audit/FakeAuditClient : misk/testing/FakeFixture, misk/audit/AuditClient {
 	public static final field Companion Lmisk/audit/FakeAuditClient$Companion;
-	public fun <init> (Ljava/lang/String;Lmisk/scope/ActionScoped;Ljava/time/Clock;Lwisp/deployment/Deployment;)V
+	public static final field DEFAULT_USER Ljava/lang/String;
+	public fun <init> (Lmisk/audit/FakeAuditClient$OptionalBinder;)V
 	public final fun getEnableLogging ()Z
-	public final fun getSentEvents ()Ljava/util/List;
+	public final fun getSentEvents ()Ljava/util/concurrent/LinkedBlockingDeque;
 	public fun logEvent (Ljava/lang/String;Ljava/lang/String;ZLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;)V
 	public final fun setEnableLogging (Z)V
 }
@@ -62,6 +76,18 @@ public final class misk/audit/FakeAuditClient$FakeAuditEvent {
 	public fun toString ()Ljava/lang/String;
 }
 
+public final class misk/audit/FakeAuditClient$OptionalBinder {
+	public fun <init> ()V
+	public final fun getAppName ()Ljava/lang/String;
+	public final fun getCallerProvider ()Lmisk/scope/ActionScoped;
+	public final fun getClock ()Ljava/time/Clock;
+	public final fun getDeployment ()Lwisp/deployment/Deployment;
+	public final fun setAppName (Ljava/lang/String;)V
+	public final fun setCallerProvider (Lmisk/scope/ActionScoped;)V
+	public final fun setClock (Ljava/time/Clock;)V
+	public final fun setDeployment (Lwisp/deployment/Deployment;)V
+}
+
 public final class misk/audit/FakeAuditClientModule : misk/inject/KAbstractModule {
 	public fun <init> ()V
 }

misk-audit-client/build.gradle.kts

@@ -24,6 +24,7 @@ dependencies {
   testFixturesImplementation(project(":misk-inject"))
   testFixturesImplementation(project(":misk-testing-api"))
   testFixturesImplementation(project(":wisp:wisp-logging"))
+  testFixturesImplementation(project(":wisp:wisp-time-testing"))
 }
 
 mavenPublishing {

misk-audit-client/src/main/kotlin/misk/audit/AuditClient.kt

@@ -23,3 +23,37 @@ interface AuditClient {
     applicationName: String? = null,
   )
 }
+
+/**
+ * Annotation indicating that request and response information should be sent to the audit client.
+ *
+ * If you would like to turn off logging for all non-successful requests, use the parameter [successOnly].
+ * otherwise, all requests will be sent to the audit client including failures.
+ *
+ * If arguments and responses may include sensitive information, it is expected that the toString()
+ * methods of these objects will redact it.
+ */
+@Retention(AnnotationRetention.RUNTIME)
+@Target(AnnotationTarget.FUNCTION)
+annotation class AuditRequestResponse(
+  /** The specific resource the event relates to. */
+  val target: String = "",
+  /** Human-readable description of the event, limited to 4000 characters. Optional, but suggested even if richDescription is provided. */
+  val description: String = "",
+  /** Whether this change is being done directly by a human (false) or by an automated system (true) */
+  val automatedChange: Boolean = false,
+  /** Rich text description of the event, limited to 4000 characters. Optional if description is provided. */
+  val richDescription: String = "",
+  /** URL to a page with more details about the event. */
+  val detailURL: String = "",
+  /** Name of the application (slug) if different from the eventSource, otherwise null. */
+  val applicationName: String = "",
+  /** If false, request body will not be included. */
+  val includeRequest: Boolean = false,
+  /** If false, response body will not be included. */
+  val includeResponse: Boolean = false,
+  /** If false, request headers will not be included. */
+  val includeRequestHeaders: Boolean = false,
+  /** If false, response headers will not be included. */
+  val includeReseponseHeaders: Boolean = false,
+)

misk-audit-client/src/test/kotlin/misk/audit/FakeAuditClientTest.kt

@@ -2,11 +2,9 @@ package misk.audit
 
 import jakarta.inject.Inject
 import misk.MiskTestingServiceModule
-import misk.config.AppNameModule
 import misk.environment.DeploymentModule
 import misk.inject.KAbstractModule
 import misk.logging.LogCollectorModule
-import misk.security.authz.AccessControlModule
 import misk.testing.MiskTest
 import misk.testing.MiskTestModule
 import misk.web.MiskWebModule
@@ -33,7 +31,7 @@ class FakeAuditClientTest {
     )
     assertThat(logCollector.takeMessages(FakeAuditClient::class).size).isEqualTo(0)
     assertThat(client.sentEvents).hasSize(1)
-    assertThat(client.sentEvents[0]).isEqualTo(
+    assertThat(client.sentEvents.take()).isEqualTo(
       FakeAuditClient.FakeAuditEvent(
         eventSource = "test-app",
         eventTarget = "the-database-001",
@@ -65,9 +63,7 @@ class FakeAuditClientTest {
 
   class TestModule : KAbstractModule() {
     override fun configure() {
-      install(AppNameModule("test-app"))
       install(MiskWebModule(WebConfig(0)))
-      install(AccessControlModule())
       install(MiskTestingServiceModule())
       install(DeploymentModule(TESTING))
       install(LogCollectorModule())

misk-audit-client/src/testFixtures/kotlin/misk/audit/FakeAuditClient.kt

@@ -7,20 +7,21 @@ import misk.config.AppName
 import misk.scope.ActionScoped
 import misk.testing.FakeFixture
 import wisp.deployment.Deployment
+import wisp.deployment.TESTING
 import wisp.logging.getLogger
+import wisp.time.FakeClock
 import java.time.Clock
+import java.util.concurrent.LinkedBlockingDeque
 import kotlin.time.Duration.Companion.nanoseconds
 import kotlin.time.DurationUnit
 
 @Singleton
 class FakeAuditClient @Inject constructor(
-  @AppName private val appName: String,
-  private val callerProvider: ActionScoped<MiskCaller?>,
-  private val clock: Clock,
-  private val deployment: Deployment,
-): AuditClient, FakeFixture() {
-  private val region: String = "us-west-2"
-  val sentEvents: MutableList<FakeAuditEvent> by resettable {  mutableListOf() }
+  private val optionalBinder: OptionalBinder,
+) : AuditClient, FakeFixture() {
+  private val region = "us-west-2"
+
+  val sentEvents: LinkedBlockingDeque<FakeAuditEvent> by resettable { LinkedBlockingDeque<FakeAuditEvent>() }
   var enableLogging: Boolean by resettable { false }
 
   data class FakeAuditEvent(
@@ -50,18 +51,24 @@ class FakeAuditClient @Inject constructor(
   ) {
     val event =
       FakeAuditEvent(
-        eventSource = appName,
+        eventSource = optionalBinder.appName,
         eventTarget = target,
-        timestampSent = clock.instant().toEpochMilli().nanoseconds.toInt(DurationUnit.NANOSECONDS),
-        applicationName = applicationName ?: appName,
+        timestampSent = optionalBinder.clock.instant()
+          .toEpochMilli().nanoseconds.toInt(DurationUnit.NANOSECONDS),
+        applicationName = applicationName ?: optionalBinder.appName,
         approverLDAP = approverLDAP,
         automatedChange = automatedChange,
         description = description,
         richDescription = richDescription,
-        environment = deployment.mapToEnvironmentName(),
+        environment = optionalBinder.deployment.mapToEnvironmentName(),
         detailURL = detailURL,
         region = region,
-        requestorLDAP = requestorLDAP ?: if (automatedChange) null else callerProvider.get()?.principal,
+        requestorLDAP = requestorLDAP
+          ?: if (automatedChange) {
+            null
+          } else {
+            optionalBinder.callerProvider.getIfInScope()?.principal ?: DEFAULT_USER
+          },
       )
 
     sentEvents.add(event)
@@ -73,6 +80,25 @@ class FakeAuditClient @Inject constructor(
 
   companion object {
     private val logger = getLogger<FakeAuditClient>()
+    const val DEFAULT_USER = "default-user"
   }
 
+  /**
+   * https://github.com/google/guice/wiki/FrequentlyAskedQuestions#how-can-i-inject-optional-parameters-into-a-constructor
+   */
+  @Singleton
+  class OptionalBinder @Inject constructor() {
+    @com.google.inject.Inject(optional = true)
+    @AppName var appName: String = "test-app"
+
+    @com.google.inject.Inject(optional = true)
+    var callerProvider: ActionScoped<MiskCaller?> =
+      ActionScoped.of(MiskCaller(user = DEFAULT_USER))
+
+    @com.google.inject.Inject(optional = true)
+    var clock: Clock = FakeClock()
+
+    @com.google.inject.Inject(optional = true)
+    var deployment: Deployment = TESTING
+  }
 }

misk-audit-client/src/testFixtures/kotlin/misk/audit/FakeAuditClientModule.kt

@@ -2,9 +2,14 @@ package misk.audit
 
 import misk.inject.KAbstractModule
 import misk.inject.asSingleton
+import misk.testing.TestFixture
+import misk.web.interceptors.hooks.AuditClientHook
+import misk.web.interceptors.hooks.RequestResponseHook
 
-class FakeAuditClientModule: KAbstractModule() {
+class FakeAuditClientModule : KAbstractModule() {
   override fun configure() {
     bind<AuditClient>().to<FakeAuditClient>().asSingleton()
+    multibind<TestFixture>().to<FakeAuditClient>()
+    multibind<RequestResponseHook.Factory>().to<AuditClientHook.Factory>()
   }
 }

misk-hibernate/build.gradle.kts

@@ -44,6 +44,7 @@ dependencies {
   implementation(project(":misk-api"))
   implementation(project(":misk-actions"))
   implementation(project(":misk-admin"))
+  implementation(project(":misk-audit-client"))
   implementation(project(":misk-backoff"))
   implementation(project(":misk-core"))
   implementation(project(":misk-crypto"))
@@ -60,6 +61,7 @@ dependencies {
   testImplementation(project(":wisp:wisp-logging-testing"))
   testImplementation(project(":wisp:wisp-time-testing"))
   testImplementation(project(":misk-hibernate-testing"))
+  testImplementation(testFixtures(project(":misk-audit-client")))
   testImplementation(testFixtures(project(":misk-jdbc")))
   testImplementation(project(":misk-testing"))
   testImplementation(testFixtures(project(":misk-crypto")))

misk-hibernate/src/main/kotlin/misk/hibernate/actions/HibernateDatabaseQueryDynamicAction.kt

@@ -24,6 +24,7 @@ import misk.web.metadata.database.DatabaseQueryMetadata
 import wisp.logging.getLogger
 import jakarta.inject.Inject
 import jakarta.inject.Singleton
+import misk.audit.AuditRequestResponse
 import misk.web.dashboard.AdminDashboardAccess
 import kotlin.reflect.KClass
 
@@ -40,6 +41,7 @@ internal class HibernateDatabaseQueryDynamicAction @Inject constructor(
   @RequestContentType(MediaTypes.APPLICATION_JSON)
   @ResponseContentType(MediaTypes.APPLICATION_JSON)
   @AdminDashboardAccess
+  @AuditRequestResponse
   fun query(@RequestBody request: Request): Response {
     val caller = callerProvider.get()!!
     val queryClass = request.queryClass

misk-hibernate/src/main/kotlin/misk/hibernate/actions/HibernateDatabaseQueryStaticAction.kt

@@ -28,6 +28,7 @@ import wisp.logging.getLogger
 import java.lang.reflect.ParameterizedType
 import jakarta.inject.Inject
 import jakarta.inject.Singleton
+import misk.audit.AuditRequestResponse
 import misk.web.dashboard.AdminDashboardAccess
 import kotlin.reflect.KClass
 
@@ -45,6 +46,7 @@ internal class HibernateDatabaseQueryStaticAction @Inject constructor(
   @RequestContentType(MediaTypes.APPLICATION_JSON)
   @ResponseContentType(MediaTypes.APPLICATION_JSON)
   @AdminDashboardAccess
+  @AuditRequestResponse
   fun query(@RequestBody request: Request): Response {
     val caller = callerProvider.get()!!
     val queryClass = request.queryClass

misk-hibernate/src/test/kotlin/misk/hibernate/actions/HibernateDatabaseQueryDynamicActionTest.kt

@@ -16,6 +16,7 @@ import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import java.time.LocalDate
 import jakarta.inject.Inject
+import misk.audit.FakeAuditClient
 import javax.persistence.Transient
 import kotlin.reflect.full.declaredMemberProperties
 import kotlin.reflect.jvm.javaField
@@ -34,6 +35,7 @@ class HibernateDatabaseQueryDynamicActionTest {
       HibernateDatabaseQueryDynamicAction.Response
       >
   @Inject @Movies lateinit var transacter: Transacter
+  @Inject lateinit var auditClient: FakeAuditClient
 
   @BeforeEach
   fun before() {
@@ -225,5 +227,20 @@ class HibernateDatabaseQueryDynamicActionTest {
       ),
       results.results
     )
+
+    assertEquals(FakeAuditClient.FakeAuditEvent(
+      eventSource = "test-app",
+      eventTarget = "HibernateDatabaseQueryDynamicAction",
+      timestampSent = 2147483647,
+      applicationName = "test-app",
+      approverLDAP = null,
+      automatedChange = false,
+      description = "HibernateDatabaseQueryDynamicAction principal=joey",
+      richDescription = "HibernateDatabaseQueryDynamicAction principal=joey time=0.000 ns code=200",
+      environment = "testing",
+      detailURL = null,
+      region = "us-west-2",
+      requestorLDAP = "joey"
+    ), auditClient.sentEvents.take())
   }
 }