Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaofficial/main'
Browse files Browse the repository at this point in the history
* giteaofficial/main:
  Set owner id to zero when GetRegistrationToken for repo (go-gitea#31725)
  fix(api): owner ID should be zero when created repo secret (go-gitea#31715)
  Fix API endpoint for registration-token (go-gitea#31722)
  Fix loadRepository error when access user dashboard (go-gitea#31719)
  Add permission check when creating PR (go-gitea#31033)
  • Loading branch information
zjjhot committed Jul 30, 2024
2 parents 5481812 + 81fa471 commit 7057df3
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 55 deletions.
4 changes: 4 additions & 0 deletions models/git/commit_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ func (status *CommitStatus) LocaleString(lang translation.Locale) string {

// HideActionsURL set `TargetURL` to an empty string if the status comes from Gitea Actions
func (status *CommitStatus) HideActionsURL(ctx context.Context) {
if status.RepoID == 0 {
return
}

if status.Repo == nil {
if err := status.loadRepository(ctx); err != nil {
log.Error("loadRepository: %v", err)
Expand Down
8 changes: 8 additions & 0 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"xorm.io/builder"
)

var ErrMustCollaborator = util.NewPermissionDeniedErrorf("user must be a collaborator")

// ErrPullRequestNotExist represents a "PullRequestNotExist" kind of error.
type ErrPullRequestNotExist struct {
ID int64
Expand Down Expand Up @@ -572,6 +574,12 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Iss
return nil
}

// ErrUserMustCollaborator represents an error that the user must be a collaborator to a given repo.
type ErrUserMustCollaborator struct {
UserID int64
RepoName string
}

// GetUnmergedPullRequest returns a pull request that is open and has not been merged
// by given head/base and repo/branch.
func GetUnmergedPullRequest(ctx context.Context, headRepoID, baseRepoID int64, headBranch, baseBranch string, flow PullRequestFlow) (*PullRequest, error) {
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1765,6 +1765,7 @@ compare.compare_head = compare
pulls.desc = Enable pull requests and code reviews.
pulls.new = New Pull Request
pulls.new.blocked_user = Cannot create pull request because you are blocked by the repository owner.
pulls.new.must_collaborator = You must be a collaborator to create pull request.
pulls.edit.already_changed = Unable to save changes to the pull request. It appears the content has already been changed by another user. Please refresh the page and try editing again to avoid overwriting their changes
pulls.view = View Pull Request
pulls.compare_changes = New Pull Request
Expand Down
10 changes: 4 additions & 6 deletions routers/api/v1/repo/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,11 @@ func (Action) CreateOrUpdateSecret(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

owner := ctx.Repo.Owner
repo := ctx.Repo.Repository

opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)

_, created, err := secret_service.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, ctx.PathParam("secretname"), opt.Data)
_, created, err := secret_service.CreateOrUpdateSecret(ctx, 0, repo.ID, ctx.PathParam("secretname"), opt.Data)
if err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
Expand Down Expand Up @@ -174,10 +173,9 @@ func (Action) DeleteSecret(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

owner := ctx.Repo.Owner
repo := ctx.Repo.Repository

err := secret_service.DeleteSecretByName(ctx, owner.ID, repo.ID, ctx.PathParam("secretname"))
err := secret_service.DeleteSecretByName(ctx, 0, repo.ID, ctx.PathParam("secretname"))
if err != nil {
if errors.Is(err, util.ErrInvalidArgument) {
ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
Expand Down Expand Up @@ -486,7 +484,7 @@ func (Action) ListVariables(ctx *context.APIContext) {

// GetRegistrationToken returns the token to register repo runners
func (Action) GetRegistrationToken(ctx *context.APIContext) {
// swagger:operation GET /repos/{owner}/{repo}/runners/registration-token repository repoGetRunnerRegistrationToken
// swagger:operation GET /repos/{owner}/{repo}/actions/runners/registration-token repository repoGetRunnerRegistrationToken
// ---
// summary: Get a repository's actions runner registration token
// produces:
Expand All @@ -506,7 +504,7 @@ func (Action) GetRegistrationToken(ctx *context.APIContext) {
// "200":
// "$ref": "#/responses/RegistrationToken"

shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID)
shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID)
}

var _ actions_service.API = new(Action)
Expand Down
2 changes: 2 additions & 0 deletions routers/api/v1/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ func CreatePullRequest(ctx *context.APIContext) {
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
} else if errors.Is(err, user_model.ErrBlockedUser) {
ctx.Error(http.StatusForbidden, "BlockedUser", err)
} else if errors.Is(err, issues_model.ErrMustCollaborator) {
ctx.Error(http.StatusForbidden, "MustCollaborator", err)
} else {
ctx.Error(http.StatusInternalServerError, "NewPullRequest", err)
}
Expand Down
10 changes: 10 additions & 0 deletions routers/web/repo/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,16 @@ func CompareAndPullRequestPost(ctx *context.Context) {
return
}
ctx.JSONError(flashError)
} else if errors.Is(err, issues_model.ErrMustCollaborator) {
flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{
"Message": ctx.Tr("repo.pulls.push_rejected"),
"Summary": ctx.Tr("repo.pulls.new.must_collaborator"),
})
if err != nil {
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
return
}
ctx.JSONError(flashError)
}
return
}
Expand Down
24 changes: 24 additions & 0 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/container"
Expand Down Expand Up @@ -48,6 +50,28 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
return user_model.ErrBlockedUser
}

// user should be a collaborator or a member of the organization for base repo
if !issue.Poster.IsAdmin {
canCreate, err := repo_model.IsOwnerMemberCollaborator(ctx, repo, issue.Poster.ID)
if err != nil {
return err
}

if !canCreate {
// or user should have write permission in the head repo
if err := pr.LoadHeadRepo(ctx); err != nil {
return err
}
perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, issue.Poster)
if err != nil {
return err
}
if !perm.CanWrite(unit.TypeCode) {
return issues_model.ErrMustCollaborator
}
}
}

prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
if err != nil {
if !git_model.IsErrBranchNotExist(err) {
Expand Down
66 changes: 33 additions & 33 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 22 additions & 16 deletions tests/integration/actions_trigger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import (
"time"

actions_model "code.gitea.io/gitea/models/actions"
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/perm"
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
"code.gitea.io/gitea/models/unittest"
Expand All @@ -34,7 +36,7 @@ import (
func TestPullRequestTargetEvent(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the base repo
org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the forked repo
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // owner of the forked repo

// create the base repo
baseRepo, err := repo_service.CreateRepository(db.DefaultContext, user2, user2, repo_service.CreateRepoOptions{
Expand All @@ -57,8 +59,12 @@ func TestPullRequestTargetEvent(t *testing.T) {
}}, nil)
assert.NoError(t, err)

// add user4 as the collaborator
ctx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository)
t.Run("AddUser4AsCollaboratorWithReadAccess", doAPIAddCollaborator(ctx, "user4", perm.AccessModeRead))

// create the forked repo
forkedRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, org3, repo_service.ForkRepoOptions{
forkedRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, user4, repo_service.ForkRepoOptions{
BaseRepo: baseRepo,
Name: "forked-repo-pull-request-target",
Description: "test pull-request-target event",
Expand Down Expand Up @@ -95,7 +101,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
assert.NotEmpty(t, addWorkflowToBaseResp)

// add a new file to the forked repo
addFileToForkedResp, err := files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, org3, &files_service.ChangeRepoFilesOptions{
addFileToForkedResp, err := files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user4, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
Expand All @@ -107,12 +113,12 @@ func TestPullRequestTargetEvent(t *testing.T) {
OldBranch: "main",
NewBranch: "fork-branch-1",
Author: &files_service.IdentityOptions{
Name: org3.Name,
Email: org3.Email,
Name: user4.Name,
Email: user4.Email,
},
Committer: &files_service.IdentityOptions{
Name: org3.Name,
Email: org3.Email,
Name: user4.Name,
Email: user4.Email,
},
Dates: &files_service.CommitDateOptions{
Author: time.Now(),
Expand All @@ -126,8 +132,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
pullIssue := &issues_model.Issue{
RepoID: baseRepo.ID,
Title: "Test pull-request-target-event",
PosterID: org3.ID,
Poster: org3,
PosterID: user4.ID,
Poster: user4,
IsPull: true,
}
pullRequest := &issues_model.PullRequest{
Expand All @@ -149,7 +155,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
assert.Equal(t, actions_module.GithubEventPullRequestTarget, actionRun.TriggerEvent)

// add another file whose name cannot match the specified path
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, org3, &files_service.ChangeRepoFilesOptions{
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user4, &files_service.ChangeRepoFilesOptions{
Files: []*files_service.ChangeRepoFile{
{
Operation: "create",
Expand All @@ -161,12 +167,12 @@ func TestPullRequestTargetEvent(t *testing.T) {
OldBranch: "main",
NewBranch: "fork-branch-2",
Author: &files_service.IdentityOptions{
Name: org3.Name,
Email: org3.Email,
Name: user4.Name,
Email: user4.Email,
},
Committer: &files_service.IdentityOptions{
Name: org3.Name,
Email: org3.Email,
Name: user4.Name,
Email: user4.Email,
},
Dates: &files_service.CommitDateOptions{
Author: time.Now(),
Expand All @@ -180,8 +186,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
pullIssue = &issues_model.Issue{
RepoID: baseRepo.ID,
Title: "A mismatched path cannot trigger pull-request-target-event",
PosterID: org3.ID,
Poster: org3,
PosterID: user4.ID,
Poster: user4,
IsPull: true,
}
pullRequest = &issues_model.PullRequest{
Expand Down
Loading

0 comments on commit 7057df3

Please sign in to comment.