Fix #2512 /api/forgejo/v1/version auth check (#2582)

Add the same auth check and middlewares as the /v1/ API. It require to export some variable from /v1 API, i am not sure if is the correct way to do Co-authored-by: oliverpool <git@olivier.pfad.fr> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2582 Reviewed-by: oliverpool <oliverpool@noreply.codeberg.org> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org> Co-authored-by: Ada <ada@gnous.eu> Co-committed-by: Ada <ada@gnous.eu>
2024-12-30 14:09:42 -05:00 · 2024-03-19 07:16:19 +00:00 · 2024-03-19 07:16:19 +00:00 · 41676a8634
commit 41676a8634
parent 1e292e9005
5 changed files with 238 additions and 144 deletions
--- a/routers/api/forgejo/v1/api.go
+++ b/routers/api/forgejo/v1/api.go
@ -5,10 +5,14 @@ package v1

 import (
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/api/shared"
 )

 func Routes() *web.Route {
 	m := web.NewRoute()
+
+	m.Use(shared.Middlewares()...)
+
 	forgejo := NewForgejo()
 	m.Get("", Root)
 	m.Get("/version", forgejo.GetVersion)
--- a/routers/api/shared/middleware.go
+++ b/routers/api/shared/middleware.go
@ -0,0 +1,152 @@
+// Copyright 2024 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package shared
+
+import (
+	"net/http"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/modules/log"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/routers/common"
+	"code.gitea.io/gitea/services/auth"
+	"code.gitea.io/gitea/services/context"
+
+	"github.com/go-chi/cors"
+)
+
+func Middlewares() (stack []any) {
+	stack = append(stack, securityHeaders())
+
+	if setting.CORSConfig.Enabled {
+		stack = append(stack, cors.Handler(cors.Options{
+			AllowedOrigins:   setting.CORSConfig.AllowDomain,
+			AllowedMethods:   setting.CORSConfig.Methods,
+			AllowCredentials: setting.CORSConfig.AllowCredentials,
+			AllowedHeaders:   append([]string{"Authorization", "X-Gitea-OTP", "X-Forgejo-OTP"}, setting.CORSConfig.Headers...),
+			MaxAge:           int(setting.CORSConfig.MaxAge.Seconds()),
+		}))
+	}
+	return append(stack,
+		context.APIContexter(),
+
+		checkDeprecatedAuthMethods,
+		// Get user from session if logged in.
+		apiAuth(buildAuthGroup()),
+		verifyAuthWithOptions(&common.VerifyOptions{
+			SignInRequired: setting.Service.RequireSignInView,
+		}),
+	)
+}
+
+func buildAuthGroup() *auth.Group {
+	group := auth.NewGroup(
+		&auth.OAuth2{},
+		&auth.HTTPSign{},
+		&auth.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API
+	)
+	if setting.Service.EnableReverseProxyAuthAPI {
+		group.Add(&auth.ReverseProxy{})
+	}
+
+	if setting.IsWindows && auth_model.IsSSPIEnabled(db.DefaultContext) {
+		group.Add(&auth.SSPI{}) // it MUST be the last, see the comment of SSPI
+	}
+
+	return group
+}
+
+func apiAuth(authMethod auth.Method) func(*context.APIContext) {
+	return func(ctx *context.APIContext) {
+		ar, err := common.AuthShared(ctx.Base, nil, authMethod)
+		if err != nil {
+			ctx.Error(http.StatusUnauthorized, "APIAuth", err)
+			return
+		}
+		ctx.Doer = ar.Doer
+		ctx.IsSigned = ar.Doer != nil
+		ctx.IsBasicAuth = ar.IsBasicAuth
+	}
+}
+
+// verifyAuthWithOptions checks authentication according to options
+func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIContext) {
+	return func(ctx *context.APIContext) {
+		// Check prohibit login users.
+		if ctx.IsSigned {
+			if !ctx.Doer.IsActive && setting.Service.RegisterEmailConfirm {
+				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "This account is not activated.",
+				})
+				return
+			}
+			if !ctx.Doer.IsActive || ctx.Doer.ProhibitLogin {
+				log.Info("Failed authentication attempt for %s from %s", ctx.Doer.Name, ctx.RemoteAddr())
+				ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "This account is prohibited from signing in, please contact your site administrator.",
+				})
+				return
+			}
+
+			if ctx.Doer.MustChangePassword {
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "You must change your password. Change it at: " + setting.AppURL + "/user/change_password",
+				})
+				return
+			}
+		}
+
+		// Redirect to dashboard if user tries to visit any non-login page.
+		if options.SignOutRequired && ctx.IsSigned && ctx.Req.URL.RequestURI() != "/" {
+			ctx.Redirect(setting.AppSubURL + "/")
+			return
+		}
+
+		if options.SignInRequired {
+			if !ctx.IsSigned {
+				// Restrict API calls with error message.
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "Only signed in user is allowed to call APIs.",
+				})
+				return
+			} else if !ctx.Doer.IsActive && setting.Service.RegisterEmailConfirm {
+				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "This account is not activated.",
+				})
+				return
+			}
+		}
+
+		if options.AdminRequired {
+			if !ctx.Doer.IsAdmin {
+				ctx.JSON(http.StatusForbidden, map[string]string{
+					"message": "You have no permission to request for this.",
+				})
+				return
+			}
+		}
+	}
+}
+
+// check for and warn against deprecated authentication options
+func checkDeprecatedAuthMethods(ctx *context.APIContext) {
+	if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
+		ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
+	}
+}
+
+func securityHeaders() func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
+			// CORB: https://www.chromium.org/Home/chromium-security/corb-for-developers
+			// http://stackoverflow.com/a/3146618/244009
+			resp.Header().Set("x-content-type-options", "nosniff")
+			next.ServeHTTP(resp, req)
+		})
+	}
+}
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@ -72,7 +72,6 @@ import (

 	actions_model "code.gitea.io/gitea/models/actions"
 	auth_model "code.gitea.io/gitea/models/auth"
-	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
@ -84,6 +83,7 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/modules/web"
+	"code.gitea.io/gitea/routers/api/shared"
 	"code.gitea.io/gitea/routers/api/v1/activitypub"
 	"code.gitea.io/gitea/routers/api/v1/admin"
 	"code.gitea.io/gitea/routers/api/v1/misc"
@ -93,7 +93,6 @@ import (
 	"code.gitea.io/gitea/routers/api/v1/repo"
 	"code.gitea.io/gitea/routers/api/v1/settings"
 	"code.gitea.io/gitea/routers/api/v1/user"
-	"code.gitea.io/gitea/routers/common"
 	"code.gitea.io/gitea/services/auth"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/forms"
@ -101,7 +100,6 @@ import (
 	_ "code.gitea.io/gitea/routers/api/v1/swagger" // for swagger generation

 	"gitea.com/go-chi/binding"
-	"github.com/go-chi/cors"
 )

 func sudo() func(ctx *context.APIContext) {
@ -731,98 +729,6 @@ func bind[T any](_ T) any {
 	}
 }

-func buildAuthGroup() *auth.Group {
-	group := auth.NewGroup(
-		&auth.OAuth2{},
-		&auth.HTTPSign{},
-		&auth.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API
-	)
-	if setting.Service.EnableReverseProxyAuthAPI {
-		group.Add(&auth.ReverseProxy{})
-	}
-
-	if setting.IsWindows && auth_model.IsSSPIEnabled(db.DefaultContext) {
-		group.Add(&auth.SSPI{}) // it MUST be the last, see the comment of SSPI
-	}
-
-	return group
-}
-
-func apiAuth(authMethod auth.Method) func(*context.APIContext) {
-	return func(ctx *context.APIContext) {
-		ar, err := common.AuthShared(ctx.Base, nil, authMethod)
-		if err != nil {
-			ctx.Error(http.StatusUnauthorized, "APIAuth", err)
-			return
-		}
-		ctx.Doer = ar.Doer
-		ctx.IsSigned = ar.Doer != nil
-		ctx.IsBasicAuth = ar.IsBasicAuth
-	}
-}
-
-// verifyAuthWithOptions checks authentication according to options
-func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.APIContext) {
-	return func(ctx *context.APIContext) {
-		// Check prohibit login users.
-		if ctx.IsSigned {
-			if !ctx.Doer.IsActive && setting.Service.RegisterEmailConfirm {
-				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "This account is not activated.",
-				})
-				return
-			}
-			if !ctx.Doer.IsActive || ctx.Doer.ProhibitLogin {
-				log.Info("Failed authentication attempt for %s from %s", ctx.Doer.Name, ctx.RemoteAddr())
-				ctx.Data["Title"] = ctx.Tr("auth.prohibit_login")
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "This account is prohibited from signing in, please contact your site administrator.",
-				})
-				return
-			}
-
-			if ctx.Doer.MustChangePassword {
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "You must change your password. Change it at: " + setting.AppURL + "/user/change_password",
-				})
-				return
-			}
-		}
-
-		// Redirect to dashboard if user tries to visit any non-login page.
-		if options.SignOutRequired && ctx.IsSigned && ctx.Req.URL.RequestURI() != "/" {
-			ctx.Redirect(setting.AppSubURL + "/")
-			return
-		}
-
-		if options.SignInRequired {
-			if !ctx.IsSigned {
-				// Restrict API calls with error message.
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "Only signed in user is allowed to call APIs.",
-				})
-				return
-			} else if !ctx.Doer.IsActive && setting.Service.RegisterEmailConfirm {
-				ctx.Data["Title"] = ctx.Tr("auth.active_your_account")
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "This account is not activated.",
-				})
-				return
-			}
-		}
-
-		if options.AdminRequired {
-			if !ctx.Doer.IsAdmin {
-				ctx.JSON(http.StatusForbidden, map[string]string{
-					"message": "You have no permission to request for this.",
-				})
-				return
-			}
-		}
-	}
-}
-
 func individualPermsChecker(ctx *context.APIContext) {
 	// org permissions have been checked in context.OrgAssignment(), but individual permissions haven't been checked.
 	if ctx.ContextUser.IsIndividual() {
@ -841,37 +747,11 @@ func individualPermsChecker(ctx *context.APIContext) {
 	}
 }

-// check for and warn against deprecated authentication options
-func checkDeprecatedAuthMethods(ctx *context.APIContext) {
-	if ctx.FormString("token") != "" || ctx.FormString("access_token") != "" {
-		ctx.Resp.Header().Set("Warning", "token and access_token API authentication is deprecated and will be removed in gitea 1.23. Please use AuthorizationHeaderToken instead. Existing queries will continue to work but without authorization.")
-	}
-}
-
 // Routes registers all v1 APIs routes to web application.
 func Routes() *web.Route {
 	m := web.NewRoute()

-	m.Use(securityHeaders())
-	if setting.CORSConfig.Enabled {
-		m.Use(cors.Handler(cors.Options{
-			AllowedOrigins:   setting.CORSConfig.AllowDomain,
-			AllowedMethods:   setting.CORSConfig.Methods,
-			AllowCredentials: setting.CORSConfig.AllowCredentials,
-			AllowedHeaders:   append([]string{"Authorization", "X-Gitea-OTP", "X-Forgejo-OTP"}, setting.CORSConfig.Headers...),
-			MaxAge:           int(setting.CORSConfig.MaxAge.Seconds()),
-		}))
-	}
-	m.Use(context.APIContexter())
-
-	m.Use(checkDeprecatedAuthMethods)
-
-	// Get user from session if logged in.
-	m.Use(apiAuth(buildAuthGroup()))
-
-	m.Use(verifyAuthWithOptions(&common.VerifyOptions{
-		SignInRequired: setting.Service.RequireSignInView,
-	}))
+	m.Use(shared.Middlewares()...)

 	m.Group("", func() {
 		// Miscellaneous (no scope required)
@ -1627,14 +1507,3 @@ func Routes() *web.Route {

 	return m
 }
-
-func securityHeaders() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
-			// CORB: https://www.chromium.org/Home/chromium-security/corb-for-developers
-			// http://stackoverflow.com/a/3146618/244009
-			resp.Header().Set("x-content-type-options", "nosniff")
-			next.ServeHTTP(resp, req)
-		})
-	}
-}
--- a/tests/integration/api_forgejo_version_test.go
+++ b/tests/integration/api_forgejo_version_test.go
@ -7,6 +7,10 @@ import (
 	"net/http"
 	"testing"

+	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/routers"
 	v1 "code.gitea.io/gitea/routers/api/forgejo/v1"
 	"code.gitea.io/gitea/tests"

@ -16,10 +20,40 @@ import (
 func TestAPIForgejoVersion(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()

-	req := NewRequest(t, "GET", "/api/forgejo/v1/version")
-	resp := MakeRequest(t, req, http.StatusOK)
+	t.Run("Version", func(t *testing.T) {
+		req := NewRequest(t, "GET", "/api/forgejo/v1/version")
+		resp := MakeRequest(t, req, http.StatusOK)

-	var version v1.Version
-	DecodeJSON(t, resp, &version)
-	assert.Equal(t, "1.0.0", *version.Version)
+		var version v1.Version
+		DecodeJSON(t, resp, &version)
+		assert.Equal(t, "1.0.0", *version.Version)
+	})
+
+	t.Run("Versions with REQUIRE_SIGNIN_VIEW enabled", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Service.RequireSignInView, true)()
+		defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
+
+		t.Run("Get forgejo version without auth", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// GET api without auth
+			req := NewRequest(t, "GET", "/api/forgejo/v1/version")
+			MakeRequest(t, req, http.StatusForbidden)
+		})
+
+		t.Run("Get forgejo version without auth", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			username := "user1"
+			session := loginUser(t, username)
+			token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+			// GET api with auth
+			req := NewRequest(t, "GET", "/api/forgejo/v1/version").AddTokenAuth(token)
+			resp := MakeRequest(t, req, http.StatusOK)
+
+			var version v1.Version
+			DecodeJSON(t, resp, &version)
+			assert.Equal(t, "1.0.0", *version.Version)
+		})
+	})
 }
--- a/tests/integration/version_test.go
+++ b/tests/integration/version_test.go
@ -7,8 +7,11 @@ import (
 	"net/http"
 	"testing"

+	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/routers"
 	"code.gitea.io/gitea/tests"

 	"github.com/stretchr/testify/assert"
@ -17,11 +20,43 @@ import (
 func TestVersion(t *testing.T) {
 	defer tests.PrepareTestEnv(t)()

-	setting.AppVer = "test-version-1"
-	req := NewRequest(t, "GET", "/api/v1/version")
-	resp := MakeRequest(t, req, http.StatusOK)
+	t.Run("Version", func(t *testing.T) {
+		setting.AppVer = "test-version-1"
+		req := NewRequest(t, "GET", "/api/v1/version")
+		resp := MakeRequest(t, req, http.StatusOK)

-	var version structs.ServerVersion
-	DecodeJSON(t, resp, &version)
-	assert.Equal(t, setting.AppVer, version.Version)
+		var version structs.ServerVersion
+		DecodeJSON(t, resp, &version)
+		assert.Equal(t, setting.AppVer, version.Version)
+	})
+
+	t.Run("Versions with REQUIRE_SIGNIN_VIEW enabled", func(t *testing.T) {
+		defer test.MockVariableValue(&setting.Service.RequireSignInView, true)()
+		defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
+
+		setting.AppVer = "test-version-1"
+
+		t.Run("Get version without auth", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+
+			// GET api without auth
+			req := NewRequest(t, "GET", "/api/v1/version")
+			MakeRequest(t, req, http.StatusForbidden)
+		})
+
+		t.Run("Get version without auth", func(t *testing.T) {
+			defer tests.PrintCurrentTest(t)()
+			username := "user1"
+			session := loginUser(t, username)
+			token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+			// GET api with auth
+			req := NewRequest(t, "GET", "/api/v1/version").AddTokenAuth(token)
+			resp := MakeRequest(t, req, http.StatusOK)
+
+			var version structs.ServerVersion
+			DecodeJSON(t, resp, &version)
+			assert.Equal(t, setting.AppVer, version.Version)
+		})
+	})
 }