From e6bbecb02d47730d3cc630d419fe27ef2fb5cb39 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 29 Oct 2024 18:01:42 +0100 Subject: [PATCH] fix: disallow basic authorization when security keys are enrolled - This unifies the security behavior of enrolling security keys with enrolling TOTP as a 2FA method. When TOTP is enrolled, you cannot use basic authorization (user:password) to make API request on behalf of the user, this is now also the case when you enroll security keys. - The usage of access tokens are the only method to make API requests on behalf of the user when a 2FA method is enrolled for the user. - Integration test added. --- services/auth/basic.go | 11 +++++++++++ tests/integration/api_twofa_test.go | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/services/auth/basic.go b/services/auth/basic.go index 382c8bc90c..d489164954 100644 --- a/services/auth/basic.go +++ b/services/auth/basic.go @@ -5,6 +5,7 @@ package auth import ( + "errors" "net/http" "strings" @@ -132,6 +133,16 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore return nil, err } + hashWebAuthn, err := auth_model.HasWebAuthnRegistrationsByUID(req.Context(), u.ID) + if err != nil { + log.Error("HasWebAuthnRegistrationsByUID: %v", err) + return nil, err + } + + if hashWebAuthn { + return nil, errors.New("Basic authorization is not allowed while having security keys enrolled") + } + if skipper, ok := source.Cfg.(LocalTwoFASkipper); !ok || !skipper.IsSkipLocalTwoFA() { if err := validateTOTP(req, u); err != nil { return nil, err diff --git a/tests/integration/api_twofa_test.go b/tests/integration/api_twofa_test.go index 0bb20255a2..fb1d2badfc 100644 --- a/tests/integration/api_twofa_test.go +++ b/tests/integration/api_twofa_test.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/tests" "github.com/pquerna/otp/totp" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -58,3 +59,24 @@ func TestAPITwoFactor(t *testing.T) { req.Header.Set("X-Forgejo-OTP", passcode) MakeRequest(t, req, http.StatusOK) } + +func TestAPIWebAuthn(t *testing.T) { + defer tests.PrepareTestEnv(t)() + + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 32}) + unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{UserID: user.ID}) + + req := NewRequest(t, "GET", "/api/v1/user") + req.SetBasicAuth(user.Name, "notpassword") + + resp := MakeRequest(t, req, http.StatusUnauthorized) + + type userResponse struct { + Message string `json:"message"` + } + var userParsed userResponse + + DecodeJSON(t, resp, &userParsed) + + assert.EqualValues(t, "Basic authorization is not allowed while having security keys enrolled", userParsed.Message) +}