From a8e505a44bcc0b15dfb18c57309e1616ad741633 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Tue, 7 Nov 2023 02:07:22 +0800 Subject: [PATCH] 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 --- modules/context/api.go | 27 -------------- routers/api/v1/api.go | 36 ------------------- services/auth/basic.go | 24 +++++++++++-- tests/integration/api_twofa_test.go | 55 +++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 65 deletions(-) create mode 100644 tests/integration/api_twofa_test.go diff --git a/modules/context/api.go b/modules/context/api.go index 044ec51b56..c6d47e3e8b 100644 --- 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 b5619c6fbe..15e687d1c8 100644 --- 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 6c3fbf595e..1184d12d1c 100644 --- 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 index 0000000000..1e5e26b8cc --- /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) +}