refactor: remove duplicate permission checks and use middleware

- Register API routes for org/repo actions permissions
- Use reqOrgOwnership and reqAdmin middleware for auth
- Remove manual usage of IsOwnedBy/IsAdmin in handlers to avoid duplication

Signed-off-by: SBALAVIGNESH123 <balavignesh449@gmail.com>
pull/36113/head
SBALAVIGNESH123 2025-12-10 03:24:05 +07:00
parent 3aa0c6f9a9
commit 442f74cf47
3 changed files with 36 additions and 58 deletions

@ -1270,6 +1270,11 @@ func Routes() *web.Router {
})
}, reqToken(), reqAdmin())
m.Group("/actions", func() {
m.Group("/permissions", func() {
m.Get("", reqAdmin(), repo.GetActionsPermissions)
m.Put("", reqAdmin(), repo.UpdateActionsPermissions)
}, reqToken())
m.Get("/tasks", repo.ListActionTasks)
m.Group("/runs", func() {
m.Group("/{run}", func() {
@ -1618,6 +1623,18 @@ func Routes() *web.Router {
m.Post("/orgs", tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization), reqToken(), bind(api.CreateOrgOption{}), org.Create)
m.Get("/orgs", org.GetAll, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization))
m.Group("/orgs/{org}", func() {
m.Group("/settings/actions", func() {
m.Group("/permissions", func() {
m.Get("", reqOrgOwnership(), org.GetActionsPermissions)
m.Put("", reqOrgOwnership(), org.UpdateActionsPermissions)
})
m.Group("/cross-repo-access", func() {
m.Get("", reqOrgOwnership(), org.ListCrossRepoAccess)
m.Post("", reqOrgOwnership(), org.AddCrossRepoAccess)
m.Delete("/{id}", reqOrgOwnership(), org.DeleteCrossRepoAccess)
})
}, reqToken())
m.Combo("").Get(org.Get).
Patch(reqToken(), reqOrgOwnership(), bind(api.EditOrgOption{}), org.Edit).
Delete(reqToken(), reqOrgOwnership(), org.Delete)

@ -34,14 +34,10 @@ func GetActionsPermissions(ctx *context.APIContext) {
// Organization settings are more sensitive than repo settings because they
// affect ALL repositories in the org. We should be extra careful here.
// Only org owners should be able to modify these settings.
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIErrorInternal(err)
return
} else if !isOwner {
ctx.APIError(http.StatusForbidden, "You must be an organization owner")
return
}
// Organization settings are more sensitive than repo settings because they
// affect ALL repositories in the org. We should be extra careful here.
// Only org owners should be able to modify these settings.
// This is enforced by the reqOrgOwnership middleware.
perms, err := actions_model.GetOrgActionPermissions(ctx, ctx.Org.Organization.ID)
if err != nil {
@ -90,14 +86,10 @@ func UpdateActionsPermissions(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
return
} else if !isOwner {
ctx.APIError(http.StatusForbidden, "Organization owner access required")
return
}
// Organization settings are more sensitive than repo settings because they
// affect ALL repositories in the org. We should be extra careful here.
// Only org owners should be able to modify these settings.
// This is enforced by the reqOrgOwnership middleware.
form := web.GetForm(ctx).(*api.OrgActionsPermissions)
@ -160,20 +152,13 @@ func ListCrossRepoAccess(ctx *context.APIContext) {
// "200":
// "$ref": "#/responses/CrossRepoAccessList"
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIErrorInternal(err)
return
}
if !isOwner {
ctx.APIError(http.StatusForbidden, "Organization owner access required")
return
}
// This is a critical security feature - cross-repo access allows one repo's
// Actions to access another repo's code/resources. We need to be very careful
// about how we implement this. See the discussion:
// https://github.com/go-gitea/gitea/issues/24635
// Permission check handled by reqOrgOwnership middleware
rules, err := actions_model.ListCrossRepoAccessRules(ctx, ctx.Org.Organization.ID)
if err != nil {
@ -214,15 +199,7 @@ func AddCrossRepoAccess(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIErrorInternal(err)
return
}
if !isOwner {
ctx.APIError(http.StatusForbidden, "Organization owner access required")
return
}
// Permission check handled by reqOrgOwnership middleware
form := web.GetForm(ctx).(*api.CrossRepoAccessRule)
@ -274,16 +251,7 @@ func DeleteCrossRepoAccess(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"
isOwner, err := ctx.Org.Organization.IsOwnedBy(ctx, ctx.Doer.ID)
if err != nil {
ctx.APIErrorInternal(err)
return
}
if !isOwner {
ctx.APIError(http.StatusForbidden, "Organization owner access required")
return
}
// Permission check handled by reqOrgOwnership middleware
ruleID := ctx.PathParamInt64("id")
// Security check: Verify the rule belongs to this org before deleting

@ -40,10 +40,8 @@ func GetActionsPermissions(ctx *context.APIContext) {
// NOTE: Only repo admins should be able to view/modify permission settings
// This is important for security - we don't want regular contributors
// to be able to grant themselves elevated permissions via Actions
if !ctx.Repo.IsAdmin() {
ctx.APIError(http.StatusForbidden, "You must be a repository admin to access this")
return
}
// Only repo admins and owners should be able to view/modify permission settings
// This is enforced by the reqAdmin middleware.
perms, err := actions_model.GetRepoActionPermissions(ctx, ctx.Repo.Repository.ID)
if err != nil {
@ -98,11 +96,8 @@ func UpdateActionsPermissions(ctx *context.APIContext) {
// "$ref": "#/responses/forbidden"
// "422":
// "$ref": "#/responses/validationError"
if !ctx.Repo.IsAdmin() {
ctx.APIError(http.StatusForbidden, "You must be a repository admin to modify this")
return
}
// Only repo admins and owners should be able to modify these settings.
// This is enforced by the reqAdmin middleware.
form := web.GetForm(ctx).(*api.ActionsPermissions)
@ -166,10 +161,8 @@ func ResetActionsPermissions(ctx *context.APIContext) {
// "403":
// "$ref": "#/responses/forbidden"
if !ctx.Repo.IsAdmin() {
ctx.APIError(http.StatusForbidden, "You must be a repository admin")
return
}
// Only repo admins and owners should be able to modify these settings.
// This is enforced by the reqAdmin middleware.
// Create default restricted permissions
// This is a "safe reset" - puts the repo back to secure defaults