Unify two factor check (#27915) (#27929)

Backport #27915 by @KN4CK3R

Fixes #27819

We have support for two factor logins with the normal web login and with
basic auth. For basic auth the two factor check was implemented at three
different places and you need to know that this check is necessary. This
PR moves the check into the basic auth itself.

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>

diff --git a/modules/context/api.go b/modules/context/api.go
index 044ec51b56599318e04c6ff98024be25413acef6..c6d47e3e8b7a7db7d16f8f7d98978ff0fe12c369 100644 (file)
--- a/modules/context/api.go
+++ b/modules/context/api.go
@@ -11,7 +11,6 @@ import (
        "net/url"
        "strings"
 
-       "code.gitea.io/gitea/models/auth"
        repo_model "code.gitea.io/gitea/models/repo"
        "code.gitea.io/gitea/models/unit"
        user_model "code.gitea.io/gitea/models/user"
@@ -205,32 +204,6 @@ func (ctx *APIContext) SetLinkHeader(total, pageSize int) {
        }
 }
 
-// CheckForOTP validates OTP
-func (ctx *APIContext) CheckForOTP() {
-       if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
-               return // Skip 2FA
-       }
-
-       otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
-       twofa, err := auth.GetTwoFactorByUID(ctx, ctx.Doer.ID)
-       if err != nil {
-               if auth.IsErrTwoFactorNotEnrolled(err) {
-                       return // No 2FA enrollment for this user
-               }
-               ctx.Error(http.StatusInternalServerError, "GetTwoFactorByUID", err)
-               return
-       }
-       ok, err := twofa.ValidateTOTP(otpHeader)
-       if err != nil {
-               ctx.Error(http.StatusInternalServerError, "ValidateTOTP", err)
-               return
-       }
-       if !ok {
-               ctx.Error(http.StatusUnauthorized, "", nil)
-               return
-       }
-}
-
 // APIContexter returns apicontext as middleware
 func APIContexter() func(http.Handler) http.Handler {
        return func(next http.Handler) http.Handler {

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index b5619c6fbe2a525ded1fd101028db749c1129f45..15e687d1c84110d81c4e4956d2b4d1abf9237395 100644 (file)
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -315,10 +315,6 @@ func reqToken() func(ctx *context.APIContext) {
                        return
                }
 
-               if ctx.IsBasicAuth {
-                       ctx.CheckForOTP()
-                       return
-               }
                if ctx.IsSigned {
                        return
                }
@@ -343,7 +339,6 @@ func reqBasicOrRevProxyAuth() func(ctx *context.APIContext) {
                        ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "auth required")
                        return
                }
-               ctx.CheckForOTP()
        }
 }
 
@@ -700,12 +695,6 @@ func bind[T any](_ T) any {
        }
 }
 
-// The OAuth2 plugin is expected to be executed first, as it must ignore the user id stored
-// in the session (if there is a user id stored in session other plugins might return the user
-// object for that id).
-//
-// The Session plugin is expected to be executed second, in order to skip authentication
-// for users that have already signed in.
 func buildAuthGroup() *auth.Group {
        group := auth.NewGroup(
                &auth.OAuth2{},
@@ -785,31 +774,6 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIC
                                })
                                return
                        }
-                       if ctx.IsSigned && ctx.IsBasicAuth {
-                               if skip, ok := ctx.Data["SkipLocalTwoFA"]; ok && skip.(bool) {
-                                       return // Skip 2FA
-                               }
-                               twofa, err := auth_model.GetTwoFactorByUID(ctx, ctx.Doer.ID)
-                               if err != nil {
-                                       if auth_model.IsErrTwoFactorNotEnrolled(err) {
-                                               return // No 2FA enrollment for this user
-                                       }
-                                       ctx.InternalServerError(err)
-                                       return
-                               }
-                               otpHeader := ctx.Req.Header.Get("X-Gitea-OTP")
-                               ok, err := twofa.ValidateTOTP(otpHeader)
-                               if err != nil {
-                                       ctx.InternalServerError(err)
-                                       return
-                               }
-                               if !ok {
-                                       ctx.JSON(http.StatusForbidden, map[string]string{
-                                               "message": "Only signed in user is allowed to call APIs.",
-                                       })
-                                       return
-                               }
-                       }
                }
 
                if options.AdminRequired {

diff --git a/services/auth/basic.go b/services/auth/basic.go
index 6c3fbf595e44cd6ea3973831ae316f636882b767..1184d12d1c4b4f3d9af15bb6040f854ef5fd9502 100644 (file)
--- a/services/auth/basic.go
+++ b/services/auth/basic.go
@@ -15,6 +15,7 @@ import (
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/timeutil"
+       "code.gitea.io/gitea/modules/util"
        "code.gitea.io/gitea/modules/web/middleware"
 )
 
@@ -131,11 +132,30 @@ func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, store DataStore
                return nil, err
        }
 
-       if skipper, ok := source.Cfg.(LocalTwoFASkipper); ok && skipper.IsSkipLocalTwoFA() {
-               store.GetData()["SkipLocalTwoFA"] = true
+       if skipper, ok := source.Cfg.(LocalTwoFASkipper); !ok || !skipper.IsSkipLocalTwoFA() {
+               if err := validateTOTP(req, u); err != nil {
+                       return nil, err
+               }
        }
 
        log.Trace("Basic Authorization: Logged in user %-v", u)
 
        return u, nil
 }
+
+func validateTOTP(req *http.Request, u *user_model.User) error {
+       twofa, err := auth_model.GetTwoFactorByUID(req.Context(), u.ID)
+       if err != nil {
+               if auth_model.IsErrTwoFactorNotEnrolled(err) {
+                       // No 2FA enrollment for this user
+                       return nil
+               }
+               return err
+       }
+       if ok, err := twofa.ValidateTOTP(req.Header.Get("X-Gitea-OTP")); err != nil {
+               return err
+       } else if !ok {
+               return util.NewInvalidArgumentErrorf("invalid provided OTP")
+       }
+       return nil
+}

diff --git a/tests/integration/api_twofa_test.go b/tests/integration/api_twofa_test.go
new file mode 100644 (file)
index 0000000..1e5e26b
--- /dev/null
+++ b/tests/integration/api_twofa_test.go
@@ -0,0 +1,55 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+       "net/http"
+       "testing"
+       "time"
+
+       auth_model "code.gitea.io/gitea/models/auth"
+       "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/models/unittest"
+       user_model "code.gitea.io/gitea/models/user"
+       "code.gitea.io/gitea/tests"
+
+       "github.com/pquerna/otp/totp"
+       "github.com/stretchr/testify/assert"
+)
+
+func TestAPITwoFactor(t *testing.T) {
+       defer tests.PrepareTestEnv(t)()
+
+       user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 16})
+
+       req := NewRequestf(t, "GET", "/api/v1/user")
+       req = AddBasicAuthHeader(req, user.Name)
+       MakeRequest(t, req, http.StatusOK)
+
+       otpKey, err := totp.Generate(totp.GenerateOpts{
+               SecretSize:  40,
+               Issuer:      "gitea-test",
+               AccountName: user.Name,
+       })
+       assert.NoError(t, err)
+
+       tfa := &auth_model.TwoFactor{
+               UID: user.ID,
+       }
+       assert.NoError(t, tfa.SetSecret(otpKey.Secret()))
+
+       assert.NoError(t, auth_model.NewTwoFactor(db.DefaultContext, tfa))
+
+       req = NewRequestf(t, "GET", "/api/v1/user")
+       req = AddBasicAuthHeader(req, user.Name)
+       MakeRequest(t, req, http.StatusUnauthorized)
+
+       passcode, err := totp.GenerateCode(otpKey.Secret(), time.Now())
+       assert.NoError(t, err)
+
+       req = NewRequestf(t, "GET", "/api/v1/user")
+       req = AddBasicAuthHeader(req, user.Name)
+       req.Header.Set("X-Gitea-OTP", passcode)
+       MakeRequest(t, req, http.StatusOK)
+}