all callers of Audit() identify which keys may contain PII

vmware-tanzu · Nov 12, 2024 · ec8ca53 · ec8ca53
1 parent 1eff79d
commit ec8ca53
Show file tree

Hide file tree

Showing 6 changed files with 113 additions and 71 deletions.
diff --git a/internal/federationdomain/downstreamsession/downstream_session.go b/internal/federationdomain/downstreamsession/downstream_session.go
@@ -52,13 +52,15 @@ func NewPinnipedSession(
 
 	auditLogger.Audit(auditevent.IdentityFromUpstreamIDP, &plog.AuditParams{
 		ReqCtx: ctx,
+		PIIKeysAndValues: []any{
+			"upstreamUsername", c.UpstreamIdentity.UpstreamUsername,
+			"upstreamGroups", c.UpstreamIdentity.UpstreamGroups,
+		},
 		KeysAndValues: []any{
 			"upstreamIDPDisplayName", c.IdentityProvider.GetDisplayName(),
 			"upstreamIDPType", c.IdentityProvider.GetSessionProviderType(),
 			"upstreamIDPResourceName", c.IdentityProvider.GetProvider().GetResourceName(),
 			"upstreamIDPResourceUID", c.IdentityProvider.GetProvider().GetResourceUID(),
-			"upstreamUsername", c.UpstreamIdentity.UpstreamUsername,
-			"upstreamGroups", c.UpstreamIdentity.UpstreamGroups,
 		},
 	})
 
@@ -118,11 +120,13 @@ func NewPinnipedSession(
 	auditLogger.Audit(auditevent.SessionStarted, &plog.AuditParams{
 		ReqCtx:  ctx,
 		Session: c.SessionIDGetter,
-		KeysAndValues: []any{
+		PIIKeysAndValues: []any{
 			"username", downstreamUsername,
 			"groups", downstreamGroups,
 			"subject", c.UpstreamIdentity.DownstreamSubject,
 			"additionalClaims", c.UpstreamLoginExtras.DownstreamAdditionalClaims,
+		},
+		KeysAndValues: []any{
 			"warnings", c.UpstreamLoginExtras.Warnings,
 		},
 	})

diff --git a/internal/federationdomain/endpoints/auth/auth_handler_test.go b/internal/federationdomain/endpoints/auth/auth_handler_test.go
@@ -1203,16 +1203,20 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 						"upstreamIDPResourceName": "some-password-granting-oidc-idp",
 						"upstreamIDPResourceUID":  "some-password-granting-resource-uid",
 						"upstreamIDPType":         "oidc",
-						"upstreamUsername":        "test-oidc-pinniped-username",
-						"upstreamGroups":          []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "test-oidc-pinniped-username",
+							"upstreamGroups":   []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "test-oidc-pinniped-username",
-						"groups":           []any{"test-pinniped-group-0", "test-pinniped-group-1"},
-						"subject":          "https://my-upstream-issuer.com?idpName=some-password-granting-oidc-idp&sub=abc123-some+guid",
-						"additionalClaims": map[string]any{}, // json: {}
-						"warnings":         []any{},          // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "test-oidc-pinniped-username",
+							"groups":           []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+							"subject":          "https://my-upstream-issuer.com?idpName=some-password-granting-oidc-idp&sub=abc123-some+guid",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},
@@ -1288,8 +1292,10 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 						"upstreamIDPResourceName": "some-password-granting-oidc-idp",
 						"upstreamIDPResourceUID":  "some-password-granting-resource-uid",
 						"upstreamIDPType":         "oidc",
-						"upstreamUsername":        "test-oidc-pinniped-username",
-						"upstreamGroups":          []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "test-oidc-pinniped-username",
+							"upstreamGroups":   []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						},
 					}),
 					testutil.WantAuditLog("Authentication Rejected By Transforms", map[string]any{
 						"reason": "configured identity policy rejected this authentication: authentication was rejected by a configured policy",
@@ -1409,16 +1415,20 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 						"upstreamIDPResourceName": "some-ldap-idp",
 						"upstreamIDPResourceUID":  "ldap-resource-uid",
 						"upstreamIDPType":         "ldap",
-						"upstreamUsername":        "some-ldap-username-from-authenticator",
-						"upstreamGroups":          []any{"group1", "group2", "group3"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "some-ldap-username-from-authenticator",
+							"upstreamGroups":   []any{"group1", "group2", "group3"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "some-ldap-username-from-authenticator",
-						"groups":           []any{"group1", "group2", "group3"},
-						"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-ldap-idp&sub=some-ldap-uid",
-						"additionalClaims": nil,     // json: null
-						"warnings":         []any{}, // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "some-ldap-username-from-authenticator",
+							"groups":           []any{"group1", "group2", "group3"},
+							"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-ldap-idp&sub=some-ldap-uid",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},
@@ -1479,16 +1489,20 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 						"upstreamIDPResourceName": "some-ldap-idp",
 						"upstreamIDPResourceUID":  "ldap-resource-uid",
 						"upstreamIDPType":         "ldap",
-						"upstreamUsername":        "some-ldap-username-from-authenticator",
-						"upstreamGroups":          []any{"group1", "group2", "group3"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "some-ldap-username-from-authenticator",
+							"upstreamGroups":   []any{"group1", "group2", "group3"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "username_prefix:some-ldap-username-from-authenticator",
-						"groups":           []any{"groups_prefix:group1", "groups_prefix:group2", "groups_prefix:group3"},
-						"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-ldap-idp&sub=some-ldap-uid",
-						"additionalClaims": nil,     // json: null
-						"warnings":         []any{}, // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "username_prefix:some-ldap-username-from-authenticator",
+							"groups":           []any{"groups_prefix:group1", "groups_prefix:group2", "groups_prefix:group3"},
+							"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-ldap-idp&sub=some-ldap-uid",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},
@@ -1794,16 +1808,20 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo
 						"upstreamIDPResourceName": "some-active-directory-idp",
 						"upstreamIDPResourceUID":  "active-directory-resource-uid",
 						"upstreamIDPType":         "activedirectory",
-						"upstreamUsername":        "some-ldap-username-from-authenticator",
-						"upstreamGroups":          []any{"group1", "group2", "group3"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "some-ldap-username-from-authenticator",
+							"upstreamGroups":   []any{"group1", "group2", "group3"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "some-ldap-username-from-authenticator",
-						"groups":           []any{"group1", "group2", "group3"},
-						"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-active-directory-idp&sub=some-ldap-uid",
-						"additionalClaims": nil,     // json: null
-						"warnings":         []any{}, // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "some-ldap-username-from-authenticator",
+							"groups":           []any{"group1", "group2", "group3"},
+							"subject":          "ldaps://some-ldap-host:123?base=ou%3Dusers%2Cdc%3Dpinniped%2Cdc%3Ddev&idpName=some-active-directory-idp&sub=some-ldap-uid",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},

diff --git a/internal/federationdomain/endpoints/callback/callback_handler_test.go b/internal/federationdomain/endpoints/callback/callback_handler_test.go
@@ -290,16 +290,20 @@ func TestCallbackEndpoint(t *testing.T) {
 						"upstreamIDPType":         "oidc",
 						"upstreamIDPResourceName": "upstream-oidc-idp-name",
 						"upstreamIDPResourceUID":  "upstream-oidc-resource-uid",
-						"upstreamUsername":        "test-pinniped-username",
-						"upstreamGroups":          []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "test-pinniped-username",
+							"upstreamGroups":   []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "test-pinniped-username",
-						"groups":           []any{"test-pinniped-group-0", "test-pinniped-group-1"},
-						"subject":          "https://my-upstream-issuer.com?idpName=upstream-oidc-idp-name&sub=abc123-some+guid",
-						"additionalClaims": map[string]any{}, // json: {}
-						"warnings":         []any{},          // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "test-pinniped-username",
+							"groups":           []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+							"subject":          "https://my-upstream-issuer.com?idpName=upstream-oidc-idp-name&sub=abc123-some+guid",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},
@@ -344,16 +348,20 @@ func TestCallbackEndpoint(t *testing.T) {
 						"upstreamIDPType":         "github",
 						"upstreamIDPResourceName": "upstream-github-idp-name",
 						"upstreamIDPResourceUID":  "upstream-github-idp-resource-uid",
-						"upstreamUsername":        "some-github-login",
-						"upstreamGroups":          []any{"org1/team1", "org2/team2"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "some-github-login",
+							"upstreamGroups":   []any{"org1/team1", "org2/team2"},
+						},
 					}),
 					testutil.WantAuditLog("Session Started", map[string]any{
-						"sessionID":        sessionID,
-						"username":         "some-github-login",
-						"groups":           []any{"org1/team1", "org2/team2"},
-						"subject":          "https://github.com?idpName=upstream-github-idp-name&sub=some-github-login",
-						"additionalClaims": nil,     // json: null
-						"warnings":         []any{}, // json: []
+						"sessionID": sessionID,
+						"warnings":  []any{}, // json: []
+						"personalInfo": map[string]any{
+							"username":         "some-github-login",
+							"groups":           []any{"org1/team1", "org2/team2"},
+							"subject":          "https://github.com?idpName=upstream-github-idp-name&sub=some-github-login",
+							"additionalClaims": map[string]any{}, // json: {}
+						},
 					}),
 				}
 			},
@@ -1788,8 +1796,10 @@ func TestCallbackEndpoint(t *testing.T) {
 						"upstreamIDPType":         "oidc",
 						"upstreamIDPResourceName": "upstream-oidc-idp-name",
 						"upstreamIDPResourceUID":  "upstream-oidc-resource-uid",
-						"upstreamUsername":        "test-pinniped-username",
-						"upstreamGroups":          []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "test-pinniped-username",
+							"upstreamGroups":   []any{"test-pinniped-group-0", "test-pinniped-group-1"},
+						},
 					}),
 					testutil.WantAuditLog("Authentication Rejected By Transforms", map[string]any{
 						"reason": "configured identity policy rejected this authentication: authentication was rejected by a configured policy",
@@ -1821,8 +1831,10 @@ func TestCallbackEndpoint(t *testing.T) {
 						"upstreamIDPType":         "github",
 						"upstreamIDPResourceName": "upstream-github-idp-name",
 						"upstreamIDPResourceUID":  "upstream-github-idp-resource-uid",
-						"upstreamUsername":        "some-github-login",
-						"upstreamGroups":          []any{"org1/team1", "org2/team2"},
+						"personalInfo": map[string]any{
+							"upstreamUsername": "some-github-login",
+							"upstreamGroups":   []any{"org1/team1", "org2/team2"},
+						},
 					}),
 					testutil.WantAuditLog("Authentication Rejected By Transforms", map[string]any{
 						"reason": "configured identity policy rejected this authentication: authentication was rejected by a configured policy",

diff --git a/internal/federationdomain/endpoints/token/token_handler.go b/internal/federationdomain/endpoints/token/token_handler.go
@@ -195,7 +195,7 @@ func upstreamRefresh(
 	auditLogger.Audit(auditevent.IdentityRefreshedFromUpstreamIDP, &plog.AuditParams{
 		ReqCtx:  ctx,
 		Session: accessRequest,
-		KeysAndValues: []any{
+		PIIKeysAndValues: []any{
 			"upstreamUsername", refreshedIdentity.UpstreamUsername,
 			"upstreamGroups", refreshedIdentity.UpstreamGroups,
 		},
@@ -250,7 +250,7 @@ func upstreamRefresh(
 	auditLogger.Audit(auditevent.SessionRefreshed, &plog.AuditParams{
 		ReqCtx:  ctx,
 		Session: accessRequest,
-		KeysAndValues: []any{
+		PIIKeysAndValues: []any{
 			"username", oldTransformedUsername, // not allowed to change above so must be the same as old
 			"groups", refreshedTransformedGroups,
 			"subject", previousIdentity.DownstreamSubject},

diff --git a/internal/federationdomain/endpoints/token/token_handler_test.go b/internal/federationdomain/endpoints/token/token_handler_test.go
@@ -2340,18 +2340,22 @@ func TestRefreshGrant(t *testing.T) {
 								},
 							}),
 							testutil.WantAuditLog("Identity Refreshed From Upstream IDP", map[string]any{
-								"sessionID":        sessionID,
-								"upstreamGroups":   []any{},
-								"upstreamUsername": "some-username",
+								"sessionID": sessionID,
+								"personalInfo": map[string]any{
+									"upstreamGroups":   []any{},
+									"upstreamUsername": "some-username",
+								},
 							}),
 							testutil.WantAuditLog("Session Refreshed", map[string]any{
 								"sessionID": sessionID,
-								"username":  "some-username",
-								"groups": []any{
-									"group1",
-									"groups2",
+								"personalInfo": map[string]any{
+									"username": "some-username",
+									"groups": []any{
+										"group1",
+										"groups2",
+									},
+									"subject": "https://issuer?sub=some-subject",
 								},
-								"subject": "https://issuer?sub=some-subject",
 							}),
 						}
 					},
@@ -2533,9 +2537,11 @@ func TestRefreshGrant(t *testing.T) {
 								},
 							}),
 							testutil.WantAuditLog("Identity Refreshed From Upstream IDP", map[string]any{
-								"sessionID":        sessionID,
-								"upstreamGroups":   []any{},
-								"upstreamUsername": "some-username",
+								"sessionID": sessionID,
+								"personalInfo": map[string]any{
+									"upstreamGroups":   []any{},
+									"upstreamUsername": "some-username",
+								},
 							}),
 							testutil.WantAuditLog("Authentication Rejected By Transforms", map[string]any{
 								"sessionID": sessionID,

diff --git a/internal/registry/credentialrequest/rest.go b/internal/registry/credentialrequest/rest.go
@@ -134,9 +134,11 @@ func (r *REST) Create(ctx context.Context, obj runtime.Object, createValidation
 
 	r.auditLogger.Audit(auditevent.TokenCredentialRequest, &plog.AuditParams{
 		ReqCtx: ctx,
-		KeysAndValues: []any{
+		PIIKeysAndValues: []any{
 			"username", userInfo.GetName(),
 			"groups", userInfo.GetGroups(),
+		},
+		KeysAndValues: []any{
 			"authenticated", true,
 			"expires", expires.Format(time.RFC3339),
 		},