From 4ef5e249d75f8da459262476eedb24d0cd119420 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 6 Mar 2022 01:49:55 +0800 Subject: [PATCH 1/5] Fix the bug: deploy key with write access can not push --- cmd/hook.go | 4 +- cmd/serv.go | 2 +- integrations/api_private_serv_test.go | 10 ++-- models/asymkey/ssh_key_deploy.go | 15 +++-- models/helper_environment.go | 4 +- models/repo.go | 2 +- modules/private/hook.go | 2 +- modules/private/serv.go | 6 +- routers/api/v1/repo/key.go | 2 +- routers/private/hook_pre_receive.go | 80 +++++++++++++++------------ routers/private/serv.go | 7 +-- routers/web/repo/http.go | 1 - 12 files changed, 71 insertions(+), 64 deletions(-) diff --git a/cmd/hook.go b/cmd/hook.go index 1dd59e819206..05fa6e56c133 100644 --- a/cmd/hook.go +++ b/cmd/hook.go @@ -185,7 +185,7 @@ Gitea or set your environment appropriately.`, "") reponame := os.Getenv(models.EnvRepoName) userID, _ := strconv.ParseInt(os.Getenv(models.EnvPusherID), 10, 64) prID, _ := strconv.ParseInt(os.Getenv(models.EnvPRID), 10, 64) - isDeployKey, _ := strconv.ParseBool(os.Getenv(models.EnvIsDeployKey)) + deployKeyID, _ := strconv.ParseInt(os.Getenv(models.EnvDeployKeyID), 10, 64) hookOptions := private.HookOptions{ UserID: userID, @@ -194,7 +194,7 @@ Gitea or set your environment appropriately.`, "") GitQuarantinePath: os.Getenv(private.GitQuarantinePath), GitPushOptions: pushOptions(), PullRequestID: prID, - IsDeployKey: isDeployKey, + DeployKeyID: deployKeyID, } scanner := bufio.NewScanner(os.Stdin) diff --git a/cmd/serv.go b/cmd/serv.go index b4ef37f1dc6c..c834ca298acf 100644 --- a/cmd/serv.go +++ b/cmd/serv.go @@ -243,7 +243,7 @@ func runServ(c *cli.Context) error { os.Setenv(models.EnvPusherID, strconv.FormatInt(results.UserID, 10)) os.Setenv(models.EnvRepoID, strconv.FormatInt(results.RepoID, 10)) os.Setenv(models.EnvPRID, fmt.Sprintf("%d", 0)) - os.Setenv(models.EnvIsDeployKey, fmt.Sprintf("%t", results.IsDeployKey)) + os.Setenv(models.EnvDeployKeyID, fmt.Sprintf("%d", results.DeployKeyID)) os.Setenv(models.EnvKeyID, fmt.Sprintf("%d", results.KeyID)) os.Setenv(models.EnvAppURL, setting.AppURL) diff --git a/integrations/api_private_serv_test.go b/integrations/api_private_serv_test.go index a58d927cb91c..fd3cb25ef2de 100644 --- a/integrations/api_private_serv_test.go +++ b/integrations/api_private_serv_test.go @@ -47,7 +47,7 @@ func TestAPIPrivateServ(t *testing.T) { results, err := private.ServCommand(ctx, 1, "user2", "repo1", perm.AccessModeWrite, "git-upload-pack", "") assert.NoError(t, err) assert.False(t, results.IsWiki) - assert.False(t, results.IsDeployKey) + assert.Zero(t, results.DeployKeyID) assert.Equal(t, int64(1), results.KeyID) assert.Equal(t, "user2@localhost", results.KeyName) assert.Equal(t, "user2", results.UserName) @@ -70,7 +70,7 @@ func TestAPIPrivateServ(t *testing.T) { results, err = private.ServCommand(ctx, 1, "user15", "big_test_public_1", perm.AccessModeRead, "git-upload-pack", "") assert.NoError(t, err) assert.False(t, results.IsWiki) - assert.False(t, results.IsDeployKey) + assert.Zero(t, results.DeployKeyID) assert.Equal(t, int64(1), results.KeyID) assert.Equal(t, "user2@localhost", results.KeyName) assert.Equal(t, "user2", results.UserName) @@ -92,7 +92,7 @@ func TestAPIPrivateServ(t *testing.T) { results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_1", perm.AccessModeRead, "git-upload-pack", "") assert.NoError(t, err) assert.False(t, results.IsWiki) - assert.True(t, results.IsDeployKey) + assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID) assert.Equal(t, "test-deploy", results.KeyName) assert.Equal(t, "user15", results.UserName) @@ -129,7 +129,7 @@ func TestAPIPrivateServ(t *testing.T) { results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeRead, "git-upload-pack", "") assert.NoError(t, err) assert.False(t, results.IsWiki) - assert.True(t, results.IsDeployKey) + assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID) assert.Equal(t, "test-deploy", results.KeyName) assert.Equal(t, "user15", results.UserName) @@ -142,7 +142,7 @@ func TestAPIPrivateServ(t *testing.T) { results, err = private.ServCommand(ctx, deployKey.KeyID, "user15", "big_test_private_2", perm.AccessModeWrite, "git-upload-pack", "") assert.NoError(t, err) assert.False(t, results.IsWiki) - assert.True(t, results.IsDeployKey) + assert.NotZero(t, results.DeployKeyID) assert.Equal(t, deployKey.KeyID, results.KeyID) assert.Equal(t, "test-deploy", results.KeyName) assert.Equal(t, "user15", results.UserName) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index fc6324792a66..e6553dcca050 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -58,7 +58,7 @@ func (key *DeployKey) GetContent() error { return nil } -// IsReadOnly checks if the key can only be used for read operations +// IsReadOnly checks if the key can only be used for read operations, used by template func (key *DeployKey) IsReadOnly() bool { return key.Mode == perm.AccessModeRead } @@ -167,7 +167,12 @@ func AddDeployKey(repoID int64, name, content string, readOnly bool) (*DeployKey } // GetDeployKeyByID returns deploy key by given ID. -func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) { +func GetDeployKeyByID(id int64) (*DeployKey, error) { + return GetDeployKeyByIDCtx(db.DefaultContext, id) +} + +// GetDeployKeyByIDCtx returns deploy key by given ID. +func GetDeployKeyByIDCtx(ctx context.Context, id int64) (*DeployKey, error) { key := new(DeployKey) has, err := db.GetEngine(ctx).ID(id).Get(key) if err != nil { @@ -203,12 +208,6 @@ func UpdateDeployKeyCols(key *DeployKey, cols ...string) error { return err } -// UpdateDeployKey updates deploy key information. -func UpdateDeployKey(key *DeployKey) error { - _, err := db.GetEngine(db.DefaultContext).ID(key.ID).AllCols().Update(key) - return err -} - // ListDeployKeysOptions are options for ListDeployKeys type ListDeployKeysOptions struct { db.ListOptions diff --git a/models/helper_environment.go b/models/helper_environment.go index 57ec3ea1e981..4cad1e5368bf 100644 --- a/models/helper_environment.go +++ b/models/helper_environment.go @@ -23,8 +23,8 @@ const ( EnvPusherName = "GITEA_PUSHER_NAME" EnvPusherEmail = "GITEA_PUSHER_EMAIL" EnvPusherID = "GITEA_PUSHER_ID" - EnvKeyID = "GITEA_KEY_ID" - EnvIsDeployKey = "GITEA_IS_DEPLOY_KEY" + EnvKeyID = "GITEA_KEY_ID" // public key ID + EnvDeployKeyID = "GITEA_DEPLOY_KEY_ID" EnvPRID = "GITEA_PR_ID" EnvIsInternal = "GITEA_INTERNAL_PUSH" EnvAppURL = "GITEA_ROOT_URL" diff --git a/models/repo.go b/models/repo.go index 38527c74dcb4..b854d7428145 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1308,7 +1308,7 @@ func LinkedRepository(a *repo_model.Attachment) (*repo_model.Repository, unit.Ty // DeleteDeployKey delete deploy keys func DeleteDeployKey(ctx context.Context, doer *user_model.User, id int64) error { - key, err := asymkey_model.GetDeployKeyByID(ctx, id) + key, err := asymkey_model.GetDeployKeyByIDCtx(ctx, id) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { return nil diff --git a/modules/private/hook.go b/modules/private/hook.go index fd864b1e6b04..559019344e45 100644 --- a/modules/private/hook.go +++ b/modules/private/hook.go @@ -56,7 +56,7 @@ type HookOptions struct { GitQuarantinePath string GitPushOptions GitPushOptions PullRequestID int64 - IsDeployKey bool + DeployKeyID int64 // if the pusher is a DeployKey, then UserID is the repo's org user. IsWiki bool } diff --git a/modules/private/serv.go b/modules/private/serv.go index e1204c23a749..2e1367e4c4f1 100644 --- a/modules/private/serv.go +++ b/modules/private/serv.go @@ -46,9 +46,9 @@ func ServNoCommand(ctx context.Context, keyID int64) (*asymkey_model.PublicKey, // ServCommandResults are the results of a call to the private route serv type ServCommandResults struct { IsWiki bool - IsDeployKey bool - KeyID int64 - KeyName string + DeployKeyID int64 + KeyID int64 // public key + KeyName string // this field is ambiguous, it can be the name of DeployKey, or the name of the PublicKey UserName string UserEmail string UserID int64 diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go index 669cc7c51cf8..8d3e6f86851d 100644 --- a/routers/api/v1/repo/key.go +++ b/routers/api/v1/repo/key.go @@ -144,7 +144,7 @@ func GetDeployKey(ctx *context.APIContext) { // "200": // "$ref": "#/responses/DeployKey" - key, err := asymkey_model.GetDeployKeyByID(db.DefaultContext, ctx.ParamsInt64(":id")) + key, err := asymkey_model.GetDeployKeyByID(ctx.ParamsInt64(":id")) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { ctx.NotFound() diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 85464deb294d..a827d03a01c4 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -12,6 +12,8 @@ import ( "strings" "code.gitea.io/gitea/models" + asymkey_model "code.gitea.io/gitea/models/asymkey" + perm_model "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" gitea_context "code.gitea.io/gitea/modules/context" @@ -24,8 +26,12 @@ import ( type preReceiveContext struct { *gitea_context.PrivateContext - user *user_model.User - perm models.Permission + + // loadedPusher indicates that where the following information are loaded + loadedPusher bool + user *user_model.User + userPerm models.Permission + deployKeyAccessMode perm_model.AccessMode canCreatePullRequest bool checkedCanCreatePullRequest bool @@ -41,62 +47,48 @@ type preReceiveContext struct { opts *private.HookOptions } -// User gets or loads User -func (ctx *preReceiveContext) User() *user_model.User { - if ctx.user == nil { - ctx.user, ctx.perm = loadUserAndPermission(ctx.PrivateContext, ctx.opts.UserID) - } - return ctx.user -} - -// Perm gets or loads Perm -func (ctx *preReceiveContext) Perm() *models.Permission { - if ctx.user == nil { - ctx.user, ctx.perm = loadUserAndPermission(ctx.PrivateContext, ctx.opts.UserID) - } - return &ctx.perm -} - -// CanWriteCode returns true if can write code +// CanWriteCode returns true if pusher can write code func (ctx *preReceiveContext) CanWriteCode() bool { if !ctx.checkedCanWriteCode { - ctx.canWriteCode = ctx.Perm().CanWrite(unit.TypeCode) + ctx.loadPusherAndPermission() + ctx.canWriteCode = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite ctx.checkedCanWriteCode = true } return ctx.canWriteCode } -// AssertCanWriteCode returns true if can write code +// AssertCanWriteCode returns true if pusher can write code func (ctx *preReceiveContext) AssertCanWriteCode() bool { if !ctx.CanWriteCode() { if ctx.Written() { return false } ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "User permission denied.", + "err": "User permission denied for writing.", }) return false } return true } -// CanCreatePullRequest returns true if can create pull requests +// CanCreatePullRequest returns true if pusher can create pull requests func (ctx *preReceiveContext) CanCreatePullRequest() bool { if !ctx.checkedCanCreatePullRequest { - ctx.canCreatePullRequest = ctx.Perm().CanRead(unit.TypePullRequests) + ctx.loadPusherAndPermission() + ctx.canCreatePullRequest = ctx.userPerm.CanRead(unit.TypePullRequests) ctx.checkedCanCreatePullRequest = true } return ctx.canCreatePullRequest } -// AssertCanCreatePullRequest returns true if can create pull requests +// AssertCreatePullRequest returns true if can create pull requests func (ctx *preReceiveContext) AssertCreatePullRequest() bool { if !ctx.CanCreatePullRequest() { if ctx.Written() { return false } ctx.JSON(http.StatusForbidden, map[string]interface{}{ - "err": "User permission denied.", + "err": "User permission denied for creating pull-request.", }) return false } @@ -246,7 +238,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN // 5. Check if the doer is allowed to push canPush := false - if ctx.opts.IsDeployKey { + if ctx.opts.DeployKeyID != 0 { canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) } else { canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx.opts.UserID) @@ -305,7 +297,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN // Now check if the user is allowed to merge PRs for this repository // Note: we can use ctx.perm and ctx.user directly as they will have been loaded above - allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.perm, ctx.user) + allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.userPerm, ctx.user) if err != nil { log.Error("Error calculating if allowed to merge: %v", err) ctx.JSON(http.StatusInternalServerError, private.Response{ @@ -323,7 +315,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN } // If we're an admin for the repository we can ignore status checks, reviews and override protected files - if ctx.perm.IsAdmin() { + if ctx.userPerm.IsAdmin() { return } @@ -450,17 +442,22 @@ func generateGitEnv(opts *private.HookOptions) (env []string) { return env } -func loadUserAndPermission(ctx *gitea_context.PrivateContext, id int64) (user *user_model.User, perm models.Permission) { - user, err := user_model.GetUserByID(id) +func (ctx *preReceiveContext) loadPusherAndPermission() { + if ctx.loadedPusher { + return + } + + user, err := user_model.GetUserByID(ctx.opts.UserID) if err != nil { - log.Error("Unable to get User id %d Error: %v", id, err) + 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", id, err), + Err: fmt.Sprintf("Unable to get User id %d Error: %v", ctx.opts.UserID, err), }) return } + ctx.user = user - perm, err = models.GetUserRepoPermission(ctx.Repo.Repository, user) + userPerm, err := models.GetUserRepoPermission(ctx.Repo.Repository, user) if err != nil { log.Error("Unable to get Repo permission of repo %s/%s of User %s", ctx.Repo.Repository.OwnerName, ctx.Repo.Repository.Name, user.Name, err) ctx.JSON(http.StatusInternalServerError, private.Response{ @@ -468,6 +465,19 @@ func loadUserAndPermission(ctx *gitea_context.PrivateContext, id int64) (user *u }) return } + ctx.userPerm = userPerm + + if ctx.opts.DeployKeyID != 0 { + deployKey, err := asymkey_model.GetDeployKeyByID(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), + }) + return + } + ctx.deployKeyAccessMode = deployKey.Mode + } - return + ctx.loadedPusher = true } diff --git a/routers/private/serv.go b/routers/private/serv.go index 65989d868be2..b0451df5d85e 100644 --- a/routers/private/serv.go +++ b/routers/private/serv.go @@ -229,8 +229,6 @@ func ServCommand(ctx *context.PrivateContext) { var deployKey *asymkey_model.DeployKey var user *user_model.User if key.Type == asymkey_model.KeyTypeDeploy { - results.IsDeployKey = true - var err error deployKey, err = asymkey_model.GetDeployKeyByRepo(key.ID, repo.ID) if err != nil { @@ -248,6 +246,7 @@ func ServCommand(ctx *context.PrivateContext) { }) return } + results.DeployKeyID = deployKey.ID results.KeyName = deployKey.Name // FIXME: Deploy keys aren't really the owner of the repo pushing changes @@ -410,9 +409,9 @@ func ServCommand(ctx *context.PrivateContext) { return } } - log.Debug("Serv Results:\nIsWiki: %t\nIsDeployKey: %t\nKeyID: %d\tKeyName: %s\nUserName: %s\nUserID: %d\nOwnerName: %s\nRepoName: %s\nRepoID: %d", + log.Debug("Serv Results:\nIsWiki: %t\nDeployKeyID: %d\nKeyID: %d\tKeyName: %s\nUserName: %s\nUserID: %d\nOwnerName: %s\nRepoName: %s\nRepoID: %d", results.IsWiki, - results.IsDeployKey, + results.DeployKeyID, results.KeyID, results.KeyName, results.UserName, diff --git a/routers/web/repo/http.go b/routers/web/repo/http.go index d5379b610ede..f3989be4b093 100644 --- a/routers/web/repo/http.go +++ b/routers/web/repo/http.go @@ -222,7 +222,6 @@ func httpBase(ctx *context.Context) (h *serviceHandler) { models.EnvRepoName + "=" + reponame, models.EnvPusherName + "=" + ctx.User.Name, models.EnvPusherID + fmt.Sprintf("=%d", ctx.User.ID), - models.EnvIsDeployKey + "=false", models.EnvAppURL + "=" + setting.AppURL, } From 34320d213d950bc969cb9bd95a812a5145ed5f03 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 6 Mar 2022 12:18:51 +0800 Subject: [PATCH 2/5] fix --- routers/private/hook_pre_receive.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index a827d03a01c4..7559481c164a 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -29,7 +29,7 @@ type preReceiveContext struct { // loadedPusher indicates that where the following information are loaded loadedPusher bool - user *user_model.User + user *user_model.User // it's the org user if a DeployKey is used userPerm models.Permission deployKeyAccessMode perm_model.AccessMode @@ -295,6 +295,8 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN return } + ctx.loadPusherAndPermission() + // Now check if the user is allowed to merge PRs for this repository // Note: we can use ctx.perm and ctx.user directly as they will have been loaded above allowedMerge, err := pull_service.IsUserAllowedToMerge(pr, ctx.userPerm, ctx.user) From 60ac321cf4b1b620f1f59362b1b2e9186488ecd8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 21 Mar 2022 02:56:53 +0800 Subject: [PATCH 3/5] improve error handling --- routers/private/hook_pre_receive.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 7559481c164a..50bf590e07a8 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -50,7 +50,9 @@ type preReceiveContext struct { // CanWriteCode returns true if pusher can write code func (ctx *preReceiveContext) CanWriteCode() bool { if !ctx.checkedCanWriteCode { - ctx.loadPusherAndPermission() + if !ctx.loadPusherAndPermission() { + return false + } ctx.canWriteCode = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite ctx.checkedCanWriteCode = true } @@ -74,7 +76,9 @@ func (ctx *preReceiveContext) AssertCanWriteCode() bool { // CanCreatePullRequest returns true if pusher can create pull requests func (ctx *preReceiveContext) CanCreatePullRequest() bool { if !ctx.checkedCanCreatePullRequest { - ctx.loadPusherAndPermission() + if !ctx.loadPusherAndPermission() { + return false + } ctx.canCreatePullRequest = ctx.userPerm.CanRead(unit.TypePullRequests) ctx.checkedCanCreatePullRequest = true } @@ -295,7 +299,11 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullN return } - ctx.loadPusherAndPermission() + // although we should have called `loadPusherAndPermission` before, here we call it explicitly again because we need to access ctx.user below + if !ctx.loadPusherAndPermission() { + // if error occurs, loadPusherAndPermission had written the error response + return + } // Now check if the user is allowed to merge PRs for this repository // Note: we can use ctx.perm and ctx.user directly as they will have been loaded above @@ -444,9 +452,10 @@ func generateGitEnv(opts *private.HookOptions) (env []string) { return env } -func (ctx *preReceiveContext) loadPusherAndPermission() { +// loadPusherAndPermission returns false if an error occurs, and it writes the error response +func (ctx *preReceiveContext) loadPusherAndPermission() bool { if ctx.loadedPusher { - return + return true } user, err := user_model.GetUserByID(ctx.opts.UserID) @@ -455,7 +464,7 @@ func (ctx *preReceiveContext) loadPusherAndPermission() { ctx.JSON(http.StatusInternalServerError, private.Response{ Err: fmt.Sprintf("Unable to get User id %d Error: %v", ctx.opts.UserID, err), }) - return + return false } ctx.user = user @@ -465,7 +474,7 @@ func (ctx *preReceiveContext) loadPusherAndPermission() { 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), }) - return + return false } ctx.userPerm = userPerm @@ -476,10 +485,11 @@ func (ctx *preReceiveContext) loadPusherAndPermission() { ctx.JSON(http.StatusInternalServerError, private.Response{ Err: fmt.Sprintf("Unable to get DeployKey id %d Error: %v", ctx.opts.DeployKeyID, err), }) - return + return false } ctx.deployKeyAccessMode = deployKey.Mode } ctx.loadedPusher = true + return true } From 828a803777021b4d733403dfa6a8b661f202e4ea Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 22 Mar 2022 10:47:58 +0800 Subject: [PATCH 4/5] use GetDeployKeyByID(ctx, id) --- models/asymkey/ssh_key_deploy.go | 7 +------ models/repo.go | 2 +- routers/api/v1/repo/key.go | 2 +- routers/private/hook_pre_receive.go | 3 ++- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index e6553dcca050..fe2ade43ae7e 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -167,12 +167,7 @@ func AddDeployKey(repoID int64, name, content string, readOnly bool) (*DeployKey } // GetDeployKeyByID returns deploy key by given ID. -func GetDeployKeyByID(id int64) (*DeployKey, error) { - return GetDeployKeyByIDCtx(db.DefaultContext, id) -} - -// GetDeployKeyByIDCtx returns deploy key by given ID. -func GetDeployKeyByIDCtx(ctx context.Context, id int64) (*DeployKey, error) { +func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) { key := new(DeployKey) has, err := db.GetEngine(ctx).ID(id).Get(key) if err != nil { diff --git a/models/repo.go b/models/repo.go index b15902d7f116..d20e5f81d31b 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1310,7 +1310,7 @@ func LinkedRepository(a *repo_model.Attachment) (*repo_model.Repository, unit.Ty // DeleteDeployKey delete deploy keys func DeleteDeployKey(ctx context.Context, doer *user_model.User, id int64) error { - key, err := asymkey_model.GetDeployKeyByIDCtx(ctx, id) + key, err := asymkey_model.GetDeployKeyByID(ctx, id) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { return nil diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go index 8d3e6f86851d..669cc7c51cf8 100644 --- a/routers/api/v1/repo/key.go +++ b/routers/api/v1/repo/key.go @@ -144,7 +144,7 @@ func GetDeployKey(ctx *context.APIContext) { // "200": // "$ref": "#/responses/DeployKey" - key, err := asymkey_model.GetDeployKeyByID(ctx.ParamsInt64(":id")) + key, err := asymkey_model.GetDeployKeyByID(db.DefaultContext, ctx.ParamsInt64(":id")) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { ctx.NotFound() diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 50bf590e07a8..ac5c044c158a 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models" asymkey_model "code.gitea.io/gitea/models/asymkey" + "code.gitea.io/gitea/models/db" perm_model "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -479,7 +480,7 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool { ctx.userPerm = userPerm if ctx.opts.DeployKeyID != 0 { - deployKey, err := asymkey_model.GetDeployKeyByID(ctx.opts.DeployKeyID) + deployKey, err := asymkey_model.GetDeployKeyByID(db.DefaultContext, 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{ From 0954a9b37bce56848ee88c2330a202297a69fd8b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Tue, 22 Mar 2022 08:16:16 +0100 Subject: [PATCH 5/5] use existing context --- routers/api/v1/repo/key.go | 2 +- routers/private/hook_pre_receive.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go index 35efa64ad3ac..568f92d7fbcb 100644 --- a/routers/api/v1/repo/key.go +++ b/routers/api/v1/repo/key.go @@ -144,7 +144,7 @@ func GetDeployKey(ctx *context.APIContext) { // "200": // "$ref": "#/responses/DeployKey" - key, err := asymkey_model.GetDeployKeyByID(db.DefaultContext, ctx.ParamsInt64(":id")) + key, err := asymkey_model.GetDeployKeyByID(ctx, ctx.ParamsInt64(":id")) if err != nil { if asymkey_model.IsErrDeployKeyNotExist(err) { ctx.NotFound() diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index ac5c044c158a..c6ea422287ed 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -13,7 +13,6 @@ import ( "code.gitea.io/gitea/models" asymkey_model "code.gitea.io/gitea/models/asymkey" - "code.gitea.io/gitea/models/db" perm_model "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -480,7 +479,7 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool { ctx.userPerm = userPerm if ctx.opts.DeployKeyID != 0 { - deployKey, err := asymkey_model.GetDeployKeyByID(db.DefaultContext, ctx.opts.DeployKeyID) + deployKey, err := asymkey_model.GetDeployKeyByID(ctx, 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{