From 611de03e013a2f47f198fe7dd66b60c517646647 Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Wed, 13 Nov 2024 13:36:25 -0600 Subject: [PATCH] Add audit event 'Incorrect Username Or Password' to auth_handler and audit event 'Using Upstream IDP' to callback_handler --- .../endpoints/auth/auth_handler.go | 10 +++- .../endpoints/auth/auth_handler_test.go | 55 +++++++++++++++++++ .../endpoints/callback/callback_handler.go | 10 +++- .../callback/callback_handler_test.go | 30 ++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) diff --git a/internal/federationdomain/endpoints/auth/auth_handler.go b/internal/federationdomain/endpoints/auth/auth_handler.go index 7baa259e9..e2454f559 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler.go +++ b/internal/federationdomain/endpoints/auth/auth_handler.go @@ -24,6 +24,7 @@ import ( "go.pinniped.dev/internal/federationdomain/formposthtml" "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/resolvedprovider" + "go.pinniped.dev/internal/federationdomain/resolvedprovider/resolvedldap" "go.pinniped.dev/internal/federationdomain/stateparam" "go.pinniped.dev/internal/httputil/responseutil" "go.pinniped.dev/internal/httputil/securityheader" @@ -261,12 +262,15 @@ func (h *authorizeHandler) authorizeWithoutBrowser( identity, loginExtras, err := idp.Login(r.Context(), submittedUsername, submittedPassword) if err != nil { + if err == resolvedldap.ErrAccessDeniedDueToUsernamePasswordNotAccepted { + h.auditLogger.Audit(auditevent.IncorrectUsernameOrPassword, &plog.AuditParams{ + ReqCtx: r.Context(), + }) + } + return err } - // TODO: Perhaps add audit event "Incorrect Username Or Password"? - // See "post_login_handler" for example - session, err := downstreamsession.NewPinnipedSession(r.Context(), h.auditLogger, &downstreamsession.SessionConfig{ UpstreamIdentity: identity, UpstreamLoginExtras: loginExtras, diff --git a/internal/federationdomain/endpoints/auth/auth_handler_test.go b/internal/federationdomain/endpoints/auth/auth_handler_test.go index 865392de4..985666210 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler_test.go +++ b/internal/federationdomain/endpoints/auth/auth_handler_test.go @@ -2178,6 +2178,33 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo wantContentType: jsonContentType, wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedErrorQuery), wantBodyString: "", + wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog { + return []testutil.WantedAuditLog{ + testutil.WantAuditLog("HTTP Request Custom Headers Used", map[string]any{ + "Pinniped-Username": true, + "Pinniped-Password": true, + }), + testutil.WantAuditLog("HTTP Request Parameters", map[string]any{ + "params": map[string]any{ + "client_id": "pinniped-cli", + "code_challenge": "redacted", + "code_challenge_method": "S256", + "nonce": "redacted", + "pinniped_idp_name": "some-password-granting-oidc-idp", + "redirect_uri": "http://127.0.0.1/callback", + "response_type": "code", + "scope": "openid profile email username groups", + "state": "redacted", + }, + }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "some-password-granting-oidc-idp", + "resourceName": "some-password-granting-oidc-idp", + "resourceUID": "some-password-granting-resource-uid", + "type": "oidc", + }), + } + }, }, { name: "wrong upstream password for LDAP authentication", @@ -2190,6 +2217,34 @@ func TestAuthorizationEndpoint(t *testing.T) { //nolint:gocyclo wantContentType: jsonContentType, wantLocationHeader: urlWithQuery(downstreamRedirectURI, fositeAccessDeniedWithBadUsernamePasswordHintErrorQuery), wantBodyString: "", + wantAuditLogs: func(encodedStateParam stateparam.Encoded, sessionID string) []testutil.WantedAuditLog { + return []testutil.WantedAuditLog{ + testutil.WantAuditLog("HTTP Request Custom Headers Used", map[string]any{ + "Pinniped-Username": true, + "Pinniped-Password": true, + }), + testutil.WantAuditLog("HTTP Request Parameters", map[string]any{ + "params": map[string]any{ + "client_id": "pinniped-cli", + "code_challenge": "redacted", + "code_challenge_method": "S256", + "nonce": "redacted", + "pinniped_idp_name": "some-ldap-idp", + "redirect_uri": "http://127.0.0.1/callback", + "response_type": "code", + "scope": "openid profile email username groups", + "state": "redacted", + }, + }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "some-ldap-idp", + "resourceName": "some-ldap-idp", + "resourceUID": "ldap-resource-uid", + "type": "ldap", + }), + testutil.WantAuditLog("Incorrect Username Or Password", map[string]any{}), + } + }, }, { name: "wrong upstream password for Active Directory authentication", diff --git a/internal/federationdomain/endpoints/callback/callback_handler.go b/internal/federationdomain/endpoints/callback/callback_handler.go index f8a84b35a..022d1569e 100644 --- a/internal/federationdomain/endpoints/callback/callback_handler.go +++ b/internal/federationdomain/endpoints/callback/callback_handler.go @@ -45,7 +45,15 @@ func NewHandler( return httperr.New(http.StatusUnprocessableEntity, "upstream provider not found") } - // TODO: Add audit event "auditevent.UsingUpstreamIDP" + auditLogger.Audit(auditevent.UsingUpstreamIDP, &plog.AuditParams{ + ReqCtx: r.Context(), + KeysAndValues: []any{ + "displayName", idp.GetDisplayName(), + "resourceName", idp.GetProvider().GetResourceName(), + "resourceUID", idp.GetProvider().GetResourceUID(), + "type", idp.GetSessionProviderType(), + }, + }) downstreamAuthParams, err := url.ParseQuery(decodedState.AuthParams) if err != nil { diff --git a/internal/federationdomain/endpoints/callback/callback_handler_test.go b/internal/federationdomain/endpoints/callback/callback_handler_test.go index 861a43465..30c59c068 100644 --- a/internal/federationdomain/endpoints/callback/callback_handler_test.go +++ b/internal/federationdomain/endpoints/callback/callback_handler_test.go @@ -285,6 +285,12 @@ func TestCallbackEndpoint(t *testing.T) { testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{ "authorizeID": encodedStateParam.AuthorizeID(), }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "upstream-oidc-idp-name", + "resourceName": "upstream-oidc-idp-name", + "resourceUID": "upstream-oidc-resource-uid", + "type": "oidc", + }), testutil.WantAuditLog("Identity From Upstream IDP", map[string]any{ "upstreamIDPDisplayName": "upstream-oidc-idp-name", "upstreamIDPType": "oidc", @@ -343,6 +349,12 @@ func TestCallbackEndpoint(t *testing.T) { testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{ "authorizeID": encodedStateParam.AuthorizeID(), }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "upstream-github-idp-name", + "resourceName": "upstream-github-idp-name", + "resourceUID": "upstream-github-idp-resource-uid", + "type": "github", + }), testutil.WantAuditLog("Identity From Upstream IDP", map[string]any{ "upstreamIDPDisplayName": "upstream-github-idp-name", "upstreamIDPType": "github", @@ -718,6 +730,12 @@ func TestCallbackEndpoint(t *testing.T) { testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{ "authorizeID": encodedStateParam.AuthorizeID(), }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "upstream-oidc-idp-name", + "resourceName": "upstream-oidc-idp-name", + "resourceUID": "upstream-oidc-resource-uid", + "type": "oidc", + }), } }, }, @@ -1791,6 +1809,12 @@ func TestCallbackEndpoint(t *testing.T) { testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{ "authorizeID": encodedStateParam.AuthorizeID(), }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "upstream-oidc-idp-name", + "resourceName": "upstream-oidc-idp-name", + "resourceUID": "upstream-oidc-resource-uid", + "type": "oidc", + }), testutil.WantAuditLog("Identity From Upstream IDP", map[string]any{ "upstreamIDPDisplayName": "upstream-oidc-idp-name", "upstreamIDPType": "oidc", @@ -1826,6 +1850,12 @@ func TestCallbackEndpoint(t *testing.T) { testutil.WantAuditLog("AuthorizeID From Parameters", map[string]any{ "authorizeID": encodedStateParam.AuthorizeID(), }), + testutil.WantAuditLog("Using Upstream IDP", map[string]any{ + "displayName": "upstream-github-idp-name", + "resourceName": "upstream-github-idp-name", + "resourceUID": "upstream-github-idp-resource-uid", + "type": "github", + }), testutil.WantAuditLog("Identity From Upstream IDP", map[string]any{ "upstreamIDPDisplayName": "upstream-github-idp-name", "upstreamIDPType": "github",