Skip to content

Commit a4148c0

Browse files
authored
Repository transfer has to be confirmed, if user can not create repo for new owner (#14792)
* make repo as "pending transfer" if on transfer start doer has no right to create repo in new destination * if new pending transfer ocured, create UI & Mail notifications
1 parent e090031 commit a4148c0

File tree

32 files changed

+898
-167
lines changed

32 files changed

+898
-167
lines changed

integrations/api_repo_test.go

+14-4
Original file line numberDiff line numberDiff line change
@@ -444,12 +444,22 @@ func TestAPIRepoTransfer(t *testing.T) {
444444
teams *[]int64
445445
expectedStatus int
446446
}{
447-
{ctxUserID: 1, newOwner: "user2", teams: nil, expectedStatus: http.StatusAccepted},
448-
{ctxUserID: 2, newOwner: "user1", teams: nil, expectedStatus: http.StatusAccepted},
449-
{ctxUserID: 2, newOwner: "user6", teams: nil, expectedStatus: http.StatusForbidden},
450-
{ctxUserID: 1, newOwner: "user2", teams: &[]int64{2}, expectedStatus: http.StatusUnprocessableEntity},
447+
// Disclaimer for test story: "user1" is an admin, "user2" is normal user and part of in owner team of org "user3"
448+
449+
// Transfer to a user with teams in another org should fail
451450
{ctxUserID: 1, newOwner: "user3", teams: &[]int64{5}, expectedStatus: http.StatusForbidden},
451+
// Transfer to a user with non-existent team IDs should fail
452+
{ctxUserID: 1, newOwner: "user2", teams: &[]int64{2}, expectedStatus: http.StatusUnprocessableEntity},
453+
// Transfer should go through
452454
{ctxUserID: 1, newOwner: "user3", teams: &[]int64{2}, expectedStatus: http.StatusAccepted},
455+
// Let user transfer it back to himself
456+
{ctxUserID: 2, newOwner: "user2", expectedStatus: http.StatusAccepted},
457+
// And revert transfer
458+
{ctxUserID: 2, newOwner: "user3", teams: &[]int64{2}, expectedStatus: http.StatusAccepted},
459+
// Cannot start transfer to an existing repo
460+
{ctxUserID: 2, newOwner: "user3", teams: nil, expectedStatus: http.StatusUnprocessableEntity},
461+
// Start transfer, repo is now in pending transfer mode
462+
{ctxUserID: 2, newOwner: "user6", teams: nil, expectedStatus: http.StatusCreated},
453463
}
454464

455465
defer prepareTestEnv(t)()

models/error.go

+34
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,40 @@ func (err ErrRepoNotExist) Error() string {
757757
err.ID, err.UID, err.OwnerName, err.Name)
758758
}
759759

760+
// ErrNoPendingRepoTransfer is an error type for repositories without a pending
761+
// transfer request
762+
type ErrNoPendingRepoTransfer struct {
763+
RepoID int64
764+
}
765+
766+
func (e ErrNoPendingRepoTransfer) Error() string {
767+
return fmt.Sprintf("repository doesn't have a pending transfer [repo_id: %d]", e.RepoID)
768+
}
769+
770+
// IsErrNoPendingTransfer is an error type when a repository has no pending
771+
// transfers
772+
func IsErrNoPendingTransfer(err error) bool {
773+
_, ok := err.(ErrNoPendingRepoTransfer)
774+
return ok
775+
}
776+
777+
// ErrRepoTransferInProgress represents the state of a repository that has an
778+
// ongoing transfer
779+
type ErrRepoTransferInProgress struct {
780+
Uname string
781+
Name string
782+
}
783+
784+
// IsErrRepoTransferInProgress checks if an error is a ErrRepoTransferInProgress.
785+
func IsErrRepoTransferInProgress(err error) bool {
786+
_, ok := err.(ErrRepoTransferInProgress)
787+
return ok
788+
}
789+
790+
func (err ErrRepoTransferInProgress) Error() string {
791+
return fmt.Sprintf("repository is already being transferred [uname: %s, name: %s]", err.Uname, err.Name)
792+
}
793+
760794
// ErrRepoAlreadyExist represents a "RepoAlreadyExist" kind of error.
761795
type ErrRepoAlreadyExist struct {
762796
Uname string

models/fixtures/repo_transfer.yml

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-
2+
id: 1
3+
doer_id: 3
4+
recipient_id: 1
5+
repo_id: 3
6+
created_unix: 1553610671
7+
updated_unix: 1553610671

models/issue.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ func (issue *Issue) ReadBy(userID int64) error {
563563
return err
564564
}
565565

566-
return setNotificationStatusReadIfUnread(x, userID, issue.ID)
566+
return setIssueNotificationStatusReadIfUnread(x, userID, issue.ID)
567567
}
568568

569569
func updateIssueCols(e Engine, issue *Issue, cols ...string) error {

models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,8 @@ var migrations = []Migration{
294294
NewMigration("Add sessions table for go-chi/session", addSessionTable),
295295
// v173 -> v174
296296
NewMigration("Add time_id column to Comment", addTimeIDCommentColumn),
297+
// v174 -> v175
298+
NewMigration("create repo transfer table", addRepoTransfer),
297299
}
298300

299301
// GetCurrentDBVersion returns the current db version

models/migrations/v174.go

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2021 The Gitea Authors. All rights reserved.
2+
// Use of this source code is governed by a MIT-style
3+
// license that can be found in the LICENSE file.
4+
5+
package migrations
6+
7+
import (
8+
"xorm.io/xorm"
9+
)
10+
11+
func addRepoTransfer(x *xorm.Engine) error {
12+
type RepoTransfer struct {
13+
ID int64 `xorm:"pk autoincr"`
14+
DoerID int64
15+
RecipientID int64
16+
RepoID int64
17+
TeamIDs []int64
18+
CreatedUnix int64 `xorm:"INDEX NOT NULL created"`
19+
UpdatedUnix int64 `xorm:"INDEX NOT NULL updated"`
20+
}
21+
22+
return x.Sync(new(RepoTransfer))
23+
}

models/models.go

+1
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ func init() {
133133
new(ProjectBoard),
134134
new(ProjectIssue),
135135
new(Session),
136+
new(RepoTransfer),
136137
)
137138

138139
gonicNames := []string{"SSL", "UID"}

models/notification.go

+70-8
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ const (
3939
NotificationSourcePullRequest
4040
// NotificationSourceCommit is a notification of a commit
4141
NotificationSourceCommit
42+
// NotificationSourceRepository is a notification for a repository
43+
NotificationSourceRepository
4244
)
4345

4446
// Notification represents a notification
@@ -119,6 +121,46 @@ func GetNotifications(opts FindNotificationOptions) (NotificationList, error) {
119121
return getNotifications(x, opts)
120122
}
121123

124+
// CreateRepoTransferNotification creates notification for the user a repository was transferred to
125+
func CreateRepoTransferNotification(doer, newOwner *User, repo *Repository) error {
126+
sess := x.NewSession()
127+
defer sess.Close()
128+
if err := sess.Begin(); err != nil {
129+
return err
130+
}
131+
var notify []*Notification
132+
133+
if newOwner.IsOrganization() {
134+
users, err := getUsersWhoCanCreateOrgRepo(sess, newOwner.ID)
135+
if err != nil || len(users) == 0 {
136+
return err
137+
}
138+
for i := range users {
139+
notify = append(notify, &Notification{
140+
UserID: users[i].ID,
141+
RepoID: repo.ID,
142+
Status: NotificationStatusUnread,
143+
UpdatedBy: doer.ID,
144+
Source: NotificationSourceRepository,
145+
})
146+
}
147+
} else {
148+
notify = []*Notification{{
149+
UserID: newOwner.ID,
150+
RepoID: repo.ID,
151+
Status: NotificationStatusUnread,
152+
UpdatedBy: doer.ID,
153+
Source: NotificationSourceRepository,
154+
}}
155+
}
156+
157+
if _, err := sess.InsertMulti(notify); err != nil {
158+
return err
159+
}
160+
161+
return sess.Commit()
162+
}
163+
122164
// CreateOrUpdateIssueNotifications creates an issue notification
123165
// for each watcher, or updates it if already exists
124166
// receiverID > 0 just send to reciver, else send to all watcher
@@ -363,7 +405,7 @@ func (n *Notification) loadRepo(e Engine) (err error) {
363405
}
364406

365407
func (n *Notification) loadIssue(e Engine) (err error) {
366-
if n.Issue == nil {
408+
if n.Issue == nil && n.IssueID != 0 {
367409
n.Issue, err = getIssueByID(e, n.IssueID)
368410
if err != nil {
369411
return fmt.Errorf("getIssueByID [%d]: %v", n.IssueID, err)
@@ -374,7 +416,7 @@ func (n *Notification) loadIssue(e Engine) (err error) {
374416
}
375417

376418
func (n *Notification) loadComment(e Engine) (err error) {
377-
if n.Comment == nil && n.CommentID > 0 {
419+
if n.Comment == nil && n.CommentID != 0 {
378420
n.Comment, err = getCommentByID(e, n.CommentID)
379421
if err != nil {
380422
return fmt.Errorf("GetCommentByID [%d] for issue ID [%d]: %v", n.CommentID, n.IssueID, err)
@@ -405,10 +447,18 @@ func (n *Notification) GetIssue() (*Issue, error) {
405447

406448
// HTMLURL formats a URL-string to the notification
407449
func (n *Notification) HTMLURL() string {
408-
if n.Comment != nil {
409-
return n.Comment.HTMLURL()
450+
switch n.Source {
451+
case NotificationSourceIssue, NotificationSourcePullRequest:
452+
if n.Comment != nil {
453+
return n.Comment.HTMLURL()
454+
}
455+
return n.Issue.HTMLURL()
456+
case NotificationSourceCommit:
457+
return n.Repository.HTMLURL() + "/commit/" + n.CommitID
458+
case NotificationSourceRepository:
459+
return n.Repository.HTMLURL()
410460
}
411-
return n.Issue.HTMLURL()
461+
return ""
412462
}
413463

414464
// APIURL formats a URL-string to the notification
@@ -562,8 +612,10 @@ func (nl NotificationList) LoadIssues() ([]int, error) {
562612
if notification.Issue == nil {
563613
notification.Issue = issues[notification.IssueID]
564614
if notification.Issue == nil {
565-
log.Error("Notification[%d]: IssueID: %d Not Found", notification.ID, notification.IssueID)
566-
failures = append(failures, i)
615+
if notification.IssueID != 0 {
616+
log.Error("Notification[%d]: IssueID: %d Not Found", notification.ID, notification.IssueID)
617+
failures = append(failures, i)
618+
}
567619
continue
568620
}
569621
notification.Issue.Repo = notification.Repository
@@ -683,7 +735,7 @@ func GetUIDsAndNotificationCounts(since, until timeutil.TimeStamp) ([]UserIDCoun
683735
return res, x.SQL(sql, since, until, NotificationStatusUnread).Find(&res)
684736
}
685737

686-
func setNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
738+
func setIssueNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
687739
notification, err := getIssueNotification(e, userID, issueID)
688740
// ignore if not exists
689741
if err != nil {
@@ -700,6 +752,16 @@ func setNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
700752
return err
701753
}
702754

755+
func setRepoNotificationStatusReadIfUnread(e Engine, userID, repoID int64) error {
756+
_, err := e.Where(builder.Eq{
757+
"user_id": userID,
758+
"status": NotificationStatusUnread,
759+
"source": NotificationSourceRepository,
760+
"repo_id": repoID,
761+
}).Cols("status").Update(&Notification{Status: NotificationStatusRead})
762+
return err
763+
}
764+
703765
// SetNotificationStatus change the notification status
704766
func SetNotificationStatus(notificationID int64, user *User, status NotificationStatus) error {
705767
notification, err := getNotificationByID(x, notificationID)

models/org.go

+14
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,20 @@ func CanCreateOrgRepo(orgID, uid int64) (bool, error) {
391391
Exist(new(Team))
392392
}
393393

394+
// GetUsersWhoCanCreateOrgRepo returns users which are able to create repo in organization
395+
func GetUsersWhoCanCreateOrgRepo(orgID int64) ([]*User, error) {
396+
return getUsersWhoCanCreateOrgRepo(x, orgID)
397+
}
398+
399+
func getUsersWhoCanCreateOrgRepo(e Engine, orgID int64) ([]*User, error) {
400+
users := make([]*User, 0, 10)
401+
return users, x.
402+
Join("INNER", "`team_user`", "`team_user`.uid=`user`.id").
403+
Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
404+
Where(builder.Eq{"team.can_create_org_repo": true}.Or(builder.Eq{"team.authorize": AccessModeOwner})).
405+
And("team_user.org_id = ?", orgID).Asc("`user`.name").Find(&users)
406+
}
407+
394408
func getOrgsByUserID(sess *xorm.Session, userID int64, showAll bool) ([]*User, error) {
395409
orgs := make([]*User, 0, 10)
396410
if !showAll {

models/org_test.go

+18
Original file line numberDiff line numberDiff line change
@@ -635,3 +635,21 @@ func TestHasOrgVisibleTypePrivate(t *testing.T) {
635635
assert.Equal(t, test2, false) // user not a part of org
636636
assert.Equal(t, test3, false) // logged out user
637637
}
638+
639+
func TestGetUsersWhoCanCreateOrgRepo(t *testing.T) {
640+
assert.NoError(t, PrepareTestDatabase())
641+
642+
users, err := GetUsersWhoCanCreateOrgRepo(3)
643+
assert.NoError(t, err)
644+
assert.Len(t, users, 2)
645+
var ids []int64
646+
for i := range users {
647+
ids = append(ids, users[i].ID)
648+
}
649+
assert.ElementsMatch(t, ids, []int64{2, 28})
650+
651+
users, err = GetUsersWhoCanCreateOrgRepo(7)
652+
assert.NoError(t, err)
653+
assert.Len(t, users, 1)
654+
assert.EqualValues(t, 5, users[0].ID)
655+
}

0 commit comments

Comments
 (0)