From 16419b6fc3b554b2c4000169b62e17ab07da5c68 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
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.

(cherry picked from commit e6bbecb02d47730d3cc630d419fe27ef2fb5cb39)
---
 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 c8cb1735ee..eebdff2bad 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)
+}