From 41676a8634fd7a0bfc994cc29236ea217f70bc01 Mon Sep 17 00:00:00 2001 From: Ada Date: Tue, 19 Mar 2024 07:16:19 +0000 Subject: [PATCH] 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 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2582 Reviewed-by: oliverpool Reviewed-by: Gusted Reviewed-by: Earl Warren Co-authored-by: Ada Co-committed-by: Ada --- routers/api/forgejo/v1/api.go | 4 + routers/api/shared/middleware.go | 152 ++++++++++++++++++ routers/api/v1/api.go | 135 +--------------- tests/integration/api_forgejo_version_test.go | 44 ++++- tests/integration/version_test.go | 47 +++++- 5 files changed, 238 insertions(+), 144 deletions(-) create mode 100644 routers/api/shared/middleware.go diff --git a/routers/api/forgejo/v1/api.go b/routers/api/forgejo/v1/api.go index 33e9eb196..88c7502e6 100644 --- 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) diff --git a/routers/api/shared/middleware.go b/routers/api/shared/middleware.go new file mode 100644 index 000000000..e2ff00402 --- /dev/null +++ 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) + }) + } +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index c296cac79..b202e32e4 100644 --- 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) - }) - } -} diff --git a/tests/integration/api_forgejo_version_test.go b/tests/integration/api_forgejo_version_test.go index b59afcbb1..5c95fd373 100644 --- 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) + }) + }) } diff --git a/tests/integration/version_test.go b/tests/integration/version_test.go index a6ae649b4..144471adb 100644 --- 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) + }) + }) }