diff --git a/modules/web/middleware/request.go b/modules/web/middleware/request.go deleted file mode 100644 index 0bb155df70..0000000000 --- a/modules/web/middleware/request.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2020 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package middleware - -import ( - "net/http" - "strings" -) - -// IsAPIPath returns true if the specified URL is an API path -func IsAPIPath(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/api/") -} diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 7106521e85..bc79669d51 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -193,9 +193,15 @@ func CommonRoutes() *web.Route { verifyAuth(r, []auth.Method{ &auth_method.OAuth2{}, &auth_method.Basic{}, - &auth_method.AccessToken{}, + &auth_method.AccessToken{ + PermitBasic: true, + PermitBearer: true, + }, &auth_method.ActionRuntimeToken{}, - &auth_method.ActionTaskToken{}, + &auth_method.ActionTaskToken{ + PermitBasic: true, + PermitBearer: true, + }, &nuget.Auth{}, &conan.Auth{}, &chef.Auth{}, @@ -842,9 +848,12 @@ func ContainerRoutes() *web.Route { verifyContainerAuth(r, []auth.Method{ &auth_method.Basic{}, - &auth_method.AccessToken{}, - &auth_method.ActionRuntimeToken{}, - &auth_method.ActionTaskToken{}, + &auth_method.AccessToken{ + PermitBasic: true, + }, + &auth_method.ActionTaskToken{ + PermitBasic: true, + }, &container.Auth{}, &auth_method.AuthorizedIntegration{ // `docker login` can't send a bearer token, so enable reading a token from the password field of diff --git a/routers/api/shared/middleware.go b/routers/api/shared/middleware.go index f6dd30f8b8..10802a89ed 100644 --- a/routers/api/shared/middleware.go +++ b/routers/api/shared/middleware.go @@ -50,9 +50,15 @@ func buildAuthGroup() *auth_method.Group { &auth_method.OAuth2{}, &auth_method.HTTPSign{}, &auth_method.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API - &auth_method.AccessToken{}, + &auth_method.AccessToken{ + PermitBasic: true, + PermitBearer: true, + }, &auth_method.ActionRuntimeToken{}, - &auth_method.ActionTaskToken{}, + &auth_method.ActionTaskToken{ + PermitBasic: true, + PermitBearer: true, + }, &auth_method.AuthorizedIntegration{}, ) if setting.Service.EnableReverseProxyAuthAPI { diff --git a/routers/web/web.go b/routers/web/web.go index df97edd558..7d69f7f978 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -105,24 +105,35 @@ func optionsCorsHandler() func(next http.Handler) http.Handler { // earlier authentication success would prevent later authentication methods from being attempted. func buildAuthGroup() *auth_method.Group { group := auth_method.NewGroup() + if setting.Service.EnableReverseProxyAuth { + // reverseproxy should before Session, otherwise the header will be ignored if user has login + group.Add(&auth_method.ReverseProxy{ + CreateSession: true, + }) + } + group.Add(&auth_method.Session{}) + return group +} - // FIXME: OAuth2, Basic, AccessToken, ActionRuntimeToken, ActionTaskToken authentication methods all use path - // matching so that they only authenticate requests in specific endpoints -- in general these auth methods aren't - // permitted for web endpoints, and aren't supported because the security models don't match. (For example, they - // can have scoped permissions, which web UI handlers don't implement.) A clearer way to do this would be to apply - // specific authentication methods to their auth middlewares. buildGitLfsAuthGroup and buildGitAuthGroup are the - // start of transitioning to that model. Eventually these methods will be fully removed from buildAuthGroup: +// Authentication methods that are applied to "mixed" web URL routes. Mixed routes are those that are primarily +// designed for web browser access, but can also be accessed by API consumers. +func buildMixedAuthGroup() *auth_method.Group { + group := auth_method.NewGroup() group.Add(&auth_method.OAuth2{}) group.Add(&auth_method.Basic{}) - group.Add(&auth_method.AccessToken{}) + group.Add(&auth_method.AccessToken{ + PermitBasic: true, + PermitBearer: true, + }) group.Add(&auth_method.ActionRuntimeToken{}) - group.Add(&auth_method.ActionTaskToken{}) - + group.Add(&auth_method.ActionTaskToken{ + PermitBasic: true, + PermitBearer: true, + }) if setting.Service.EnableReverseProxyAuth { group.Add(&auth_method.ReverseProxy{}) // reverseproxy should before Session, otherwise the header will be ignored if user has login } group.Add(&auth_method.Session{}) - return group } @@ -132,8 +143,17 @@ func buildGitLfsAuthGroup() *auth_method.Group { group := auth_method.NewGroup() group.Add(&auth_method.LFSToken{}) group.Add(&auth_method.Basic{}) - group.Add(&auth_method.AccessToken{}) - group.Add(&auth_method.ActionTaskToken{}) + group.Add(&auth_method.AccessToken{ + PermitBasic: true, + // PermitBearer is left at default `false`. This behaviour is maintained from when one auth method performed + // all the "Basic ..." handling, and one performed all the "Bearer ..." handling, and access to LFS paths + // weren't permitted in the bearer codepath. There isn't a clear reason from Forgejo's perspective to deny + // usage in this case, it's just maintained because it previously worked that way. + }) + group.Add(&auth_method.ActionTaskToken{ + PermitBasic: true, + // PermitBearer is left at default `false` -- same explanation as above for AccessToken. + }) group.Add(&auth_method.AuthorizedIntegration{ // "Authorization: Basic ..." is easier to use for git operations, and already supported for other tokens, so it // is enabled for Authorized Integrations as well: @@ -148,9 +168,15 @@ func buildGitAuthGroup() *auth_method.Group { group := auth_method.NewGroup() group.Add(&auth_method.OAuth2{}) group.Add(&auth_method.Basic{}) - group.Add(&auth_method.AccessToken{}) + group.Add(&auth_method.AccessToken{ + PermitBasic: true, + PermitBearer: true, + }) group.Add(&auth_method.ActionRuntimeToken{}) - group.Add(&auth_method.ActionTaskToken{}) + group.Add(&auth_method.ActionTaskToken{ + PermitBasic: true, + PermitBearer: true, + }) group.Add(&auth_method.AuthorizedIntegration{ // "Authorization: Basic ..." is easier to use for git operations, and already supported for other tokens, so it // is enabled for Authorized Integrations as well: @@ -360,6 +386,11 @@ func Routes() *web.Route { // TODO: GetNotificationCount & GetActiveStopwatch really seem like things that could be folded into Contexter or as helper functions user.GetNotificationCount, repo.GetActiveStopwatch, goGet) + routes.Group("", + func() { + registerMixedRoutes(routes) + }, + gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildMixedAuthGroup()), goGet) routes.Group("", func() { registerGitLFSRoutes(routes) @@ -369,6 +400,15 @@ func Routes() *web.Route { registerGitRoutes(routes) }, gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildGitAuthGroup()), goGet) + // The only endpoint which can only be accessed with the OAuth2 authentication method is /userinfo, extracted here + // so that other auth methods can't be applied to it + routes.Methods( + "GET, POST, OPTIONS", + "/login/oauth/userinfo", + gzipMid, common.Sessioner(), context.Contexter(), + oauth2Enabled, optionsCorsHandler(), ignoreCSRF, webAuth(&auth_method.OAuth2{}), + auth.InfoOAuth) + routes.NotFound( gzipMid, common.Sessioner(), context.Contexter(), webAuth(buildAuthGroup()), // TODO: GetNotificationCount & GetActiveStopwatch really seem like things that could be folded into Contexter or as helper functions @@ -385,26 +425,39 @@ func Routes() *web.Route { return routes } -var ignoreCSRF = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) - -// registerRoutes register routes -func registerRoutes(m *web.Route) { - reqSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: true}) - reqSignOut := verifyAuthWithOptions(&common.VerifyOptions{SignOutRequired: true}) +var ( + ignoreCSRF = verifyAuthWithOptions(&common.VerifyOptions{DisableCSRF: true}) + reqSignIn = verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: true}) + reqSignOut = verifyAuthWithOptions(&common.VerifyOptions{SignOutRequired: true}) // TODO: rename them to "optSignIn", which means that the "sign-in" could be optional, depends on the VerifyOptions (RequireSignInView) - ignSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) - ignExploreSignIn := verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) + ignSignIn = verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView}) + ignExploreSignIn = verifyAuthWithOptions(&common.VerifyOptions{SignInRequired: setting.Service.RequireSignInView || setting.Service.Explore.RequireSigninView}) - validation.AddBindingRules() + reqRepoAdmin = context.RequireRepoAdmin() + reqRepoCodeWriter = context.RequireRepoWriter(unit.TypeCode) + canEnableEditor = context.CanEnableEditor() + reqRepoCodeReader = context.RequireRepoReader(unit.TypeCode) + reqRepoReleaseWriter = context.RequireRepoWriter(unit.TypeReleases) + reqRepoReleaseReader = context.RequireRepoReader(unit.TypeReleases) + reqRepoWikiWriter = context.RequireRepoWriter(unit.TypeWiki) + reqRepoIssueReader = context.RequireRepoReader(unit.TypeIssues) + reqRepoPullsReader = context.RequireRepoReader(unit.TypePullRequests) + reqRepoIssuesOrPullsWriter = context.RequireRepoWriterOr(unit.TypeIssues, unit.TypePullRequests) + reqRepoIssuesOrPullsReader = context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests) + reqRepoProjectsReader = context.RequireRepoReader(unit.TypeProjects) + reqRepoProjectsWriter = context.RequireRepoWriter(unit.TypeProjects) + reqRepoActionsReader = context.RequireRepoReader(unit.TypeActions) + reqRepoActionsWriter = context.RequireRepoWriter(unit.TypeActions) + reqRepoDelegateActionTrust = context.RequireRepoDelegateActionTrust() - linkAccountEnabled := func(ctx *context.Context) { + linkAccountEnabled = func(ctx *context.Context) { if !setting.Service.EnableOpenIDSignIn && !setting.Service.EnableOpenIDSignUp && !setting.OAuth2.Enabled { ctx.Error(http.StatusForbidden) return } } - requiredTwoFactor := func(ctx *context.Context) { + requiredTwoFactor = func(ctx *context.Context) { if !ctx.Doer.MustHaveTwoFactor() { return } @@ -418,28 +471,28 @@ func registerRoutes(m *web.Route) { ctx.Data["HideNavbarLinks"] = !hasTwoFactor } - openIDSignInEnabled := func(ctx *context.Context) { + openIDSignInEnabled = func(ctx *context.Context) { if !setting.Service.EnableOpenIDSignIn { ctx.Error(http.StatusForbidden) return } } - openIDSignUpEnabled := func(ctx *context.Context) { + openIDSignUpEnabled = func(ctx *context.Context) { if !setting.Service.EnableOpenIDSignUp { ctx.Error(http.StatusForbidden) return } } - oauth2Enabled := func(ctx *context.Context) { + oauth2Enabled = func(ctx *context.Context) { if !setting.OAuth2.Enabled { ctx.Error(http.StatusForbidden) return } } - reqMilestonesDashboardPageEnabled := func(ctx *context.Context) { + reqMilestonesDashboardPageEnabled = func(ctx *context.Context) { if !setting.Service.ShowMilestonesDashboardPage { ctx.Error(http.StatusForbidden) return @@ -447,49 +500,42 @@ func registerRoutes(m *web.Route) { } // webhooksEnabled requires webhooks to be enabled by admin. - webhooksEnabled := func(ctx *context.Context) { + webhooksEnabled = func(ctx *context.Context) { if setting.DisableWebhooks { ctx.Error(http.StatusForbidden) return } } - federationEnabled := func(ctx *context.Context) { + federationEnabled = func(ctx *context.Context) { if !setting.Federation.Enabled { ctx.Error(http.StatusNotFound) return } } - dlSourceEnabled := func(ctx *context.Context) { - if setting.Repository.DisableDownloadSourceArchives { - ctx.Error(http.StatusNotFound) - return - } - } - - sitemapEnabled := func(ctx *context.Context) { + sitemapEnabled = func(ctx *context.Context) { if !setting.Other.EnableSitemap { ctx.Error(http.StatusNotFound) return } } - packagesEnabled := func(ctx *context.Context) { + packagesEnabled = func(ctx *context.Context) { if !setting.Packages.Enabled { ctx.Error(http.StatusForbidden) return } } - feedEnabled := func(ctx *context.Context) { + feedEnabled = func(ctx *context.Context) { if !setting.Other.EnableFeed { ctx.Error(http.StatusNotFound) return } } - reqUnitAccess := func(unitType unit.Type, accessMode perm.AccessMode, ignoreGlobal bool) func(ctx *context.Context) { + reqUnitAccess = func(unitType unit.Type, accessMode perm.AccessMode, ignoreGlobal bool) func(ctx *context.Context) { return func(ctx *context.Context) { // only check global disabled units when ignoreGlobal is false if !ignoreGlobal && unitType.UnitGlobalDisabled() { @@ -510,6 +556,11 @@ func registerRoutes(m *web.Route) { } } } +) + +// registerRoutes register routes +func registerRoutes(m *web.Route) { + validation.AddBindingRules() addSettingsVariablesRoutes := func() { m.Group("/variables", func() { @@ -640,7 +691,6 @@ func registerRoutes(m *web.Route) { }, reqSignIn) m.Group("", func() { - m.Methods("GET, POST, OPTIONS", "/userinfo", auth.InfoOAuth) m.Methods("POST, OPTIONS", "/access_token", web.Bind(forms.AccessTokenForm{}), auth.AccessTokenOAuth) m.Methods("GET, OPTIONS", "/keys", auth.OIDCKeys) m.Methods("POST, OPTIONS", "/introspect", web.Bind(forms.IntrospectTokenForm{}), auth.IntrospectOAuth) @@ -932,28 +982,10 @@ func registerRoutes(m *web.Route) { m.Group("", func() { m.Get("/{username}", user.UsernameSubRoute) - m.Methods("GET, OPTIONS", "/attachments/{uuid}", optionsCorsHandler(), repo.GetAttachment) }, ignSignIn) m.Post("/{username}", reqSignIn, context.UserAssignmentWeb(), user.Action) - reqRepoAdmin := context.RequireRepoAdmin() - reqRepoCodeWriter := context.RequireRepoWriter(unit.TypeCode) - canEnableEditor := context.CanEnableEditor() - reqRepoCodeReader := context.RequireRepoReader(unit.TypeCode) - reqRepoReleaseWriter := context.RequireRepoWriter(unit.TypeReleases) - reqRepoReleaseReader := context.RequireRepoReader(unit.TypeReleases) - reqRepoWikiWriter := context.RequireRepoWriter(unit.TypeWiki) - reqRepoIssueReader := context.RequireRepoReader(unit.TypeIssues) - reqRepoPullsReader := context.RequireRepoReader(unit.TypePullRequests) - reqRepoIssuesOrPullsWriter := context.RequireRepoWriterOr(unit.TypeIssues, unit.TypePullRequests) - reqRepoIssuesOrPullsReader := context.RequireRepoReaderOr(unit.TypeIssues, unit.TypePullRequests) - reqRepoProjectsReader := context.RequireRepoReader(unit.TypeProjects) - reqRepoProjectsWriter := context.RequireRepoWriter(unit.TypeProjects) - reqRepoActionsReader := context.RequireRepoReader(unit.TypeActions) - reqRepoActionsWriter := context.RequireRepoWriter(unit.TypeActions) - reqRepoDelegateActionTrust := context.RequireRepoDelegateActionTrust() - reqPackageAccess := func(accessMode perm.AccessMode) func(ctx *context.Context) { return func(ctx *context.Context) { if ctx.Package.AccessMode < accessMode && !ctx.IsUserSiteAdmin() { @@ -1478,7 +1510,6 @@ func registerRoutes(m *web.Route) { }, ctxDataSet("EnableFeed", setting.Other.EnableFeed), repo.MustBeNotEmpty, context.RepoRefByType(context.RepoRefTag, true)) m.Get("/releases/attachments/{uuid}", repo.MustBeNotEmpty, repo.GetAttachment) - m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload) m.Group("/releases", func() { m.Combo("/new", context.EnforceQuotaWeb(quota_model.LimitSubjectSizeReposAll, context.QuotaTargetRepo)). Get(repo.NewRelease). @@ -1493,11 +1524,6 @@ func registerRoutes(m *web.Route) { }, reqSignIn, repo.MustBeNotEmpty, context.RepoMustNotBeArchived(), reqRepoReleaseWriter, repo.CommitInfoCache, context.EnforceQuotaWeb(quota_model.LimitSubjectSizeReposAll, context.QuotaTargetRepo)) }, ignSignIn, context.RepoAssignment, context.UnitTypes(), reqRepoReleaseReader) - // to maintain compatibility with old attachments - m.Group("/{username}/{reponame}", func() { - m.Get("/attachments/{uuid}", repo.GetAttachment) - }, ignSignIn, context.RepoAssignment, context.UnitTypes()) - m.Group("/{username}/{reponame}", func() { m.Post("/topics", repo.TopicsPost) }, context.RepoAssignment, context.RepoMustNotBeArchived(), reqRepoAdmin) @@ -1646,11 +1672,6 @@ func registerRoutes(m *web.Route) { m.Get("/{period}", repo.ActivityAuthors) }, context.RepoRef(), repo.MustBeNotEmpty, context.RequireRepoReaderOr(unit.TypeCode)) - m.Group("/archive", func() { - m.Get("/*", repo.Download) - m.Post("/*", repo.InitiateDownload) - }, repo.MustBeNotEmpty, dlSourceEnabled, reqRepoCodeReader) - m.Group("/branches", func() { m.Get("/list", repo.GetBranchesList) m.Get("", repo.Branches) @@ -1721,15 +1742,6 @@ func registerRoutes(m *web.Route) { m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownloadOrLFS) }, repo.MustBeNotEmpty, reqRepoCodeReader) - m.Group("/raw", func() { - m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload) - m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload) - m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload) - m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByID) - // "/*" route is deprecated, and kept for backward compatibility - m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload) - }, repo.MustBeNotEmpty, reqRepoCodeReader) - m.Group("/render", func() { m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.RenderFile) m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.RenderFile) @@ -1870,6 +1882,44 @@ func registerGitRoutes(m *web.Route) { }) } +func registerMixedRoutes(m *web.Route) { + dlSourceEnabled := func(ctx *context.Context) { + if setting.Repository.DisableDownloadSourceArchives { + ctx.Error(http.StatusNotFound) + return + } + } + + m.Group("/{username}/{reponame}", func() { + m.Group("/raw", func() { + m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.SingleDownload) + m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.SingleDownload) + m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.SingleDownload) + m.Get("/blob/{sha}", context.RepoRefByType(context.RepoRefBlob), repo.DownloadByID) + // "/*" route is deprecated, and kept for backward compatibility + m.Get("/*", context.RepoRefByType(context.RepoRefLegacy), repo.SingleDownload) + }, repo.MustBeNotEmpty, reqRepoCodeReader) + + m.Group("/archive", func() { + m.Get("/*", repo.Download) + m.Post("/*", repo.InitiateDownload) + }, repo.MustBeNotEmpty, dlSourceEnabled, reqRepoCodeReader) + }, ignSignIn, context.RepoAssignment, context.UnitTypes()) + + m.Group("/{username}/{reponame}", func() { + m.Get("/releases/download/{vTag}/{fileName}", repo.MustBeNotEmpty, repo.RedirectDownload) + }, ignSignIn, context.RepoAssignment, context.UnitTypes(), reqRepoReleaseReader) + + // to maintain compatibility with old attachments + m.Group("/{username}/{reponame}", func() { + m.Get("/attachments/{uuid}", repo.GetAttachment) + }, ignSignIn, context.RepoAssignment, context.UnitTypes()) + + m.Group("", func() { + m.Methods("GET, OPTIONS", "/attachments/{uuid}", optionsCorsHandler(), repo.GetAttachment) + }, ignSignIn) +} + func BindUpload() http.HandlerFunc { return func(resp http.ResponseWriter, req *http.Request) { theObj := new(forms.UploadRepoFileForm) // create a new form obj for every request but not use obj directly diff --git a/services/auth/method/access_token.go b/services/auth/method/access_token.go index f1a5e8e766..7f8174336d 100644 --- a/services/auth/method/access_token.go +++ b/services/auth/method/access_token.go @@ -11,24 +11,21 @@ import ( user_model "forgejo.org/models/user" "forgejo.org/modules/log" "forgejo.org/modules/optional" - "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" "forgejo.org/services/authz" ) var _ auth.Method = &AccessToken{} -type AccessToken struct{} +type AccessToken struct { + // Permit the use of `Authorization: Basic ...` to include an access token. + PermitBasic bool + // Permit the use of `Authorization: Bearer ...`, `Authorization: Token ...`, and form-based tokens. + PermitBearer bool +} func (a *AccessToken) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput { - // Authentication previously was performed in a single routine for `Authorization: Basic ...` and `Authorization: - // Bearer ...`, and both routines had separate URL exclusion lists onto which they wouldn't apply. That behaviour - // is maintained by cloning those conditions here and deciding whether to look at basic/bearer auth, or not. In the - // future this should be removed and migrated to route-specific middleware. - legacySkipBasic := !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) - legacySkipFormAndBearer := !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && !isGitRawOrAttachPath(req) && !isArchivePath(req) - - maybeAuthToken := a.getTokenFromRequest(req, legacySkipBasic, legacySkipFormAndBearer) + maybeAuthToken := a.getTokenFromRequest(req) if !maybeAuthToken.Has() { return &auth.AuthenticationNotAttempted{} } @@ -60,8 +57,8 @@ func (a *AccessToken) Verify(req *http.Request, w http.ResponseWriter, _ auth.Se return &auth.AuthenticationSuccess{Result: &accessTokenAuthenticationResult{user: u, scope: token.Scope, reducer: reducer}} } -func (a *AccessToken) getTokenFromRequest(req *http.Request, skipBasic, skipFormAndBearer bool) optional.Option[string] { - if !skipFormAndBearer { +func (a *AccessToken) getTokenFromRequest(req *http.Request) optional.Option[string] { + if a.PermitBearer { if has, token := tokenFromForm(req).Get(); has { return optional.Some(token) } @@ -69,7 +66,7 @@ func (a *AccessToken) getTokenFromRequest(req *http.Request, skipBasic, skipForm return optional.Some(token) } } - if !skipBasic { + if a.PermitBasic { if has, token := tokenFromAuthorizationBasic(req).Get(); has { return optional.Some(token) } diff --git a/services/auth/method/action_runtime_token.go b/services/auth/method/action_runtime_token.go index 2d15476424..f58e9129a9 100644 --- a/services/auth/method/action_runtime_token.go +++ b/services/auth/method/action_runtime_token.go @@ -11,7 +11,6 @@ import ( actions_model "forgejo.org/models/actions" user_model "forgejo.org/models/user" "forgejo.org/modules/optional" - "forgejo.org/modules/web/middleware" "forgejo.org/services/actions" "forgejo.org/services/auth" ) @@ -21,11 +20,6 @@ var _ auth.Method = &ActionRuntimeToken{} type ActionRuntimeToken struct{} func (a *ActionRuntimeToken) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput { - // In the future this should be removed and migrated to route-specific middleware: - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && !isGitRawOrAttachPath(req) && !isArchivePath(req) { - return &auth.AuthenticationNotAttempted{} - } - maybeAuthToken := a.getTokenFromRequest(req) if !maybeAuthToken.Has() { return &auth.AuthenticationNotAttempted{} diff --git a/services/auth/method/action_task_token.go b/services/auth/method/action_task_token.go index b5e1ab4f59..d78fa5346d 100644 --- a/services/auth/method/action_task_token.go +++ b/services/auth/method/action_task_token.go @@ -13,23 +13,20 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/util" - "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" ) var _ auth.Method = &ActionTaskToken{} -type ActionTaskToken struct{} +type ActionTaskToken struct { + // Permit the use of `Authorization: Basic ...` to include an access token. + PermitBasic bool + // Permit the use of `Authorization: Bearer ...`, `Authorization: Token ...`, and form-based tokens. + PermitBearer bool +} func (a *ActionTaskToken) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput { - // Authentication previously was performed in a single routine for `Authorization: Basic ...` and `Authorization: - // Bearer ...`, and both routines had separate URL exclusion lists onto which they wouldn't apply. That behaviour - // is maintained by cloning those conditions here and deciding whether to look at basic/bearer auth, or not. In the - // future this should be removed and migrated to route-specific middleware. - legacySkipBasic := !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) - legacySkipFormAndBearer := !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && !isGitRawOrAttachPath(req) && !isArchivePath(req) - - maybeAuthToken := a.getTokenFromRequest(req, legacySkipBasic, legacySkipFormAndBearer) + maybeAuthToken := a.getTokenFromRequest(req) if !maybeAuthToken.Has() { return &auth.AuthenticationNotAttempted{} } @@ -49,8 +46,8 @@ func (a *ActionTaskToken) Verify(req *http.Request, w http.ResponseWriter, _ aut return &auth.AuthenticationSuccess{Result: &actionsTaskTokenAuthenticationResult{user: user_model.NewActionsUser(), taskID: task.ID}} } -func (a *ActionTaskToken) getTokenFromRequest(req *http.Request, skipBasic, skipFormAndBearer bool) optional.Option[string] { - if !skipFormAndBearer { +func (a *ActionTaskToken) getTokenFromRequest(req *http.Request) optional.Option[string] { + if a.PermitBearer { if has, token := tokenFromForm(req).Get(); has { return optional.Some(token) } @@ -58,7 +55,7 @@ func (a *ActionTaskToken) getTokenFromRequest(req *http.Request, skipBasic, skip return optional.Some(token) } } - if !skipBasic { + if a.PermitBasic { if has, token := tokenFromAuthorizationBasic(req).Get(); has { return optional.Some(token) } diff --git a/services/auth/method/auth.go b/services/auth/method/auth.go index b2e89d51e9..7d6a0d6722 100644 --- a/services/auth/method/auth.go +++ b/services/auth/method/auth.go @@ -7,15 +7,12 @@ package method import ( "fmt" "net/http" - "regexp" - "strings" user_model "forgejo.org/models/user" "forgejo.org/modules/auth/webauthn" "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/session" - "forgejo.org/modules/setting" "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" user_service "forgejo.org/services/user" @@ -27,42 +24,6 @@ func Init() { webauthn.Init() } -// isAttachmentDownload check if request is a file download (GET) with URL to an attachment -func isAttachmentDownload(req *http.Request) bool { - return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET" -} - -// isContainerPath checks if the request targets the container endpoint -func isContainerPath(req *http.Request) bool { - // Go's URL omits trailing slashes from `Path`. That means that `/v2/`, the top-level endpoint, appears as `/v2`. - // strings.HasPrefix(req.URL.Path, "/v2") would be inappropriate because it would match paths like `/v2-abcd`, too. - return req.URL.Path == "/v2" || strings.HasPrefix(req.URL.Path, "/v2/") -} - -var ( - gitRawOrAttachPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)|(?:raw/)|(?:releases/download/)|(?:attachments/))`) - lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`) - archivePathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/archive/`) -) - -func isGitRawOrAttachPath(req *http.Request) bool { - return gitRawOrAttachPathRe.MatchString(req.URL.Path) -} - -func isGitRawOrAttachOrLFSPath(req *http.Request) bool { - if isGitRawOrAttachPath(req) { - return true - } - if setting.LFS.StartServer { - return lfsPathRe.MatchString(req.URL.Path) - } - return false -} - -func isArchivePath(req *http.Request) bool { - return archivePathRe.MatchString(req.URL.Path) -} - // handleSignIn clears existing session variables and stores new ones for the specified user object func handleSignIn(resp http.ResponseWriter, req *http.Request, sess auth.SessionStore, user *user_model.User) { // We need to regenerate the session... diff --git a/services/auth/method/auth_test.go b/services/auth/method/auth_test.go deleted file mode 100644 index 6e686feab8..0000000000 --- a/services/auth/method/auth_test.go +++ /dev/null @@ -1,198 +0,0 @@ -// Copyright 2014 The Gogs Authors. All rights reserved. -// Copyright 2019 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package method - -import ( - "net/http" - "net/url" - "testing" - - "forgejo.org/modules/setting" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_isGitRawOrLFSPath(t *testing.T) { - tests := []struct { - path string - - want bool - }{ - { - "/owner/repo/git-upload-pack", - true, - }, - { - "/owner/repo/git-receive-pack", - true, - }, - { - "/owner/repo/info/refs", - true, - }, - { - "/owner/repo/HEAD", - true, - }, - { - "/owner/repo/objects/info/alternates", - true, - }, - { - "/owner/repo/objects/info/http-alternates", - true, - }, - { - "/owner/repo/objects/info/packs", - true, - }, - { - "/owner/repo/objects/info/blahahsdhsdkla", - true, - }, - { - "/owner/repo/objects/01/23456789abcdef0123456789abcdef01234567", - true, - }, - { - "/owner/repo/objects/pack/pack-123456789012345678921234567893124567894.pack", - true, - }, - { - "/owner/repo/objects/pack/pack-0123456789abcdef0123456789abcdef0123456.idx", - true, - }, - { - "/owner/repo/raw/branch/foo/fanaso", - true, - }, - { - "/owner/repo/stars", - false, - }, - { - "/notowner", - false, - }, - { - "/owner/repo", - false, - }, - { - "/owner/repo/commit/123456789012345678921234567893124567894", - false, - }, - { - "/owner/repo/releases/download/tag/repo.tar.gz", - true, - }, - { - "/owner/repo/attachments/6d92a9ee-5d8b-4993-97c9-6181bdaa8955", - true, - }, - } - lfsTests := []string{ - "/owner/repo/info/lfs/", - "/owner/repo/info/lfs/objects/batch", - "/owner/repo/info/lfs/objects/oid/filename", - "/owner/repo/info/lfs/objects/oid", - "/owner/repo/info/lfs/objects", - "/owner/repo/info/lfs/verify", - "/owner/repo/info/lfs/locks", - "/owner/repo/info/lfs/locks/verify", - "/owner/repo/info/lfs/locks/123/unlock", - } - - origLFSStartServer := setting.LFS.StartServer - - for _, tt := range tests { - t.Run(tt.path, func(t *testing.T) { - req, _ := http.NewRequest("POST", "http://localhost"+tt.path, nil) - setting.LFS.StartServer = false - if got := isGitRawOrAttachOrLFSPath(req); got != tt.want { - t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) - } - setting.LFS.StartServer = true - if got := isGitRawOrAttachOrLFSPath(req); got != tt.want { - t.Errorf("isGitOrLFSPath() = %v, want %v", got, tt.want) - } - }) - } - for _, tt := range lfsTests { - t.Run(tt, func(t *testing.T) { - req, _ := http.NewRequest("POST", tt, nil) - setting.LFS.StartServer = false - if got := isGitRawOrAttachOrLFSPath(req); got != setting.LFS.StartServer { - t.Errorf("isGitOrLFSPath(%q) = %v, want %v, %v", tt, got, setting.LFS.StartServer, gitRawOrAttachPathRe.MatchString(tt)) - } - setting.LFS.StartServer = true - if got := isGitRawOrAttachOrLFSPath(req); got != setting.LFS.StartServer { - t.Errorf("isGitOrLFSPath(%q) = %v, want %v", tt, got, setting.LFS.StartServer) - } - }) - } - setting.LFS.StartServer = origLFSStartServer -} - -func TestAuth_isContainerPath(t *testing.T) { - testCases := []struct { - name string - input string - isContainerPath bool - }{ - { - name: "without trailing slash", - input: "https://example.com/v2", - isContainerPath: true, - }, - { - name: "with trailing slash", - input: "https://example.com/v2/", - isContainerPath: true, - }, - { - name: "with additional path components", - input: "https://example.com/v2/example/blobs/uploads/", - isContainerPath: true, - }, - { - name: "without v2", - input: "https://example.com/", - isContainerPath: false, - }, - { - name: "v2 not at the beginning", - input: "https://example.com/something/v2/", - isContainerPath: false, - }, - { - name: "v2 with prefix", - input: "https://example.com/abcd-v2/", - isContainerPath: false, - }, - { - name: "v2 with suffix", - input: "https://example.com/v2-abcd/", - isContainerPath: false, - }, - { - name: "v1", - input: "https://example.com/v1/", - isContainerPath: false, - }, - } - - for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - inputURL, err := url.Parse(testCase.input) - require.NoError(t, err) - - request := http.Request{URL: inputURL} - - assert.Equal(t, testCase.isContainerPath, isContainerPath(&request)) - }) - } -} diff --git a/services/auth/method/basic.go b/services/auth/method/basic.go index 8385bea2fd..abc3387efe 100644 --- a/services/auth/method/basic.go +++ b/services/auth/method/basic.go @@ -16,7 +16,6 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/setting" "forgejo.org/modules/util" - "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" "forgejo.org/services/auth/source/db" ) @@ -34,11 +33,6 @@ type Basic struct{} // Verify extracts and validates Basic data (username and password/token) from the "Authorization" header of the request // and returns the corresponding user object for that name/token on successful validation. func (b *Basic) Verify(req *http.Request, w http.ResponseWriter, _ auth.SessionStore) auth.MethodOutput { - // Basic authentication should only fire on API, Download or on Git or LFSPaths - if !middleware.IsAPIPath(req) && !isContainerPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { - return &auth.AuthenticationNotAttempted{} - } - baHead := req.Header.Get("Authorization") if len(baHead) == 0 { return &auth.AuthenticationNotAttempted{} diff --git a/services/auth/method/oauth2.go b/services/auth/method/oauth2.go index 9eeef6a6c1..acfd534fb7 100644 --- a/services/auth/method/oauth2.go +++ b/services/auth/method/oauth2.go @@ -17,7 +17,6 @@ import ( "forgejo.org/modules/log" "forgejo.org/modules/optional" "forgejo.org/modules/setting" - "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" "forgejo.org/services/auth/source/oauth2" ) @@ -74,11 +73,6 @@ func (o *OAuth2) Verify(req *http.Request, w http.ResponseWriter, _ auth.Session if !setting.OAuth2.Enabled { return &auth.AuthenticationNotAttempted{} } - // These paths are not API paths, but we still want to check for tokens because they maybe in the API returned URLs - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isAuthenticatedTokenRequest(req) && - !isGitRawOrAttachPath(req) && !isArchivePath(req) { - return &auth.AuthenticationNotAttempted{} - } maybeAuthToken := o.getTokenFromRequest(req) if !maybeAuthToken.Has() { @@ -145,7 +139,3 @@ func (*OAuth2) getTokenFromRequest(req *http.Request) optional.Option[string] { } return optional.None[string]() } - -func isAuthenticatedTokenRequest(req *http.Request) bool { - return req.URL.Path == "/login/oauth/userinfo" -} diff --git a/services/auth/method/reverseproxy.go b/services/auth/method/reverseproxy.go index 0fda466875..483f7ed724 100644 --- a/services/auth/method/reverseproxy.go +++ b/services/auth/method/reverseproxy.go @@ -15,7 +15,6 @@ import ( "forgejo.org/modules/optional" "forgejo.org/modules/setting" "forgejo.org/modules/util" - "forgejo.org/modules/web/middleware" "forgejo.org/services/auth" gouuid "github.com/google/uuid" @@ -34,7 +33,10 @@ const ReverseProxyMethodName = "reverse_proxy" // On successful authentication the proxy is expected to populate the username in the // "setting.ReverseProxyAuthUser" header. Optionally it can also populate the email of the // user in the "setting.ReverseProxyAuthEmail" header. -type ReverseProxy struct{} +type ReverseProxy struct { + // If true, create a session once a user authenticates. + CreateSession bool +} // getUserName extracts the username from the "setting.ReverseProxyAuthUser" header func (r *ReverseProxy) getUserName(req *http.Request) string { @@ -120,8 +122,7 @@ func (r *ReverseProxy) Verify(req *http.Request, w http.ResponseWriter, sess aut } } - // Make sure requests to API paths, attachment downloads, git and LFS do not create a new session - if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitRawOrAttachOrLFSPath(req) { + if r.CreateSession { if sess != nil && (sess.Get("uid") == nil || sess.Get("uid").(int64) != user.ID) { handleSignIn(w, req, sess, user) }