Skip to content

Commit 7057df3

Browse files
zjjhotzjjhot
committed
Merge remote-tracking branch 'giteaofficial/main'
* 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)
2 parents 5481812 + 81fa471 commit 7057df3

File tree

10 files changed

+168
-55
lines changed

10 files changed

+168
-55
lines changed

‎models/git/commit_status.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,10 @@ func (status *CommitStatus) LocaleString(lang translation.Locale) string {
211211

212212
// HideActionsURL set `TargetURL` to an empty string if the status comes from Gitea Actions
213213
func (status *CommitStatus) HideActionsURL(ctx context.Context) {
214+
if status.RepoID == 0 {
215+
return
216+
}
217+
214218
if status.Repo == nil {
215219
if err := status.loadRepository(ctx); err != nil {
216220
log.Error("loadRepository: %v", err)

‎models/issues/pull.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ import (
2727
"xorm.io/builder"
2828
)
2929

30+
var ErrMustCollaborator = util.NewPermissionDeniedErrorf("user must be a collaborator")
31+
3032
// ErrPullRequestNotExist represents a "PullRequestNotExist" kind of error.
3133
type ErrPullRequestNotExist struct {
3234
ID int64
@@ -572,6 +574,12 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *Iss
572574
return nil
573575
}
574576

577+
// ErrUserMustCollaborator represents an error that the user must be a collaborator to a given repo.
578+
type ErrUserMustCollaborator struct {
579+
UserID int64
580+
RepoName string
581+
}
582+
575583
// GetUnmergedPullRequest returns a pull request that is open and has not been merged
576584
// by given head/base and repo/branch.
577585
func GetUnmergedPullRequest(ctx context.Context, headRepoID, baseRepoID int64, headBranch, baseBranch string, flow PullRequestFlow) (*PullRequest, error) {

‎options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,6 +1765,7 @@ compare.compare_head = compare
17651765
pulls.desc = Enable pull requests and code reviews.
17661766
pulls.new = New Pull Request
17671767
pulls.new.blocked_user = Cannot create pull request because you are blocked by the repository owner.
1768+
pulls.new.must_collaborator = You must be a collaborator to create pull request.
17681769
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
17691770
pulls.view = View Pull Request
17701771
pulls.compare_changes = New Pull Request

‎routers/api/v1/repo/action.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,11 @@ func (Action) CreateOrUpdateSecret(ctx *context.APIContext) {
117117
// "404":
118118
// "$ref": "#/responses/notFound"
119119

120-
owner := ctx.Repo.Owner
121120
repo := ctx.Repo.Repository
122121

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

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

177-
owner := ctx.Repo.Owner
178176
repo := ctx.Repo.Repository
179177

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

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

509-
shared.GetRegistrationToken(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID)
507+
shared.GetRegistrationToken(ctx, 0, ctx.Repo.Repository.ID)
510508
}
511509

512510
var _ actions_service.API = new(Action)

‎routers/api/v1/repo/pull.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,8 @@ func CreatePullRequest(ctx *context.APIContext) {
535535
ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err)
536536
} else if errors.Is(err, user_model.ErrBlockedUser) {
537537
ctx.Error(http.StatusForbidden, "BlockedUser", err)
538+
} else if errors.Is(err, issues_model.ErrMustCollaborator) {
539+
ctx.Error(http.StatusForbidden, "MustCollaborator", err)
538540
} else {
539541
ctx.Error(http.StatusInternalServerError, "NewPullRequest", err)
540542
}

‎routers/web/repo/pull.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1337,6 +1337,16 @@ func CompareAndPullRequestPost(ctx *context.Context) {
13371337
return
13381338
}
13391339
ctx.JSONError(flashError)
1340+
} else if errors.Is(err, issues_model.ErrMustCollaborator) {
1341+
flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{
1342+
"Message": ctx.Tr("repo.pulls.push_rejected"),
1343+
"Summary": ctx.Tr("repo.pulls.new.must_collaborator"),
1344+
})
1345+
if err != nil {
1346+
ctx.ServerError("CompareAndPullRequest.HTMLString", err)
1347+
return
1348+
}
1349+
ctx.JSONError(flashError)
13401350
}
13411351
return
13421352
}

‎services/pull/pull.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@ import (
1717
"code.gitea.io/gitea/models/db"
1818
git_model "code.gitea.io/gitea/models/git"
1919
issues_model "code.gitea.io/gitea/models/issues"
20+
access_model "code.gitea.io/gitea/models/perm/access"
2021
repo_model "code.gitea.io/gitea/models/repo"
22+
"code.gitea.io/gitea/models/unit"
2123
user_model "code.gitea.io/gitea/models/user"
2224
"code.gitea.io/gitea/modules/base"
2325
"code.gitea.io/gitea/modules/container"
@@ -48,6 +50,28 @@ func NewPullRequest(ctx context.Context, repo *repo_model.Repository, issue *iss
4850
return user_model.ErrBlockedUser
4951
}
5052

53+
// user should be a collaborator or a member of the organization for base repo
54+
if !issue.Poster.IsAdmin {
55+
canCreate, err := repo_model.IsOwnerMemberCollaborator(ctx, repo, issue.Poster.ID)
56+
if err != nil {
57+
return err
58+
}
59+
60+
if !canCreate {
61+
// or user should have write permission in the head repo
62+
if err := pr.LoadHeadRepo(ctx); err != nil {
63+
return err
64+
}
65+
perm, err := access_model.GetUserRepoPermission(ctx, pr.HeadRepo, issue.Poster)
66+
if err != nil {
67+
return err
68+
}
69+
if !perm.CanWrite(unit.TypeCode) {
70+
return issues_model.ErrMustCollaborator
71+
}
72+
}
73+
}
74+
5175
prCtx, cancel, err := createTemporaryRepoForPR(ctx, pr)
5276
if err != nil {
5377
if !git_model.IsErrBranchNotExist(err) {

‎templates/swagger/v1_json.tmpl

Lines changed: 33 additions & 33 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/integration/actions_trigger_test.go

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import (
1111
"time"
1212

1313
actions_model "code.gitea.io/gitea/models/actions"
14+
auth_model "code.gitea.io/gitea/models/auth"
1415
"code.gitea.io/gitea/models/db"
1516
git_model "code.gitea.io/gitea/models/git"
1617
issues_model "code.gitea.io/gitea/models/issues"
18+
"code.gitea.io/gitea/models/perm"
1719
repo_model "code.gitea.io/gitea/models/repo"
1820
unit_model "code.gitea.io/gitea/models/unit"
1921
"code.gitea.io/gitea/models/unittest"
@@ -34,7 +36,7 @@ import (
3436
func TestPullRequestTargetEvent(t *testing.T) {
3537
onGiteaRun(t, func(t *testing.T, u *url.URL) {
3638
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the base repo
37-
org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) // owner of the forked repo
39+
user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // owner of the forked repo
3840

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

62+
// add user4 as the collaborator
63+
ctx := NewAPITestContext(t, baseRepo.OwnerName, baseRepo.Name, auth_model.AccessTokenScopeWriteRepository)
64+
t.Run("AddUser4AsCollaboratorWithReadAccess", doAPIAddCollaborator(ctx, "user4", perm.AccessModeRead))
65+
6066
// create the forked repo
61-
forkedRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, org3, repo_service.ForkRepoOptions{
67+
forkedRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, user4, repo_service.ForkRepoOptions{
6268
BaseRepo: baseRepo,
6369
Name: "forked-repo-pull-request-target",
6470
Description: "test pull-request-target event",
@@ -95,7 +101,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
95101
assert.NotEmpty(t, addWorkflowToBaseResp)
96102

97103
// add a new file to the forked repo
98-
addFileToForkedResp, err := files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, org3, &files_service.ChangeRepoFilesOptions{
104+
addFileToForkedResp, err := files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user4, &files_service.ChangeRepoFilesOptions{
99105
Files: []*files_service.ChangeRepoFile{
100106
{
101107
Operation: "create",
@@ -107,12 +113,12 @@ func TestPullRequestTargetEvent(t *testing.T) {
107113
OldBranch: "main",
108114
NewBranch: "fork-branch-1",
109115
Author: &files_service.IdentityOptions{
110-
Name: org3.Name,
111-
Email: org3.Email,
116+
Name: user4.Name,
117+
Email: user4.Email,
112118
},
113119
Committer: &files_service.IdentityOptions{
114-
Name: org3.Name,
115-
Email: org3.Email,
120+
Name: user4.Name,
121+
Email: user4.Email,
116122
},
117123
Dates: &files_service.CommitDateOptions{
118124
Author: time.Now(),
@@ -126,8 +132,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
126132
pullIssue := &issues_model.Issue{
127133
RepoID: baseRepo.ID,
128134
Title: "Test pull-request-target-event",
129-
PosterID: org3.ID,
130-
Poster: org3,
135+
PosterID: user4.ID,
136+
Poster: user4,
131137
IsPull: true,
132138
}
133139
pullRequest := &issues_model.PullRequest{
@@ -149,7 +155,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
149155
assert.Equal(t, actions_module.GithubEventPullRequestTarget, actionRun.TriggerEvent)
150156

151157
// add another file whose name cannot match the specified path
152-
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, org3, &files_service.ChangeRepoFilesOptions{
158+
addFileToForkedResp, err = files_service.ChangeRepoFiles(git.DefaultContext, forkedRepo, user4, &files_service.ChangeRepoFilesOptions{
153159
Files: []*files_service.ChangeRepoFile{
154160
{
155161
Operation: "create",
@@ -161,12 +167,12 @@ func TestPullRequestTargetEvent(t *testing.T) {
161167
OldBranch: "main",
162168
NewBranch: "fork-branch-2",
163169
Author: &files_service.IdentityOptions{
164-
Name: org3.Name,
165-
Email: org3.Email,
170+
Name: user4.Name,
171+
Email: user4.Email,
166172
},
167173
Committer: &files_service.IdentityOptions{
168-
Name: org3.Name,
169-
Email: org3.Email,
174+
Name: user4.Name,
175+
Email: user4.Email,
170176
},
171177
Dates: &files_service.CommitDateOptions{
172178
Author: time.Now(),
@@ -180,8 +186,8 @@ func TestPullRequestTargetEvent(t *testing.T) {
180186
pullIssue = &issues_model.Issue{
181187
RepoID: baseRepo.ID,
182188
Title: "A mismatched path cannot trigger pull-request-target-event",
183-
PosterID: org3.ID,
184-
Poster: org3,
189+
PosterID: user4.ID,
190+
Poster: user4,
185191
IsPull: true,
186192
}
187193
pullRequest = &issues_model.PullRequest{

0 commit comments

Comments
 (0)