From 943f026844c34113d09feba7a13a539ae5a64f4c Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 20 Aug 2026 23:57:17 +0800 Subject: [PATCH] refactor: deploy key and private route handlers (#38999) clean up legacy code, fix various bugs: * add missing "return" --- models/asymkey/ssh_key.go | 6 +- models/asymkey/ssh_key_deploy.go | 119 +++++-------- routers/api/v1/api.go | 2 +- routers/api/v1/repo/key.go | 58 ++----- routers/private/actions.go | 25 +-- routers/private/hook_pre_receive.go | 159 ++++-------------- routers/private/hook_proc_receive.go | 14 +- routers/private/internal_repo.go | 15 +- routers/private/key.go | 24 +-- routers/private/mail.go | 23 +-- routers/private/manager.go | 14 +- routers/private/manager_process.go | 12 +- routers/private/manager_windows.go | 5 +- routers/private/restore_repo.go | 13 +- routers/private/serv.go | 2 +- routers/web/repo/setting/deploy_key.go | 14 +- routers/web/repo/setting/settings_test.go | 4 +- services/asymkey/deploy_key.go | 9 +- services/convert/convert.go | 23 +-- templates/repo/settings/deploy_keys.tmpl | 26 ++- tests/integration/api_keys_test.go | 14 +- tests/integration/api_private_serv_test.go | 13 +- .../integration/api_repo_files_change_test.go | 2 +- 23 files changed, 185 insertions(+), 411 deletions(-) diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index e72a88ae8f6..ff7a9b12039 100644 --- a/models/asymkey/ssh_key.go +++ b/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) } diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index e7cbc9253eb..41ec6d82dce 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/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}) } diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index b2c932f4af6..e457c886fcb 100644 --- a/routers/api/v1/api.go +++ b/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) diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go index 0104fd40116..465a775c931 100644 --- a/routers/api/v1/repo/key.go +++ b/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 diff --git a/routers/private/actions.go b/routers/private/actions.go index ea69b953bf7..fa4c200fa60 100644 --- a/routers/private/actions.go +++ b/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 } diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index ae04a93a95f..d33b1a32ffb 100644 --- a/routers/private/hook_pre_receive.go +++ b/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 diff --git a/routers/private/hook_proc_receive.go b/routers/private/hook_proc_receive.go index 4d2b04ee800..73542c6dd76 100644 --- a/routers/private/hook_proc_receive.go +++ b/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 diff --git a/routers/private/internal_repo.go b/routers/private/internal_repo.go index 2b22d810170..c4563c1433e 100644 --- a/routers/private/internal_repo.go +++ b/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 == "" { diff --git a/routers/private/key.go b/routers/private/key.go index 7f3804df6e0..2a89e2790a5 100644 --- a/routers/private/key.go +++ b/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) diff --git a/routers/private/mail.go b/routers/private/mail.go index 3f7fbb3966d..684366a9dc7 100644 --- a/routers/private/mail.go +++ b/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 } } diff --git a/routers/private/manager.go b/routers/private/manager.go index b31e3bfea02..10f80756be7 100644 --- a/routers/private/manager.go +++ b/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") diff --git a/routers/private/manager_process.go b/routers/private/manager_process.go index 47cddf77067..1352edb8902 100644 --- a/routers/private/manager_process.go +++ b/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 } diff --git a/routers/private/manager_windows.go b/routers/private/manager_windows.go index c5201a03e95..d5df4414f30 100644 --- a/routers/private/manager_windows.go +++ b/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 diff --git a/routers/private/restore_repo.go b/routers/private/restore_repo.go index b88982f30a2..5b14586c118 100644 --- a/routers/private/restore_repo.go +++ b/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, ¶ms); 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") } diff --git a/routers/private/serv.go b/routers/private/serv.go index 61a91a4d99b..dc1c7c3dd69 100644 --- a/routers/private/serv.go +++ b/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) diff --git a/routers/web/repo/setting/deploy_key.go b/routers/web/repo/setting/deploy_key.go index 5909cdc1003..3620b01a2dc 100644 --- a/routers/web/repo/setting/deploy_key.go +++ b/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") } diff --git a/routers/web/repo/setting/settings_test.go b/routers/web/repo/setting/settings_test.go index 3beecf7b6f3..12573c11ab3 100644 --- a/routers/web/repo/setting/settings_test.go +++ b/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}) }) } diff --git a/services/asymkey/deploy_key.go b/services/asymkey/deploy_key.go index 17c409dc012..66c22407a14 100644 --- a/services/asymkey/deploy_key.go +++ b/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 diff --git a/services/convert/convert.go b/services/convert/convert.go index 84f9cc1b317..814837b13e5 100644 --- a/services/convert/convert.go +++ b/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 diff --git a/templates/repo/settings/deploy_keys.tmpl b/templates/repo/settings/deploy_keys.tmpl index 588968f877c..f5ce3fe3785 100644 --- a/templates/repo/settings/deploy_keys.tmpl +++ b/templates/repo/settings/deploy_keys.tmpl @@ -41,24 +41,36 @@ - {{if .Deploykeys}} + {{if .RepoDeployKeys}}
- {{range .Deploykeys}} + {{range $deployKey := .RepoDeployKeys}}
- {{svg "octicon-key" 32}} + {{svg "octicon-key" 32}}
-
{{.Name}}
+
{{$deployKey.Name}}
- {{.Fingerprint}} + {{$deployKey.Fingerprint}}
- {{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort .CreatedUnix)}} — {{svg "octicon-info"}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} {{DateUtils.AbsoluteShort .UpdatedUnix}}{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}} - {{ctx.Locale.Tr "settings.can_read_info"}}{{if not .IsReadOnly}} / {{ctx.Locale.Tr "settings.can_write_info"}} {{end}} + {{ctx.Locale.Tr "settings.added_on" (DateUtils.AbsoluteShort $deployKey.CreatedUnix)}} + - + {{svg "octicon-info"}} + {{if $deployKey.HasUsed}} + {{ctx.Locale.Tr "settings.last_used"}} + {{DateUtils.AbsoluteShort $deployKey.UpdatedUnix}} + {{else}} + {{ctx.Locale.Tr "settings.no_activity"}} + {{end}} + - + {{ctx.Locale.Tr "settings.can_read_info"}}{{if not $deployKey.IsReadOnly}} / {{ctx.Locale.Tr "settings.can_write_info"}} {{end}}
-
diff --git a/tests/integration/api_keys_test.go b/tests/integration/api_keys_test.go index d2c5d389284..40a3533890f 100644 --- a/tests/integration/api_keys_test.go +++ b/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, }) } diff --git a/tests/integration/api_private_serv_test.go b/tests/integration/api_private_serv_test.go index fcac2174af8..a8f1308448d 100644 --- a/tests/integration/api_private_serv_test.go +++ b/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 diff --git a/tests/integration/api_repo_files_change_test.go b/tests/integration/api_repo_files_change_test.go index 74c31862823..4b3b5ccf7b3 100644 --- a/tests/integration/api_repo_files_change_test.go +++ b/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()