From 76f6b725b89e7dcf7a289bdc473f670c0c2d2b7e Mon Sep 17 00:00:00 2001
From: Joshua Casey <joshua.casey@broadcom.com>
Date: Mon, 11 Nov 2024 10:33:01 -0600
Subject: [PATCH] Fix some rebase conflicts

---
 .../requestlogger/request_logger.go           | 59 ++++++++++---------
 .../requestlogger/request_logger_test.go      |  2 +-
 internal/plog/plog.go                         |  3 +-
 internal/plog/plog_test.go                    |  1 -
 4 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/internal/federationdomain/requestlogger/request_logger.go b/internal/federationdomain/requestlogger/request_logger.go
index 4ea6fdbeb..899ebceda 100644
--- a/internal/federationdomain/requestlogger/request_logger.go
+++ b/internal/federationdomain/requestlogger/request_logger.go
@@ -116,6 +116,35 @@ func (rl *requestLogger) logRequestReceived() {
 	)
 }
 
+func getLocationForAuditLogs(location string) string {
+	if location == "" {
+		return "no location header"
+	}
+
+	parsedLocation, err := url.Parse(location)
+	if err != nil {
+		return "unparsable location header"
+	}
+
+	// We don't know what this `Location` header is used for, so redact nearly all query parameters
+	redactedParams := parsedLocation.Query()
+	for k, v := range redactedParams {
+		// Due to https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1,
+		// authorize errors can have an 'error' and an 'error_description' parameter
+		// which should never contain PII and is safe to log.
+		// The 'err' parameter may be populated by the post_login_handler to indicate issues
+		// when using Supervisor's built-in login page.
+		if k == "error" || k == "error_description" || k == "err" {
+			continue
+		}
+		for i := range v {
+			redactedParams[k][i] = "redacted"
+		}
+	}
+	parsedLocation.RawQuery = redactedParams.Encode()
+	return parsedLocation.String()
+}
+
 func (rl *requestLogger) logRequestComplete() {
 	r := rl.req
 
@@ -123,41 +152,13 @@ func (rl *requestLogger) logRequestComplete() {
 		return
 	}
 
-	location := rl.Header().Get("Location")
-	if location == "" {
-		location = "no location header"
-	} else {
-		parsedLocation, err := url.Parse(location)
-		if err != nil {
-			location = "unparsable location header"
-		} else {
-			// We don't know what this `Location` header is used for, so redact all query params
-			redactedParams := parsedLocation.Query()
-			for k, v := range redactedParams {
-				// Due to https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1,
-				// authorize errors can have an 'error' and an 'error_description' parameter
-				// which should never contain PII and is safe to log.
-				// The 'err' parameter may be populated by the post_login_handler to indicate issues
-				// when using Supervisor's built-in login page.
-				if k == "error" || k == "error_description" || k == "err" {
-					continue
-				}
-				for i := range v {
-					redactedParams[k][i] = "redacted"
-				}
-			}
-			parsedLocation.RawQuery = redactedParams.Encode()
-			location = parsedLocation.String()
-		}
-	}
-
 	rl.auditLogger.Audit(auditevent.HTTPRequestCompleted,
 		r.Context(),
 		plog.NoSessionPersisted(),
 		"path", r.URL.Path, // include the path again to make it easy to "grep -v healthz" to watch all other audit events
 		"latency", rl.clock.Since(rl.startTime),
 		"responseStatus", rl.status,
-		"location", location,
+		"location", getLocationForAuditLogs(rl.Header().Get("Location")),
 	)
 }
 
diff --git a/internal/federationdomain/requestlogger/request_logger_test.go b/internal/federationdomain/requestlogger/request_logger_test.go
index 37bf208b9..1ad3e2653 100644
--- a/internal/federationdomain/requestlogger/request_logger_test.go
+++ b/internal/federationdomain/requestlogger/request_logger_test.go
@@ -156,7 +156,7 @@ func TestLogRequestComplete(t *testing.T) {
 			auditCfg: supervisor.AuditSpec{
 				InternalPaths: "Enabled",
 			},
-			wantAuditLogs: happyAuditEventWanted("/healthz", ""),
+			wantAuditLogs: happyAuditEventWanted("/healthz", "no location header"),
 		},
 		{
 			name:     "when internal paths are Enabled, audits external paths",
diff --git a/internal/plog/plog.go b/internal/plog/plog.go
index 8768071e4..d78022d10 100644
--- a/internal/plog/plog.go
+++ b/internal/plog/plog.go
@@ -33,8 +33,9 @@ import (
 	"slices"
 
 	"github.com/go-logr/logr"
-	"go.pinniped.dev/internal/auditevent"
 	"k8s.io/apiserver/pkg/audit"
+
+	"go.pinniped.dev/internal/auditevent"
 )
 
 const errorKey = "error" // this matches zapr's default for .Error calls (which is asserted via tests)
diff --git a/internal/plog/plog_test.go b/internal/plog/plog_test.go
index c2df44c7f..894a28c22 100644
--- a/internal/plog/plog_test.go
+++ b/internal/plog/plog_test.go
@@ -4,7 +4,6 @@
 package plog
 
 import (
-	"bytes"
 	"fmt"
 	"runtime"
 	"strings"