finn/forgejo
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Add "Allow edits from maintainer" feature (#18002)

Browse source 
Adds a feature [like GitHub has](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request-from-a-fork) (step 7).
If you create a new PR from a forked repo, you can select (and change later, but only if you are the PR creator/poster) the "Allow edits from maintainers" option.
Then users with write access to the base branch get more permissions on this branch:
* use the update pull request button
* push directly from the command line (`git push`)
* edit/delete/upload files via web UI
* use related API endpoints

You can't merge PRs to this branch with this enabled, you'll need "full" code write permissions.

This feature has a pretty big impact on the permission system. I might forgot changing some things or didn't find security vulnerabilities. In this case, please leave a review or comment on this PR.

Closes #17728

Co-authored-by: 6543 <6543@obermui.de>
This commit is contained in:
qwerty287 2022-04-28 17:45:33 +02:00 • committed by GitHub
parent 92dfbada37
commit 8eb1cd9264
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
29 changed files with 359 additions and 60 deletions
Download patch file Download diff file Expand all files Collapse all files

17
routers/api/v1/api.go
View file

@ -283,6 +283,15 @@ func reqRepoWriter(unitTypes ...unit.Type) func(ctx *context.APIContext) {
}
}
// reqRepoBranchWriter user should have a permission to write to a branch, or be a site admin
func reqRepoBranchWriter(ctx *context.APIContext) {
options, ok := web.GetForm(ctx).(api.FileOptionInterface)
if !ok || (!ctx.Repo.CanWriteToBranch(ctx.Doer, options.Branch()) && !ctx.IsUserSiteAdmin()) {
ctx.Error(http.StatusForbidden, "reqRepoBranchWriter", "user should have a permission to write to this branch")
return
}
}
// reqRepoReader user should have specific read permission or be a repo admin or a site admin
func reqRepoReader(unitType unit.Type) func(ctx *context.APIContext) {
return func(ctx *context.APIContext) {
@ -1021,10 +1030,10 @@ func Routes() *web.Route {
m.Get("", repo.GetContentsList)
m.Get("/*", repo.GetContents)
m.Group("/*", func() {
m.Post("", bind(api.CreateFileOptions{}), repo.CreateFile)
m.Put("", bind(api.UpdateFileOptions{}), repo.UpdateFile)
m.Delete("", bind(api.DeleteFileOptions{}), repo.DeleteFile)
}, reqRepoWriter(unit.TypeCode), reqToken())
m.Post("", bind(api.CreateFileOptions{}), reqRepoBranchWriter, repo.CreateFile)
m.Put("", bind(api.UpdateFileOptions{}), reqRepoBranchWriter, repo.UpdateFile)
m.Delete("", bind(api.DeleteFileOptions{}), reqRepoBranchWriter, repo.DeleteFile)
}, reqToken())
}, reqRepoReader(unit.TypeCode))
m.Get("/signing-key.gpg", misc.SigningKey)
m.Group("/topics", func() {

10
routers/api/v1/repo/file.go
View file

@ -173,8 +173,10 @@ func GetEditorconfig(ctx *context.APIContext) {
}
// canWriteFiles returns true if repository is editable and user has proper access level.
func canWriteFiles(r *context.Repository) bool {
return r.Permission.CanWrite(unit.TypeCode) && !r.Repository.IsMirror && !r.Repository.IsArchived
func canWriteFiles(ctx *context.APIContext, branch string) bool {
return ctx.Repo.Permission.CanWriteToBranch(ctx.Doer, branch) &&
!ctx.Repo.Repository.IsMirror &&
!ctx.Repo.Repository.IsArchived
}
// canReadFiles returns true if repository is readable and user has proper access level.
@ -376,7 +378,7 @@ func handleCreateOrUpdateFileError(ctx *context.APIContext, err error) {
// Called from both CreateFile or UpdateFile to handle both
func createOrUpdateFile(ctx *context.APIContext, opts *files_service.UpdateRepoFileOptions) (*api.FileResponse, error) {
if !canWriteFiles(ctx.Repo) {
if !canWriteFiles(ctx, opts.OldBranch) {
return nil, models.ErrUserDoesNotHaveAccessToRepo{
UserID: ctx.Doer.ID,
RepoName: ctx.Repo.Repository.LowerName,
@ -433,7 +435,7 @@ func DeleteFile(ctx *context.APIContext) {
// "$ref": "#/responses/error"
apiOpts := web.GetForm(ctx).(*api.DeleteFileOptions)
if !canWriteFiles(ctx.Repo) {
if !canWriteFiles(ctx, apiOpts.BranchName) {
ctx.Error(http.StatusForbidden, "DeleteFile", models.ErrUserDoesNotHaveAccessToRepo{
UserID: ctx.Doer.ID,
RepoName: ctx.Repo.Repository.LowerName,

2
routers/api/v1/repo/patch.go
View file

@ -77,7 +77,7 @@ func ApplyDiffPatch(ctx *context.APIContext) {
opts.Message = "apply-patch"
}
if !canWriteFiles(ctx.Repo) {
if !canWriteFiles(ctx, apiOpts.BranchName) {
ctx.Error(http.StatusInternalServerError, "ApplyPatch", models.ErrUserDoesNotHaveAccessToRepo{
UserID: ctx.Doer.ID,
RepoName: ctx.Repo.Repository.LowerName,

12
routers/api/v1/repo/pull.go
View file

@ -616,6 +616,18 @@ func EditPullRequest(ctx *context.APIContext) {
notification.NotifyPullRequestChangeTargetBranch(ctx.Doer, pr, form.Base)
}
// update allow edits
if form.AllowMaintainerEdit != nil {
if err := pull_service.SetAllowEdits(ctx, ctx.Doer, pr, *form.AllowMaintainerEdit); err != nil {
if errors.Is(pull_service.ErrUserHasNoPermissionForAction, err) {
ctx.Error(http.StatusForbidden, "SetAllowEdits", fmt.Sprintf("SetAllowEdits: %s", err))
return
}
ctx.ServerError("SetAllowEdits", err)
return
}
}
// Refetch from database
pr, err = models.GetPullRequestByIndex(ctx.Repo.Repository.ID, pr.Index)
if err != nil {

8
routers/private/hook_pre_receive.go
View file

@ -45,6 +45,8 @@ type preReceiveContext struct {
env []string
opts *private.HookOptions
branchName string
}
// CanWriteCode returns true if pusher can write code
@ -53,7 +55,7 @@ func (ctx *preReceiveContext) CanWriteCode() bool {
if !ctx.loadPusherAndPermission() {
return false
}
ctx.canWriteCode = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
ctx.canWriteCode = ctx.userPerm.CanWriteToBranch(ctx.user, ctx.branchName) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
ctx.checkedCanWriteCode = true
}
return ctx.canWriteCode
@ -134,13 +136,15 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
}
func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID, refFullName string) {
branchName := strings.TrimPrefix(refFullName, git.BranchPrefix)
ctx.branchName = branchName
if !ctx.AssertCanWriteCode() {
return
}
repo := ctx.Repo.Repository
gitRepo := ctx.Repo.GitRepo
branchName := strings.TrimPrefix(refFullName, git.BranchPrefix)
if branchName == repo.DefaultBranch && newCommitID == git.EmptySHA {
log.Warn("Forbidden: Branch: %s is the default branch in %-v and cannot be deleted", branchName, repo)

2
routers/web/repo/compare.go
View file

@ -398,6 +398,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
}
ctx.Data["HeadRepo"] = ci.HeadRepo
ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
// Now we need to assert that the ctx.Doer has permission to read
// the baseRepo's code and pulls
@ -436,6 +437,7 @@ func ParseCompareInfo(ctx *context.Context) *CompareInfo {
ctx.NotFound("ParseCompareInfo", nil)
return nil
}
ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
}
// If we have a rootRepo and it's different from:

15
routers/web/repo/issue.go
View file

@ -1529,7 +1529,7 @@ func ViewIssue(ctx *context.Context) {
if ctx.IsSigned {
if err := pull.LoadHeadRepoCtx(ctx); err != nil {
log.Error("LoadHeadRepo: %v", err)
} else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch {
} else if pull.HeadRepo != nil {
perm, err := models.GetUserRepoPermission(ctx, pull.HeadRepo, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserRepoPermission", err)
@ -1537,12 +1537,15 @@ func ViewIssue(ctx *context.Context) {
}
if perm.CanWrite(unit.TypeCode) {
// Check if branch is not protected
if protected, err := models.IsProtectedBranch(pull.HeadRepo.ID, pull.HeadBranch); err != nil {
log.Error("IsProtectedBranch: %v", err)
} else if !protected {
canDelete = true
ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup"
if pull.HeadBranch != pull.HeadRepo.DefaultBranch {
if protected, err := models.IsProtectedBranch(pull.HeadRepo.ID, pull.HeadBranch); err != nil {
log.Error("IsProtectedBranch: %v", err)
} else if !protected {
canDelete = true
ctx.Data["DeleteBranchLink"] = issue.Link() + "/cleanup"
}
}
ctx.Data["CanWriteToHeadRepo"] = true
}
}

45
routers/web/repo/pull.go
View file

@ -1132,14 +1132,15 @@ func CompareAndPullRequestPost(ctx *context.Context) {
Content: form.Content,
}
pullRequest := &models.PullRequest{
HeadRepoID: ci.HeadRepo.ID,
BaseRepoID: repo.ID,
HeadBranch: ci.HeadBranch,
BaseBranch: ci.BaseBranch,
HeadRepo: ci.HeadRepo,
BaseRepo: repo,
MergeBase: ci.CompareInfo.MergeBase,
Type: models.PullRequestGitea,
HeadRepoID: ci.HeadRepo.ID,
BaseRepoID: repo.ID,
HeadBranch: ci.HeadBranch,
BaseBranch: ci.BaseBranch,
HeadRepo: ci.HeadRepo,
BaseRepo: repo,
MergeBase: ci.CompareInfo.MergeBase,
Type: models.PullRequestGitea,
AllowMaintainerEdit: form.AllowMaintainerEdit,
}
// FIXME: check error in the case two people send pull request at almost same time, give nice error prompt
// instead of 500.
@ -1411,3 +1412,31 @@ func UpdatePullRequestTarget(ctx *context.Context) {
"base_branch": pr.BaseBranch,
})
}
// SetAllowEdits allow edits from maintainers to PRs
func SetAllowEdits(ctx *context.Context) {
form := web.GetForm(ctx).(*forms.UpdateAllowEditsForm)
pr, err := models.GetPullRequestByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
if err != nil {
if models.IsErrPullRequestNotExist(err) {
ctx.NotFound("GetPullRequestByIndex", err)
} else {
ctx.ServerError("GetPullRequestByIndex", err)
}
return
}
if err := pull_service.SetAllowEdits(ctx, ctx.Doer, pr, form.AllowMaintainerEdit); err != nil {
if errors.Is(pull_service.ErrUserHasNoPermissionForAction, err) {
ctx.Error(http.StatusForbidden)
return
}
ctx.ServerError("SetAllowEdits", err)
return
}
ctx.JSON(http.StatusOK, map[string]interface{}{
"allow_maintainer_edit": pr.AllowMaintainerEdit,
})
}

8
routers/web/repo/view.go
View file

@ -579,7 +579,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
ctx.Data["LineEscapeStatus"] = statuses
}
if !isLFSFile {
if ctx.Repo.CanEnableEditor() {
if ctx.Repo.CanEnableEditor(ctx.Doer) {
if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID {
ctx.Data["CanEditFile"] = false
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked")
@ -589,7 +589,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
}
} else if !ctx.Repo.IsViewBranch {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
} else if !ctx.Repo.CanWrite(unit_model.TypeCode) {
} else if !ctx.Repo.CanWriteToBranch(ctx.Doer, ctx.Repo.BranchName) {
ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit")
}
}
@ -629,7 +629,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
}
}
if ctx.Repo.CanEnableEditor() {
if ctx.Repo.CanEnableEditor(ctx.Doer) {
if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID {
ctx.Data["CanDeleteFile"] = false
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.this_file_locked")
@ -639,7 +639,7 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry, treeLink, rawLink st
}
} else if !ctx.Repo.IsViewBranch {
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch")
} else if !ctx.Repo.CanWrite(unit_model.TypeCode) {
} else if !ctx.Repo.CanWriteToBranch(ctx.Doer, ctx.Repo.BranchName) {
ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_have_write_access")
}
}

8
routers/web/web.go
View file

@ -574,6 +574,7 @@ func RegisterRoutes(m *web.Route) {
reqRepoAdmin := context.RequireRepoAdmin()
reqRepoCodeWriter := context.RequireRepoWriter(unit.TypeCode)
canEnableEditor := context.CanEnableEditor()
reqRepoCodeReader := context.RequireRepoReader(unit.TypeCode)
reqRepoReleaseWriter := context.RequireRepoWriter(unit.TypeReleases)
reqRepoReleaseReader := context.RequireRepoReader(unit.TypeReleases)
@ -925,12 +926,12 @@ func RegisterRoutes(m *web.Route) {
Post(bindIgnErr(forms.EditRepoFileForm{}), repo.NewDiffPatchPost)
m.Combo("/_cherrypick/{sha:([a-f0-9]{7,40})}/*").Get(repo.CherryPick).
Post(bindIgnErr(forms.CherryPickForm{}), repo.CherryPickPost)
}, context.RepoRefByType(context.RepoRefBranch), repo.MustBeEditable)
}, repo.MustBeEditable)
m.Group("", func() {
m.Post("/upload-file", repo.UploadFileToServer)
m.Post("/upload-remove", bindIgnErr(forms.RemoveUploadFileForm{}), repo.RemoveUploadFileFromServer)
}, context.RepoRef(), repo.MustBeEditable, repo.MustBeAbleToUpload)
}, context.RepoMustNotBeArchived(), reqRepoCodeWriter, repo.MustBeNotEmpty)
}, repo.MustBeEditable, repo.MustBeAbleToUpload)
}, context.RepoRef(), canEnableEditor, context.RepoMustNotBeArchived(), repo.MustBeNotEmpty)
m.Group("/branches", func() {
m.Group("/_new", func() {
@ -1105,6 +1106,7 @@ func RegisterRoutes(m *web.Route) {
m.Get("/commits", context.RepoRef(), repo.ViewPullCommits)
m.Post("/merge", context.RepoMustNotBeArchived(), bindIgnErr(forms.MergePullRequestForm{}), repo.MergePullRequest)
m.Post("/update", repo.UpdatePullRequest)
m.Post("/set_allow_maintainer_edit", bindIgnErr(forms.UpdateAllowEditsForm{}), repo.SetAllowEdits)
m.Post("/cleanup", context.RepoMustNotBeArchived(), context.RepoRef(), repo.CleanUpPullRequest)
m.Group("/files", func() {
m.Get("", context.RepoRef(), repo.SetEditorconfigIfExists, repo.SetDiffViewStyle, repo.SetWhitespaceBehavior, repo.ViewPullFiles)