mirror of
https://github.com/netbirdio/netbird.git
synced 2026-04-18 08:16:39 +00:00
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
This commit is contained in:
@@ -114,6 +114,7 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler {
|
|||||||
var requestID string
|
var requestID string
|
||||||
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
||||||
cd.SetOrigin(proxy.OriginAuth)
|
cd.SetOrigin(proxy.OriginAuth)
|
||||||
|
cd.SetAuthMethod(auth.MethodOIDC.String())
|
||||||
requestID = cd.GetRequestID()
|
requestID = cd.GetRequestID()
|
||||||
}
|
}
|
||||||
errDesc := r.URL.Query().Get("error_description")
|
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.
|
// Try to authenticate with each scheme.
|
||||||
methods := make(map[string]string)
|
methods := make(map[string]string)
|
||||||
|
var attemptedMethod string
|
||||||
for _, scheme := range config.Schemes {
|
for _, scheme := range config.Schemes {
|
||||||
token, promptData := scheme.Authenticate(r)
|
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 != "" {
|
if token != "" {
|
||||||
result, err := mw.validateSessionToken(r.Context(), host, token, config.SessionPublicKey, scheme.Type())
|
result, err := mw.validateSessionToken(r.Context(), host, token, config.SessionPublicKey, scheme.Type())
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
||||||
cd.SetOrigin(proxy.OriginAuth)
|
cd.SetOrigin(proxy.OriginAuth)
|
||||||
|
cd.SetAuthMethod(scheme.Type().String())
|
||||||
}
|
}
|
||||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
@@ -190,11 +199,27 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler {
|
|||||||
|
|
||||||
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
if cd := proxy.CapturedDataFromContext(r.Context()); cd != nil {
|
||||||
cd.SetOrigin(proxy.OriginAuth)
|
cd.SetOrigin(proxy.OriginAuth)
|
||||||
|
if attemptedMethod != "" {
|
||||||
|
cd.SetAuthMethod(attemptedMethod)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
web.ServeHTTP(w, r, map[string]any{"methods": methods}, http.StatusUnauthorized)
|
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.
|
// AddDomain registers authentication schemes for the given domain.
|
||||||
// If schemes are provided, a valid session public key is required to sign/verify
|
// 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.
|
// session JWTs. Returns an error if the key is missing or invalid.
|
||||||
|
|||||||
@@ -509,3 +509,152 @@ func TestAddDomain_InvalidKeyDoesNotCorruptExistingConfig(t *testing.T) {
|
|||||||
assert.Len(t, config.Schemes, 1)
|
assert.Len(t, config.Schemes, 1)
|
||||||
assert.Equal(t, time.Hour, config.SessionExpiration)
|
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)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user