Audit event 'HTTP Request Completed' will now log the location with err, error, and error_description query parameters

Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2024-11-08 15:36:04 -06:00
parent 2db5dda266
commit f4f393e5de
2 changed files with 30 additions and 23 deletions

View File

@@ -134,6 +134,14 @@ func (rl *requestLogger) logRequestComplete() {
// 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"
}

View File

@@ -142,7 +142,7 @@ func TestLogRequestComplete(t *testing.T) {
wantAuditLogs: noAuditEventsWanted,
},
{
name: "when internal paths are not Enabled, audits external path with location (redacting all query params)",
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{
@@ -151,31 +151,12 @@ func TestLogRequestComplete(t *testing.T) {
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 without location",
path: "/pretend-to-login",
location: "", // make it obvious
auditCfg: supervisor.AuditSpec{
InternalPaths: "Disabled",
},
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "no location header"),
},
{
name: "when internal paths are not Enabled, audits external path with invalid location",
path: "/pretend-to-login",
location: "http://e x a m p l e.com",
auditCfg: supervisor.AuditSpec{
InternalPaths: "Disabled",
},
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "unparsable location header"),
},
{
name: "when internal paths are Enabled, audits internal paths",
path: "/healthz",
location: "some-location",
name: "when internal paths are Enabled, audits internal paths",
path: "/healthz",
auditCfg: supervisor.AuditSpec{
InternalPaths: "Enabled",
},
wantAuditLogs: happyAuditEventWanted("/healthz", "some-location"),
wantAuditLogs: happyAuditEventWanted("/healthz", ""),
},
{
name: "when internal paths are Enabled, audits external paths",
@@ -186,6 +167,24 @@ func TestLogRequestComplete(t *testing.T) {
},
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "some-location"),
},
{
name: "audits path without location",
path: "/pretend-to-login",
location: "", // make it obvious
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "no location header"),
},
{
name: "audits path with invalid location",
path: "/pretend-to-login",
location: "http://e x a m p l e.com",
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "unparsable location header"),
},
{
name: "audits path with location redacting all query params except err, error, and error_description",
path: "/pretend-to-login",
location: "http://127.0.0.1:1234?code=pin_ac_FAKE&foo=bar&foo=quz&lorem=ipsum&err=some-err&error=some-error&error_description=some-error-description&zzlast=some-value",
wantAuditLogs: happyAuditEventWanted("/pretend-to-login", "http://127.0.0.1:1234?code=redacted&err=some-err&error=some-error&error_description=some-error-description&foo=redacted&foo=redacted&lorem=redacted&zzlast=redacted"),
},
}
nowDoesntMatter := time.Date(1122, time.September, 33, 4, 55, 56, 778899, time.Local)