refactor: deploy key and private route handlers (#38999)

clean up legacy code, fix various bugs:

* add missing "return"

models/asymkey/ssh_key.go

@@ -38,7 +38,7 @@ const (
 // PublicKey represents a user or deploy SSH public key.
 type PublicKey struct {
 	ID            int64           `xorm:"pk autoincr"`
-	OwnerID       int64           `xorm:"INDEX NOT NULL"`
+	OwnerID       int64           `xorm:"INDEX NOT NULL"` // deploy-key doesn't have owner
 	Name          string          `xorm:"NOT NULL"`
 	Fingerprint   string          `xorm:"INDEX NOT NULL"`
 	Content       string          `xorm:"MEDIUMTEXT NOT NULL"`
@@ -73,7 +73,7 @@ func (key *PublicKey) OmitEmail() string {
 	return strings.Join(fields[:2], " ")
 }
 
-func addKey(ctx context.Context, key *PublicKey) (err error) {
+func addPublicKey(ctx context.Context, key *PublicKey) (err error) {
 	if len(key.Fingerprint) == 0 {
 		key.Fingerprint, err = CalcFingerprint(key.Content)
 		if err != nil {
@@ -123,7 +123,7 @@ func AddPublicKey(ctx context.Context, ownerID int64, name, content string, auth
 			LoginSourceID: authSourceID,
 			Verified:      verified,
 		}
-		if err = addKey(ctx, key); err != nil {
+		if err = addPublicKey(ctx, key); err != nil {
 			return nil, fmt.Errorf("addKey: %w", err)
 		}
 

models/asymkey/ssh_key_deploy.go

@@ -11,19 +11,11 @@ import (
 	"gitea.dev/models/db"
 	"gitea.dev/models/perm"
 	"gitea.dev/modules/timeutil"
+	"gitea.dev/modules/util"
 
 	"xorm.io/builder"
 )
 
-// ________                .__                 ____  __.
-// \______ \   ____ ______ |  |   ____ ___.__.|    |/ _|____ ___.__.
-//  |    |  \_/ __ \\____ \|  |  /  _ <   |  ||      <_/ __ <   |  |
-//  |    `   \  ___/|  |_> >  |_(  <_> )___  ||    |  \  ___/\___  |
-// /_______  /\___  >   __/|____/\____// ____||____|__ \___  > ____|
-//         \/     \/|__|               \/             \/   \/\/
-//
-// This file contains functions specific to DeployKeys
-
 // DeployKey represents deploy key information and its relation with repository.
 type DeployKey struct {
 	ID          int64 `xorm:"pk autoincr"`
@@ -31,30 +23,29 @@ type DeployKey struct {
 	RepoID      int64 `xorm:"UNIQUE(s) INDEX"`
 	Name        string
 	Fingerprint string
-	Content     string `xorm:"-"`
 
 	Mode perm.AccessMode `xorm:"NOT NULL DEFAULT 1"`
 
-	CreatedUnix       timeutil.TimeStamp `xorm:"created"`
-	UpdatedUnix       timeutil.TimeStamp `xorm:"updated"`
-	HasRecentActivity bool               `xorm:"-"`
-	HasUsed           bool               `xorm:"-"`
+	CreatedUnix timeutil.TimeStamp `xorm:"created"`
+	UpdatedUnix timeutil.TimeStamp `xorm:"updated"`
+
+	PublicKey *PublicKey `xorm:"-"`
 }
 
-// AfterLoad is invoked from XORM after setting the values of all fields of this object.
-func (key *DeployKey) AfterLoad() {
-	key.HasUsed = key.UpdatedUnix > key.CreatedUnix
-	key.HasRecentActivity = key.UpdatedUnix.AddDuration(7*24*time.Hour) > timeutil.TimeStampNow()
+func (key *DeployKey) HasUsed() bool {
+	return key.UpdatedUnix > key.CreatedUnix
 }
 
-// GetContent gets associated public key content.
-func (key *DeployKey) GetContent(ctx context.Context) error {
-	pkey, err := GetPublicKeyByID(ctx, key.KeyID)
-	if err != nil {
-		return err
+func (key *DeployKey) HasRecentActivity() bool {
+	return key.UpdatedUnix.AddDuration(7*24*time.Hour) > timeutil.TimeStampNow()
+}
+
+func (key *DeployKey) LoadPublicKey(ctx context.Context) (err error) {
+	if key.PublicKey != nil {
+		return nil
 	}
-	key.Content = pkey.Content
-	return nil
+	key.PublicKey, err = GetPublicKeyByID(ctx, key.KeyID)
+	return err
 }
 
 // IsReadOnly checks if the key can only be used for read operations, used by template
@@ -66,57 +57,39 @@ func init() {
 	db.RegisterModel(new(DeployKey))
 }
 
-func checkDeployKey(ctx context.Context, keyID, repoID int64, name string) error {
+func checkDeployKey(ctx context.Context, repoID, publicKeyID int64, name string) error {
 	// Note: We want error detail, not just true or false here.
 	has, err := db.GetEngine(ctx).
-		Where("key_id = ? AND repo_id = ?", keyID, repoID).
+		Where("repo_id=? AND (key_id=? OR name=?)", repoID, publicKeyID, name).
 		Get(new(DeployKey))
 	if err != nil {
 		return err
 	} else if has {
-		return ErrDeployKeyAlreadyExist{keyID, repoID}
+		return ErrDeployKeyAlreadyExist{publicKeyID, repoID}
 	}
-
-	has, err = db.GetEngine(ctx).
-		Where("repo_id = ? AND name = ?", repoID, name).
-		Get(new(DeployKey))
-	if err != nil {
-		return err
-	} else if has {
-		return ErrDeployKeyNameAlreadyUsed{repoID, name}
-	}
-
 	return nil
 }
 
 // addDeployKey adds new key-repo relation.
-func addDeployKey(ctx context.Context, keyID, repoID int64, name, fingerprint string, mode perm.AccessMode) (*DeployKey, error) {
-	if err := checkDeployKey(ctx, keyID, repoID, name); err != nil {
+func addDeployKey(ctx context.Context, repoID, publicKeyID int64, name, fingerprint string, mode perm.AccessMode) (*DeployKey, error) {
+	if err := checkDeployKey(ctx, repoID, publicKeyID, name); err != nil {
 		return nil, err
 	}
 
-	key := &DeployKey{
-		KeyID:       keyID,
-		RepoID:      repoID,
-		Name:        name,
-		Fingerprint: fingerprint,
-		Mode:        mode,
-	}
+	key := &DeployKey{KeyID: publicKeyID, RepoID: repoID, Name: name, Fingerprint: fingerprint, Mode: mode}
 	return key, db.Insert(ctx, key)
 }
 
 // AddDeployKey add new deploy key to database and authorized_keys file.
-func AddDeployKey(ctx context.Context, repoID int64, name, content string, readOnly bool) (*DeployKey, error) {
+func AddDeployKey(ctx context.Context, repoID int64, name, content string, accessMode perm.AccessMode) (*DeployKey, error) {
 	fingerprint, err := CalcFingerprint(content)
 	if err != nil {
 		return nil, err
 	}
 
-	accessMode := perm.AccessModeRead
-	if !readOnly {
-		accessMode = perm.AccessModeWrite
+	if accessMode != perm.AccessModeRead && accessMode != perm.AccessModeWrite {
+		return nil, util.NewInvalidArgumentErrorf("invalid access mode")
 	}
-
 	return db.WithTx2(ctx, func(ctx context.Context) (*DeployKey, error) {
 		pkey, exist, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint})
 		if err != nil {
@@ -126,52 +99,46 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO
 				return nil, ErrKeyAlreadyExist{0, fingerprint, ""}
 			}
 		} else {
-			// First time use this deploy key.
+			// First time use this deploy key, add a shared public key
 			pkey = &PublicKey{
-				Fingerprint: fingerprint,
-				Mode:        accessMode,
+				Mode:        perm.AccessModeNone,
 				Type:        KeyTypeDeploy,
+				Name:        "(DeployKey)",
 				Content:     content,
-				Name:        name,
+				Fingerprint: fingerprint,
 			}
-			if err = addKey(ctx, pkey); err != nil {
-				return nil, fmt.Errorf("addKey: %w", err)
+			if err = addPublicKey(ctx, pkey); err != nil {
+				return nil, fmt.Errorf("addPublicKey: %w", err)
 			}
 		}
-
-		key, err := addDeployKey(ctx, pkey.ID, repoID, name, pkey.Fingerprint, accessMode)
-		if err != nil {
-			return nil, err
-		}
-
-		return key, nil
+		return addDeployKey(ctx, repoID, pkey.ID, name, fingerprint, accessMode)
 	})
 }
 
 // GetDeployKeyByID returns deploy key by given ID.
-func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) {
-	key, exist, err := db.GetByID[DeployKey](ctx, id)
+func GetDeployKeyByID(ctx context.Context, repoID, deployKeyID int64) (*DeployKey, error) {
+	key, exist, err := db.Get[DeployKey](ctx, builder.Eq{"id": deployKeyID, "repo_id": repoID})
 	if err != nil {
 		return nil, err
 	} else if !exist {
-		return nil, ErrDeployKeyNotExist{id, 0, 0}
+		return nil, ErrDeployKeyNotExist{deployKeyID, 0, repoID}
 	}
 	return key, nil
 }
 
-// GetDeployKeyByRepo returns deploy key by given public key ID and repository ID.
-func GetDeployKeyByRepo(ctx context.Context, keyID, repoID int64) (*DeployKey, error) {
-	key, exist, err := db.Get[DeployKey](ctx, builder.Eq{"key_id": keyID, "repo_id": repoID})
+// GetDeployKeyByRepoPublicKey returns deploy key by given public key ID and repository ID.
+func GetDeployKeyByRepoPublicKey(ctx context.Context, repoID, publicKeyID int64) (*DeployKey, error) {
+	key, exist, err := db.Get[DeployKey](ctx, builder.Eq{"key_id": publicKeyID, "repo_id": repoID})
 	if err != nil {
 		return nil, err
 	} else if !exist {
-		return nil, ErrDeployKeyNotExist{0, keyID, repoID}
+		return nil, ErrDeployKeyNotExist{0, publicKeyID, repoID}
 	}
 	return key, nil
 }
 
-// IsDeployKeyExistByKeyID return true if there is at least one deploykey with the key id
-func IsDeployKeyExistByKeyID(ctx context.Context, keyID int64) (bool, error) {
+// IsDeployKeyExistByPublicKeyID return true if there is at least one deploy-key with the key id
+func IsDeployKeyExistByPublicKeyID(ctx context.Context, keyID int64) (bool, error) {
 	return db.GetEngine(ctx).
 		Where("key_id = ?", keyID).
 		Get(new(DeployKey))
@@ -193,9 +160,7 @@ type ListDeployKeysOptions struct {
 
 func (opt ListDeployKeysOptions) ToConds() builder.Cond {
 	cond := builder.NewCond()
-	if opt.RepoID != 0 {
-		cond = cond.And(builder.Eq{"repo_id": opt.RepoID})
-	}
+	cond = cond.And(builder.Eq{"repo_id": opt.RepoID}) // repo ID must be used
 	if opt.KeyID != 0 {
 		cond = cond.And(builder.Eq{"key_id": opt.KeyID})
 	}

routers/api/v1/api.go

@@ -1451,7 +1451,7 @@ func Routes() *web.Router {
 					m.Combo("").Get(repo.ListDeployKeys).
 						Post(bind(api.CreateKeyOption{}), repo.CreateDeployKey)
 					m.Combo("/{id}").Get(repo.GetDeployKey).
-						Delete(repo.DeleteDeploykey)
+						Delete(repo.DeleteDeployKey)
 				}, reqToken(), reqAdmin())
 				m.Group("/times", func() {
 					m.Combo("").Get(repo.ListTrackedTimesByRepository)

routers/api/v1/repo/key.go

@@ -8,15 +8,14 @@ import (
 	stdCtx "context"
 	"fmt"
 	"net/http"
-	"net/url"
 
 	asymkey_model "gitea.dev/models/asymkey"
 	"gitea.dev/models/db"
 	"gitea.dev/models/perm"
 	access_model "gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
-	"gitea.dev/modules/setting"
 	api "gitea.dev/modules/structs"
+	"gitea.dev/modules/util"
 	"gitea.dev/modules/web"
 	"gitea.dev/routers/api/v1/utils"
 	asymkey_service "gitea.dev/services/asymkey"
@@ -39,10 +38,6 @@ func appendPrivateInformation(ctx stdCtx.Context, apiKey *api.DeployKey, key *as
 	return apiKey, nil
 }
 
-func composeDeployKeysAPILink(owner, name string) string {
-	return setting.AppURL + "api/v1/repos/" + url.PathEscape(owner) + "/" + url.PathEscape(name) + "/keys/"
-}
-
 // ListDeployKeys list all the deploy keys of a repository
 func ListDeployKeys(ctx *context.APIContext) {
 	// swagger:operation GET /repos/{owner}/{repo}/keys repository repoListKeys
@@ -96,21 +91,16 @@ func ListDeployKeys(ctx *context.APIContext) {
 		return
 	}
 
-	apiLink := composeDeployKeysAPILink(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-	apiKeys := make([]*api.DeployKey, len(keys))
+	apiDeployKeys := make([]*api.DeployKey, len(keys))
 	for i := range keys {
-		if err := keys[i].GetContent(ctx); err != nil {
-			ctx.APIErrorInternal(err)
-			return
-		}
-		apiKeys[i] = convert.ToDeployKey(apiLink, keys[i])
+		apiDeployKeys[i] = convert.ToDeployKey(ctx, ctx.Repo.Repository, keys[i])
 		if ctx.Doer.IsAdmin || ((ctx.Repo.Repository.ID == keys[i].RepoID) && (ctx.Doer.ID == ctx.Repo.Owner.ID)) {
-			apiKeys[i], _ = appendPrivateInformation(ctx, apiKeys[i], keys[i], ctx.Repo.Repository)
+			apiDeployKeys[i], _ = appendPrivateInformation(ctx, apiDeployKeys[i], keys[i], ctx.Repo.Repository)
 		}
 	}
 
 	ctx.SetTotalCountHeader(count)
-	ctx.JSON(http.StatusOK, &apiKeys)
+	ctx.JSON(http.StatusOK, &apiDeployKeys)
 }
 
 // GetDeployKey get a deploy key by id
@@ -143,33 +133,17 @@ func GetDeployKey(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	key, err := asymkey_model.GetDeployKeyByID(ctx, ctx.PathParamInt64("id"))
+	key, err := asymkey_model.GetDeployKeyByID(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("id"))
 	if err != nil {
-		if asymkey_model.IsErrDeployKeyNotExist(err) {
-			ctx.APIErrorNotFound()
-		} else {
-			ctx.APIErrorInternal(err)
-		}
+		ctx.APIErrorAuto(err)
 		return
 	}
 
-	// this check make it more consistent
-	if key.RepoID != ctx.Repo.Repository.ID {
-		ctx.APIErrorNotFound()
-		return
-	}
-
-	if err = key.GetContent(ctx); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
-
-	apiLink := composeDeployKeysAPILink(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-	apiKey := convert.ToDeployKey(apiLink, key)
+	apiDeployKey := convert.ToDeployKey(ctx, ctx.Repo.Repository, key)
 	if ctx.Doer.IsAdmin || ((ctx.Repo.Repository.ID == key.RepoID) && (ctx.Doer.ID == ctx.Repo.Owner.ID)) {
-		apiKey, _ = appendPrivateInformation(ctx, apiKey, key, ctx.Repo.Repository)
+		apiDeployKey, _ = appendPrivateInformation(ctx, apiDeployKey, key, ctx.Repo.Repository)
 	}
-	ctx.JSON(http.StatusOK, apiKey)
+	ctx.JSON(http.StatusOK, apiDeployKey)
 }
 
 // HandleCheckKeyStringError handle check key error
@@ -238,19 +212,17 @@ func CreateDeployKey(ctx *context.APIContext) {
 		return
 	}
 
-	key, err := asymkey_model.AddDeployKey(ctx, ctx.Repo.Repository.ID, form.Title, content, form.ReadOnly)
+	accessMode := util.Iif(form.ReadOnly, perm.AccessModeRead, perm.AccessModeWrite)
+	key, err := asymkey_model.AddDeployKey(ctx, ctx.Repo.Repository.ID, form.Title, content, accessMode)
 	if err != nil {
 		HandleAddKeyError(ctx, err)
 		return
 	}
-
-	key.Content = content
-	apiLink := composeDeployKeysAPILink(ctx.Repo.Owner.Name, ctx.Repo.Repository.Name)
-	ctx.JSON(http.StatusCreated, convert.ToDeployKey(apiLink, key))
+	ctx.JSON(http.StatusCreated, convert.ToDeployKey(ctx, ctx.Repo.Repository, key))
 }
 
-// DeleteDeploykey delete deploy key for a repository
-func DeleteDeploykey(ctx *context.APIContext) {
+// DeleteDeployKey delete deploy key for a repository
+func DeleteDeployKey(ctx *context.APIContext) {
 	// swagger:operation DELETE /repos/{owner}/{repo}/keys/{id} repository repoDeleteKey
 	// ---
 	// summary: Delete a key from a repository

routers/private/actions.go

@@ -5,7 +5,6 @@ package private
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
 	"strings"
 
@@ -13,7 +12,6 @@ import (
 	repo_model "gitea.dev/models/repo"
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/json"
-	"gitea.dev/modules/log"
 	"gitea.dev/modules/private"
 	"gitea.dev/modules/util"
 	"gitea.dev/services/context"
@@ -26,38 +24,25 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
 	defer rd.Close()
 
 	if err := json.NewDecoder(rd).Decode(&genRequest); err != nil {
-		log.Error("JSON Decode failed: %v", err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("JSON Decode failed: %v", err)
 		return
 	}
 
 	owner, repo, err := parseScope(ctx, genRequest.Scope)
 	if err != nil {
-		log.Error("parseScope failed: %v", err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("parseScope failed: %v", err)
+		return
 	}
 
 	token, err := actions_model.GetLatestRunnerToken(ctx, owner, repo)
 	if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
 		token, err = actions_model.NewRunnerToken(ctx, owner, repo)
 		if err != nil {
-			errMsg := fmt.Sprintf("error while creating runner token: %v", err)
-			log.Error("NewRunnerToken failed: %v", errMsg)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: errMsg,
-			})
+			ctx.PrivateInternalErrorf("error while creating runner token: %v", err)
 			return
 		}
 	} else if err != nil {
-		errMsg := fmt.Sprintf("could not get unactivated runner token: %v", err)
-		log.Error("GetLatestRunnerToken failed: %v", errMsg)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: errMsg,
-		})
+		ctx.PrivateInternalErrorf("could not get unactivated runner token: %v", err)
 		return
 	}
 

routers/private/hook_pre_receive.go

@@ -5,7 +5,6 @@ package private
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
 	"os"
 
@@ -18,7 +17,6 @@ import (
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/git"
 	"gitea.dev/modules/git/gitcmd"
-	"gitea.dev/modules/log"
 	"gitea.dev/modules/private"
 	"gitea.dev/modules/util"
 	"gitea.dev/modules/web"
@@ -74,9 +72,7 @@ func (ctx *preReceiveContext) assertCanWriteRef(refFullName git.RefName) bool {
 		if ctx.Written() {
 			return false
 		}
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: "User permission denied for writing.",
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "User permission denied for writing.")
 		return false
 	}
 	return true
@@ -97,9 +93,7 @@ func (ctx *preReceiveContext) AssertCreatePullRequest() bool {
 		if ctx.Written() {
 			return false
 		}
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: "User permission denied for creating pull-request.",
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "User permission denied for creating pull-request.")
 		return false
 	}
 	return true
@@ -159,19 +153,13 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		defaultBranch = repo.DefaultWikiBranch
 	}
 	if branchName == defaultBranch && newCommitID == objectFormat.EmptyObjectID().String() {
-		log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo)
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: fmt.Sprintf("branch %s is the default branch and cannot be deleted", branchName),
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is the default branch and cannot be deleted", branchName)
 		return
 	}
 
 	protectBranch, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repo.ID, branchName)
 	if err != nil {
-		log.Error("Unable to get protected branch: %s in %-v Error: %v", branchName, repo, err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("Unable to get protected branch: %v", err)
 		return
 	}
 
@@ -187,10 +175,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 	//
 	// 1. Detect and prevent deletion of the branch
 	if newCommitID == objectFormat.EmptyObjectID().String() {
-		log.Warn("Forbidden: Branch: %s in %-v is protected from deletion", branchName, repo)
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: fmt.Sprintf("branch %s is protected from deletion", branchName),
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from deletion", branchName)
 		return
 	}
 
@@ -202,19 +187,13 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 			AddDynamicArguments(oldCommitID, "^"+newCommitID).
 			WithEnv(ctx.env).WithRepo(repo).RunStdString(ctx)
 		if err != nil {
-			log.Error("Unable to detect force push between: %s and %s in %-v Error: %v", oldCommitID, newCommitID, repo, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Fail to detect force push: %v", err),
-			})
+			ctx.PrivateInternalErrorf("Unable to detect force push between %s and %s in %s: %v", oldCommitID, newCommitID, repo.FullName(), err)
 			return
 		} else if len(output) > 0 {
 			if protectBranch.CanForcePush {
 				isForcePush = true
 			} else {
-				log.Warn("Forbidden: Branch: %s in %-v is protected from force push", branchName, repo)
-				ctx.JSON(http.StatusForbidden, private.Response{
-					UserMsg: fmt.Sprintf("branch %s is protected from force push", branchName),
-				})
+				ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from force push", branchName)
 				return
 			}
 		}
@@ -226,16 +205,10 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		if err != nil {
 			errUnverified, ok := err.(*errUnverifiedCommit)
 			if !ok {
-				log.Error("Unable to check commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
-				ctx.JSON(http.StatusInternalServerError, private.Response{
-					Err: fmt.Sprintf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err),
-				})
+				ctx.PrivateInternalErrorf("Unable to check commits from %s to %s: %v", oldCommitID, newCommitID, err)
 				return
 			}
-			log.Warn("Forbidden: Branch: %s in %-v is protected from unverified commit %s", branchName, repo, errUnverified.sha)
-			ctx.JSON(http.StatusForbidden, private.Response{
-				UserMsg: fmt.Sprintf("branch %s is protected from unverified commit %s", branchName, errUnverified.sha),
-			})
+			ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from unverified commit %s", branchName, errUnverified.sha)
 			return
 		}
 	}
@@ -252,10 +225,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		if err != nil {
 			errFilePathProtected, ok := errors.AsType[pull_service.ErrFilePathProtected](err)
 			if !ok {
-				log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
-				ctx.JSON(http.StatusInternalServerError, private.Response{
-					Err: fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err),
-				})
+				ctx.PrivateInternalErrorf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err)
 				return
 			}
 
@@ -287,12 +257,9 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		if ctx.opts.PullRequestID == 0 {
 			// 6a. If we're not merging from the UI/API then there are two ways we got here:
 			//
-			// We are changing a protected file and we're not allowed to do that
+			// We are changing a protected file, and we're not allowed to do that
 			if changedProtectedfiles {
-				log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
-				ctx.JSON(http.StatusForbidden, private.Response{
-					UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
-				})
+				ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from changing file %s", branchName, protectedFilePath)
 				return
 			}
 
@@ -301,10 +268,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 			if len(globs) > 0 {
 				unprotectedFilesOnly, err := pull_service.CheckUnprotectedFiles(ctx, gitRepo, branchName, oldCommitID, newCommitID, globs, ctx.env)
 				if err != nil {
-					log.Error("Unable to check file protection for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err)
-					ctx.JSON(http.StatusInternalServerError, private.Response{
-						Err: fmt.Sprintf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err),
-					})
+					ctx.PrivateInternalErrorf("Unable to check file protection for commits from %s to %s: %v", oldCommitID, newCommitID, err)
 					return
 				}
 				if unprotectedFilesOnly {
@@ -315,16 +279,10 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 
 			// Or we're simply not able to push to this protected branch
 			if isForcePush {
-				log.Warn("Forbidden: User %d is not allowed to force-push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo)
-				ctx.JSON(http.StatusForbidden, private.Response{
-					UserMsg: "Not allowed to force-push to protected branch " + branchName,
-				})
+				ctx.PrivateUserErrorf(http.StatusForbidden, "Not allowed to force-push to protected branch %s", branchName)
 				return
 			}
-			log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v", ctx.opts.UserID, branchName, repo)
-			ctx.JSON(http.StatusForbidden, private.Response{
-				UserMsg: "Not allowed to push to protected branch " + branchName,
-			})
+			ctx.PrivateUserErrorf(http.StatusForbidden, "Not allowed to push to protected branch %s", branchName)
 			return
 		}
 		// 6b. Merge (from UI or API)
@@ -332,10 +290,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		// Get the PR, user and permissions for the user in the repository
 		pr, err := issues_model.GetPullRequestByID(ctx, ctx.opts.PullRequestID)
 		if err != nil {
-			log.Error("Unable to get PullRequest %d Error: %v", ctx.opts.PullRequestID, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get PullRequest %d Error: %v", ctx.opts.PullRequestID, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get PullRequest %d Error: %v", ctx.opts.PullRequestID, err)
 			return
 		}
 
@@ -343,18 +298,12 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 		// Note: we can use ctx.perm and ctx.user directly as they will have been loaded above
 		allowedMerge, err := pull_service.IsUserAllowedToMerge(ctx, pr, ctx.userPerm, ctx.user)
 		if err != nil {
-			log.Error("Error calculating if allowed to merge: %v", err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Error calculating if allowed to merge: %v", err),
-			})
+			ctx.PrivateInternalErrorf("Error calculating if allowed to merge: %v", err)
 			return
 		}
 
 		if !allowedMerge {
-			log.Warn("Forbidden: User %d is not allowed to push to protected branch: %s in %-v and is not allowed to merge pr #%d", ctx.opts.UserID, branchName, repo, pr.Index)
-			ctx.JSON(http.StatusForbidden, private.Response{
-				UserMsg: "Not allowed to push to protected branch " + branchName,
-			})
+			ctx.PrivateUserErrorf(http.StatusForbidden, "Not allowed to push to protected branch %s", branchName)
 			return
 		}
 
@@ -365,26 +314,17 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
 
 		// Now if we're not an admin - we can't overwrite protected files so fail now
 		if changedProtectedfiles {
-			log.Warn("Forbidden: Branch: %s in %-v is protected from changing file %s", branchName, repo, protectedFilePath)
-			ctx.JSON(http.StatusForbidden, private.Response{
-				UserMsg: fmt.Sprintf("branch %s is protected from changing file %s", branchName, protectedFilePath),
-			})
+			ctx.PrivateUserErrorf(http.StatusForbidden, "Branch %s is protected from changing file %s", branchName, protectedFilePath)
 			return
 		}
 
 		// Check all status checks and reviews are ok
 		if err := pull_service.CheckPullBranchProtections(ctx, pr, true); err != nil {
 			if errors.Is(err, pull_service.ErrNotReadyToMerge) {
-				log.Warn("Forbidden: User %d is not allowed push to protected branch %s in %-v and pr #%d is not ready to be merged: %s", ctx.opts.UserID, branchName, repo, pr.Index, err.Error())
-				ctx.JSON(http.StatusForbidden, private.Response{
-					UserMsg: fmt.Sprintf("Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error()),
-				})
+				ctx.PrivateUserErrorf(http.StatusForbidden, "Not allowed to push to protected branch %s and pr #%d is not ready to be merged: %s", branchName, ctx.opts.PullRequestID, err.Error())
 				return
 			}
-			log.Error("Unable to check if mergeable: protected branch %s in %-v and pr #%d. Error: %v", ctx.opts.UserID, branchName, repo, pr.Index, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get status of pull request %d. Error: %v", ctx.opts.PullRequestID, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get status of pull request %d: %v", ctx.opts.PullRequestID, err)
 			return
 		}
 	}
@@ -401,10 +341,7 @@ func preReceiveTag(ctx *preReceiveContext, refFullName git.RefName) {
 		var err error
 		ctx.protectedTags, err = git_model.GetProtectedTags(ctx, ctx.Repo.Repository.ID)
 		if err != nil {
-			log.Error("Unable to get protected tags for %-v Error: %v", ctx.Repo.Repository, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: err.Error(),
-			})
+			ctx.PrivateInternalErrorf("Unable to get protected tags: %v", err)
 			return
 		}
 		ctx.gotProtectedTags = true
@@ -412,16 +349,11 @@ func preReceiveTag(ctx *preReceiveContext, refFullName git.RefName) {
 
 	isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, ctx.protectedTags, tagName, ctx.opts.UserID)
 	if err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("unable to check allowed tags: %v", err)
 		return
 	}
 	if !isAllowed {
-		log.Warn("Forbidden: Tag %s in %-v is protected", tagName, ctx.Repo.Repository)
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: fmt.Sprintf("Tag %s is protected", tagName),
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "Tag %s is protected", tagName)
 		return
 	}
 }
@@ -432,29 +364,21 @@ func preReceiveFor(ctx *preReceiveContext, refFullName git.RefName) {
 	}
 
 	if ctx.Repo.Repository.IsEmpty {
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: "Can't create pull request for an empty repository.",
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "Can't create pull request for an empty repository.")
 		return
 	}
 
 	if ctx.opts.IsWiki {
-		ctx.JSON(http.StatusForbidden, private.Response{
-			UserMsg: "Pull requests are not supported on the wiki.",
-		})
+		ctx.PrivateUserErrorf(http.StatusForbidden, "Pull requests are not supported on the wiki.")
 		return
 	}
 
 	_, _, err := agit.GetAgitBranchInfo(ctx, ctx.Repo.Repository.ID, refFullName.ForBranchName())
 	if err != nil {
 		if !errors.Is(err, util.ErrNotExist) {
-			ctx.JSON(http.StatusForbidden, private.Response{
-				UserMsg: fmt.Sprintf("Unexpected ref: %s", refFullName),
-			})
+			ctx.PrivateUserErrorf(http.StatusForbidden, "Unexpected ref: %s", refFullName)
 		} else {
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: err.Error(),
-			})
+			ctx.PrivateInternalErrorf("Unable to get branch info for ref %s: %v", refFullName, err)
 		}
 	}
 }
@@ -482,50 +406,35 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool {
 		taskID := ctx.opts.ActionsTaskID
 		ctx.user = user_model.NewActionsUserWithTaskID(taskID)
 		if taskID == 0 {
-			log.Error("HookPreReceive: ActionsUser with task ID 0")
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: "ActionsUser with task ID 0",
-			})
+			ctx.PrivateUserErrorf(http.StatusInternalServerError, "ActionsUser with task ID 0")
 			return false
 		}
 
 		userPerm, err := access_model.GetActionsUserRepoPermission(ctx, ctx.Repo.Repository, ctx.user, taskID)
 		if err != nil {
-			log.Error("Unable to get Actions user repo permission for task %d Error: %v", taskID, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get Actions user repo permission for task %d Error: %v", taskID, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get Actions user repo permission for task %d Error: %v", taskID, err)
 			return false
 		}
 		ctx.userPerm = userPerm
 	} else {
 		user, err := user_model.GetUserByID(ctx, ctx.opts.UserID)
 		if err != nil {
-			log.Error("Unable to get User id %d Error: %v", ctx.opts.UserID, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get User id %d Error: %v", ctx.opts.UserID, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get User id %d Error: %v", ctx.opts.UserID, err)
 			return false
 		}
 		ctx.user = user
 		userPerm, err := access_model.GetDoerRepoPermission(ctx, ctx.Repo.Repository, user)
 		if err != nil {
-			log.Error("Unable to get Repo permission of repo %s/%s of User %s: %v", ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name, user.Name, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get Repo permission of repo %s/%s of User %s: %v", ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name, user.Name, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get Repo permission of repo %s/%s of User %s: %v", ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name, user.Name, err)
 			return false
 		}
 		ctx.userPerm = userPerm
 	}
 
 	if ctx.opts.DeployKeyID != 0 {
-		deployKey, err := asymkey_model.GetDeployKeyByID(ctx, ctx.opts.DeployKeyID)
+		deployKey, err := asymkey_model.GetDeployKeyByID(ctx, ctx.Repo.Repository.ID, ctx.opts.DeployKeyID)
 		if err != nil {
-			log.Error("Unable to get DeployKey id %d Error: %v", ctx.opts.DeployKeyID, err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Unable to get DeployKey id %d Error: %v", ctx.opts.DeployKeyID, err),
-			})
+			ctx.PrivateInternalErrorf("Unable to get DeployKey id %d Error: %v", ctx.opts.DeployKeyID, err)
 			return false
 		}
 		ctx.deployKeyAccessMode = deployKey.Mode

routers/private/hook_proc_receive.go

@@ -10,7 +10,6 @@ import (
 	issues_model "gitea.dev/models/issues"
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/log"
 	"gitea.dev/modules/private"
 	"gitea.dev/modules/web"
 	"gitea.dev/services/agit"
@@ -28,18 +27,11 @@ func HookProcReceive(ctx *gitea_context.PrivateContext) {
 	results, err := agit.ProcReceive(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, opts)
 	if err != nil {
 		if errors.Is(err, issues_model.ErrMustCollaborator) {
-			ctx.JSON(http.StatusUnauthorized, private.Response{
-				Err: err.Error(), UserMsg: "You must be a collaborator to create pull request.",
-			})
+			ctx.PrivateUserErrorf(http.StatusUnauthorized, "You must be a collaborator to create pull request.")
 		} else if errors.Is(err, user_model.ErrBlockedUser) {
-			ctx.JSON(http.StatusUnauthorized, private.Response{
-				Err: err.Error(), UserMsg: "Cannot create pull request because you are blocked by the repository owner.",
-			})
+			ctx.PrivateUserErrorf(http.StatusUnauthorized, "Cannot create pull request because you are blocked by the repository owner.")
 		} else {
-			log.Error("agit.ProcReceive failed: %v", err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: err.Error(),
-			})
+			ctx.PrivateInternalErrorf("agit.ProcReceive failed: %v", err)
 		}
 
 		return

routers/private/internal_repo.go

@@ -4,13 +4,8 @@
 package private
 
 import (
-	"fmt"
-	"net/http"
-
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/modules/git"
-	"gitea.dev/modules/log"
-	"gitea.dev/modules/private"
 	gitea_context "gitea.dev/services/context"
 )
 
@@ -29,10 +24,7 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) {
 
 	gitRepo, err := git.RepositoryFromRequestContextOrOpen(ctx, repo)
 	if err != nil {
-		log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
-		})
+		ctx.PrivateInternalErrorf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
 		return
 	}
 	ctx.Repo = &gitea_context.Repository{
@@ -44,10 +36,7 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) {
 func loadRepository(ctx *gitea_context.PrivateContext, ownerName, repoName string) *repo_model.Repository {
 	repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ownerName, repoName)
 	if err != nil {
-		log.Error("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: fmt.Sprintf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err),
-		})
+		ctx.PrivateInternalErrorf("Failed to get repository: %s/%s Error: %v", ownerName, repoName, err)
 		return nil
 	}
 	if repo.OwnerName == "" {

routers/private/key.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 
 	asymkey_model "gitea.dev/models/asymkey"
-	"gitea.dev/modules/private"
 	"gitea.dev/modules/timeutil"
 	"gitea.dev/services/context"
 )
@@ -17,28 +16,22 @@ func UpdatePublicKeyInRepo(ctx *context.PrivateContext) {
 	keyID := ctx.PathParamInt64("id")
 	repoID := ctx.PathParamInt64("repoid")
 	if err := asymkey_model.UpdatePublicKeyUpdated(ctx, keyID); err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 
-	deployKey, err := asymkey_model.GetDeployKeyByRepo(ctx, keyID, repoID)
+	deployKey, err := asymkey_model.GetDeployKeyByRepoPublicKey(ctx, repoID, keyID)
 	if err != nil {
 		if asymkey_model.IsErrDeployKeyNotExist(err) {
 			ctx.PlainText(http.StatusOK, "success")
 			return
 		}
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 	deployKey.UpdatedUnix = timeutil.TimeStampNow()
 	if err = asymkey_model.UpdateDeployKeyCols(ctx, deployKey, "updated_unix"); err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 
@@ -52,18 +45,13 @@ func AuthorizedPublicKeyByContent(ctx *context.PrivateContext) {
 
 	publicKey, err := asymkey_model.SearchPublicKeyByContent(ctx, content)
 	if err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 
 	authorizedString, err := asymkey_model.AuthorizedStringForKey(publicKey)
 	if err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err:     err.Error(),
-			UserMsg: "invalid public key",
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 	ctx.PlainText(http.StatusOK, authorizedString)

routers/private/mail.go

@@ -5,14 +5,12 @@ package private
 
 import (
 	stdCtx "context"
-	"fmt"
 	"net/http"
 	"strconv"
 
 	"gitea.dev/models/db"
 	user_model "gitea.dev/models/user"
 	"gitea.dev/modules/json"
-	"gitea.dev/modules/log"
 	"gitea.dev/modules/private"
 	"gitea.dev/modules/setting"
 	"gitea.dev/services/context"
@@ -25,9 +23,7 @@ import (
 // It doesn't wait before each message will be processed
 func SendEmail(ctx *context.PrivateContext) {
 	if setting.MailService == nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: "Mail service is not enabled.",
-		})
+		ctx.PrivateInternalErrorf("Mail service is not enabled.")
 		return
 	}
 
@@ -36,10 +32,7 @@ func SendEmail(ctx *context.PrivateContext) {
 	defer rd.Close()
 
 	if err := json.NewDecoder(rd).Decode(&mail); err != nil {
-		log.Error("JSON Decode failed: %v", err)
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("JSON Decode failed: %v", err)
 		return
 	}
 
@@ -48,11 +41,7 @@ func SendEmail(ctx *context.PrivateContext) {
 		for _, uname := range mail.To {
 			user, err := user_model.GetUserByName(ctx, uname)
 			if err != nil {
-				err := fmt.Sprintf("Failed to get user information: %v", err)
-				log.Error(err)
-				ctx.JSON(http.StatusInternalServerError, private.Response{
-					Err: err,
-				})
+				ctx.PrivateInternalErrorf("Failed to get user information: %v", err)
 				return
 			}
 
@@ -68,11 +57,7 @@ func SendEmail(ctx *context.PrivateContext) {
 			return nil
 		})
 		if err != nil {
-			err := fmt.Sprintf("Failed to find users: %v", err)
-			log.Error(err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: err,
-			})
+			ctx.PrivateInternalErrorf("Failed to find users: %v", err)
 			return
 		}
 	}

routers/private/manager.go

@@ -4,7 +4,6 @@
 package private
 
 import (
-	"fmt"
 	"net/http"
 
 	"gitea.dev/models/db"
@@ -22,9 +21,7 @@ import (
 func ReloadTemplates(ctx *context.PrivateContext) {
 	err := templates.ReloadAllTemplates()
 	if err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			UserMsg: fmt.Sprintf("Template error: %v", err),
-		})
+		ctx.PrivateInternalErrorf("Template error: %v", err)
 		return
 	}
 	ctx.PlainText(http.StatusOK, "success")
@@ -49,9 +46,8 @@ func FlushQueues(ctx *context.PrivateContext) {
 	}
 	err := queue.GetManager().FlushAll(ctx, opts.Timeout)
 	if err != nil {
-		ctx.JSON(http.StatusRequestTimeout, private.Response{
-			UserMsg: fmt.Sprintf("%v", err),
-		})
+		ctx.PrivateUserErrorf(http.StatusRequestTimeout, "%v", err)
+		return
 	}
 	ctx.PlainText(http.StatusOK, "success")
 }
@@ -71,9 +67,7 @@ func ResumeLogging(ctx *context.PrivateContext) {
 // ReleaseReopenLogging releases and reopens logging files
 func ReleaseReopenLogging(ctx *context.PrivateContext) {
 	if err := releasereopen.GetManager().ReleaseReopen(); err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: fmt.Sprintf("Error during release and reopen: %v", err),
-		})
+		ctx.PrivateInternalErrorf("Error during release and reopen: %v", err)
 		return
 	}
 	ctx.PlainText(http.StatusOK, "success")

routers/private/manager_process.go

@@ -11,8 +11,6 @@ import (
 	"runtime"
 	"time"
 
-	"gitea.dev/modules/log"
-	"gitea.dev/modules/private"
 	process_module "gitea.dev/modules/process"
 	"gitea.dev/services/context"
 )
@@ -38,10 +36,7 @@ func Processes(ctx *context.PrivateContext) {
 	if stacktraces {
 		processes, processCount, goroutineCount, err = process_module.GetManager().ProcessStacktraces(flat, noSystem)
 		if err != nil {
-			log.Error("Unable to get stacktrace: %v", err)
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Failed to get stacktraces: %v", err),
-			})
+			ctx.PrivateInternalErrorf("Failed to get stacktraces: %v", err)
 			return
 		}
 	} else {
@@ -61,11 +56,8 @@ func Processes(ctx *context.PrivateContext) {
 	ctx.Resp.WriteHeader(http.StatusOK)
 
 	if err := writeProcesses(ctx.Resp, processes, processCount, goroutineCount, "", flat); err != nil {
-		log.Error("Unable to write out process stacktrace: %v", err)
 		if !ctx.Written() {
-			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: fmt.Sprintf("Failed to get stacktraces: %v", err),
-			})
+			ctx.PrivateInternalErrorf("Failed to get stacktraces: %v", err)
 		}
 		return
 	}

routers/private/manager_windows.go

@@ -9,15 +9,12 @@ import (
 	"net/http"
 
 	"gitea.dev/modules/graceful"
-	"gitea.dev/modules/private"
 	"gitea.dev/services/context"
 )
 
 // Restart is not implemented for Windows based servers as they can't fork
 func Restart(ctx *context.PrivateContext) {
-	ctx.JSON(http.StatusNotImplemented, private.Response{
-		UserMsg: "windows servers cannot be gracefully restarted - shutdown and restart manually",
-	})
+	ctx.PrivateUserErrorf(http.StatusNotImplemented, "windows servers cannot be gracefully restarted - shutdown and restart manually")
 }
 
 // Shutdown causes the server to perform a graceful shutdown

routers/private/restore_repo.go

@@ -8,7 +8,6 @@ import (
 	"net/http"
 
 	"gitea.dev/modules/json"
-	"gitea.dev/modules/private"
 	myCtx "gitea.dev/services/context"
 	"gitea.dev/services/migrations"
 )
@@ -17,9 +16,7 @@ import (
 func RestoreRepo(ctx *myCtx.PrivateContext) {
 	bs, err := io.ReadAll(ctx.Req.Body)
 	if err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 	params := struct {
@@ -30,9 +27,7 @@ func RestoreRepo(ctx *myCtx.PrivateContext) {
 		Validation bool
 	}{}
 	if err = json.Unmarshal(bs, &params); err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 		return
 	}
 
@@ -44,9 +39,7 @@ func RestoreRepo(ctx *myCtx.PrivateContext) {
 		params.Units,
 		params.Validation,
 	); err != nil {
-		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err.Error(),
-		})
+		ctx.PrivateInternalErrorf("%v", err)
 	} else {
 		ctx.PlainText(http.StatusOK, "success")
 	}

routers/private/serv.go

@@ -198,7 +198,7 @@ func ServCommand(ctx *context.PrivateContext) {
 			ctx.PrivateUserErrorf(http.StatusNotFound, "Cannot find repository %s", repoLogName)
 			return
 		}
-		deployKey, err = asymkey_model.GetDeployKeyByRepo(ctx, key.ID, repo.ID)
+		deployKey, err = asymkey_model.GetDeployKeyByRepoPublicKey(ctx, repo.ID, key.ID)
 		if err != nil {
 			if asymkey_model.IsErrDeployKeyNotExist(err) {
 				ctx.PrivateUserErrorf(http.StatusNotFound, "Deploy key %d:%s has no %q permission for %s.", key.ID, key.Name, modeString, repoLogName)

routers/web/repo/setting/deploy_key.go

@@ -9,13 +9,15 @@ import (
 
 	asymkey_model "gitea.dev/models/asymkey"
 	"gitea.dev/models/db"
+	"gitea.dev/models/perm"
 	"gitea.dev/modules/setting"
+	"gitea.dev/modules/util"
 	asymkey_service "gitea.dev/services/asymkey"
 	"gitea.dev/services/context"
 	"gitea.dev/services/forms"
 )
 
-// DeployKeys render the deploy keys list of a repository page
+// DeployKeys render the deploy-keys list of a repository page
 func DeployKeys(ctx *context.Context) {
 	ctx.Data["Title"] = ctx.Tr("repo.settings.deploy_keys") + " / " + ctx.Tr("secrets.secrets")
 	ctx.Data["PageIsSettingsKeys"] = true
@@ -26,7 +28,7 @@ func DeployKeys(ctx *context.Context) {
 		ctx.ServerError("ListDeployKeys", err)
 		return
 	}
-	ctx.Data["Deploykeys"] = keys
+	ctx.Data["RepoDeployKeys"] = keys
 
 	ctx.HTML(http.StatusOK, tplDeployKeys)
 }
@@ -51,7 +53,8 @@ func DeployKeysPost(ctx *context.Context) {
 		return
 	}
 
-	key, err := asymkey_model.AddDeployKey(ctx, ctx.Repo.Repository.ID, form.Title, content, !form.IsWritable)
+	accessMode := util.Iif(form.IsWritable, perm.AccessModeWrite, perm.AccessModeRead)
+	key, err := asymkey_model.AddDeployKey(ctx, ctx.Repo.Repository.ID, form.Title, content, accessMode)
 	if err != nil {
 		switch {
 		case asymkey_model.IsErrDeployKeyAlreadyExist(err):
@@ -72,13 +75,12 @@ func DeployKeysPost(ctx *context.Context) {
 	ctx.JSONRedirect(ctx.Repo.RepoLink + "/settings/keys")
 }
 
-// DeleteDeployKey response for deleting a deploy key
+// DeleteDeployKey response for deleting a deploy-key
 func DeleteDeployKey(ctx *context.Context) {
 	if err := asymkey_service.DeleteDeployKey(ctx, ctx.Repo.Repository, ctx.FormInt64("id")); err != nil {
-		ctx.Flash.Error("DeleteDeployKey: " + err.Error())
+		ctx.ServerError("DeleteDeployKey", err)
 	} else {
 		ctx.Flash.Success(ctx.Tr("repo.settings.deploy_key_deletion_success"))
 	}
-
 	ctx.JSONRedirect(ctx.Repo.RepoLink + "/settings/keys")
 }

routers/web/repo/setting/settings_test.go

@@ -37,7 +37,7 @@ func TestAddDeployKey(t *testing.T) {
 		contexttest.LoadRepo(t, ctx, 2)
 		DeployKeysPost(ctx)
 		assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())
-		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-only", Content: testKey, Mode: perm.AccessModeRead})
+		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-only", Mode: perm.AccessModeRead})
 	})
 	t.Run("ReadWrite", func(t *testing.T) {
 		const testKey = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIEHjnNEfE88W1pvBLdV3otv28x760gdmPao3lVD5uAt9\n"
@@ -46,7 +46,7 @@ func TestAddDeployKey(t *testing.T) {
 		contexttest.LoadRepo(t, ctx, 2)
 		DeployKeysPost(ctx)
 		assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())
-		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-write", Content: testKey, Mode: perm.AccessModeWrite})
+		unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{Name: "read-write", Mode: perm.AccessModeWrite})
 	})
 }
 

services/asymkey/deploy_key.go

@@ -34,7 +34,7 @@ func deleteDeployKeyFromDB(ctx context.Context, key *asymkey_model.DeployKey) er
 	}
 
 	// Check if this is the last reference to same key content.
-	has, err := asymkey_model.IsDeployKeyExistByKeyID(ctx, key.KeyID)
+	has, err := asymkey_model.IsDeployKeyExistByPublicKeyID(ctx, key.KeyID)
 	if err != nil {
 		return err
 	} else if !has {
@@ -50,18 +50,13 @@ func deleteDeployKeyFromDB(ctx context.Context, key *asymkey_model.DeployKey) er
 // Permissions check should be done outside.
 func DeleteDeployKey(ctx context.Context, repo *repo_model.Repository, id int64) error {
 	if err := db.WithTx(ctx, func(ctx context.Context) error {
-		key, err := asymkey_model.GetDeployKeyByID(ctx, id)
+		key, err := asymkey_model.GetDeployKeyByID(ctx, repo.ID, id)
 		if err != nil {
 			if asymkey_model.IsErrDeployKeyNotExist(err) {
 				return nil
 			}
 			return fmt.Errorf("GetDeployKeyByID: %w", err)
 		}
-
-		if key.RepoID != repo.ID {
-			return fmt.Errorf("deploy key %d does not belong to repository %d", id, repo.ID)
-		}
-
 		return deleteDeployKeyFromDB(ctx, key)
 	}); err != nil {
 		return err

services/convert/convert.go

@@ -847,17 +847,20 @@ func ToGitHook(h *git.Hook) *api.GitHook {
 }
 
 // ToDeployKey convert asymkey_model.DeployKey to api.DeployKey
-func ToDeployKey(apiLink string, key *asymkey_model.DeployKey) *api.DeployKey {
-	return &api.DeployKey{
-		ID:          key.ID,
-		KeyID:       key.KeyID,
-		Key:         key.Content,
-		Fingerprint: key.Fingerprint,
-		URL:         fmt.Sprintf("%s%d", apiLink, key.ID),
-		Title:       key.Name,
-		Created:     key.CreatedUnix.AsTime(),
-		ReadOnly:    key.Mode == perm.AccessModeRead, // All deploy keys are read-only.
+func ToDeployKey(ctx context.Context, repo *repo_model.Repository, deployKey *asymkey_model.DeployKey) *api.DeployKey {
+	k := &api.DeployKey{
+		ID:       deployKey.ID,
+		KeyID:    deployKey.KeyID,
+		URL:      repo.APIURL(ctx) + fmt.Sprintf("/keys/%d", deployKey.ID),
+		Title:    deployKey.Name,
+		Created:  deployKey.CreatedUnix.AsTime(),
+		ReadOnly: deployKey.Mode == perm.AccessModeRead, // All deploy keys are read-only.
 	}
+	if err := deployKey.LoadPublicKey(ctx); err == nil {
+		k.Key = deployKey.PublicKey.Content
+		k.Fingerprint = deployKey.PublicKey.Fingerprint
+	}
+	return k
 }
 
 // ToOrganization convert user_model.User to api.Organization

templates/repo/settings/deploy_keys.tmpl

@@ -41,24 +41,36 @@
 					</button>
 				</form>
 			</div>
-			{{if .Deploykeys}}
+			{{if .RepoDeployKeys}}
 				<div class="flex-divided-list items-with-main">
-					{{range .Deploykeys}}
+					{{range $deployKey := .RepoDeployKeys}}
 						<div class="item">
 							<div class="item-leading">
-								<span class="{{if .HasRecentActivity}}tw-text-green{{end}}" {{if .HasRecentActivity}}data-tooltip-content="{{ctx.Locale.Tr "settings.key_state_desc"}}"{{end}}>{{svg "octicon-key" 32}}</span>
+								<span class="{{if $deployKey.HasRecentActivity}}tw-text-green{{end}}"
+									{{if $deployKey.HasRecentActivity}}data-tooltip-content="{{ctx.Locale.Tr "settings.key_state_desc"}}"{{end}}
+								>{{svg "octicon-key" 32}}</span>
 							</div>
 							<div class="item-main">
-								<div class="item-title">{{.Name}}</div>
+								<div class="item-title">{{$deployKey.Name}}</div>
 								<div class="item-body">
-									{{.Fingerprint}}
+									{{$deployKey.Fingerprint}}
 								</div>
 								<div class="item-body">
-									<i>{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .CreatedUnix)}} —  {{svg "octicon-info"}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} <span {{if .HasRecentActivity}}class="tw-text-green"{{end}}>{{DateUtils.AbsoluteShort .UpdatedUnix}}</span>{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}} - <span>{{ctx.Locale.Tr "settings.can_read_info"}}{{if not .IsReadOnly}} / {{ctx.Locale.Tr "settings.can_write_info"}} {{end}}</span></i>
+									{{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort $deployKey.CreatedUnix)}}
+									<span class="tw-mx-2">-</span>
+									{{svg "octicon-info"}}
+									{{if $deployKey.HasUsed}}
+										{{ctx.Locale.Tr "settings.last_used"}}
+										<span {{if $deployKey.HasRecentActivity}}class="tw-text-green"{{end}}>{{DateUtils.AbsoluteShort $deployKey.UpdatedUnix}}</span>
+									{{else}}
+										{{ctx.Locale.Tr "settings.no_activity"}}
+									{{end}}
+									<span class="tw-mx-2">-</span>
+									<span>{{ctx.Locale.Tr "settings.can_read_info"}}{{if not $deployKey.IsReadOnly}} / {{ctx.Locale.Tr "settings.can_write_info"}} {{end}}</span>
 								</div>
 							</div>
 							<div class="item-trailing">
-								<button class="ui red tiny button link-action" data-modal-confirm="#repo-deploy-key-delete-modal" data-url="{{$.Link}}/delete?id={{.ID}}">
+								<button class="ui red tiny button link-action" data-modal-confirm="#repo-deploy-key-delete-modal" data-url="{{$.Link}}/delete?id={{$deployKey.ID}}">
 									{{ctx.Locale.Tr "settings.delete_key"}}
 								</button>
 							</div>

tests/integration/api_keys_test.go

@@ -67,10 +67,9 @@ func TestCreateReadOnlyDeployKey(t *testing.T) {
 
 	newDeployKey := DecodeJSON(t, resp, &api.DeployKey{})
 	unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{
-		ID:      newDeployKey.ID,
-		Name:    rawKeyBody.Title,
-		Content: rawKeyBody.Key,
-		Mode:    perm.AccessModeRead,
+		ID:   newDeployKey.ID,
+		Name: rawKeyBody.Title,
+		Mode: perm.AccessModeRead,
 	})
 
 	// Using the ID of a key that does not belong to the repository must fail
@@ -105,10 +104,9 @@ func TestCreateReadWriteDeployKey(t *testing.T) {
 
 	newDeployKey := DecodeJSON(t, resp, &api.DeployKey{})
 	unittest.AssertExistsAndLoadBean(t, &asymkey_model.DeployKey{
-		ID:      newDeployKey.ID,
-		Name:    rawKeyBody.Title,
-		Content: rawKeyBody.Key,
-		Mode:    perm.AccessModeWrite,
+		ID:   newDeployKey.ID,
+		Name: rawKeyBody.Title,
+		Mode: perm.AccessModeWrite,
 	})
 }
 

tests/integration/api_private_serv_test.go

@@ -26,14 +26,15 @@ func TestAPIPrivateNoServ(t *testing.T) {
 		assert.Equal(t, int64(1), key.ID)
 		assert.Equal(t, "user2@localhost", key.Name)
 
-		deployKey, err := asymkey_model.AddDeployKey(ctx, 1, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", false)
+		keyContent := "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment"
+		deployKey, err := asymkey_model.AddDeployKey(ctx, 1, "test-deploy", keyContent, perm.AccessModeRead)
 		assert.NoError(t, err)
 
 		key, user, err = private.ServNoCommand(ctx, deployKey.KeyID)
 		assert.NoError(t, err)
 		assert.Empty(t, user)
 		assert.Equal(t, deployKey.KeyID, key.ID)
-		assert.Equal(t, "test-deploy", key.Name)
+		assert.Equal(t, "(DeployKey)", key.Name)
 	})
 }
 
@@ -84,10 +85,11 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Empty(t, results)
 
 		// Add reading deploy key
-		deployKey, err := asymkey_model.AddDeployKey(ctx, 19 /* repo id */, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", true)
+		testContent := "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment"
+		deployKey, err := asymkey_model.AddDeployKey(ctx, 19 /* repo id */, "test-deploy", testContent, perm.AccessModeRead)
 		assert.NoError(t, err)
 
-		// Can pull from repo we're a deploy key for
+		// Can pull from repo we're a deploy-key for
 		results, extra = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "")
 		assert.NoError(t, extra.Error)
 		assert.False(t, results.IsWiki)
@@ -116,7 +118,8 @@ func TestAPIPrivateServ(t *testing.T) {
 		assert.Empty(t, results)
 
 		// Add writing deploy key
-		deployKey, err = asymkey_model.AddDeployKey(ctx, 20 /* repo id */, "test-deploy", "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment", false)
+		testContent = "sk-ecdsa-sha2-nistp256@openssh.com AAAAInNrLWVjZHNhLXNoYTItbmlzdHAyNTZAb3BlbnNzaC5jb20AAAAIbmlzdHAyNTYAAABBBGXEEzWmm1dxb+57RoK5KVCL0w2eNv9cqJX2AGGVlkFsVDhOXHzsadS3LTK4VlEbbrDMJdoti9yM8vclA8IeRacAAAAEc3NoOg== nocomment"
+		deployKey, err = asymkey_model.AddDeployKey(ctx, 20 /* repo id */, "test-deploy", testContent, perm.AccessModeWrite)
 		assert.NoError(t, err)
 
 		// Cannot push to a private repo with reading key

tests/integration/api_repo_files_change_test.go

@@ -239,7 +239,7 @@ func TestAPIChangeFiles(t *testing.T) {
 		req = NewRequestWithJSON(t, "POST", url, &changeFilesOptions).
 			AddTokenAuth(token2)
 		resp = MakeRequest(t, req, http.StatusForbidden)
-		assert.Contains(t, resp.Body.String(), `"message":"branch develop is protected from force push"`)
+		assert.Contains(t, resp.Body.String(), `"message":"Branch develop is protected from force push"`)
 
 		// Test updating a file and renaming it
 		changeFilesOptions = getChangeFilesOptions()