From ce230ec68e39a200b40e00cd22a128c5e2a2b623 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 02:34:23 +0000 Subject: [PATCH 01/15] fix --- routers/api/packages/container/auth.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index 33f439ec3e588..a301483edd32c 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -33,6 +33,9 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS if uid == -1 { return user_model.NewGhostUser(), nil } + if uid == -2 { + return user_model.NewActionsUser(), nil + } u, err := user_model.GetUserByID(req.Context(), uid) if err != nil { From 78b383a47d1dc7d527a21173f14439849140e849 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 02:45:08 +0000 Subject: [PATCH 02/15] improve --- routers/api/packages/container/auth.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index a301483edd32c..a491289e0e094 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -27,14 +27,16 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS return nil, err } - if uid == 0 { - return nil, nil - } - if uid == -1 { - return user_model.NewGhostUser(), nil - } - if uid == -2 { - return user_model.NewActionsUser(), nil + if uid <= 0 { + switch uid { + case -1: + return user_model.NewGhostUser(), nil + case -2: + return user_model.NewActionsUser(), nil + default: + log.Error("Invaild uid: %d", uid) + return nil, nil + } } u, err := user_model.GetUserByID(req.Context(), uid) From f86ac6f0cdc67903aa01c1c394305acd34572865 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 13:02:42 +0900 Subject: [PATCH 03/15] Update routers/api/packages/container/auth.go Co-authored-by: wxiaoguang --- routers/api/packages/container/auth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index a491289e0e094..a5ef58a8abf07 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -31,7 +31,7 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS switch uid { case -1: return user_model.NewGhostUser(), nil - case -2: + case user_model.ActionsUserID: return user_model.NewActionsUser(), nil default: log.Error("Invaild uid: %d", uid) From b12b1c9e7fe04e9e889e084fe25ec833ceab5879 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 04:12:19 +0000 Subject: [PATCH 04/15] use GetPossibleUserByID instead of GetUserByID --- routers/api/packages/container/auth.go | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index a5ef58a8abf07..abe8f68ad8c16 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -27,19 +27,7 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS return nil, err } - if uid <= 0 { - switch uid { - case -1: - return user_model.NewGhostUser(), nil - case user_model.ActionsUserID: - return user_model.NewActionsUser(), nil - default: - log.Error("Invaild uid: %d", uid) - return nil, nil - } - } - - u, err := user_model.GetUserByID(req.Context(), uid) + u, err := user_model.GetPossibleUserByID(req.Context(), uid) if err != nil { log.Error("GetUserByID: %v", err) return nil, err From ad0b2c2be1d5bd80b8a1498b75c9bdcea15828b8 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 04:17:20 +0000 Subject: [PATCH 05/15] fix --- routers/api/packages/container/auth.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index abe8f68ad8c16..5f2a4272b7dc8 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -29,7 +29,10 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS u, err := user_model.GetPossibleUserByID(req.Context(), uid) if err != nil { - log.Error("GetUserByID: %v", err) + if user_model.IsErrUserNotExist(err) { + return nil, nil + } + log.Error("GetPossibleUserByID: %v", err) return nil, err } From 953a2487ca2126f2475a9ff1049eb6940b1e87f7 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 27 Mar 2023 04:21:49 +0000 Subject: [PATCH 06/15] fix --- routers/api/packages/container/auth.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go index 5f2a4272b7dc8..6fb32c389d861 100644 --- a/routers/api/packages/container/auth.go +++ b/routers/api/packages/container/auth.go @@ -27,11 +27,12 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS return nil, err } + if uid == 0 { + return nil, nil + } + u, err := user_model.GetPossibleUserByID(req.Context(), uid) if err != nil { - if user_model.IsErrUserNotExist(err) { - return nil, nil - } log.Error("GetPossibleUserByID: %v", err) return nil, err } From b95d72f2f486cb54578aae7936e879f02adfafc5 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 28 Mar 2023 00:22:51 +0000 Subject: [PATCH 07/15] check triggeruser's permission --- models/actions/run.go | 24 +++++++++++++++------ routers/api/packages/container/container.go | 17 +++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 1af8f897fa08a..e217d592960c7 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -101,15 +101,27 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { if err := run.Repo.LoadAttributes(ctx); err != nil { return err } + if err := run.LoadTriggerUser(ctx); err != nil { + return err + } - if run.TriggerUser == nil { - u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) - if err != nil { - return err - } - run.TriggerUser = u + return nil +} + +// LoadAttributes load Repo TriggerUser if not loaded +func (run *ActionRun) LoadTriggerUser(ctx context.Context) error { + if run == nil { + return nil + } + if run.TriggerUser != nil { + return nil } + u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) + if err != nil { + return err + } + run.TriggerUser = u return nil } diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 883fe73cbdf2d..ff8f1bab4e852 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -14,6 +14,7 @@ import ( "strconv" "strings" + actions_model "code.gitea.io/gitea/models/actions" packages_model "code.gitea.io/gitea/models/packages" container_model "code.gitea.io/gitea/models/packages/container" user_model "code.gitea.io/gitea/models/user" @@ -143,6 +144,22 @@ func Authenticate(ctx *context.Context) { u := ctx.Doer if u == nil { u = user_model.NewGhostUser() + } else if u.IsActions() { + task, err := actions_model.GetTaskByID(ctx, ctx.Data["ActionsTaskID"].(int64)) + if err != nil { + apiError(ctx, http.StatusInternalServerError, err) + } + if err := task.LoadJob(ctx); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + } + if err := task.Job.LoadRun(ctx); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + } + if err := task.Job.Run.LoadTriggerUser(ctx); err != nil { + apiError(ctx, http.StatusInternalServerError, err) + } + + u = task.Job.Run.TriggerUser } token, err := packages_service.CreateAuthorizationToken(u) From 60abfe85da28aa5fab9abb1000ca49a7a43c3093 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 28 Mar 2023 00:58:06 +0000 Subject: [PATCH 08/15] improve permission check --- modules/context/package.go | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/modules/context/package.go b/modules/context/package.go index 2a55db3a77fa6..c48cb4bab9efe 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -96,29 +96,21 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { if ctx.Package.Owner.IsOrganization() { org := organization.OrgFromUser(ctx.Package.Owner) - // 1. Get user max authorize level for the org (may be none, if user is not member of the org) + // 1. If user is logined if ctx.Doer != nil { - var err error - accessMode, err = org.GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID) + // check every team permissions + teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) if err != nil { return accessMode, err } - // If access mode is less than write check every team for more permissions - if accessMode < perm.AccessModeWrite { - teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) - if err != nil { - return accessMode, err - } - for _, t := range teams { - perm := t.UnitAccessMode(ctx, unit.TypePackages) - if accessMode < perm { - accessMode = perm - } + for _, t := range teams { + perm := t.UnitAccessMode(ctx, unit.TypePackages) + if accessMode < perm { + accessMode = perm } } - } - // 2. If authorize level is none, check if org is visible to user - if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { + } else if organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { + // 2. If user is non-login, check if org is visible to non-login user accessMode = perm.AccessModeRead } } else { From 4b4026be0269dbf5cb5fc939ff53e55700d1e387 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 28 Mar 2023 01:40:48 +0000 Subject: [PATCH 09/15] fix lint --- models/actions/run.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index e217d592960c7..708b51928c429 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -101,11 +101,8 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { if err := run.Repo.LoadAttributes(ctx); err != nil { return err } - if err := run.LoadTriggerUser(ctx); err != nil { - return err - } - return nil + return run.LoadTriggerUser(ctx) } // LoadAttributes load Repo TriggerUser if not loaded From b04e219a4dc8d805240e0c40c20cf4a678b8b71b Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 28 Mar 2023 04:03:19 +0000 Subject: [PATCH 10/15] add return --- routers/api/packages/container/container.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index ff8f1bab4e852..2b31dcdbe1046 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -148,15 +148,19 @@ func Authenticate(ctx *context.Context) { task, err := actions_model.GetTaskByID(ctx, ctx.Data["ActionsTaskID"].(int64)) if err != nil { apiError(ctx, http.StatusInternalServerError, err) + return } if err := task.LoadJob(ctx); err != nil { apiError(ctx, http.StatusInternalServerError, err) + return } if err := task.Job.LoadRun(ctx); err != nil { apiError(ctx, http.StatusInternalServerError, err) + return } if err := task.Job.Run.LoadTriggerUser(ctx); err != nil { apiError(ctx, http.StatusInternalServerError, err) + return } u = task.Job.Run.TriggerUser From 1e2b185b1a29c8301b4f016fc7c94124aa69e9de Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 28 Mar 2023 08:44:54 +0000 Subject: [PATCH 11/15] remove unneccesary line --- routers/api/packages/container/container.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 2b31dcdbe1046..8d6587c61368a 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -162,7 +162,6 @@ func Authenticate(ctx *context.Context) { apiError(ctx, http.StatusInternalServerError, err) return } - u = task.Job.Run.TriggerUser } From 9520c129e41f09e8a195540367e60ae9820574ef Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 00:41:49 +0000 Subject: [PATCH 12/15] fix --- modules/context/package.go | 2 +- routers/api/packages/api.go | 53 ++++++++------------- routers/api/packages/container/container.go | 20 -------- 3 files changed, 22 insertions(+), 53 deletions(-) diff --git a/modules/context/package.go b/modules/context/package.go index c48cb4bab9efe..0a1fffe6292e2 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -97,7 +97,7 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { org := organization.OrgFromUser(ctx.Package.Owner) // 1. If user is logined - if ctx.Doer != nil { + if ctx.Doer != nil && !ctx.Doer.IsGhost() { // check every team permissions teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) if err != nil { diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index c0c7b117f696b..6e6939dc00789 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -44,35 +44,39 @@ func reqPackageAccess(accessMode perm.AccessMode) func(ctx *context.Context) { } } -// CommonRoutes provide endpoints for most package managers (except containers - see below) -// These are mounted on `/api/packages` (not `/api/v1/packages`) -func CommonRoutes(ctx gocontext.Context) *web.Route { - r := web.NewRoute() - - r.Use(context.PackageContexter(ctx)) - - authMethods := []auth.Method{ - &auth.OAuth2{}, - &auth.Basic{}, - &nuget.Auth{}, - &conan.Auth{}, - &chef.Auth{}, - } +func verifyAuth(r *web.Route, authMethods []auth.Method) { if setting.Service.EnableReverseProxyAuth { authMethods = append(authMethods, &auth.ReverseProxy{}) } - authGroup := auth.NewGroup(authMethods...) + r.Use(func(ctx *context.Context) { var err error ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) if err != nil { - log.Error("Verify: %v", err) + log.Error("Failed to verify user: %v", err) ctx.Error(http.StatusUnauthorized, "authGroup.Verify") return } + // TODO: check ActionUser's access permission ctx.IsSigned = ctx.Doer != nil }) +} + +// CommonRoutes provide endpoints for most package managers (except containers - see below) +// These are mounted on `/api/packages` (not `/api/v1/packages`) +func CommonRoutes(ctx gocontext.Context) *web.Route { + r := web.NewRoute() + + r.Use(context.PackageContexter(ctx)) + + verifyAuth(r, []auth.Method{ + &auth.OAuth2{}, + &auth.Basic{}, + &nuget.Auth{}, + &conan.Auth{}, + &chef.Auth{}, + }) r.Group("/{username}", func() { r.Group("/cargo", func() { @@ -437,24 +441,9 @@ func ContainerRoutes(ctx gocontext.Context) *web.Route { r.Use(context.PackageContexter(ctx)) - authMethods := []auth.Method{ + verifyAuth(r, []auth.Method{ &auth.Basic{}, &container.Auth{}, - } - if setting.Service.EnableReverseProxyAuth { - authMethods = append(authMethods, &auth.ReverseProxy{}) - } - - authGroup := auth.NewGroup(authMethods...) - r.Use(func(ctx *context.Context) { - var err error - ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session) - if err != nil { - log.Error("Failed to verify user: %v", err) - ctx.Error(http.StatusUnauthorized, "Verify") - return - } - ctx.IsSigned = ctx.Doer != nil }) r.Get("", container.ReqContainerAccess, container.DetermineSupport) diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go index 8d6587c61368a..883fe73cbdf2d 100644 --- a/routers/api/packages/container/container.go +++ b/routers/api/packages/container/container.go @@ -14,7 +14,6 @@ import ( "strconv" "strings" - actions_model "code.gitea.io/gitea/models/actions" packages_model "code.gitea.io/gitea/models/packages" container_model "code.gitea.io/gitea/models/packages/container" user_model "code.gitea.io/gitea/models/user" @@ -144,25 +143,6 @@ func Authenticate(ctx *context.Context) { u := ctx.Doer if u == nil { u = user_model.NewGhostUser() - } else if u.IsActions() { - task, err := actions_model.GetTaskByID(ctx, ctx.Data["ActionsTaskID"].(int64)) - if err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - if err := task.LoadJob(ctx); err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - if err := task.Job.LoadRun(ctx); err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - if err := task.Job.Run.LoadTriggerUser(ctx); err != nil { - apiError(ctx, http.StatusInternalServerError, err) - return - } - u = task.Job.Run.TriggerUser } token, err := packages_service.CreateAuthorizationToken(u) From 1144abec863c1ab5abba95cfa8d54a9529749b63 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 00:59:37 +0000 Subject: [PATCH 13/15] revert improve permission check --- modules/context/package.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/modules/context/package.go b/modules/context/package.go index 0a1fffe6292e2..2a55db3a77fa6 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -96,21 +96,29 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { if ctx.Package.Owner.IsOrganization() { org := organization.OrgFromUser(ctx.Package.Owner) - // 1. If user is logined - if ctx.Doer != nil && !ctx.Doer.IsGhost() { - // check every team permissions - teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) + // 1. Get user max authorize level for the org (may be none, if user is not member of the org) + if ctx.Doer != nil { + var err error + accessMode, err = org.GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID) if err != nil { return accessMode, err } - for _, t := range teams { - perm := t.UnitAccessMode(ctx, unit.TypePackages) - if accessMode < perm { - accessMode = perm + // If access mode is less than write check every team for more permissions + if accessMode < perm.AccessModeWrite { + teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) + if err != nil { + return accessMode, err + } + for _, t := range teams { + perm := t.UnitAccessMode(ctx, unit.TypePackages) + if accessMode < perm { + accessMode = perm + } } } - } else if organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { - // 2. If user is non-login, check if org is visible to non-login user + } + // 2. If authorize level is none, check if org is visible to user + if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { accessMode = perm.AccessModeRead } } else { From 623626610be4638fa8a82f168374d46cc57d2bb5 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 01:01:46 +0000 Subject: [PATCH 14/15] revert add LoadTriggerUser --- models/actions/run.go | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index 708b51928c429..1af8f897fa08a 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -102,23 +102,14 @@ func (run *ActionRun) LoadAttributes(ctx context.Context) error { return err } - return run.LoadTriggerUser(ctx) -} - -// LoadAttributes load Repo TriggerUser if not loaded -func (run *ActionRun) LoadTriggerUser(ctx context.Context) error { - if run == nil { - return nil - } - if run.TriggerUser != nil { - return nil + if run.TriggerUser == nil { + u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) + if err != nil { + return err + } + run.TriggerUser = u } - u, err := user_model.GetPossibleUserByID(ctx, run.TriggerUserID) - if err != nil { - return err - } - run.TriggerUser = u return nil } From 59c79977b52699122036a360ec4d6c7d1a080bb9 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 01:03:04 +0000 Subject: [PATCH 15/15] remove todo --- routers/api/packages/api.go | 1 - 1 file changed, 1 deletion(-) diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go index 6e6939dc00789..4cebabecf02ba 100644 --- a/routers/api/packages/api.go +++ b/routers/api/packages/api.go @@ -58,7 +58,6 @@ func verifyAuth(r *web.Route, authMethods []auth.Method) { ctx.Error(http.StatusUnauthorized, "authGroup.Verify") return } - // TODO: check ActionUser's access permission ctx.IsSigned = ctx.Doer != nil }) }