Skip to content

Commit d42ace7

Browse files
lunnylunny
committed
Fix bugs
1 parent b2c24a7 commit d42ace7

File tree

8 files changed

+88
-32
lines changed

8 files changed

+88
-32
lines changed

‎models/repo/transfer.go

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ type RepoTransfer struct { //nolint
6868
RecipientID int64
6969
Recipient *user_model.User `xorm:"-"`
7070
RepoID int64
71+
Repo *Repository `xorm:"-"`
7172
TeamIDs []int64
7273
Teams []*organization.Team `xorm:"-"`
7374

@@ -79,48 +80,65 @@ func init() {
7980
db.RegisterModel(new(RepoTransfer))
8081
}
8182

82-
// LoadAttributes fetches the transfer recipient from the database
83-
func (r *RepoTransfer) LoadAttributes(ctx context.Context) error {
83+
func (r *RepoTransfer) LoadRecipient(ctx context.Context) error {
8484
if r.Recipient == nil {
8585
u, err := user_model.GetUserByID(ctx, r.RecipientID)
8686
if err != nil {
8787
return err
8888
}
89-
9089
r.Recipient = u
9190
}
9291

93-
if r.Recipient.IsOrganization() && len(r.TeamIDs) != len(r.Teams) {
94-
for _, v := range r.TeamIDs {
95-
team, err := organization.GetTeamByID(ctx, v)
96-
if err != nil {
97-
return err
98-
}
92+
return nil
93+
}
9994

100-
if team.OrgID != r.Recipient.ID {
101-
return fmt.Errorf("team %d belongs not to org %d", v, r.Recipient.ID)
102-
}
95+
func (r *RepoTransfer) LoadRepo(ctx context.Context) error {
96+
if r.Repo == nil {
97+
repo, err := GetRepositoryByID(ctx, r.RepoID)
98+
if err != nil {
99+
return err
100+
}
101+
r.Repo = repo
102+
}
103+
104+
return nil
105+
}
106+
107+
// LoadAttributes fetches the transfer recipient from the database
108+
func (r *RepoTransfer) LoadAttributes(ctx context.Context) error {
109+
if err := r.LoadRecipient(ctx); err != nil {
110+
return err
111+
}
103112

113+
if r.Recipient.IsOrganization() && r.Teams == nil {
114+
teamsMap, err := organization.GetTeamsByIDs(ctx, r.TeamIDs)
115+
if err != nil {
116+
return err
117+
}
118+
for _, team := range teamsMap {
104119
r.Teams = append(r.Teams, team)
105120
}
106121
}
107122

123+
if err := r.LoadRepo(ctx); err != nil {
124+
return err
125+
}
126+
108127
if r.Doer == nil {
109128
u, err := user_model.GetUserByID(ctx, r.DoerID)
110129
if err != nil {
111130
return err
112131
}
113-
114132
r.Doer = u
115133
}
116134

117135
return nil
118136
}
119137

120-
// CanUserAcceptOrRejectTransfer checks if the user has the rights to accept/decline a repo transfer.
138+
// CanUserAcceptTransfer checks if the user has the rights to accept/decline a repo transfer.
121139
// For user, it checks if it's himself
122140
// For organizations, it checks if the user is able to create repos
123-
func (r *RepoTransfer) CanUserAcceptOrRejectTransfer(ctx context.Context, u *user_model.User) bool {
141+
func (r *RepoTransfer) CanUserAcceptTransfer(ctx context.Context, u *user_model.User) bool {
124142
if err := r.LoadAttributes(ctx); err != nil {
125143
log.Error("LoadAttributes: %v", err)
126144
return false
@@ -139,6 +157,32 @@ func (r *RepoTransfer) CanUserAcceptOrRejectTransfer(ctx context.Context, u *use
139157
return allowed
140158
}
141159

160+
func (r *RepoTransfer) CanUserCancelTransfer(ctx context.Context, u *user_model.User) bool {
161+
if r.DoerID == u.ID { // sender can cancel the transfer
162+
return true
163+
}
164+
if err := r.Repo.LoadOwner(ctx); err != nil {
165+
log.Error("LoadOwner: %v", err)
166+
return false
167+
}
168+
if !r.Repo.Owner.IsOrganization() {
169+
if u.ID == r.Repo.OwnerID { // owner can cancel the transfer
170+
return true
171+
}
172+
} else {
173+
allowed, err := organization.CanCreateOrgRepo(ctx, r.Repo.OwnerID, u.ID)
174+
if err != nil {
175+
log.Error("CanCreateOrgRepo: %v", err)
176+
return false
177+
}
178+
if allowed {
179+
return true
180+
}
181+
}
182+
183+
return r.CanUserAcceptTransfer(ctx, u) // the user who can accept the transfer can also reject it
184+
}
185+
142186
type PendingRepositoryTransferOptions struct {
143187
RepoID int64
144188
SenderID int64

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

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
user_model "code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/log"
1717
api "code.gitea.io/gitea/modules/structs"
18+
"code.gitea.io/gitea/modules/util"
1819
"code.gitea.io/gitea/modules/web"
1920
"code.gitea.io/gitea/services/context"
2021
"code.gitea.io/gitea/services/convert"
@@ -162,11 +163,15 @@ func AcceptTransfer(ctx *context.APIContext) {
162163
// "$ref": "#/responses/notFound"
163164

164165
err := repo_service.AcceptTransferOwnership(ctx, ctx.Repo.Repository, ctx.Doer)
165-
if ctx.Written() {
166-
return
167-
}
168166
if err != nil {
169-
ctx.Error(http.StatusInternalServerError, "acceptOrRejectRepoTransfer", err)
167+
switch {
168+
case repo_model.IsErrNoPendingTransfer(err):
169+
ctx.Error(http.StatusNotFound, "AcceptTransferOwnership", err)
170+
case errors.Is(err, util.ErrPermissionDenied):
171+
ctx.Error(http.StatusForbidden, "AcceptTransferOwnership", err)
172+
default:
173+
ctx.Error(http.StatusInternalServerError, "AcceptTransferOwnership", err)
174+
}
170175
return
171176
}
172177

@@ -199,9 +204,16 @@ func RejectTransfer(ctx *context.APIContext) {
199204
// "404":
200205
// "$ref": "#/responses/notFound"
201206

202-
err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
207+
err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
203208
if err != nil {
204-
ctx.Error(http.StatusInternalServerError, "acceptOrRejectRepoTransfer", err)
209+
switch {
210+
case repo_model.IsErrNoPendingTransfer(err):
211+
ctx.Error(http.StatusNotFound, "CancelRepositoryTransfer", err)
212+
case errors.Is(err, util.ErrPermissionDenied):
213+
ctx.Error(http.StatusForbidden, "CancelRepositoryTransfer", err)
214+
default:
215+
ctx.Error(http.StatusInternalServerError, "CancelRepositoryTransfer", err)
216+
}
205217
return
206218
}
207219

‎routers/web/repo/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ func Action(ctx *context.Context) {
328328
}
329329
ctx.Redirect(ctx.Repo.Repository.Link())
330330
case "reject_transfer":
331-
err = repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
331+
err = repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
332332
if err == nil {
333333
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected"))
334334
}

‎routers/web/repo/setting/setting.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -821,7 +821,6 @@ func SettingsPost(ctx *context.Context) {
821821
} else {
822822
ctx.ServerError("GetPendingRepositoryTransfer", err)
823823
}
824-
825824
return
826825
}
827826

@@ -830,7 +829,7 @@ func SettingsPost(ctx *context.Context) {
830829
return
831830
}
832831

833-
if err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer); err != nil {
832+
if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer); err != nil {
834833
ctx.ServerError("CancelRepositoryTransfer", err)
835834
return
836835
}

‎services/context/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ func RepoAssignment(ctx *Context) {
719719

720720
ctx.Data["RepoTransfer"] = repoTransfer
721721
if ctx.Doer != nil {
722-
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer)
722+
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer)
723723
}
724724
}
725725

‎services/repository/transfer.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, d
4848
return err
4949
}
5050

51-
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
51+
if !repoTransfer.CanUserAcceptTransfer(ctx, doer) {
5252
return util.ErrPermissionDenied
5353
}
5454

@@ -452,9 +452,10 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
452452
return nil
453453
}
454454

455-
// RejectRepositoryTransfer marks the repository as ready and remove pending transfer entry,
455+
// CancelRepositoryTransfer marks the repository as ready and remove pending transfer entry,
456456
// thus cancel the transfer process.
457-
func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
457+
// Both the sender and the accepter can cancel the transfer.
458+
func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
458459
return db.WithTx(ctx, func(ctx context.Context) error {
459460
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo)
460461
if err != nil {
@@ -465,7 +466,7 @@ func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository,
465466
return err
466467
}
467468

468-
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
469+
if !repoTransfer.CanUserCancelTransfer(ctx, doer) {
469470
return util.ErrPermissionDenied
470471
}
471472

‎services/repository/transfer_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func TestRepositoryTransfer(t *testing.T) {
9393
assert.NotNil(t, transfer)
9494

9595
// Cancel transfer
96-
assert.NoError(t, RejectRepositoryTransfer(db.DefaultContext, repo, doer))
96+
assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo, doer))
9797

9898
transfer, err = repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
9999
assert.Error(t, err)
@@ -122,6 +122,6 @@ func TestRepositoryTransfer(t *testing.T) {
122122
assert.Error(t, err)
123123

124124
// Cancel transfer
125-
err = RejectRepositoryTransfer(db.DefaultContext, repo2, doer)
125+
err = CancelRepositoryTransfer(db.DefaultContext, repo2, doer)
126126
assert.True(t, repo_model.IsErrNoPendingTransfer(err))
127127
}

‎services/user/block.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func cancelRepositoryTransfers(ctx context.Context, doer, sender, recipient *use
207207
return err
208208
}
209209

210-
if err := repo_service.RejectRepositoryTransfer(ctx, repo, doer); err != nil {
210+
if err := repo_service.CancelRepositoryTransfer(ctx, repo, doer); err != nil {
211211
return err
212212
}
213213
}

0 commit comments

Comments
 (0)