From 95d672c9df7474723f2ebd4646e9d9a79dbb3085 Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Tue, 10 Feb 2026 21:33:15 +0100 Subject: [PATCH] fix: capture auth method in access logs for failed authentication - Add wasCredentialSubmitted helper to detect when credentials were submitted but authentication failed - Set auth method in CapturedData when wrong PIN/password is entered - Set auth method for OAuth callback errors and token validation errors - Add tests for failed auth method capture --- proxy/internal/auth/middleware.go | 25 +++++ proxy/internal/auth/middleware_test.go | 149 +++++++++++++++++++++++++ 2 files changed, 174 insertions(+) diff --git a/proxy/internal/auth/middleware.go b/proxy/internal/auth/middleware.go index e4f937019..06fba4272 100644 --- a/proxy/internal/auth/middleware.go +++ b/proxy/internal/auth/middleware.go @@ -114,6 +114,7 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler { var requestID string if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { cd.SetOrigin(proxy.OriginAuth) + cd.SetAuthMethod(auth.MethodOIDC.String()) requestID = cd.GetRequestID() } errDesc := r.URL.Query().Get("error_description") @@ -138,13 +139,21 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler { // Try to authenticate with each scheme. methods := make(map[string]string) + var attemptedMethod string for _, scheme := range config.Schemes { token, promptData := scheme.Authenticate(r) + + // Track if credentials were submitted but auth failed + if token == "" && wasCredentialSubmitted(r, scheme.Type()) { + attemptedMethod = scheme.Type().String() + } + if token != "" { result, err := mw.validateSessionToken(r.Context(), host, token, config.SessionPublicKey, scheme.Type()) if err != nil { if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { cd.SetOrigin(proxy.OriginAuth) + cd.SetAuthMethod(scheme.Type().String()) } http.Error(w, err.Error(), http.StatusBadRequest) return @@ -190,11 +199,27 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler { if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil { cd.SetOrigin(proxy.OriginAuth) + if attemptedMethod != "" { + cd.SetAuthMethod(attemptedMethod) + } } web.ServeHTTP(w, r, map[string]any{"methods": methods}, http.StatusUnauthorized) }) } +// wasCredentialSubmitted checks if credentials were submitted for the given auth method. +func wasCredentialSubmitted(r *http.Request, method auth.Method) bool { + switch method { + case auth.MethodPIN: + return r.FormValue("pin") != "" + case auth.MethodPassword: + return r.FormValue("password") != "" + case auth.MethodOIDC: + return r.URL.Query().Get("session_token") != "" + } + return false +} + // AddDomain registers authentication schemes for the given domain. // If schemes are provided, a valid session public key is required to sign/verify // session JWTs. Returns an error if the key is missing or invalid. diff --git a/proxy/internal/auth/middleware_test.go b/proxy/internal/auth/middleware_test.go index dd6529164..c987dcd3a 100644 --- a/proxy/internal/auth/middleware_test.go +++ b/proxy/internal/auth/middleware_test.go @@ -509,3 +509,152 @@ func TestAddDomain_InvalidKeyDoesNotCorruptExistingConfig(t *testing.T) { assert.Len(t, config.Schemes, 1) assert.Equal(t, time.Hour, config.SessionExpiration) } + +func TestProtect_FailedPinAuthCapturesAuthMethod(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil) + kp := generateTestKeyPair(t) + + // Scheme that always fails authentication (returns empty token) + scheme := &stubScheme{ + method: auth.MethodPIN, + authFn: func(_ *http.Request) (string, string) { + return "", "pin" + }, + } + require.NoError(t, mw.AddDomain("example.com", []Scheme{scheme}, kp.PublicKey, time.Hour, "", "")) + + capturedData := &proxy.CapturedData{} + handler := mw.Protect(newPassthroughHandler()) + + // Submit wrong PIN - should capture auth method + form := url.Values{"pin": {"wrong-pin"}} + req := httptest.NewRequest(http.MethodPost, "http://example.com/", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req = req.WithContext(proxy.WithCapturedData(req.Context(), capturedData)) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.Equal(t, "pin", capturedData.GetAuthMethod(), "Auth method should be captured for failed PIN auth") +} + +func TestProtect_FailedPasswordAuthCapturesAuthMethod(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil) + kp := generateTestKeyPair(t) + + scheme := &stubScheme{ + method: auth.MethodPassword, + authFn: func(_ *http.Request) (string, string) { + return "", "password" + }, + } + require.NoError(t, mw.AddDomain("example.com", []Scheme{scheme}, kp.PublicKey, time.Hour, "", "")) + + capturedData := &proxy.CapturedData{} + handler := mw.Protect(newPassthroughHandler()) + + // Submit wrong password - should capture auth method + form := url.Values{"password": {"wrong-password"}} + req := httptest.NewRequest(http.MethodPost, "http://example.com/", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req = req.WithContext(proxy.WithCapturedData(req.Context(), capturedData)) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.Equal(t, "password", capturedData.GetAuthMethod(), "Auth method should be captured for failed password auth") +} + +func TestProtect_NoCredentialsDoesNotCaptureAuthMethod(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil) + kp := generateTestKeyPair(t) + + scheme := &stubScheme{ + method: auth.MethodPIN, + authFn: func(_ *http.Request) (string, string) { + return "", "pin" + }, + } + require.NoError(t, mw.AddDomain("example.com", []Scheme{scheme}, kp.PublicKey, time.Hour, "", "")) + + capturedData := &proxy.CapturedData{} + handler := mw.Protect(newPassthroughHandler()) + + // No credentials submitted - should not capture auth method + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req = req.WithContext(proxy.WithCapturedData(req.Context(), capturedData)) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code) + assert.Empty(t, capturedData.GetAuthMethod(), "Auth method should not be captured when no credentials submitted") +} + +func TestWasCredentialSubmitted(t *testing.T) { + tests := []struct { + name string + method auth.Method + formData url.Values + query url.Values + expected bool + }{ + { + name: "PIN submitted", + method: auth.MethodPIN, + formData: url.Values{"pin": {"123456"}}, + expected: true, + }, + { + name: "PIN not submitted", + method: auth.MethodPIN, + formData: url.Values{}, + expected: false, + }, + { + name: "Password submitted", + method: auth.MethodPassword, + formData: url.Values{"password": {"secret"}}, + expected: true, + }, + { + name: "Password not submitted", + method: auth.MethodPassword, + formData: url.Values{}, + expected: false, + }, + { + name: "OIDC token in query", + method: auth.MethodOIDC, + query: url.Values{"session_token": {"abc123"}}, + expected: true, + }, + { + name: "OIDC token not in query", + method: auth.MethodOIDC, + query: url.Values{}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reqURL := "http://example.com/" + if len(tt.query) > 0 { + reqURL += "?" + tt.query.Encode() + } + + var body *strings.Reader + if len(tt.formData) > 0 { + body = strings.NewReader(tt.formData.Encode()) + } else { + body = strings.NewReader("") + } + + req := httptest.NewRequest(http.MethodPost, reqURL, body) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + result := wasCredentialSubmitted(req, tt.method) + assert.Equal(t, tt.expected, result) + }) + } +}