refactor: change authentication to return structured data (#12202)

Currently authentication methods return information in two forms: they return who was authenticated as a `*user_model.User`, and then they insert key-values into `ctx.Data` which has critical impact on how the authenticated request is treated.  This PR changes the authentication methods to return structured data in the form of an `AuthenticationResult`, with all the key-value information in `ctx.Data` being moved into methods on the `AuthenticationResult` interface.

Authentication workflows in Forgejo are a real mess.  This is the first step in trying to clean it up and make the code predictable and reasonable, and is both follow-up work that was identified from the repo-specific access tokens (where the `"ApiTokenReducer"` key-value was added), and is pre-requisite work to future JWT enhancements that are [being discussed](https://codeberg.org/forgejo/forgejo/issues/3571#issuecomment-13268004).

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. All work and communication must conform to Forgejo's [AI Agreement](https://codeberg.org/forgejo/governance/src/branch/main/AIAgreement.md). There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests for Go changes

- I added test coverage for Go changes...
  - [ ] in their respective `*_test.go` for unit tests.
  - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
  - All changes, at least in theory, are refactors of existing logic and are not expected to have functional deviations -- existing regression tests are the only planned testing.
- I ran...
  - [x] `make pr-go` before pushing

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change.
- [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12202
Reviewed-by: Andreas Ahlenstorf <aahlenst@noreply.codeberg.org>
This commit is contained in:
Mathieu Fenniak
2026-04-22 21:00:26 +02:00
committed by Mathieu Fenniak
parent 2ed98ac848
commit 1ddd5faa5c
45 changed files with 620 additions and 348 deletions
+52 -44
View File
@@ -38,48 +38,46 @@ import (
"forgejo.org/routers/api/packages/swift"
"forgejo.org/routers/api/packages/vagrant"
"forgejo.org/services/auth"
auth_method "forgejo.org/services/auth/method"
"forgejo.org/services/context"
)
func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) {
return func(ctx *context.Context) {
if ctx.Data["IsApiToken"] == true {
scope, ok := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
if ok { // it's a personal access token but not oauth2 token
scopeMatched := false
var err error
switch accessMode {
case perm.AccessModeRead:
scopeMatched, err = scope.HasScope(auth_model.AccessTokenScopeReadPackage)
if err != nil {
ctx.Error(http.StatusInternalServerError, "HasScope", err.Error())
return
}
case perm.AccessModeWrite:
scopeMatched, err = scope.HasScope(auth_model.AccessTokenScopeWritePackage)
if err != nil {
ctx.Error(http.StatusInternalServerError, "HasScope", err.Error())
return
}
}
if !scopeMatched {
ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea Package API"`)
ctx.Error(http.StatusUnauthorized, "reqPackageAccess", "user should have specific permission or be a site admin")
return
}
// check if scope only applies to public resources
publicOnly, err := scope.PublicOnly()
if hasScope, scope := ctx.Authentication.Scope().Get(); hasScope {
scopeMatched := false
var err error
switch accessMode {
case perm.AccessModeRead:
scopeMatched, err = scope.HasScope(auth_model.AccessTokenScopeReadPackage)
if err != nil {
ctx.Error(http.StatusForbidden, "tokenRequiresScope", "parsing public resource scope failed: "+err.Error())
ctx.Error(http.StatusInternalServerError, "HasScope", err.Error())
return
}
case perm.AccessModeWrite:
scopeMatched, err = scope.HasScope(auth_model.AccessTokenScopeWritePackage)
if err != nil {
ctx.Error(http.StatusInternalServerError, "HasScope", err.Error())
return
}
}
if !scopeMatched {
ctx.Resp.Header().Set("WWW-Authenticate", `Basic realm="Gitea Package API"`)
ctx.Error(http.StatusUnauthorized, "reqPackageAccess", "user should have specific permission or be a site admin")
return
}
if publicOnly {
if ctx.Package != nil && ctx.Package.Owner.Visibility.IsPrivate() {
ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public packages")
return
}
// check if scope only applies to public resources
publicOnly, err := scope.PublicOnly()
if err != nil {
ctx.Error(http.StatusForbidden, "tokenRequiresScope", "parsing public resource scope failed: "+err.Error())
return
}
if publicOnly {
if ctx.Package != nil && ctx.Package.Owner.Visibility.IsPrivate() {
ctx.Error(http.StatusForbidden, "reqToken", "token scope is limited to public packages")
return
}
}
}
@@ -116,38 +114,48 @@ func enforcePackagesQuota() func(ctx *context.Context) {
func verifyAuth(r *web.Route, authMethods []auth.Method) {
if setting.Service.EnableReverseProxyAuth {
authMethods = append(authMethods, &auth.ReverseProxy{})
authMethods = append(authMethods, &auth_method.ReverseProxy{})
}
authGroup := auth.NewGroup(authMethods...)
authGroup := auth_method.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) {
var err error
ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
authResult, err := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session)
if err != nil {
log.Info("Failed to verify user: %v", err)
ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
return
}
if authResult == nil {
ctx.Error(http.StatusInternalServerError, "verifyAuth nil authentication result")
return
}
ctx.Doer = authResult.User()
ctx.IsSigned = ctx.Doer != nil
ctx.Authentication = authResult
})
}
func verifyContainerAuth(r *web.Route, authMethods []auth.Method) {
if setting.Service.EnableReverseProxyAuth {
authMethods = append(authMethods, &auth.ReverseProxy{})
authMethods = append(authMethods, &auth_method.ReverseProxy{})
}
authGroup := auth.NewGroup(authMethods...)
authGroup := auth_method.NewGroup(authMethods...)
r.Use(func(ctx *context.Context) {
var err error
ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
authResult, err := authGroup.Verify(ctx.Req, ctx.Resp, ctx.Session)
if err != nil {
log.Info("Failed to verify user: %v", err)
container.APIUnauthorizedError(ctx)
ctx.Error(http.StatusUnauthorized, "authGroup.Verify")
return
}
if authResult == nil {
ctx.Error(http.StatusInternalServerError, "verifyContainerAuth nil authentication result")
return
}
ctx.Doer = authResult.User()
ctx.IsSigned = ctx.Doer != nil
ctx.Authentication = authResult
})
}
@@ -159,8 +167,8 @@ func CommonRoutes() *web.Route {
r.Use(context.PackageContexter())
verifyAuth(r, []auth.Method{
&auth.OAuth2{},
&auth.Basic{},
&auth_method.OAuth2{},
&auth_method.Basic{},
&nuget.Auth{},
&conan.Auth{},
&chef.Auth{},
@@ -804,7 +812,7 @@ func ContainerRoutes() *web.Route {
r.Use(context.PackageContexter())
verifyContainerAuth(r, []auth.Method{&auth.Basic{}, &container.Auth{}})
verifyContainerAuth(r, []auth.Method{&auth_method.Basic{}, &container.Auth{}})
r.Get("", container.ReqContainerAccess, container.DetermineSupport)
r.Group("/token", func() {
+14 -4
View File
@@ -39,9 +39,19 @@ var (
versionPattern = regexp.MustCompile(`version=(\d+\.\d+)`)
authorizationPattern = regexp.MustCompile(`\AX-Ops-Authorization-(\d+)`)
_ auth.Method = &Auth{}
_ auth.Method = &Auth{}
_ auth.AuthenticationResult = &chefAuthenticationResult{}
)
type chefAuthenticationResult struct {
*auth.BaseAuthenticationResult
user *user_model.User
}
func (r *chefAuthenticationResult) User() *user_model.User {
return r.user
}
// Documentation:
// https://docs.chef.io/server/api_chef_server/#required-headers
// https://github.com/chef-boneyard/chef-rfc/blob/master/rfc065-sign-v1.3.md
@@ -55,13 +65,13 @@ func (a *Auth) Name() string {
// Verify extracts the user from the signed request
// If the request is signed with the user private key the user is verified.
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) {
u, err := getUserFromRequest(req)
if err != nil {
return nil, err
}
if u == nil {
return nil, nil
return &auth.UnauthenticatedResult{}, nil
}
pub, err := getUserPublicKey(req.Context(), u)
@@ -82,7 +92,7 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
return nil, err
}
return u, nil
return &chefAuthenticationResult{user: u}, nil
}
func getUserFromRequest(req *http.Request) (*user_model.User, error) {
+25 -6
View File
@@ -6,13 +6,32 @@ package conan
import (
"net/http"
auth_model "forgejo.org/models/auth"
user_model "forgejo.org/models/user"
"forgejo.org/modules/log"
"forgejo.org/modules/optional"
"forgejo.org/services/auth"
"forgejo.org/services/packages"
)
var _ auth.Method = &Auth{}
var (
_ auth.Method = &Auth{}
_ auth.AuthenticationResult = &conanAuthenticationResult{}
)
type conanAuthenticationResult struct {
*auth.BaseAuthenticationResult
user *user_model.User
scope optional.Option[auth_model.AccessTokenScope]
}
func (r *conanAuthenticationResult) Scope() optional.Option[auth_model.AccessTokenScope] {
return r.scope
}
func (r *conanAuthenticationResult) User() *user_model.User {
return r.user
}
type Auth struct{}
@@ -21,7 +40,7 @@ func (a *Auth) Name() string {
}
// Verify extracts the user from the Bearer token
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) {
uid, scope, err := packages.ParseAuthorizationToken(req)
if err != nil {
log.Trace("ParseAuthorizationToken: %v", err)
@@ -29,13 +48,13 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
}
if uid == 0 {
return nil, nil
return &auth.UnauthenticatedResult{}, nil
}
// Propagate scope of the authorization token.
authScope := optional.None[auth_model.AccessTokenScope]()
if scope != "" {
store.GetData()["IsApiToken"] = true
store.GetData()["ApiTokenScope"] = scope
authScope = optional.Some(scope)
}
u, err := user_model.GetUserByID(req.Context(), uid)
@@ -44,5 +63,5 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
return nil, err
}
return u, nil
return &conanAuthenticationResult{user: u, scope: authScope}, nil
}
+1 -2
View File
@@ -11,7 +11,6 @@ import (
"strings"
"time"
auth_model "forgejo.org/models/auth"
"forgejo.org/models/db"
packages_model "forgejo.org/models/packages"
conan_model "forgejo.org/models/packages/conan"
@@ -119,7 +118,7 @@ func Authenticate(ctx *context.Context) {
}
// If there's an API scope, ensure it propagates.
scope, _ := ctx.Data.GetData()["ApiTokenScope"].(auth_model.AccessTokenScope)
scope := ctx.Authentication.Scope().ValueOrZeroValue()
token, err := packages_service.CreateAuthorizationToken(ctx.Doer, scope)
if err != nil {
+25 -6
View File
@@ -6,13 +6,32 @@ package container
import (
"net/http"
auth_model "forgejo.org/models/auth"
user_model "forgejo.org/models/user"
"forgejo.org/modules/log"
"forgejo.org/modules/optional"
"forgejo.org/services/auth"
"forgejo.org/services/packages"
)
var _ auth.Method = &Auth{}
var (
_ auth.Method = &Auth{}
_ auth.AuthenticationResult = &containerAuthenticationResult{}
)
type containerAuthenticationResult struct {
*auth.BaseAuthenticationResult
user *user_model.User
scope optional.Option[auth_model.AccessTokenScope]
}
func (r *containerAuthenticationResult) Scope() optional.Option[auth_model.AccessTokenScope] {
return r.scope
}
func (r *containerAuthenticationResult) User() *user_model.User {
return r.user
}
type Auth struct{}
@@ -22,7 +41,7 @@ func (a *Auth) Name() string {
// Verify extracts the user from the Bearer token
// If it's an anonymous session a ghost user is returned
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) {
uid, scope, err := packages.ParseAuthorizationToken(req)
if err != nil {
log.Trace("ParseAuthorizationToken: %v", err)
@@ -30,13 +49,13 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
}
if uid == 0 {
return nil, nil
return &auth.UnauthenticatedResult{}, nil
}
// Propagate scope of the authorization token.
authScope := optional.None[auth_model.AccessTokenScope]()
if scope != "" {
store.GetData()["IsApiToken"] = true
store.GetData()["ApiTokenScope"] = scope
authScope = optional.Some(scope)
}
u, err := user_model.GetPossibleUserByID(req.Context(), uid)
@@ -45,5 +64,5 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
return nil, err
}
return u, nil
return &containerAuthenticationResult{user: u, scope: authScope}, nil
}
+1 -2
View File
@@ -13,7 +13,6 @@ import (
"regexp"
"strconv"
auth_model "forgejo.org/models/auth"
packages_model "forgejo.org/models/packages"
container_model "forgejo.org/models/packages/container"
user_model "forgejo.org/models/user"
@@ -158,7 +157,7 @@ func Authenticate(ctx *context.Context) {
}
// If there's an API scope, ensure it propagates.
scope, _ := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
scope := ctx.Authentication.Scope().ValueOrZeroValue()
token, err := packages_service.CreateAuthorizationToken(u, scope)
if err != nil {
+17 -3
View File
@@ -12,6 +12,20 @@ import (
"forgejo.org/services/auth"
)
var (
_ auth.Method = &Auth{}
_ auth.AuthenticationResult = &nugetAuthenticationResult{}
)
type nugetAuthenticationResult struct {
*auth.BaseAuthenticationResult
user *user_model.User
}
func (r *nugetAuthenticationResult) User() *user_model.User {
return r.user
}
var _ auth.Method = &Auth{}
type Auth struct{}
@@ -21,14 +35,14 @@ func (a *Auth) Name() string {
}
// https://docs.microsoft.com/en-us/nuget/api/package-publish-resource#request-parameters
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataStore, sess auth.SessionStore) (*user_model.User, error) {
func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, sess auth.SessionStore) (auth.AuthenticationResult, error) {
token, err := auth_model.GetAccessTokenBySHA(req.Context(), req.Header.Get("X-NuGet-ApiKey"))
if err != nil {
if !auth_model.IsErrAccessTokenNotExist(err) && !auth_model.IsErrAccessTokenEmpty(err) {
log.Error("GetAccessTokenBySHA: %v", err)
return nil, err
}
return nil, nil
return &auth.UnauthenticatedResult{}, nil
}
u, err := user_model.GetUserByID(req.Context(), token.UID)
@@ -41,5 +55,5 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
log.Error("UpdateLastUsed: %v", err)
}
return u, nil
return &nugetAuthenticationResult{user: u}, nil
}
+21 -16
View File
@@ -4,6 +4,7 @@
package shared
import (
"errors"
"fmt"
"net/http"
@@ -12,6 +13,7 @@ import (
"forgejo.org/modules/setting"
"forgejo.org/routers/common"
"forgejo.org/services/auth"
auth_method "forgejo.org/services/auth/method"
"forgejo.org/services/authz"
"forgejo.org/services/context"
@@ -43,14 +45,14 @@ func Middlewares() (stack []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
func buildAuthGroup() *auth_method.Group {
group := auth_method.NewGroup(
&auth_method.OAuth2{},
&auth_method.HTTPSign{},
&auth_method.Basic{}, // FIXME: this should be removed once we don't allow basic auth in API
)
if setting.Service.EnableReverseProxyAuthAPI {
group.Add(&auth.ReverseProxy{})
group.Add(&auth_method.ReverseProxy{})
}
return group
@@ -63,15 +65,18 @@ func apiAuthentication(authMethod auth.Method) func(*context.APIContext) {
ctx.Error(http.StatusUnauthorized, "APIAuth", err)
return
}
ctx.Doer = ar.Doer
ctx.IsSigned = ar.Doer != nil
ctx.IsBasicAuth = ar.IsBasicAuth
if ar == nil {
ctx.Error(http.StatusInternalServerError, "apiAuthentication nil authentication result", errors.New("nil authentication result"))
return
}
ctx.Doer = ar.User()
ctx.IsSigned = ctx.Doer != nil
ctx.Authentication = ar
}
}
func apiAuthorization(ctx *context.APIContext) {
scope, scopeExists := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
if scopeExists {
if hasScope, scope := ctx.Authentication.Scope().Get(); hasScope {
publicOnly, err := scope.PublicOnly()
if err != nil {
ctx.Error(http.StatusForbidden, "tokenRequiresScope", "parsing public resource scope failed: "+err.Error())
@@ -80,13 +85,13 @@ func apiAuthorization(ctx *context.APIContext) {
ctx.PublicOnly = publicOnly
}
reducer, reducerExists := ctx.Data["ApiTokenReducer"].(authz.AuthorizationReducer)
if reducerExists {
reducer := ctx.Authentication.Reducer()
if reducer != nil {
ctx.Reducer = reducer
} else {
// No "ApiTokenReducer" will be populated if the auth method wasn't an PAT. In this case, we populate
// `ctx.Reducer` so no nil checks are needed, and we respect the scope `PublicOnly()` so that it it's safe to
// just rely on `ctx.Reducer` to account for public-only access:
// No Reducer will be populated if the auth method wasn't an PAT. In this case, we populate `ctx.Reducer` so no
// nil checks are needed, and we respect the scope `PublicOnly()` so that it it's safe to just rely on
// `ctx.Reducer` to account for public-only access:
if ctx.PublicOnly {
ctx.Reducer = &authz.PublicReposAuthorizationReducer{}
} else {
+7 -8
View File
@@ -85,7 +85,6 @@ import (
"forgejo.org/routers/api/v1/settings"
"forgejo.org/routers/api/v1/user"
"forgejo.org/services/actions"
"forgejo.org/services/auth"
"forgejo.org/services/context"
"forgejo.org/services/forms"
redirect_service "forgejo.org/services/redirect"
@@ -180,8 +179,8 @@ func repoAssignment() func(ctx *context.APIContext) {
repo.Owner = owner
ctx.Repo.Repository = repo
if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID {
taskID := ctx.Data["ActionsTaskID"].(int64)
if ctx.Doer != nil && ctx.Doer.ID == user_model.ActionsUserID && ctx.Authentication.ActionsTaskID().Has() {
_, taskID := ctx.Authentication.ActionsTaskID().Get()
task, err := actions_model.GetTaskByID(ctx, taskID)
if err != nil {
ctx.Error(http.StatusInternalServerError, "actions_model.GetTaskByID", err)
@@ -330,8 +329,8 @@ func tokenRequiresScopes(requiredScopeCategories ...auth_model.AccessTokenScopeC
}
// Need OAuth2 token to be present.
scope, scopeExists := ctx.Data["ApiTokenScope"].(auth_model.AccessTokenScope)
if ctx.Data["IsApiToken"] != true || !scopeExists {
hasScope, scope := ctx.Authentication.Scope().Get()
if !hasScope {
return
}
@@ -372,7 +371,7 @@ func tokenRequiresRepoOwnerScope(ctx *context.APIContext) {
func reqToken() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
// If actions token is present
if true == ctx.Data["IsActionsToken"] {
if ctx.Authentication.ActionsTaskID().Has() {
return
}
@@ -401,13 +400,13 @@ func reqUsersExploreEnabled() func(ctx *context.APIContext) {
func reqBasicOrRevProxyAuth() func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
if ctx.IsSigned && setting.Service.EnableReverseProxyAuthAPI && ctx.Data["AuthedMethod"].(string) == auth.ReverseProxyMethodName {
if ctx.IsSigned && setting.Service.EnableReverseProxyAuthAPI && ctx.Authentication.IsReverseProxyAuthentication() {
return
}
// Require basic authorization method to be used and that basic
// authorization used password login to verify the user.
if passwordLogin, ok := ctx.Data["IsPasswordLogin"].(bool); !ok || !passwordLogin {
if !ctx.Authentication.IsPasswordAuthentication() {
ctx.Error(http.StatusUnauthorized, "reqBasicAuth", "auth method not allowed")
return
}