Improve and fix bugs in runner management page

models/actions/runner.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/timeutil"
@@ -52,6 +53,30 @@ type ActionRunner struct {
 	Deleted timeutil.TimeStamp `xorm:"deleted"`
 }
 
+// RunnerType defines the runner type
+type RunnerType int //revive:disable-line:exported
+
+const (
+	// RunnerTypeGlobal defines a global runner
+	RunnerTypeGlobal RunnerType = iota
+
+	// RunnerTypeOrganization defines an organization runner
+	RunnerTypeOrganization
+
+	// RunnerTypeRepository defines a repository runner
+	RunnerTypeRepository
+)
+
+func (r *ActionRunner) Type() RunnerType {
+	if r.RepoID != 0 {
+		return RunnerTypeRepository
+	}
+	if r.OwnerID != 0 {
+		return RunnerTypeOrganization
+	}
+	return RunnerTypeGlobal
+}
+
 func (r *ActionRunner) OwnType() string {
 	if r.RepoID != 0 {
 		return fmt.Sprintf("Repo(%s)", r.Repo.FullName())
@@ -94,14 +119,40 @@ func (r *ActionRunner) AllLabels() []string {
 }
 
 // Editable checks if the runner is editable by the user
-func (r *ActionRunner) Editable(ownerID, repoID int64) bool {
-	if ownerID == 0 && repoID == 0 {
-		return true
+func (r *ActionRunner) Editable(doer, owner *user_model.User, repo *repo_model.Repository) (bool, error) {
+	if doer == nil {
+		return false, nil
 	}
-	if ownerID > 0 && r.OwnerID == ownerID {
-		return true
+
+	// global runner in admin settings
+	if doer.IsAdmin {
+		return true, nil
+	}
+
+	if repo != nil {
+		// repo runner in repo runners list
+		if r.Type() == RunnerTypeRepository && r.RepoID == repo.ID {
+			return true, nil
+		}
+
+		if err := repo.LoadOwner(db.DefaultContext); err != nil {
+			return false, err
+		}
+		// org runner in repo runners list
+		if r.Type() == RunnerTypeOrganization && repo.Owner.IsOrganization() {
+			isOrgOwner, err := (*organization.Organization)(repo.Owner).IsOwnedBy(doer.ID)
+			if err != nil {
+				return false, err
+			}
+			return repo.OwnerID == r.OwnerID && isOrgOwner, nil
+		}
+	}
+
+	if owner == nil {
+		return false, nil
 	}
-	return repoID > 0 && r.RepoID == repoID
+	// org runner in org runners list
+	return r.Type() == RunnerTypeOrganization && r.OwnerID == owner.ID && owner.IsOrganization(), nil
 }
 
 // LoadAttributes loads the attributes of the runner

options/locale/locale_en-US.ini

@@ -3382,6 +3382,7 @@ runners.owner_type = Type
 runners.description = Description
 runners.labels = Labels
 runners.last_online = Last Online Time
+runners.no_permission_to_edit = You have no permission to edit this runner.
 runners.agent_labels = Agent Labels
 runners.custom_labels = Custom Labels
 runners.custom_labels_helper = Custom labels are labels that are added manually by an administrator. A comma separates labels, whitespace at the start and end of each label is ignored.

routers/web/admin/runners.go

@@ -39,7 +39,7 @@ func Runners(ctx *context.Context) {
 		Filter: ctx.Req.URL.Query().Get("q"),
 	}
 
-	actions_shared.RunnersList(ctx, tplRunners, opts)
+	actions_shared.RunnersList(ctx, tplRunners, opts, nil, nil)
 }
 
 // EditRunner show editing runner page
@@ -53,16 +53,16 @@ func EditRunner(ctx *context.Context) {
 		page = 1
 	}
 
-	actions_shared.RunnerDetails(ctx, tplRunnerEdit, page, ctx.ParamsInt64(":runnerid"), 0, 0)
+	actions_shared.RunnerDetails(ctx, tplRunnerEdit, page, ctx.ParamsInt64(":runnerid"), nil, nil)
 }
 
 // EditRunnerPost response for editing runner
 func EditRunnerPost(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("actions.runners.edit")
 	ctx.Data["PageIsAdmin"] = true
 	ctx.Data["PageIsAdminRunners"] = true
-	actions_shared.RunnerDetailsEditPost(ctx, ctx.ParamsInt64(":runnerid"), 0, 0,
-		setting.AppSubURL+"/admin/runners/"+url.PathEscape(ctx.Params(":runnerid")))
+	actions_shared.RunnerDetailsEditPost(ctx, ctx.ParamsInt64(":runnerid"),
+		setting.AppSubURL+"/admin/runners/"+url.PathEscape(ctx.Params(":runnerid")), nil, nil)
 }
 
 // DeleteRunnerPost response for deleting a runner

routers/web/org/org_runners.go

@@ -34,7 +34,7 @@ func Runners(ctx *context.Context) {
 		WithAvailable: true,
 	}
 
-	actions_shared.RunnersList(ctx, tplSettingsRunners, opts)
+	actions_shared.RunnersList(ctx, tplSettingsRunners, opts, ctx.Org.Organization.AsUser(), nil)
 }
 
 // ResetRunnerRegistrationToken reset runner registration token
@@ -54,9 +54,7 @@ func RunnersEdit(ctx *context.Context) {
 		page = 1
 	}
 
-	actions_shared.RunnerDetails(ctx, tplSettingsRunnersEdit, page,
-		ctx.ParamsInt64(":runnerid"), ctx.Org.Organization.ID, 0,
-	)
+	actions_shared.RunnerDetails(ctx, tplSettingsRunnersEdit, page, ctx.ParamsInt64(":runnerid"), ctx.Org.Organization.AsUser(), nil)
 }
 
 // RunnersEditPost response for editing runner
@@ -65,8 +63,7 @@ func RunnersEditPost(ctx *context.Context) {
 	ctx.Data["PageIsOrgSettings"] = true
 	ctx.Data["PageIsOrgSettingsRunners"] = true
 	actions_shared.RunnerDetailsEditPost(ctx, ctx.ParamsInt64(":runnerid"),
-		ctx.Org.Organization.ID, 0,
-		ctx.Org.OrgLink+"/settings/runners/"+url.PathEscape(ctx.Params(":runnerid")))
+		ctx.Org.OrgLink+"/settings/runners/"+url.PathEscape(ctx.Params(":runnerid")), ctx.Org.Organization.AsUser(), nil)
 }
 
 // RunnerDeletePost response for deleting runner

routers/web/repo/runners.go

@@ -38,7 +38,7 @@ func Runners(ctx *context.Context) {
 		WithAvailable: true,
 	}
 
-	actions_shared.RunnersList(ctx, tplRunners, opts)
+	actions_shared.RunnersList(ctx, tplRunners, opts, nil, ctx.Repo.Repository)
 }
 
 func RunnersEdit(ctx *context.Context) {
@@ -49,17 +49,14 @@ func RunnersEdit(ctx *context.Context) {
 		page = 1
 	}
 
-	actions_shared.RunnerDetails(ctx, tplRunnerEdit, page,
-		ctx.ParamsInt64(":runnerid"), 0, ctx.Repo.Repository.ID,
-	)
+	actions_shared.RunnerDetails(ctx, tplRunnerEdit, page, ctx.ParamsInt64(":runnerid"), nil, ctx.Repo.Repository)
 }
 
 func RunnersEditPost(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("actions.runners")
 	ctx.Data["PageIsSettingsRunners"] = true
 	actions_shared.RunnerDetailsEditPost(ctx, ctx.ParamsInt64(":runnerid"),
-		0, ctx.Repo.Repository.ID,
-		ctx.Repo.RepoLink+"/settings/runners/"+url.PathEscape(ctx.Params(":runnerid")))
+		ctx.Repo.RepoLink+"/settings/runners/"+url.PathEscape(ctx.Params(":runnerid")), nil, ctx.Repo.Repository)
 }
 
 func ResetRunnerRegistrationToken(ctx *context.Context) {

routers/web/shared/actions/runners.go

@@ -10,6 +10,8 @@ import (
 
 	actions_model "code.gitea.io/gitea/models/actions"
 	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
@@ -19,7 +21,7 @@ import (
 )
 
 // RunnersList render common runners list page
-func RunnersList(ctx *context.Context, tplName base.TplName, opts actions_model.FindRunnerOptions) {
+func RunnersList(ctx *context.Context, tplName base.TplName, opts actions_model.FindRunnerOptions, owner *user.User, repo *repo.Repository) {
 	count, err := actions_model.CountRunners(ctx, opts)
 	if err != nil {
 		ctx.ServerError("CountRunners", err)
@@ -54,8 +56,8 @@ func RunnersList(ctx *context.Context, tplName base.TplName, opts actions_model.
 	ctx.Data["Runners"] = runners
 	ctx.Data["Total"] = count
 	ctx.Data["RegistrationToken"] = token.Token
-	ctx.Data["RunnerOnwerID"] = opts.OwnerID
-	ctx.Data["RunnerRepoID"] = opts.RepoID
+	ctx.Data["RunnerOwner"] = owner
+	ctx.Data["RunnerRepo"] = repo
 
 	pager := context.NewPagination(int(count), opts.PageSize, opts.Page, 5)
 	ctx.Data["Page"] = pager
@@ -64,7 +66,7 @@ func RunnersList(ctx *context.Context, tplName base.TplName, opts actions_model.
 }
 
 // RunnerDetails render runner details page
-func RunnerDetails(ctx *context.Context, tplName base.TplName, page int, runnerID, ownerID, repoID int64) {
+func RunnerDetails(ctx *context.Context, tplName base.TplName, page int, runnerID int64, owner *user.User, repo *repo.Repository) {
 	runner, err := actions_model.GetRunnerByID(ctx, runnerID)
 	if err != nil {
 		ctx.ServerError("GetRunnerByID", err)
@@ -74,7 +76,10 @@ func RunnerDetails(ctx *context.Context, tplName base.TplName, page int, runnerI
 		ctx.ServerError("LoadAttributes", err)
 		return
 	}
-	if !runner.Editable(ownerID, repoID) {
+	if editable, err := runner.Editable(ctx.Doer, owner, repo); err != nil {
+		ctx.ServerError("Editable", err)
+		return
+	} else if !editable {
 		err = errors.New("no permission to edit this runner")
 		ctx.NotFound("RunnerDetails", err)
 		return
@@ -116,14 +121,17 @@ func RunnerDetails(ctx *context.Context, tplName base.TplName, page int, runnerI
 }
 
 // RunnerDetailsEditPost response for edit runner details
-func RunnerDetailsEditPost(ctx *context.Context, runnerID, ownerID, repoID int64, redirectTo string) {
+func RunnerDetailsEditPost(ctx *context.Context, runnerID int64, redirectTo string, owner *user.User, repo *repo.Repository) {
 	runner, err := actions_model.GetRunnerByID(ctx, runnerID)
 	if err != nil {
 		log.Warn("RunnerDetailsEditPost.GetRunnerByID failed: %v, url: %s", err, ctx.Req.URL)
 		ctx.ServerError("RunnerDetailsEditPost.GetRunnerByID", err)
 		return
 	}
-	if !runner.Editable(ownerID, repoID) {
+	if editable, err := runner.Editable(ctx.Doer, owner, repo); err != nil {
+		ctx.ServerError("Editable", err)
+		return
+	} else if !editable {
 		ctx.NotFound("RunnerDetailsEditPost.Editable", util.NewPermissionDeniedErrorf("no permission to edit this runner"))
 		return
 	}

templates/shared/actions/runner_list.tmpl

@@ -71,8 +71,10 @@
 						</td>
 						<td>{{if .LastOnline}}{{TimeSinceUnix .LastOnline $.locale}}{{else}}{{$.locale.Tr "never"}}{{end}}</td>
 						<td class="runner-ops">
-							{{if .Editable $.RunnerOnwerID $.RunnerRepoID}}
+							{{if .Editable $.Context.Doer $.RunnerOwner $.RunnerRepo}}
 							<a href="{{$.Link}}/{{.ID}}">{{svg "octicon-pencil"}}</a>
+							{{else}}
+							<div data-tooltip-content="{{$.locale.Tr "actions.runners.no_permission_to_edit"}}">{{svg "octicon-x"}}</div>
 							{{end}}
 						</td>
 					</tr>