diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 39836ab0165..7ea399dbfd9 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -31,9 +31,7 @@ import ( type preReceiveContext struct { *gitea_context.PrivateContext - // loadedPusher indicates that where the following information are loaded - loadedPusher bool - user *user_model.User // it's the org user if a DeployKey is used + user *user_model.User // the "pusher", it's the org user if a DeployKey is used userPerm access_model.Permission deployKeyAccessMode perm_model.AccessMode @@ -53,10 +51,7 @@ type preReceiveContext struct { func (ctx *preReceiveContext) canWriteCodeUnit() bool { if ctx.canWriteCodeUnitCached == nil { - var canWrite bool - if ctx.loadPusherAndPermission() { - canWrite = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite - } + canWrite := ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite ctx.canWriteCodeUnitCached = &canWrite } return *ctx.canWriteCodeUnitCached @@ -91,9 +86,6 @@ func (ctx *preReceiveContext) assertCanWriteRef(refFullName git.RefName) bool { // CanCreatePullRequest returns true if pusher can create pull requests func (ctx *preReceiveContext) CanCreatePullRequest() bool { if !ctx.checkedCanCreatePullRequest { - if !ctx.loadPusherAndPermission() { - return false - } ctx.canCreatePullRequest = ctx.userPerm.CanRead(unit.TypePullRequests) ctx.checkedCanCreatePullRequest = true } @@ -124,6 +116,10 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) { opts: opts, } + if !ourCtx.loadPusherAndPermission() { + return // if error occurs, loadPusherAndPermission had written the error response + } + // Iterate across the provided old commit IDs for i := range opts.OldCommitIDs { oldCommitID := opts.OldCommitIDs[i] @@ -281,18 +277,10 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r canPush = !changedProtectedfiles && protectBranch.CanPush && (!protectBranch.EnableWhitelist || protectBranch.WhitelistDeployKeys) } } else { - user, err := user_model.GetUserByID(ctx, ctx.opts.UserID) - if err != nil { - log.Error("Unable to GetUserByID for commits from %s to %s in %-v: %v", oldCommitID, newCommitID, repo, err) - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to GetUserByID for commits from %s to %s: %v", oldCommitID, newCommitID, err), - }) - return - } if isForcePush { - canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, user) + canPush = !changedProtectedfiles && protectBranch.CanUserForcePush(ctx, ctx.user) } else { - canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, user) + canPush = !changedProtectedfiles && protectBranch.CanUserPush(ctx, ctx.user) } } @@ -354,12 +342,6 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r return } - // 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 allowedMerge, err := pull_service.IsUserAllowedToMerge(ctx, pr, ctx.userPerm, ctx.user) @@ -499,10 +481,6 @@ func generateGitEnv(opts *private.HookOptions) (env []string) { // loadPusherAndPermission returns false if an error occurs, and it writes the error response func (ctx *preReceiveContext) loadPusherAndPermission() bool { - if ctx.loadedPusher { - return true - } - if ctx.opts.UserID == user_model.ActionsUserID { taskID := ctx.opts.ActionsTaskID ctx.user = user_model.NewActionsUserWithTaskID(taskID) @@ -555,7 +533,5 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool { } ctx.deployKeyAccessMode = deployKey.Mode } - - ctx.loadedPusher = true return true } diff --git a/routers/private/hook_pre_receive_test.go b/routers/private/hook_pre_receive_test.go index bea5e8974d8..3c1c21673f2 100644 --- a/routers/private/hook_pre_receive_test.go +++ b/routers/private/hook_pre_receive_test.go @@ -52,7 +52,6 @@ func TestPreReceiveCanWriteCodePerBranch(t *testing.T) { mockCtx, _ := contexttest.MockPrivateContext(t, "/") ctx := &preReceiveContext{ PrivateContext: mockCtx, - loadedPusher: true, user: maintainer, userPerm: headPerm, }