Skip to content

Commit

Permalink
Repository transfer has to be confirmed, if user can not create repo …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
6543 authored Mar 1, 2021
1 parent e090031 commit a4148c0
Show file tree
Hide file tree
Showing 32 changed files with 898 additions and 167 deletions.
18 changes: 14 additions & 4 deletions integrations/api_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,12 +444,22 @@ func TestAPIRepoTransfer(t *testing.T) {
teams *[]int64
expectedStatus int
}{
{ctxUserID: 1, newOwner: "user2", teams: nil, expectedStatus: http.StatusAccepted},
{ctxUserID: 2, newOwner: "user1", teams: nil, expectedStatus: http.StatusAccepted},
{ctxUserID: 2, newOwner: "user6", teams: nil, expectedStatus: http.StatusForbidden},
{ctxUserID: 1, newOwner: "user2", teams: &[]int64{2}, expectedStatus: http.StatusUnprocessableEntity},
// Disclaimer for test story: "user1" is an admin, "user2" is normal user and part of in owner team of org "user3"

// Transfer to a user with teams in another org should fail
{ctxUserID: 1, newOwner: "user3", teams: &[]int64{5}, expectedStatus: http.StatusForbidden},
// Transfer to a user with non-existent team IDs should fail
{ctxUserID: 1, newOwner: "user2", teams: &[]int64{2}, expectedStatus: http.StatusUnprocessableEntity},
// Transfer should go through
{ctxUserID: 1, newOwner: "user3", teams: &[]int64{2}, expectedStatus: http.StatusAccepted},
// Let user transfer it back to himself
{ctxUserID: 2, newOwner: "user2", expectedStatus: http.StatusAccepted},
// And revert transfer
{ctxUserID: 2, newOwner: "user3", teams: &[]int64{2}, expectedStatus: http.StatusAccepted},
// Cannot start transfer to an existing repo
{ctxUserID: 2, newOwner: "user3", teams: nil, expectedStatus: http.StatusUnprocessableEntity},
// Start transfer, repo is now in pending transfer mode
{ctxUserID: 2, newOwner: "user6", teams: nil, expectedStatus: http.StatusCreated},
}

defer prepareTestEnv(t)()
Expand Down
34 changes: 34 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,40 @@ func (err ErrRepoNotExist) Error() string {
err.ID, err.UID, err.OwnerName, err.Name)
}

// ErrNoPendingRepoTransfer is an error type for repositories without a pending
// transfer request
type ErrNoPendingRepoTransfer struct {
RepoID int64
}

func (e ErrNoPendingRepoTransfer) Error() string {
return fmt.Sprintf("repository doesn't have a pending transfer [repo_id: %d]", e.RepoID)
}

// IsErrNoPendingTransfer is an error type when a repository has no pending
// transfers
func IsErrNoPendingTransfer(err error) bool {
_, ok := err.(ErrNoPendingRepoTransfer)
return ok
}

// ErrRepoTransferInProgress represents the state of a repository that has an
// ongoing transfer
type ErrRepoTransferInProgress struct {
Uname string
Name string
}

// IsErrRepoTransferInProgress checks if an error is a ErrRepoTransferInProgress.
func IsErrRepoTransferInProgress(err error) bool {
_, ok := err.(ErrRepoTransferInProgress)
return ok
}

func (err ErrRepoTransferInProgress) Error() string {
return fmt.Sprintf("repository is already being transferred [uname: %s, name: %s]", err.Uname, err.Name)
}

// ErrRepoAlreadyExist represents a "RepoAlreadyExist" kind of error.
type ErrRepoAlreadyExist struct {
Uname string
Expand Down
7 changes: 7 additions & 0 deletions models/fixtures/repo_transfer.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-
id: 1
doer_id: 3
recipient_id: 1
repo_id: 3
created_unix: 1553610671
updated_unix: 1553610671
2 changes: 1 addition & 1 deletion models/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ func (issue *Issue) ReadBy(userID int64) error {
return err
}

return setNotificationStatusReadIfUnread(x, userID, issue.ID)
return setIssueNotificationStatusReadIfUnread(x, userID, issue.ID)
}

func updateIssueCols(e Engine, issue *Issue, cols ...string) error {
Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,8 @@ var migrations = []Migration{
NewMigration("Add sessions table for go-chi/session", addSessionTable),
// v173 -> v174
NewMigration("Add time_id column to Comment", addTimeIDCommentColumn),
// v174 -> v175
NewMigration("create repo transfer table", addRepoTransfer),
}

// GetCurrentDBVersion returns the current db version
Expand Down
23 changes: 23 additions & 0 deletions models/migrations/v174.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Copyright 2021 The Gitea Authors. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"xorm.io/xorm"
)

func addRepoTransfer(x *xorm.Engine) error {
type RepoTransfer struct {
ID int64 `xorm:"pk autoincr"`
DoerID int64
RecipientID int64
RepoID int64
TeamIDs []int64
CreatedUnix int64 `xorm:"INDEX NOT NULL created"`
UpdatedUnix int64 `xorm:"INDEX NOT NULL updated"`
}

return x.Sync(new(RepoTransfer))
}
1 change: 1 addition & 0 deletions models/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ func init() {
new(ProjectBoard),
new(ProjectIssue),
new(Session),
new(RepoTransfer),
)

gonicNames := []string{"SSL", "UID"}
Expand Down
78 changes: 70 additions & 8 deletions models/notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ const (
NotificationSourcePullRequest
// NotificationSourceCommit is a notification of a commit
NotificationSourceCommit
// NotificationSourceRepository is a notification for a repository
NotificationSourceRepository
)

// Notification represents a notification
Expand Down Expand Up @@ -119,6 +121,46 @@ func GetNotifications(opts FindNotificationOptions) (NotificationList, error) {
return getNotifications(x, opts)
}

// CreateRepoTransferNotification creates notification for the user a repository was transferred to
func CreateRepoTransferNotification(doer, newOwner *User, repo *Repository) error {
sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}
var notify []*Notification

if newOwner.IsOrganization() {
users, err := getUsersWhoCanCreateOrgRepo(sess, newOwner.ID)
if err != nil || len(users) == 0 {
return err
}
for i := range users {
notify = append(notify, &Notification{
UserID: users[i].ID,
RepoID: repo.ID,
Status: NotificationStatusUnread,
UpdatedBy: doer.ID,
Source: NotificationSourceRepository,
})
}
} else {
notify = []*Notification{{
UserID: newOwner.ID,
RepoID: repo.ID,
Status: NotificationStatusUnread,
UpdatedBy: doer.ID,
Source: NotificationSourceRepository,
}}
}

if _, err := sess.InsertMulti(notify); err != nil {
return err
}

return sess.Commit()
}

// CreateOrUpdateIssueNotifications creates an issue notification
// for each watcher, or updates it if already exists
// receiverID > 0 just send to reciver, else send to all watcher
Expand Down Expand Up @@ -363,7 +405,7 @@ func (n *Notification) loadRepo(e Engine) (err error) {
}

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

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

// HTMLURL formats a URL-string to the notification
func (n *Notification) HTMLURL() string {
if n.Comment != nil {
return n.Comment.HTMLURL()
switch n.Source {
case NotificationSourceIssue, NotificationSourcePullRequest:
if n.Comment != nil {
return n.Comment.HTMLURL()
}
return n.Issue.HTMLURL()
case NotificationSourceCommit:
return n.Repository.HTMLURL() + "/commit/" + n.CommitID
case NotificationSourceRepository:
return n.Repository.HTMLURL()
}
return n.Issue.HTMLURL()
return ""
}

// APIURL formats a URL-string to the notification
Expand Down Expand Up @@ -562,8 +612,10 @@ func (nl NotificationList) LoadIssues() ([]int, error) {
if notification.Issue == nil {
notification.Issue = issues[notification.IssueID]
if notification.Issue == nil {
log.Error("Notification[%d]: IssueID: %d Not Found", notification.ID, notification.IssueID)
failures = append(failures, i)
if notification.IssueID != 0 {
log.Error("Notification[%d]: IssueID: %d Not Found", notification.ID, notification.IssueID)
failures = append(failures, i)
}
continue
}
notification.Issue.Repo = notification.Repository
Expand Down Expand Up @@ -683,7 +735,7 @@ func GetUIDsAndNotificationCounts(since, until timeutil.TimeStamp) ([]UserIDCoun
return res, x.SQL(sql, since, until, NotificationStatusUnread).Find(&res)
}

func setNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
func setIssueNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
notification, err := getIssueNotification(e, userID, issueID)
// ignore if not exists
if err != nil {
Expand All @@ -700,6 +752,16 @@ func setNotificationStatusReadIfUnread(e Engine, userID, issueID int64) error {
return err
}

func setRepoNotificationStatusReadIfUnread(e Engine, userID, repoID int64) error {
_, err := e.Where(builder.Eq{
"user_id": userID,
"status": NotificationStatusUnread,
"source": NotificationSourceRepository,
"repo_id": repoID,
}).Cols("status").Update(&Notification{Status: NotificationStatusRead})
return err
}

// SetNotificationStatus change the notification status
func SetNotificationStatus(notificationID int64, user *User, status NotificationStatus) error {
notification, err := getNotificationByID(x, notificationID)
Expand Down
14 changes: 14 additions & 0 deletions models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,20 @@ func CanCreateOrgRepo(orgID, uid int64) (bool, error) {
Exist(new(Team))
}

// GetUsersWhoCanCreateOrgRepo returns users which are able to create repo in organization
func GetUsersWhoCanCreateOrgRepo(orgID int64) ([]*User, error) {
return getUsersWhoCanCreateOrgRepo(x, orgID)
}

func getUsersWhoCanCreateOrgRepo(e Engine, orgID int64) ([]*User, error) {
users := make([]*User, 0, 10)
return users, x.
Join("INNER", "`team_user`", "`team_user`.uid=`user`.id").
Join("INNER", "`team`", "`team`.id=`team_user`.team_id").
Where(builder.Eq{"team.can_create_org_repo": true}.Or(builder.Eq{"team.authorize": AccessModeOwner})).
And("team_user.org_id = ?", orgID).Asc("`user`.name").Find(&users)
}

func getOrgsByUserID(sess *xorm.Session, userID int64, showAll bool) ([]*User, error) {
orgs := make([]*User, 0, 10)
if !showAll {
Expand Down
18 changes: 18 additions & 0 deletions models/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -635,3 +635,21 @@ func TestHasOrgVisibleTypePrivate(t *testing.T) {
assert.Equal(t, test2, false) // user not a part of org
assert.Equal(t, test3, false) // logged out user
}

func TestGetUsersWhoCanCreateOrgRepo(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())

users, err := GetUsersWhoCanCreateOrgRepo(3)
assert.NoError(t, err)
assert.Len(t, users, 2)
var ids []int64
for i := range users {
ids = append(ids, users[i].ID)
}
assert.ElementsMatch(t, ids, []int64{2, 28})

users, err = GetUsersWhoCanCreateOrgRepo(7)
assert.NoError(t, err)
assert.Len(t, users, 1)
assert.EqualValues(t, 5, users[0].ID)
}
Loading

0 comments on commit a4148c0

Please sign in to comment.