Skip to content

Commit d426213

Browse files
wxiaoguangwxiaoguang
committed
fix various bugs
1 parent 4c51acb commit d426213

File tree

15 files changed

+356
-49
lines changed

15 files changed

+356
-49
lines changed

‎routers/api/v1/api.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ func apiAuth(authMethod auth.Method) func(*context.APIContext) {
774774
return func(ctx *context.APIContext) {
775775
ar, err := common.AuthShared(ctx.Base, nil, authMethod)
776776
if err != nil {
777-
ctx.APIError(http.StatusUnauthorized, err)
777+
ctx.APIError(http.StatusUnauthorized, "invalid username or password")
778778
return
779779
}
780780
ctx.Doer = ar.Doer

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func CreateIssueDependency(ctx *context.APIContext) {
201201
return
202202
}
203203

204-
dependencyPerm := getPermissionForRepo(ctx, target.Repo)
204+
dependencyPerm := getPermissionForRepo(ctx, dependency.Repo)
205205
if ctx.Written() {
206206
return
207207
}
@@ -262,7 +262,7 @@ func RemoveIssueDependency(ctx *context.APIContext) {
262262
return
263263
}
264264

265-
dependencyPerm := getPermissionForRepo(ctx, target.Repo)
265+
dependencyPerm := getPermissionForRepo(ctx, dependency.Repo)
266266
if ctx.Written() {
267267
return
268268
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http"
88

99
repo_model "code.gitea.io/gitea/models/repo"
10+
unit_model "code.gitea.io/gitea/models/unit"
1011
"code.gitea.io/gitea/services/context"
1112
"code.gitea.io/gitea/services/convert"
1213
release_service "code.gitea.io/gitea/services/release"
@@ -58,6 +59,13 @@ func GetReleaseByTag(ctx *context.APIContext) {
5859
return
5960
}
6061

62+
if release.IsDraft { // only the users with write access can see draft releases
63+
if !ctx.IsSigned || !ctx.Repo.CanWrite(unit_model.TypeReleases) {
64+
ctx.APIErrorNotFound()
65+
return
66+
}
67+
}
68+
6169
if err = release.LoadAttributes(ctx); err != nil {
6270
ctx.APIErrorInternal(err)
6371
return

‎routers/web/repo/issue_content_history.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,12 +206,11 @@ func SoftDeleteContentHistory(ctx *context.Context) {
206206
ctx.NotFound(issues_model.ErrCommentNotExist{})
207207
return
208208
}
209+
if history.CommentID != commentID {
210+
ctx.NotFound(issues_model.ErrCommentNotExist{})
211+
return
212+
}
209213
if commentID != 0 {
210-
if history.CommentID != commentID {
211-
ctx.NotFound(issues_model.ErrCommentNotExist{})
212-
return
213-
}
214-
215214
if comment, err = issues_model.GetCommentByID(ctx, commentID); err != nil {
216215
log.Error("can not get comment for issue content history %v. err=%v", historyID, err)
217216
return

‎routers/web/web.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func webAuth(authMethod auth_service.Method) func(*context.Context) {
119119
ar, err := common.AuthShared(ctx.Base, ctx.Session, authMethod)
120120
if err != nil {
121121
log.Error("Failed to verify user: %v", err)
122-
ctx.HTTPError(http.StatusUnauthorized, "Failed to authenticate user")
122+
ctx.HTTPError(http.StatusUnauthorized, "invalid username or password")
123123
return
124124
}
125125
ctx.Doer = ar.Doer

‎services/convert/convert.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,9 @@ func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerifi
542542
}
543543
if verif.SigningUser != nil {
544544
commitVerification.Signer = &api.PayloadUser{
545-
Name: verif.SigningUser.Name,
546-
Email: verif.SigningUser.Email,
545+
UserName: verif.SigningUser.Name,
546+
Name: verif.SigningUser.DisplayName(),
547+
Email: verif.SigningEmail, // Use the email from the signature, not from the user profile
547548
}
548549
}
549550
return commitVerification

‎services/pull/merge.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,11 +547,15 @@ var escapedSymbols = regexp.MustCompile(`([*[?! \\])`)
547547

548548
// IsUserAllowedToMerge check if user is allowed to merge PR with given permissions and branch protections
549549
func IsUserAllowedToMerge(ctx context.Context, pr *issues_model.PullRequest, p access_model.Permission, user *user_model.User) (bool, error) {
550+
return isUserAllowedToMergeInRepoBranch(ctx, pr.BaseRepoID, pr.BaseBranch, p, user)
551+
}
552+
553+
func isUserAllowedToMergeInRepoBranch(ctx context.Context, repoID int64, branch string, p access_model.Permission, user *user_model.User) (bool, error) {
550554
if user == nil {
551555
return false, nil
552556
}
553557

554-
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
558+
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, repoID, branch)
555559
if err != nil {
556560
return false, err
557561
}

‎services/pull/update.go‎

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.
101101
}
102102

103103
// IsUserAllowedToUpdate check if user is allowed to update PR with given permissions and branch protections
104+
// update PR means send new commits to PR head branch from base branch
104105
func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest, user *user_model.User) (mergeAllowed, rebaseAllowed bool, err error) {
105106
if pull.Flow == issues_model.PullRequestFlowAGit {
106107
return false, false, nil
107108
}
108-
109109
if user == nil {
110110
return false, false, nil
111111
}
@@ -121,61 +121,55 @@ func IsUserAllowedToUpdate(ctx context.Context, pull *issues_model.PullRequest,
121121
return false, false, err
122122
}
123123

124-
pr := &issues_model.PullRequest{
125-
HeadRepoID: pull.BaseRepoID,
126-
HeadRepo: pull.BaseRepo,
127-
BaseRepoID: pull.HeadRepoID,
128-
BaseRepo: pull.HeadRepo,
129-
HeadBranch: pull.BaseBranch,
130-
BaseBranch: pull.HeadBranch,
131-
}
132-
133-
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch)
134-
if err != nil {
135-
return false, false, err
124+
// 1. check base repository's AllowRebaseUpdate configuration
125+
// it is a config in base repo but controls the head (fork) repo's "Update" behavior
126+
prBaseUnit, err := pull.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
127+
if repo_model.IsErrUnitTypeNotExist(err) {
128+
return false, false, nil // the PR unit is disabled in base repo
129+
} else if err != nil {
130+
return false, false, fmt.Errorf("get base repo unit: %v", err)
131+
} else {
132+
rebaseAllowed = prBaseUnit.PullRequestsConfig().AllowRebaseUpdate
136133
}
137134

138-
if err := pr.LoadBaseRepo(ctx); err != nil {
139-
return false, false, err
140-
}
141-
prUnit, err := pr.BaseRepo.GetUnit(ctx, unit.TypePullRequests)
142-
if err != nil {
143-
if repo_model.IsErrUnitTypeNotExist(err) {
144-
return false, false, nil
135+
// 2. check head branch protection whether rebase is allowed, if pb not found then rebase depends on the above setting
136+
{
137+
pb, err := git_model.GetFirstMatchProtectedBranchRule(ctx, pull.HeadRepoID, pull.HeadBranch)
138+
if err != nil {
139+
return false, false, err
145140
}
146-
log.Error("pr.BaseRepo.GetUnit(unit.TypePullRequests): %v", err)
147-
return false, false, err
148-
}
149-
150-
rebaseAllowed = prUnit.PullRequestsConfig().AllowRebaseUpdate
151-
152-
// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
153-
if pb != nil {
154-
pb.Repo = pull.BaseRepo
155-
if !pb.CanUserForcePush(ctx, user) {
156-
rebaseAllowed = false
141+
// If branch protected, disable rebase unless user is whitelisted to force push (which extends regular push)
142+
if pb != nil {
143+
pb.Repo = pull.HeadRepo
144+
rebaseAllowed = rebaseAllowed && pb.CanUserForcePush(ctx, user)
157145
}
158146
}
159147

148+
// 3. check whether user has write access to head branch
160149
baseRepoPerm, err := access_model.GetUserRepoPermission(ctx, pull.BaseRepo, user)
161150
if err != nil {
162151
return false, false, err
163152
}
164153

165-
mergeAllowed, err = IsUserAllowedToMerge(ctx, pr, headRepoPerm, user)
154+
mergeAllowed, err = isUserAllowedToMergeInRepoBranch(ctx, pull.HeadRepoID, pull.HeadBranch, headRepoPerm, user)
166155
if err != nil {
167156
return false, false, err
168157
}
169158

159+
// 4. if the pull creator allows maintainer to edit, it means the write permissions of the head branch has been
160+
// granted to the user with write permission of the base repository
170161
if pull.AllowMaintainerEdit {
171-
mergeAllowedMaintainer, err := IsUserAllowedToMerge(ctx, pr, baseRepoPerm, user)
162+
mergeAllowedMaintainer, err := isUserAllowedToMergeInRepoBranch(ctx, pull.BaseRepoID, pull.BaseBranch, baseRepoPerm, user)
172163
if err != nil {
173164
return false, false, err
174165
}
175166

176167
mergeAllowed = mergeAllowed || mergeAllowedMaintainer
177168
}
178169

170+
// if merge is not allowed, rebase is also not allowed
171+
rebaseAllowed = rebaseAllowed && mergeAllowed
172+
179173
return mergeAllowed, rebaseAllowed, nil
180174
}
181175

‎services/release/release.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re
361361
if err != nil {
362362
return fmt.Errorf("GetProtectedTags: %w", err)
363363
}
364-
isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, rel.PublisherID)
364+
isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, doer.ID)
365365
if err != nil {
366366
return err
367367
}

‎tests/integration/api_auth_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright 2025 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package integration
5+
6+
import (
7+
"net/http"
8+
"testing"
9+
10+
"code.gitea.io/gitea/tests"
11+
12+
"github.com/stretchr/testify/assert"
13+
)
14+
15+
func TestAPIAuth(t *testing.T) {
16+
defer tests.PrepareTestEnv(t)()
17+
18+
req := NewRequestf(t, "GET", "/api/v1/user").AddBasicAuth("user2")
19+
MakeRequest(t, req, http.StatusOK)
20+
21+
req = NewRequestf(t, "GET", "/api/v1/user").AddBasicAuth("user2", "wrong-password")
22+
resp := MakeRequest(t, req, http.StatusUnauthorized)
23+
assert.Contains(t, resp.Body.String(), `{"message":"invalid username or password"`)
24+
25+
req = NewRequestf(t, "GET", "/api/v1/user").AddBasicAuth("user-not-exist")
26+
resp = MakeRequest(t, req, http.StatusUnauthorized)
27+
assert.Contains(t, resp.Body.String(), `{"message":"invalid username or password"`)
28+
}

0 commit comments

Comments
 (0)