Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure deploy key with write access can push #19010

Merged
merged 12 commits into from
Mar 22, 2022
4 changes: 2 additions & 2 deletions cmd/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cmd/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
6543 marked this conversation as resolved.
Show resolved Hide resolved
os.Setenv(models.EnvAppURL, setting.AppURL)

Expand Down
10 changes: 5 additions & 5 deletions integrations/api_private_serv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
15 changes: 7 additions & 8 deletions models/asymkey/ssh_key_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
6543 marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions models/helper_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.GetDeployKeyByID(ctx, id)
key, err := asymkey_model.GetDeployKeyByIDCtx(ctx, id)
if err != nil {
if asymkey_model.IsErrDeployKeyNotExist(err) {
return nil
Expand Down
2 changes: 1 addition & 1 deletion modules/private/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions modules/private/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
if asymkey_model.IsErrDeployKeyNotExist(err) {
ctx.NotFound()
Expand Down
82 changes: 47 additions & 35 deletions routers/private/hook_pre_receive.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 // it's the org user if a DeployKey is used
userPerm models.Permission
deployKeyAccessMode perm_model.AccessMode

canCreatePullRequest bool
checkedCanCreatePullRequest bool
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -303,9 +295,11 @@ 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.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{
Expand All @@ -323,7 +317,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
}

Expand Down Expand Up @@ -450,24 +444,42 @@ 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
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
}
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{
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
}
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
}
7 changes: 3 additions & 4 deletions routers/private/serv.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion routers/web/repo/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down