Skip to content

Commit

Permalink
Fix bug
Browse files Browse the repository at this point in the history
  • Loading branch information
lunny committed Jan 13, 2025
1 parent 07d1a37 commit 45ffedd
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 48 deletions.
30 changes: 2 additions & 28 deletions models/repo/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,10 @@ func (r *RepoTransfer) LoadAttributes(ctx context.Context) error {
return nil
}

// CanUserAcceptTransfer checks if the user has the rights to accept/decline a repo transfer.
// CanUserAcceptOrRejectTransfer checks if the user has the rights to accept/decline a repo transfer.
// For user, it checks if it's himself
// For organizations, it checks if the user is able to create repos
func (r *RepoTransfer) CanUserAcceptTransfer(ctx context.Context, u *user_model.User) bool {
func (r *RepoTransfer) CanUserAcceptOrRejectTransfer(ctx context.Context, u *user_model.User) bool {
if err := r.LoadAttributes(ctx); err != nil {
log.Error("LoadAttributes: %v", err)
return false
Expand All @@ -157,32 +157,6 @@ func (r *RepoTransfer) CanUserAcceptTransfer(ctx context.Context, u *user_model.
return allowed
}

func (r *RepoTransfer) CanUserCancelTransfer(ctx context.Context, u *user_model.User) bool {
if r.DoerID == u.ID { // sender can cancel the transfer
return true
}
if err := r.Repo.LoadOwner(ctx); err != nil {
log.Error("LoadOwner: %v", err)
return false
}
if !r.Repo.Owner.IsOrganization() {
if u.ID == r.Repo.OwnerID { // owner can cancel the transfer
return true
}
} else {
allowed, err := organization.CanCreateOrgRepo(ctx, r.Repo.OwnerID, u.ID)
if err != nil {
log.Error("CanCreateOrgRepo: %v", err)
return false
}
if allowed {
return true
}
}

return r.CanUserAcceptTransfer(ctx, u) // the user who can accept the transfer can also reject it
}

type PendingRepositoryTransferOptions struct {
RepoID int64
SenderID int64
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func RejectTransfer(ctx *context.APIContext) {
// "404":
// "$ref": "#/responses/notFound"

err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
err := repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
if err != nil {
switch {
case repo_model.IsErrNoPendingTransfer(err):
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ func Action(ctx *context.Context) {
}
ctx.Redirect(ctx.Repo.Repository.Link())
case "reject_transfer":
err = repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
err = repo_service.RejectRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer)
if err == nil {
ctx.Flash.Success(ctx.Tr("repo.settings.transfer.rejected"))
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ func SettingsPost(ctx *context.Context) {
return
}

if err := repo_service.CancelRepositoryTransfer(ctx, ctx.Repo.Repository, ctx.Doer); err != nil {
if err := repo_service.CancelRepositoryTransfer(ctx, repoTransfer, ctx.Doer); err != nil {
ctx.ServerError("CancelRepositoryTransfer", err)
return
}
Expand Down
3 changes: 2 additions & 1 deletion services/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,8 @@ func RepoAssignment(ctx *Context) {

ctx.Data["RepoTransfer"] = repoTransfer
if ctx.Doer != nil {
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptTransfer(ctx, ctx.Doer)
ctx.Data["CanUserAcceptTransfer"] = repoTransfer.CanUserAcceptOrRejectTransfer(ctx, ctx.Doer)
ctx.Data["CanUserRejectTransfer"] = ctx.Data["CanUserAcceptTransfer"]
}
}

Expand Down
58 changes: 53 additions & 5 deletions services/repository/transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func AcceptTransferOwnership(ctx context.Context, repo *repo_model.Repository, d
return err
}

if !repoTransfer.CanUserAcceptTransfer(ctx, doer) {
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
return util.ErrPermissionDenied
}

Expand Down Expand Up @@ -452,10 +452,10 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
return nil
}

// CancelRepositoryTransfer marks the repository as ready and remove pending transfer entry,
// RejectRepositoryTransfer marks the repository as ready and remove pending transfer entry,
// thus cancel the transfer process.
// Both the sender and the accepter can cancel the transfer.
func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
// The accepter can reject the transfer.
func RejectRepositoryTransfer(ctx context.Context, repo *repo_model.Repository, doer *user_model.User) error {
return db.WithTx(ctx, func(ctx context.Context) error {
repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, repo)
if err != nil {
Expand All @@ -466,7 +466,7 @@ func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository,
return err
}

if !repoTransfer.CanUserCancelTransfer(ctx, doer) {
if !repoTransfer.CanUserAcceptOrRejectTransfer(ctx, doer) {
return util.ErrPermissionDenied
}

Expand All @@ -478,3 +478,51 @@ func CancelRepositoryTransfer(ctx context.Context, repo *repo_model.Repository,
return repo_model.DeleteRepositoryTransfer(ctx, repo.ID)
})
}

func canUserCancelTransfer(ctx context.Context, r *repo_model.RepoTransfer, u *user_model.User) bool {
if u.ID == r.DoerID {
return true
}

if err := r.LoadAttributes(ctx); err != nil {
log.Error("LoadAttributes: %v", err)
return false
}

if err := r.Repo.LoadOwner(ctx); err != nil {
log.Error("LoadOwner: %v", err)
return false
}

if !r.Repo.Owner.IsOrganization() {
return r.Repo.OwnerID == u.ID
}

perm, err := access_model.GetUserRepoPermission(ctx, r.Repo, u)
if err != nil {
log.Error("GetUserRepoPermission: %v", err)
return false
}
return perm.IsAdmin()
}

// CancelRepositoryTransfer cancels the repository transfer process. The sender or
// the users who have admin permission of the original repository can cancel the transfer
func CancelRepositoryTransfer(ctx context.Context, repoTransfer *repo_model.RepoTransfer, doer *user_model.User) error {
return db.WithTx(ctx, func(ctx context.Context) error {
if err := repoTransfer.LoadAttributes(ctx); err != nil {
return err
}

if !canUserCancelTransfer(ctx, repoTransfer, doer) {
return util.ErrPermissionDenied
}

repoTransfer.Repo.Status = repo_model.RepositoryReady
if err := repo_model.UpdateRepositoryCols(ctx, repoTransfer.Repo, "status"); err != nil {
return err
}

return repo_model.DeleteRepositoryTransfer(ctx, repoTransfer.RepoID)
})
}
6 changes: 3 additions & 3 deletions services/repository/transfer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func TestRepositoryTransfer(t *testing.T) {
assert.NotNil(t, transfer)

// Cancel transfer
assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, repo, doer))
assert.NoError(t, CancelRepositoryTransfer(db.DefaultContext, transfer, doer))

transfer, err = repo_model.GetPendingRepositoryTransfer(db.DefaultContext, repo)
assert.Error(t, err)
Expand Down Expand Up @@ -121,7 +121,7 @@ func TestRepositoryTransfer(t *testing.T) {
err = repo_model.CreatePendingRepositoryTransfer(db.DefaultContext, doer, &user_model.User{ID: 1000, LowerName: "user1000"}, repo2.ID, nil)
assert.Error(t, err)

// Cancel transfer
err = CancelRepositoryTransfer(db.DefaultContext, repo2, doer)
// Reject transfer
err = RejectRepositoryTransfer(db.DefaultContext, repo2, doer)
assert.True(t, repo_model.IsErrNoPendingTransfer(err))
}
7 changes: 1 addition & 6 deletions services/user/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,7 @@ func cancelRepositoryTransfers(ctx context.Context, doer, sender, recipient *use
}

for _, transfer := range transfers {
repo, err := repo_model.GetRepositoryByID(ctx, transfer.RepoID)
if err != nil {
return err
}

if err := repo_service.CancelRepositoryTransfer(ctx, repo, doer); err != nil {
if err := repo_service.CancelRepositoryTransfer(ctx, transfer, doer); err != nil {
return err
}
}
Expand Down
4 changes: 2 additions & 2 deletions templates/repo/header.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@
</form>
<form method="post" action="{{$.RepoLink}}/action/reject_transfer?redirect_to={{$.RepoLink}}">
{{$.CsrfTokenHtml}}
<div data-tooltip-content="{{if $.CanUserAcceptTransfer}}{{ctx.Locale.Tr "repo.transfer.reject_desc" $.RepoTransfer.Recipient.DisplayName}}{{else}}{{ctx.Locale.Tr "repo.transfer.no_permission_to_reject"}}{{end}}">
<button type="submit" class="ui basic button {{if $.CanUserAcceptTransfer}}red {{end}}ok small"{{if not $.CanUserAcceptTransfer}} disabled{{end}}>
<div data-tooltip-content="{{if $.CanUserRejectTransfer}}{{ctx.Locale.Tr "repo.transfer.reject_desc" $.RepoTransfer.Recipient.DisplayName}}{{else}}{{ctx.Locale.Tr "repo.transfer.no_permission_to_reject"}}{{end}}">
<button type="submit" class="ui basic button {{if $.CanUserRejectTransfer}}red {{end}}ok small"{{if not $.CanUserRejectTransfer}} disabled{{end}}>
{{ctx.Locale.Tr "repo.transfer.reject"}}
</button>
</div>
Expand Down

0 comments on commit 45ffedd

Please sign in to comment.