Fix some rebase conflicts

This commit is contained in:
Joshua Casey
2024-11-11 10:33:01 -06:00
parent f9e1dd4bec
commit 76f6b725b8
4 changed files with 33 additions and 32 deletions

View File

@@ -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")),
)
}

View File

@@ -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",

View File

@@ -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)

View File

@@ -4,7 +4,6 @@
package plog
import (
"bytes"
"fmt"
"runtime"
"strings"