add config for audit logging, remove Audit() from Logger interface

Co-authored-by: Joshua Casey <joshuatcasey@gmail.com>

deploy/concierge/deployment.yaml

@@ -103,6 +103,8 @@ data:
     tls:
       onedottwo:
         allowedCiphers: (@= str(data.values.allowed_ciphers_for_tls_onedottwo) @)
+    audit:
+      logUsernamesAndGroups: (@= data.values.audit.log_usernames_and_groups @)
 ---
 #@ if data.values.image_pull_dockerconfigjson and data.values.image_pull_dockerconfigjson != "":
 apiVersion: v1

deploy/concierge/values.yaml

@@ -231,3 +231,15 @@ no_proxy: "$(KUBERNETES_SERVICE_HOST),169.254.169.254,127.0.0.1,localhost,.svc,.
 #! An empty array is perfectly valid, as is any array of strings.
 allowed_ciphers_for_tls_onedottwo:
 - ""
+
+#@schema/title "Audit logging configuration"
+#@schema/desc "Customize the content of audit log events."
+audit:
+
+  #@schema/title "Log usernames and groups"
+  #@ log_usernames_and_groups_desc = "Enables or disables printing usernames and group names in audit logs. Options are 'enabled' or 'disabled'. \
+  #@ If enabled, usernames are group names may be printed in audit log events. \
+  #@ If disabled, usernames and group names will be redacted from audit logs because they might contain personally identifiable information."
+  #@schema/desc log_usernames_and_groups_desc
+  #@schema/validation one_of=["enabled", "disabled"]
+  log_usernames_and_groups: disabled

deploy/supervisor/helpers.lib.yaml

@@ -57,6 +57,10 @@ _: #@ template.replace(data.values.custom_labels)
 #@       "onedottwo": {
 #@         "allowedCiphers": data.values.allowed_ciphers_for_tls_onedottwo
 #@       }
+#@     },
+#@     "audit": {
+#@       "logUsernamesAndGroups": data.values.audit.log_usernames_and_groups,
+#@       "logInternalPaths": data.values.audit.log_internal_paths
 #@     }
 #@   }
 #@   if data.values.log_level:

deploy/supervisor/values.yaml

@@ -220,3 +220,23 @@ endpoints: { }
 #! An empty array is perfectly valid, as is any array of strings.
 allowed_ciphers_for_tls_onedottwo:
 - ""
+
+#@schema/title "Audit logging configuration"
+#@schema/desc "Customize the content of audit log events."
+audit:
+
+  #@schema/title "Log usernames and groups"
+  #@ log_usernames_and_groups_desc = "Enables or disables printing usernames and group names in audit logs. Options are 'enabled' or 'disabled'. \
+  #@ If enabled, usernames are group names may be printed in audit log events. \
+  #@ If disabled, usernames and group names will be redacted from audit logs because they might contain personally identifiable information."
+  #@schema/desc log_usernames_and_groups_desc
+  #@schema/validation one_of=["enabled", "disabled"]
+  log_usernames_and_groups: disabled
+
+  #@schema/title "Log HTTPS requests for internal paths"
+  #@ log_internal_paths = "Enables or disables request logging for internal paths in audit logs. Options are 'enabled' or 'disabled'. \
+  #@ If enabled, requests to certain paths that are typically only used internal to the cluster (e.g. /healthz) will be enabled, which can be very verbose. \
+  #@ If disabled, requests to those paths will not be audit logged."
+  #@schema/desc log_internal_paths
+  #@schema/validation one_of=["enabled", "disabled"]
+  log_internal_paths: disabled

internal/concierge/apiserver/apiserver.go

@@ -39,6 +39,7 @@ type ExtraConfig struct {
 	LoginConciergeGroupVersion    schema.GroupVersion
 	IdentityConciergeGroupVersion schema.GroupVersion
 	TokenClient                   *tokenclient.TokenClient
+	AuditLogger                   plog.AuditLogger
 }
 
 type PinnipedServer struct {
@@ -82,7 +83,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	for _, f := range []func() (schema.GroupVersionResource, rest.Storage){
 		func() (schema.GroupVersionResource, rest.Storage) {
 			tokenCredReqGVR := c.ExtraConfig.LoginConciergeGroupVersion.WithResource("tokencredentialrequests")
-			tokenCredStorage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer, tokenCredReqGVR.GroupResource(), plog.New())
+			tokenCredStorage := credentialrequest.NewREST(c.ExtraConfig.Authenticator, c.ExtraConfig.Issuer, tokenCredReqGVR.GroupResource(), c.ExtraConfig.AuditLogger)
 			return tokenCredReqGVR, tokenCredStorage
 		},
 		func() (schema.GroupVersionResource, rest.Storage) {

internal/concierge/server/server.go

@@ -180,21 +180,9 @@ func (a *App) runServer(ctx context.Context) error {
 		dynamiccertauthority.New(impersonationProxySigningCertProvider), // fallback to our internal CA if we need to
 	}
 
-	// Get the aggregated API server config.
-	aggregatedAPIServerConfig, err := getAggregatedAPIServerConfig(
-		dynamicServingCertProvider,
-		authenticators,
-		certIssuer,
-		buildControllers,
-		*cfg.APIGroupSuffix,
-		*cfg.AggregatedAPIServerPort,
-		scheme,
-		loginGV,
-		identityGV,
-	)
-	if err != nil {
-		return fmt.Errorf("could not configure aggregated API server: %w", err)
-	}
+	auditLogger := plog.NewAuditLogger(plog.AuditLogConfig{
+		LogUsernamesAndGroupNames: cfg.Audit.LogUsernamesAndGroups.Enabled(),
+	})
 
 	// Configure a token client that retrieves relatively short-lived tokens from the API server.
 	// It uses a k8s client without leader election because all pods need tokens.
@@ -206,13 +194,31 @@ func (a *App) runServer(ctx context.Context) error {
 	if err != nil {
 		return fmt.Errorf("could not create default kubernetes client: %w", err)
 	}
-	aggregatedAPIServerConfig.ExtraConfig.TokenClient = tokenclient.New(
+	tokenClient := tokenclient.New(
 		cfg.NamesConfig.ImpersonationProxyServiceAccount,
 		k8sClient.Kubernetes.CoreV1().ServiceAccounts(podInfo.Namespace),
 		impersonationProxyTokenCache.Set,
 		plog.New(),
 		tokenclient.WithExpirationSeconds(oneDayInSeconds))
 
+	// Get the aggregated API server config.
+	aggregatedAPIServerConfig, err := getAggregatedAPIServerConfig(
+		dynamicServingCertProvider,
+		authenticators,
+		certIssuer,
+		buildControllers,
+		*cfg.APIGroupSuffix,
+		*cfg.AggregatedAPIServerPort,
+		scheme,
+		loginGV,
+		identityGV,
+		auditLogger,
+		tokenClient,
+	)
+	if err != nil {
+		return fmt.Errorf("could not configure aggregated API server: %w", err)
+	}
+
 	// Complete the aggregated API server config and make a server instance.
 	server, err := aggregatedAPIServerConfig.Complete().New()
 	if err != nil {
@@ -235,6 +241,8 @@ func getAggregatedAPIServerConfig(
 	aggregatedAPIServerPort int64,
 	scheme *runtime.Scheme,
 	loginConciergeGroupVersion, identityConciergeGroupVersion schema.GroupVersion,
+	auditLogger plog.AuditLogger,
+	tokenClient *tokenclient.TokenClient,
 ) (*apiserver.Config, error) {
 	codecs := serializer.NewCodecFactory(scheme)
 
@@ -301,6 +309,8 @@ func getAggregatedAPIServerConfig(
 			NegotiatedSerializer:          codecs,
 			LoginConciergeGroupVersion:    loginConciergeGroupVersion,
 			IdentityConciergeGroupVersion: identityConciergeGroupVersion,
+			TokenClient:                   tokenClient,
+			AuditLogger:                   auditLogger,
 		},
 	}
 	return apiServerConfig, nil

internal/config/concierge/config.go

@@ -88,6 +88,10 @@ func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowe
 		return nil, fmt.Errorf("validate tls: %w", err)
 	}
 
+	if err := validateAudit(&config.Audit); err != nil {
+		return nil, fmt.Errorf("validate audit: %w", err)
+	}
+
 	if config.Labels == nil {
 		config.Labels = make(map[string]string)
 	}
@@ -200,3 +204,11 @@ func validateServerPort(port *int64) error {
 	}
 	return nil
 }
+
+func validateAudit(auditConfig *AuditSpec) error {
+	v := auditConfig.LogUsernamesAndGroups
+	if v != "" && v != Enabled && v != Disabled {
+		return constable.Error("invalid logUsernamesAndGroups format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')")
+	}
+	return nil
+}

internal/config/concierge/config_test.go

@@ -67,6 +67,8 @@ func TestFromPath(t *testing.T) {
 				    - foo
 				    - bar
 					- TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
+				audit:
+				  logUsernamesAndGroups: enabled
 			`),
 			wantConfig: &Config{
 				DiscoveryInfo: DiscoveryInfoSpec{
@@ -115,6 +117,9 @@ func TestFromPath(t *testing.T) {
 						},
 					},
 				},
+				Audit: AuditSpec{
+					LogUsernamesAndGroups: "enabled",
+				},
 			},
 		},
 		{
@@ -155,6 +160,8 @@ func TestFromPath(t *testing.T) {
 				log:
 				  level: all
 				  format: json
+				audit:
+				  logUsernamesAndGroups: disabled
 			`),
 			wantConfig: &Config{
 				DiscoveryInfo: DiscoveryInfoSpec{
@@ -195,6 +202,9 @@ func TestFromPath(t *testing.T) {
 					Level:  plog.LevelAll,
 					Format: plog.FormatJSON,
 				},
+				Audit: AuditSpec{
+					LogUsernamesAndGroups: "disabled",
+				},
 			},
 		},
 		{
@@ -287,6 +297,7 @@ func TestFromPath(t *testing.T) {
 					NamePrefix: ptr.To("pinniped-kube-cert-agent-"),
 					Image:      ptr.To("debian:latest"),
 				},
+				Audit: AuditSpec{LogUsernamesAndGroups: ""},
 			},
 		},
 		{
@@ -629,6 +640,28 @@ func TestFromPath(t *testing.T) {
 			allowedCiphersError: fmt.Errorf("some error from setAllowedCiphers"),
 			wantError:           "validate tls: some error from setAllowedCiphers",
 		},
+		{
+			name: "invalid audit.logUsernamesAndGroups format",
+			yaml: here.Doc(`
+				---
+				names:
+				  servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
+				  credentialIssuer: pinniped-config
+				  apiService: pinniped-api
+				  impersonationLoadBalancerService: impersonationLoadBalancerService-value
+				  impersonationClusterIPService: impersonationClusterIPService-value
+				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
+				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  agentServiceAccount: agentServiceAccount-value
+				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
+				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
+				audit:
+				  logUsernamesAndGroups: this-value-is-not-allowed
+			`),
+			wantError: "validate audit: invalid logUsernamesAndGroups format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')",
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

internal/config/concierge/types.go

@@ -5,6 +5,11 @@ package concierge
 
 import "go.pinniped.dev/internal/plog"
 
+const (
+	Enabled  = "enabled"
+	Disabled = "disabled"
+)
+
 // Config contains knobs to set up an instance of the Pinniped Concierge.
 type Config struct {
 	DiscoveryInfo                DiscoveryInfoSpec `json:"discovery"`
@@ -17,6 +22,17 @@ type Config struct {
 	Labels                       map[string]string `json:"labels"`
 	Log                          plog.LogSpec      `json:"log"`
 	TLS                          TLSSpec           `json:"tls"`
+	Audit                        AuditSpec         `json:"audit"`
+}
+
+type AuditUsernamesAndGroups string
+
+func (l AuditUsernamesAndGroups) Enabled() bool {
+	return l == Enabled
+}
+
+type AuditSpec struct {
+	LogUsernamesAndGroups AuditUsernamesAndGroups `json:"logUsernamesAndGroups"`
 }
 
 type TLSSpec struct {

internal/config/supervisor/config.go

@@ -100,6 +100,10 @@ func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowe
 		return nil, fmt.Errorf("validate tls: %w", err)
 	}
 
+	if err := validateAudit(&config.Audit); err != nil {
+		return nil, fmt.Errorf("validate audit: %w", err)
+	}
+
 	return &config, nil
 }
 
@@ -214,3 +218,23 @@ func validateServerPort(port *int64) error {
 	}
 	return nil
 }
+
+func validateAudit(auditConfig *AuditSpec) error {
+	const errFmt = "invalid %s format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')"
+
+	switch auditConfig.LogUsernamesAndGroups {
+	case Enabled, Disabled, "":
+		// no-op
+	default:
+		return fmt.Errorf(errFmt, "logUsernamesAndGroups")
+	}
+
+	switch auditConfig.LogInternalPaths {
+	case Enabled, Disabled, "":
+		// no-op
+	default:
+		return fmt.Errorf(errFmt, "logInternalPaths")
+	}
+
+	return nil
+}

internal/config/supervisor/config_test.go

@@ -52,6 +52,9 @@ func TestFromPath(t *testing.T) {
 				    - foo
 				    - bar
 				    - TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
+				audit:
+				  logUsernamesAndGroups: enabled
+				  logInternalPaths: enabled
 			`),
 			wantConfig: &Config{
 				APIGroupSuffix: ptr.To("some.suffix.com"),
@@ -86,6 +89,10 @@ func TestFromPath(t *testing.T) {
 						},
 					},
 				},
+				Audit: AuditSpec{
+					LogUsernamesAndGroups: "enabled",
+					LogInternalPaths:      "enabled",
+				},
 			},
 		},
 		{
@@ -123,6 +130,42 @@ func TestFromPath(t *testing.T) {
 					},
 				},
 				AggregatedAPIServerPort: ptr.To[int64](10250),
+				Audit: AuditSpec{
+					LogInternalPaths:      "",
+					LogUsernamesAndGroups: "",
+				},
+			},
+		},
+		{
+			name: "audit settings can be disabled explicitly",
+			yaml: here.Doc(`
+				---
+				names:
+				  defaultTLSCertificateSecret: my-secret-name
+				audit:
+				  logInternalPaths: disabled
+				  logUsernamesAndGroups: disabled
+			`),
+			wantConfig: &Config{
+				APIGroupSuffix: ptr.To("pinniped.dev"),
+				Labels:         map[string]string{},
+				NamesConfig: NamesConfigSpec{
+					DefaultTLSCertificateSecret: "my-secret-name",
+				},
+				Endpoints: &Endpoints{
+					HTTPS: &Endpoint{
+						Network: "tcp",
+						Address: ":8443",
+					},
+					HTTP: &Endpoint{
+						Network: "disabled",
+					},
+				},
+				AggregatedAPIServerPort: ptr.To[int64](10250),
+				Audit: AuditSpec{
+					LogInternalPaths:      "disabled",
+					LogUsernamesAndGroups: "disabled",
+				},
 			},
 		},
 		{
@@ -267,6 +310,28 @@ func TestFromPath(t *testing.T) {
 			`),
 			wantError: "validate aggregatedAPIServerPort: must be within range 1024 to 65535",
 		},
+		{
+			name: "invalid audit.logUsernamesAndGroups format",
+			yaml: here.Doc(`
+				---
+				names:
+				  defaultTLSCertificateSecret: my-secret-name
+				audit:
+				  logUsernamesAndGroups: this-is-not-a-valid-value
+			`),
+			wantError: "validate audit: invalid logUsernamesAndGroups format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')",
+		},
+		{
+			name: "invalid audit.logInternalPaths format",
+			yaml: here.Doc(`
+				---
+				names:
+				  defaultTLSCertificateSecret: my-secret-name
+				audit:
+				  logInternalPaths: this-is-not-a-valid-value
+			`),
+			wantError: "validate audit: invalid logInternalPaths format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')",
+		},
 		{
 			name: "returns setAllowedCiphers errors",
 			yaml: here.Doc(`

internal/config/supervisor/types.go

@@ -7,6 +7,11 @@ import (
 	"go.pinniped.dev/internal/plog"
 )
 
+const (
+	Enabled  = "enabled"
+	Disabled = "disabled"
+)
+
 // Config contains knobs to set up an instance of the Pinniped Supervisor.
 type Config struct {
 	APIGroupSuffix          *string           `json:"apiGroupSuffix,omitempty"`
@@ -20,11 +25,18 @@ type Config struct {
 }
 
 type AuditInternalPaths string
+type AuditUsernamesAndGroups string
 
-const AuditInternalPathsEnabled = "Enabled"
+func (l AuditInternalPaths) Enabled() bool {
+	return l == Enabled
+}
+func (l AuditUsernamesAndGroups) Enabled() bool {
+	return l == Enabled
+}
 
 type AuditSpec struct {
-	InternalPaths AuditInternalPaths `json:"internalPaths"`
+	LogInternalPaths      AuditInternalPaths      `json:"logInternalPaths"`
+	LogUsernamesAndGroups AuditUsernamesAndGroups `json:"logUsernamesAndGroups"`
 }
 
 type TLSSpec struct {

internal/controller/supervisorstorage/garbage_collector_test.go

@@ -57,7 +57,7 @@ func TestGarbageCollectorControllerInformerFilters(t *testing.T) {
 				nil,
 				secretsInformer,
 				observableWithInformerOption.WithInformer, // make it possible to observe the behavior of the Filters
-				plog.New(),
+				nil,
 			)
 			secretsInformerFilter = observableWithInformerOption.GetFilterForInformer(secretsInformer)
 		})
@@ -139,7 +139,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 			syncContext             *controllerlib.Context
 			fakeClock               *clocktesting.FakeClock
 			frozenNow               time.Time
-			auditLog                *bytes.Buffer
+			actualAuditLog          *bytes.Buffer
 			wantAuditLogs           []testutil.WantedAuditLog
 		)
 
@@ -148,7 +148,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		var startInformersAndController = func(idpCache dynamicupstreamprovider.DynamicUpstreamIDPProvider) {
 			// Set this at the last second to allow for injection of server override.
 			var auditLogger plog.AuditLogger
-			auditLogger, auditLog = plog.TestLogger(t)
+			auditLogger, actualAuditLog = plog.TestAuditLogger(t)
 			subject = GarbageCollectorController(
 				idpCache,
 				fakeClock,
@@ -198,7 +198,7 @@ func TestGarbageCollectorControllerSync(t *testing.T) {
 		it.After(func() {
 			cancelContextCancelFunc()
 
-			testutil.CompareAuditLogs(t, wantAuditLogs, auditLog.String())
+			testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
 		})
 
 		when("there are secrets without the garbage-collect-after annotation", func() {

internal/federationdomain/endpoints/auth/auth_handler_test.go

@@ -4027,7 +4027,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 		supervisorClient *supervisorfake.Clientset,
 		kubeClient *fake.Clientset,
 		secretsClient v1.SecretInterface,
-		auditLog *bytes.Buffer,
+		actualAuditLog *bytes.Buffer,
 	) {
 		if test.kubeResources != nil {
 			test.kubeResources(t, supervisorClient, kubeClient)
@@ -4118,7 +4118,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 		if test.wantAuditLogs != nil {
 			wantAuditLogs := test.wantAuditLogs(stateparam.Encoded(actualQueryStateParam), sessionID)
 			testutil.WantAuditIDOnEveryAuditLog(wantAuditLogs, "fake-audit-id")
-			testutil.CompareAuditLogs(t, wantAuditLogs, auditLog.String())
+			testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
 		}
 
 		switch {
@@ -4177,7 +4177,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 			if len(test.wantDownstreamAdditionalClaims) > 0 {
 				require.True(t, oidcIDPsCount > 0, "wantDownstreamAdditionalClaims requires at least one OIDC IDP")
 			}
-			auditLogger, auditLog := plog.TestLogger(t)
+			auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 			subject := NewHandler(
 				downstreamIssuer,
 				idps,
@@ -4186,7 +4186,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 				test.stateEncoder, test.cookieEncoder,
 				auditLogger,
 			)
-			runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, auditLog)
+			runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, actualAuditLog)
 		})
 	}
 
@@ -4202,7 +4202,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 		oauthHelperWithRealStorage, kubeOauthStore := createOauthHelperWithRealStorage(secretsClient, oidcClientsClient)
 		oauthHelperWithNullStorage, _ := createOauthHelperWithNullStorage(secretsClient, oidcClientsClient)
 		idpLister := test.idps.BuildFederationDomainIdentityProvidersListerFinder()
-		auditLogger, auditLog := plog.TestLogger(t)
+		auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 		subject := NewHandler(
 			downstreamIssuer,
 			idpLister,
@@ -4212,8 +4212,8 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 			auditLogger,
 		)
 
-		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, auditLog)
-		auditLog.Reset() // clear the log for the next authorize call
+		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, actualAuditLog)
+		actualAuditLog.Reset() // clear the log for the next authorize call
 
 		// Call the idpLister's setter to change the upstream IDP settings.
 		newProviderSettings := oidctestutil.NewTestUpstreamOIDCIdentityProviderBuilder().
@@ -4288,7 +4288,7 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 		// modified expectations. This should ensure that the implementation is using the in-memory cache
 		// of upstream IDP settings appropriately in terms of always getting the values from the cache
 		// on every request.
-		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, auditLog)
+		runOneTestCase(t, test, subject, kubeOauthStore, supervisorClient, kubeClient, secretsClient, actualAuditLog)
 	})
 }
 

internal/federationdomain/endpoints/callback/callback_handler_test.go

@@ -1853,7 +1853,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			jwksProviderIsUnused := jwks.NewDynamicJWKSProvider()
 			oauthHelper := oidc.FositeOauth2Helper(oauthStore, downstreamIssuer, hmacSecretFunc, jwksProviderIsUnused, timeoutsConfiguration, nil)
 
-			logger, log := plog.TestLogger(t)
+			auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 
 			subject := NewHandler(
 				test.idps.BuildFederationDomainIdentityProvidersListerFinder(),
@@ -1861,7 +1861,7 @@ func TestCallbackEndpoint(t *testing.T) {
 				happyStateCodec,
 				happyCookieCodec,
 				happyUpstreamRedirectURI,
-				logger,
+				auditLogger,
 			)
 
 			reqContext := context.WithValue(context.Background(), struct{ name string }{name: "test"}, "request-context")
@@ -1959,7 +1959,7 @@ func TestCallbackEndpoint(t *testing.T) {
 			if test.wantAuditLogs != nil {
 				wantAuditLogs := test.wantAuditLogs(testutil.GetStateParam(t, test.path), sessionID)
 				testutil.WantAuditIDOnEveryAuditLog(wantAuditLogs, "fake-audit-id")
-				testutil.CompareAuditLogs(t, wantAuditLogs, log.String())
+				testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
 			}
 		})
 	}

internal/federationdomain/endpoints/login/login_handler_test.go

@@ -449,9 +449,9 @@ func TestLoginEndpoint(t *testing.T) {
 				return test.postHandlerErr
 			}
 
-			logger, log := plog.TestLogger(t)
+			auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 
-			subject := NewHandler(happyStateCodec, happyCookieCodec, testGetHandler, testPostHandler, logger)
+			subject := NewHandler(happyStateCodec, happyCookieCodec, testGetHandler, testPostHandler, auditLogger)
 
 			subject.ServeHTTP(rsp, req)
 
@@ -468,7 +468,7 @@ func TestLoginEndpoint(t *testing.T) {
 			if test.wantAuditLogs != nil {
 				wantAuditLogs := test.wantAuditLogs(testutil.GetStateParam(t, test.path))
 				testutil.WantAuditIDOnEveryAuditLog(wantAuditLogs, "fake-audit-id")
-				testutil.CompareAuditLogs(t, wantAuditLogs, log.String())
+				testutil.CompareAuditLogs(t, wantAuditLogs, actualAuditLog.String())
 			}
 		})
 	}

internal/federationdomain/endpoints/login/post_login_handler_test.go

@@ -1147,7 +1147,9 @@ func TestPostLoginEndpoint(t *testing.T) {
 
 			rsp := httptest.NewRecorder()
 
-			subject := NewPostHandler(downstreamIssuer, tt.idps.BuildFederationDomainIdentityProvidersListerFinder(), oauthHelper, plog.New())
+			auditLogger, _ := plog.TestAuditLogger(t)
+
+			subject := NewPostHandler(downstreamIssuer, tt.idps.BuildFederationDomainIdentityProvidersListerFinder(), oauthHelper, auditLogger)
 
 			err := subject(rsp, req, happyEncodedUpstreamState, tt.decodedState)
 			if tt.wantErr != "" {

internal/federationdomain/endpoints/token/token_handler_test.go

@@ -5107,18 +5107,18 @@ func exchangeAuthcodeForTokens(
 		test.makeJwksSigningKeyAndProvider = generateJWTSigningKeyAndJWKSProvider
 	}
 
-	logger, actualAuditLog := plog.TestLogger(t)
+	auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 
 	var oauthHelper fosite.OAuth2Provider
 	// Note that makeHappyOauthHelper() calls simulateAuthEndpointHavingAlreadyRun() to preload the session storage.
-	oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore, test.makeJwksSigningKeyAndProvider, test.customSessionData, test.modifySession, logger)
+	oauthHelper, authCode, jwtSigningKey = makeHappyOauthHelper(t, authRequest, oauthStore, test.makeJwksSigningKeyAndProvider, test.customSessionData, test.modifySession, auditLogger)
 
 	subject = NewHandler(
 		idps,
 		oauthHelper,
 		timeoutsConfiguration.OverrideDefaultAccessTokenLifespan,
 		timeoutsConfiguration.OverrideDefaultIDTokenLifespan,
-		logger,
+		auditLogger,
 	)
 
 	authorizeEndpointGrantedOpenIDScope := strings.Contains(authRequest.Form.Get("scope"), "openid")

internal/federationdomain/endpointsmanager/manager.go

@@ -62,7 +62,7 @@ func NewManager(
 	secretsClient corev1client.SecretInterface,
 	oidcClientsClient v1alpha1.OIDCClientInterface,
 	auditLogger plog.AuditLogger,
-	auditCfg supervisor.AuditSpec,
+	auditInternalPathsCfg supervisor.AuditInternalPaths,
 ) *Manager {
 	m := &Manager{
 		providerHandlers:    make(map[string]http.Handler),
@@ -74,7 +74,7 @@ func NewManager(
 		auditLogger:         auditLogger,
 	}
 	// nextHandler is the next handler in the chain, called when this manager didn't know how to handle a request
-	m.buildHandlerChain(nextHandler, auditCfg)
+	m.buildHandlerChain(nextHandler, auditInternalPathsCfg)
 	return m
 }
 
@@ -193,11 +193,11 @@ func (m *Manager) SetFederationDomains(federationDomains ...*federationdomainpro
 	}
 }
 
-func (m *Manager) buildHandlerChain(nextHandler http.Handler, auditCfg supervisor.AuditSpec) {
+func (m *Manager) buildHandlerChain(nextHandler http.Handler, auditInternalPathsCfg supervisor.AuditInternalPaths) {
 	// Build the basic handler for FederationDomain endpoints.
 	handler := m.buildManagerHandler(nextHandler)
 	// Log all requests, including audit ID.
-	handler = requestlogger.WithHTTPRequestAuditLogging(handler, m.auditLogger, auditCfg)
+	handler = requestlogger.WithHTTPRequestAuditLogging(handler, m.auditLogger, auditInternalPathsCfg)
 	// Add random audit ID to request context and response headers.
 	handler = requestlogger.WithAuditID(handler)
 	m.handlerChain = handler

internal/federationdomain/endpointsmanager/manager_test.go

@@ -360,6 +360,8 @@ func TestManager(t *testing.T) {
 			cache.SetStateEncoderHashKey(issuer2, []byte("some-state-encoder-hash-key-2"))
 			cache.SetStateEncoderBlockKey(issuer2, []byte("16-bytes-STATE02"))
 
+			auditLogger, _ := plog.TestAuditLogger(t)
+
 			subject = NewManager(
 				nextHandler,
 				dynamicJWKSProvider,
@@ -367,8 +369,8 @@ func TestManager(t *testing.T) {
 				&cache,
 				secretsClient,
 				oidcClientsClient,
-				plog.New(),
-				supervisor.AuditSpec{},
+				auditLogger,
+				supervisor.Enabled,
 			)
 		})
 

internal/federationdomain/requestlogger/request_logger.go

@@ -49,9 +49,9 @@ func WithAuditID(handler http.Handler) http.Handler {
 	})
 }
 
-func WithHTTPRequestAuditLogging(handler http.Handler, auditLogger plog.AuditLogger, auditCfg supervisor.AuditSpec) http.Handler {
+func WithHTTPRequestAuditLogging(handler http.Handler, auditLogger plog.AuditLogger, auditInternalPathsCfg supervisor.AuditInternalPaths) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		rl := newRequestLogger(req, w, auditLogger, time.Now(), auditCfg)
+		rl := newRequestLogger(req, w, auditLogger, time.Now(), auditInternalPathsCfg)
 
 		rl.logRequestReceived()
 		defer rl.logRequestComplete()
@@ -73,19 +73,25 @@ type requestLogger struct {
 	userAgent string
 	w         http.ResponseWriter
 
-	auditLogger plog.AuditLogger
-	auditCfg    supervisor.AuditSpec
+	auditLogger        plog.AuditLogger
+	auditInternalPaths bool
 }
 
-func newRequestLogger(req *http.Request, w http.ResponseWriter, auditLogger plog.AuditLogger, startTime time.Time, auditCfg supervisor.AuditSpec) *requestLogger {
+func newRequestLogger(
+	req *http.Request,
+	w http.ResponseWriter,
+	auditLogger plog.AuditLogger,
+	startTime time.Time,
+	auditInternalPathsCfg supervisor.AuditInternalPaths,
+) *requestLogger {
 	return &requestLogger{
-		req:         req,
-		w:           w,
-		startTime:   startTime,
-		clock:       clock.RealClock{},
-		userAgent:   req.UserAgent(), // cache this from the req to avoid any possibility of concurrent read/write problems with headers map
-		auditLogger: auditLogger,
-		auditCfg:    auditCfg,
+		req:                req,
+		w:                  w,
+		startTime:          startTime,
+		clock:              clock.RealClock{},
+		userAgent:          req.UserAgent(), // cache this from the req to avoid any possibility of concurrent read/write problems with headers map
+		auditLogger:        auditLogger,
+		auditInternalPaths: auditInternalPathsCfg.Enabled(),
 	}
 }
 
@@ -98,7 +104,7 @@ func internalPaths() []string {
 func (rl *requestLogger) logRequestReceived() {
 	r := rl.req
 
-	if rl.auditCfg.InternalPaths != supervisor.AuditInternalPathsEnabled && slices.Contains(internalPaths(), r.URL.Path) {
+	if !rl.auditInternalPaths && slices.Contains(internalPaths(), r.URL.Path) {
 		return
 	}
 
@@ -148,7 +154,7 @@ func getLocationForAuditLogs(location string) string {
 func (rl *requestLogger) logRequestComplete() {
 	r := rl.req
 
-	if rl.auditCfg.InternalPaths != supervisor.AuditInternalPathsEnabled && slices.Contains(internalPaths(), r.URL.Path) {
+	if !rl.auditInternalPaths && slices.Contains(internalPaths(), r.URL.Path) {
 		return
 	}
 

internal/federationdomain/requestlogger/request_logger_test.go

@@ -13,7 +13,6 @@ import (
 	"go.uber.org/mock/gomock"
 	clocktesting "k8s.io/utils/clock/testing"
 
-	"go.pinniped.dev/internal/config/supervisor"
 	"go.pinniped.dev/internal/mocks/mockresponsewriter"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/testutil"
@@ -39,52 +38,44 @@ func TestLogRequestReceived(t *testing.T) {
 	}
 
 	tests := []struct {
-		name          string
-		path          string
-		auditCfg      supervisor.AuditSpec
-		wantAuditLogs []testutil.WantedAuditLog
+		name               string
+		path               string
+		auditInternalPaths bool
+		wantAuditLogs      []testutil.WantedAuditLog
 	}{
 		{
-			name: "when internal paths are not Enabled, ignores internal paths",
-			path: "/healthz",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Disabled",
-			},
-			wantAuditLogs: noAuditEventsWanted,
+			name:               "when internal paths are not enabled, ignores internal paths",
+			path:               "/healthz",
+			auditInternalPaths: false,
+			wantAuditLogs:      noAuditEventsWanted,
 		},
 		{
-			name: "when internal paths are not Enabled, audits external path",
-			path: "/pretend-to-login",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Disabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/pretend-to-login"),
+			name:               "when internal paths are not enabled, audits external path",
+			path:               "/pretend-to-login",
+			auditInternalPaths: false,
+			wantAuditLogs:      happyAuditEventWanted("/pretend-to-login"),
 		},
 		{
-			name: "when internal paths are Enabled, audits internal paths",
-			path: "/healthz",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Enabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/healthz"),
+			name:               "when internal paths are enabled, audits internal paths",
+			path:               "/healthz",
+			auditInternalPaths: true,
+			wantAuditLogs:      happyAuditEventWanted("/healthz"),
 		},
 		{
-			name: "when internal paths are Enabled, audits external paths",
-			path: "/pretend-to-login",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Enabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/pretend-to-login"),
+			name:               "when internal paths are enabled, audits external paths",
+			path:               "/pretend-to-login",
+			auditInternalPaths: true,
+			wantAuditLogs:      happyAuditEventWanted("/pretend-to-login"),
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			t.Parallel()
-			logger, log := plog.TestLogger(t)
+			auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 
 			subject := requestLogger{
-				auditLogger: logger,
+				auditLogger: auditLogger,
 				req: &http.Request{
 					Method: "some-method",
 					Proto:  "some-proto",
@@ -97,13 +88,13 @@ func TestLogRequestReceived(t *testing.T) {
 						ServerName: "some-sni-server-name",
 					},
 				},
-				userAgent: "some-user-agent",
-				auditCfg:  test.auditCfg,
+				userAgent:          "some-user-agent",
+				auditInternalPaths: test.auditInternalPaths,
 			}
 
 			subject.logRequestReceived()
 
-			testutil.CompareAuditLogs(t, test.wantAuditLogs, log.String())
+			testutil.CompareAuditLogs(t, test.wantAuditLogs, actualAuditLog.String())
 		})
 	}
 }
@@ -127,45 +118,37 @@ func TestLogRequestComplete(t *testing.T) {
 	}
 
 	tests := []struct {
-		name          string
-		path          string
-		location      string
-		auditCfg      supervisor.AuditSpec
-		wantAuditLogs []testutil.WantedAuditLog
+		name               string
+		path               string
+		location           string
+		auditInternalPaths bool
+		wantAuditLogs      []testutil.WantedAuditLog
 	}{
 		{
-			name: "when internal paths are not Enabled, ignores internal paths",
-			path: "/healthz",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Disabled",
-			},
-			wantAuditLogs: noAuditEventsWanted,
+			name:               "when internal paths are not enabled, ignores internal paths",
+			path:               "/healthz",
+			auditInternalPaths: false,
+			wantAuditLogs:      noAuditEventsWanted,
 		},
 		{
-			name:     "when internal paths are not Enabled, audits external path with location (redacting unknown query params)",
-			path:     "/pretend-to-login",
-			location: "http://127.0.0.1?foo=bar&foo=quz&lorem=ipsum",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Disabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "http://127.0.0.1?foo=redacted&foo=redacted&lorem=redacted"),
+			name:               "when internal paths are not enabled, audits external path with location (redacting unknown query params)",
+			path:               "/pretend-to-login",
+			location:           "http://127.0.0.1?foo=bar&foo=quz&lorem=ipsum",
+			auditInternalPaths: false,
+			wantAuditLogs:      happyAuditEventWanted("/pretend-to-login", "http://127.0.0.1?foo=redacted&foo=redacted&lorem=redacted"),
 		},
 		{
-			name: "when internal paths are Enabled, audits internal paths",
-			path: "/healthz",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Enabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/healthz", "no location header"),
+			name:               "when internal paths are enabled, audits internal paths",
+			path:               "/healthz",
+			auditInternalPaths: true,
+			wantAuditLogs:      happyAuditEventWanted("/healthz", "no location header"),
 		},
 		{
-			name:     "when internal paths are Enabled, audits external paths",
-			path:     "/pretend-to-login",
-			location: "some-location",
-			auditCfg: supervisor.AuditSpec{
-				InternalPaths: "Enabled",
-			},
-			wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "some-location"),
+			name:               "when internal paths are enabled, audits external paths",
+			path:               "/pretend-to-login",
+			location:           "some-location",
+			auditInternalPaths: true,
+			wantAuditLogs:      happyAuditEventWanted("/pretend-to-login", "some-location"),
 		},
 		{
 			name:          "audits path without location",
@@ -203,10 +186,10 @@ func TestLogRequestComplete(t *testing.T) {
 				})
 			}
 
-			logger, log := plog.TestLogger(t)
+			auditLogger, actualAuditLog := plog.TestAuditLogger(t)
 
 			subject := requestLogger{
-				auditLogger: logger,
+				auditLogger: auditLogger,
 				startTime:   startTime,
 				clock:       frozenClock,
 				req: &http.Request{
@@ -214,14 +197,14 @@ func TestLogRequestComplete(t *testing.T) {
 						Path: test.path,
 					},
 				},
-				status:   777,
-				w:        mockResponseWriter,
-				auditCfg: test.auditCfg,
+				status:             777,
+				w:                  mockResponseWriter,
+				auditInternalPaths: test.auditInternalPaths,
 			}
 
 			subject.logRequestComplete()
 
-			testutil.CompareAuditLogs(t, test.wantAuditLogs, log.String())
+			testutil.CompareAuditLogs(t, test.wantAuditLogs, actualAuditLog.String())
 		})
 	}
 }

internal/plog/plog.go

@@ -71,8 +71,6 @@ type AuditLogger interface {
 // If test assertions are desired, Logger should be passed in as an input.  New should be used as the
 // production implementation and TestLogger should be used to write test assertions.
 type Logger interface {
-	AuditLogger
-
 	Error(msg string, err error, keysAndValues ...any)
 	Warning(msg string, keysAndValues ...any)
 	WarningErr(msg string, err error, keysAndValues ...any)
@@ -92,6 +90,7 @@ type Logger interface {
 	// for internal and test use only
 	withDepth(d int) Logger
 	withLogrMod(mod func(logr.Logger) logr.Logger) Logger
+	audit(msg string, keysAndValues ...any)
 }
 
 // MinLogger is the overlap between Logger and logr.Logger.
@@ -107,28 +106,34 @@ type pLogger struct {
 	depth int
 }
 
+type AuditLogConfig struct {
+	LogUsernamesAndGroupNames bool
+}
+
+type auditLogger struct {
+	cfg    AuditLogConfig
+	logger Logger
+}
+
 func New() Logger {
 	return pLogger{}
 }
 
-// Error logs show in the pod log output as `"level":"error","message":"some error msg"`
-// where the message text comes from the err parameter.
-// They also contain the standard `timestamp` and `caller` keys, along with any other keysAndValues.
-// Only when the global log level is configured to "trace" or "all", then they will also include a `stacktrace` key.
-// Error logs cannot be suppressed by the global log level configuration.
-func (p pLogger) Error(msg string, err error, keysAndValues ...any) {
-	p.logr().WithCallDepth(p.depth+1).Error(err, msg, keysAndValues...)
+func NewAuditLogger(cfg AuditLogConfig) AuditLogger {
+	return &auditLogger{
+		cfg:    cfg,
+		logger: New(),
+	}
 }
 
 // Audit logs show in the pod log output as `"level":"info","message":"some msg","auditEvent":true`
 // where the message text comes from the msg parameter.
 // They also contain the standard `timestamp` and `caller` keys, along with any other keysAndValues.
 // Only when the global log level is configured to "trace" or "all", then they will also include a `stacktrace` key.
-// Audit logs cannot be suppressed by the global log level configuration, but rather can be disabled
-// by their own separate configuration. This is because Audit logs should always be printed when they are desired
-// by the admin, regardless of global log level, yet the admin should also have a way to entirely disable them
-// when they want to avoid potential PII (e.g. usernames) in their pod logs.
-func (p pLogger) Audit(msg auditevent.Message, reqCtx context.Context, session SessionIDGetter, keysAndValues ...any) {
+// Audit logs cannot be suppressed by the global log level configuration. This is because Audit logs should always
+// be printed, regardless of global log level. Audit logs offer their own configuration options, such as a way to
+// avoid potential PII (e.g. usernames and group names) in their pod logs.
+func (a *auditLogger) Audit(msg auditevent.Message, reqCtx context.Context, session SessionIDGetter, keysAndValues ...any) {
 	// Always add a key/value auditEvent=true.
 	keysAndValues = slices.Concat([]any{"auditEvent", true}, keysAndValues)
 
@@ -148,7 +153,23 @@ func (p pLogger) Audit(msg auditevent.Message, reqCtx context.Context, session S
 		keysAndValues = slices.Concat([]any{"sessionID", sessionID}, keysAndValues)
 	}
 
-	p.logr().V(klogLevelWarning).WithCallDepth(p.depth+1).Info(string(msg), keysAndValues...)
+	a.logger.audit(string(msg), keysAndValues...)
+}
+
+// audit is used internally by AuditLogger to print an audit log event to the pLogger's output.
+func (p pLogger) audit(msg string, keysAndValues ...any) {
+	// Always print log message (klogLevelWarning cannot be suppressed by configuration),
+	// and always use the Info function because audit logs are not warnings or errors.
+	p.logr().V(klogLevelWarning).WithCallDepth(p.depth+1).Info(msg, keysAndValues...)
+}
+
+// Error logs show in the pod log output as `"level":"error","message":"some error msg"`
+// where the message text comes from the err parameter.
+// They also contain the standard `timestamp` and `caller` keys, along with any other keysAndValues.
+// Only when the global log level is configured to "trace" or "all", then they will also include a `stacktrace` key.
+// Error logs cannot be suppressed by the global log level configuration.
+func (p pLogger) Error(msg string, err error, keysAndValues ...any) {
+	p.logr().WithCallDepth(p.depth+1).Error(err, msg, keysAndValues...)
 }
 
 func (p pLogger) warningDepth(msg string, depth int, keysAndValues ...any) {

internal/plog/testing.go

@@ -71,6 +71,13 @@ func TestLogger(t *testing.T) (Logger, *bytes.Buffer) {
 		&log
 }
 
+func TestAuditLogger(t *testing.T) (AuditLogger, *bytes.Buffer) {
+	t.Helper()
+
+	underlyingLogger, logBuf := TestLogger(t)
+	return &auditLogger{logger: underlyingLogger, cfg: AuditLogConfig{LogUsernamesAndGroupNames: true}}, logBuf
+}
+
 func TestConsoleLogger(t *testing.T, w io.Writer) Logger {
 	t.Helper()
 

internal/registry/credentialrequest/rest_test.go

@@ -33,7 +33,7 @@ import (
 )
 
 func TestNew(t *testing.T) {
-	r := NewREST(nil, nil, schema.GroupResource{Group: "bears", Resource: "panda"}, plog.New())
+	r := NewREST(nil, nil, schema.GroupResource{Group: "bears", Resource: "panda"}, nil)
 	require.NotNil(t, r)
 	require.False(t, r.NamespaceScoped())
 	require.Equal(t, []string{"pinniped"}, r.Categories())
@@ -70,6 +70,7 @@ func TestCreate(t *testing.T) {
 		var ctrl *gomock.Controller
 		var logger *testutil.TranscriptLogger
 		var originalKLogLevel klog.Level
+		var auditLogger plog.AuditLogger
 
 		it.Before(func() {
 			r = require.New(t)
@@ -79,6 +80,7 @@ func TestCreate(t *testing.T) {
 			originalKLogLevel = testutil.GetGlobalKlogLevel()
 			// trace.Log() utility will only log at level 2 or above, so set that for this test.
 			testutil.SetGlobalKlogLevel(t, 2) //nolint:staticcheck // old test of code using trace.Log()
+			auditLogger, _ = plog.TestAuditLogger(t)
 		})
 
 		it.After(func() {
@@ -104,7 +106,7 @@ func TestCreate(t *testing.T) {
 				5*time.Minute,
 			).Return([]byte("test-cert"), []byte("test-key"), nil)
 
-			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -143,7 +145,7 @@ func TestCreate(t *testing.T) {
 				IssueClientCertPEM(gomock.Any(), gomock.Any(), gomock.Any()).
 				Return(nil, nil, fmt.Errorf("some certificate authority error"))
 
-			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, clientCertIssuer, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 			requireSuccessfulResponseWithAuthenticationFailureMessage(t, err, response)
@@ -156,7 +158,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator := mockcredentialrequest.NewMockTokenCredentialRequestAuthenticator(ctrl)
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).Return(nil, nil)
 
-			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -171,7 +173,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
 				Return(nil, errors.New("some webhook error"))
 
-			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -186,7 +188,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req).
 				Return(&user.DefaultInfo{Name: ""}, nil)
 
-			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -205,7 +207,7 @@ func TestCreate(t *testing.T) {
 					Groups: []string{"test-group-1", "test-group-2"},
 				}, nil)
 
-			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -224,7 +226,7 @@ func TestCreate(t *testing.T) {
 					Extra:  map[string][]string{"test-key": {"test-val-1", "test-val-2"}},
 				}, nil)
 
-			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, nil, schema.GroupResource{}, auditLogger)
 
 			response, err := callCreate(context.Background(), storage, req)
 
@@ -234,7 +236,7 @@ func TestCreate(t *testing.T) {
 
 		it("CreateFailsWhenGivenTheWrongInputType", func() {
 			notACredentialRequest := runtime.Unknown{}
-			response, err := NewREST(nil, nil, schema.GroupResource{}, plog.New()).Create(
+			response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create(
 				genericapirequest.NewContext(),
 				&notACredentialRequest,
 				rest.ValidateAllObjectFunc,
@@ -245,7 +247,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenTokenValueIsEmptyInRequest", func() {
-			storage := NewREST(nil, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger)
 			response, err := callCreate(context.Background(), storage, credentialRequest(loginapi.TokenCredentialRequestSpec{
 				Token: "",
 			}))
@@ -256,7 +258,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenValidationFails", func() {
-			storage := NewREST(nil, nil, schema.GroupResource{}, plog.New())
+			storage := NewREST(nil, nil, schema.GroupResource{}, auditLogger)
 			response, err := storage.Create(
 				context.Background(),
 				validCredentialRequest(),
@@ -276,7 +278,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()).
 				Return(&user.DefaultInfo{Name: "test-user"}, nil)
 
-			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, auditLogger)
 			response, err := storage.Create(
 				context.Background(),
 				req,
@@ -297,7 +299,7 @@ func TestCreate(t *testing.T) {
 			requestAuthenticator.EXPECT().AuthenticateTokenCredentialRequest(gomock.Any(), req.DeepCopy()).
 				Return(&user.DefaultInfo{Name: "test-user"}, nil)
 
-			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, plog.New())
+			storage := NewREST(requestAuthenticator, successfulIssuer(ctrl), schema.GroupResource{}, auditLogger)
 			validationFunctionWasCalled := false
 			var validationFunctionSawTokenValue string
 			response, err := storage.Create(
@@ -317,7 +319,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenRequestOptionsDryRunIsNotEmpty", func() {
-			response, err := NewREST(nil, nil, schema.GroupResource{}, plog.New()).Create(
+			response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create(
 				genericapirequest.NewContext(),
 				validCredentialRequest(),
 				rest.ValidateAllObjectFunc,
@@ -331,7 +333,7 @@ func TestCreate(t *testing.T) {
 		})
 
 		it("CreateFailsWhenNamespaceIsNotEmpty", func() {
-			response, err := NewREST(nil, nil, schema.GroupResource{}, plog.New()).Create(
+			response, err := NewREST(nil, nil, schema.GroupResource{}, auditLogger).Create(
 				genericapirequest.WithNamespace(genericapirequest.NewContext(), "some-ns"),
 				validCredentialRequest(),
 				rest.ValidateAllObjectFunc,

internal/supervisor/server/server.go

@@ -149,6 +149,7 @@ func prepareControllers(
 	pinnipedInformers supervisorinformers.SharedInformerFactory,
 	leaderElector controllerinit.RunnerWrapper,
 	podInfo *downward.PodInfo,
+	auditLogger plog.AuditLogger,
 ) controllerinit.RunnerBuilder {
 	const certificateName string = "pinniped-supervisor-api-tls-serving-certificate"
 	clientSecretSupervisorGroupData := groupsuffix.SupervisorAggregatedGroups(*cfg.APIGroupSuffix)
@@ -167,7 +168,7 @@ func prepareControllers(
 				kubeClient,
 				secretInformer,
 				controllerlib.WithInformer,
-				plog.New(),
+				auditLogger,
 			),
 			singletonWorker,
 		).
@@ -451,6 +452,10 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
 		return fmt.Errorf("cannot create k8s client without leader election: %w", err)
 	}
 
+	auditLogger := plog.NewAuditLogger(plog.AuditLogConfig{
+		LogUsernamesAndGroupNames: cfg.Audit.LogUsernamesAndGroups.Enabled(),
+	})
+
 	kubeInformers := k8sinformers.NewSharedInformerFactoryWithOptions(
 		client.Kubernetes,
 		defaultResyncInterval,
@@ -484,8 +489,8 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
 		&secretCache,
 		clientWithoutLeaderElection.Kubernetes.CoreV1().Secrets(serverInstallationNamespace), // writes to kube storage are allowed for non-leaders
 		client.PinnipedSupervisor.ConfigV1alpha1().OIDCClients(serverInstallationNamespace),
-		plog.New(),
-		cfg.Audit,
+		auditLogger,
+		cfg.Audit.LogInternalPaths,
 	)
 
 	// Get the "real" name of the client secret supervisor API group (i.e., the API group name with the
@@ -508,6 +513,7 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
 		pinnipedInformers,
 		leaderElector,
 		podInfo,
+		auditLogger,
 	)
 
 	shutdown := &sync.WaitGroup{}