Browse Source

Fix so that user can still fork his own repository to owned organizations (#2699)

* Fix so that user can still fork his own repository to his organizations

* Fix to only use owned organizations

* Add integration test for forking own repository to owned organization
Lauris BH 2 years ago
parent
commit
1ec4dc6c1d

+ 1 - 1
integrations/pull_create_test.go

@@ -46,7 +46,7 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch strin
46 46
 func TestPullCreate(t *testing.T) {
47 47
 	prepareTestEnv(t)
48 48
 	session := loginUser(t, "user1")
49
-	testRepoFork(t, session)
49
+	testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
50 50
 	testEditFile(t, session, "user1", "repo1", "master", "README.md")
51 51
 	testPullCreate(t, session, "user1", "repo1", "master")
52 52
 }

+ 2 - 2
integrations/pull_merge_test.go

@@ -48,7 +48,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
48 48
 func TestPullMerge(t *testing.T) {
49 49
 	prepareTestEnv(t)
50 50
 	session := loginUser(t, "user1")
51
-	testRepoFork(t, session)
51
+	testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
52 52
 	testEditFile(t, session, "user1", "repo1", "master", "README.md")
53 53
 
54 54
 	resp := testPullCreate(t, session, "user1", "repo1", "master")
@@ -61,7 +61,7 @@ func TestPullMerge(t *testing.T) {
61 61
 func TestPullCleanUpAfterMerge(t *testing.T) {
62 62
 	prepareTestEnv(t)
63 63
 	session := loginUser(t, "user1")
64
-	testRepoFork(t, session)
64
+	testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
65 65
 	testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md")
66 66
 
67 67
 	resp := testPullCreate(t, session, "user1", "repo1", "feature/test")

+ 28 - 7
integrations/repo_fork_test.go

@@ -5,19 +5,24 @@
5 5
 package integrations
6 6
 
7 7
 import (
8
+	"fmt"
8 9
 	"net/http"
9 10
 	"testing"
10 11
 
12
+	"code.gitea.io/gitea/models"
13
+
11 14
 	"github.com/stretchr/testify/assert"
12 15
 )
13 16
 
14
-func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
17
+func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName string) *TestResponse {
18
+	forkOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: forkOwnerName}).(*models.User)
19
+
15 20
 	// Step0: check the existence of the to-fork repo
16
-	req := NewRequest(t, "GET", "/user1/repo1")
21
+	req := NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
17 22
 	resp := session.MakeRequest(t, req, http.StatusNotFound)
18 23
 
19 24
 	// Step1: go to the main page of repo
20
-	req = NewRequest(t, "GET", "/user2/repo1")
25
+	req = NewRequestf(t, "GET", "/%s/%s", ownerName, repoName)
21 26
 	resp = session.MakeRequest(t, req, http.StatusOK)
22 27
 
23 28
 	// Step2: click the fork button
@@ -31,15 +36,17 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
31 36
 	htmlDoc = NewHTMLParser(t, resp.Body)
32 37
 	link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/fork/\"]").Attr("action")
33 38
 	assert.True(t, exists, "The template has changed")
39
+	_, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", forkOwner.ID)).Attr("data-value")
40
+	assert.True(t, exists, fmt.Sprintf("Fork owner '%s' is not present in select box", forkOwnerName))
34 41
 	req = NewRequestWithValues(t, "POST", link, map[string]string{
35 42
 		"_csrf":     htmlDoc.GetCSRF(),
36
-		"uid":       "1",
37
-		"repo_name": "repo1",
43
+		"uid":       fmt.Sprintf("%d", forkOwner.ID),
44
+		"repo_name": forkRepoName,
38 45
 	})
39 46
 	resp = session.MakeRequest(t, req, http.StatusFound)
40 47
 
41 48
 	// Step4: check the existence of the forked repo
42
-	req = NewRequest(t, "GET", "/user1/repo1")
49
+	req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
43 50
 	resp = session.MakeRequest(t, req, http.StatusOK)
44 51
 
45 52
 	return resp
@@ -48,5 +55,19 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
48 55
 func TestRepoFork(t *testing.T) {
49 56
 	prepareTestEnv(t)
50 57
 	session := loginUser(t, "user1")
51
-	testRepoFork(t, session)
58
+	testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
59
+}
60
+
61
+func TestRepoForkToOrg(t *testing.T) {
62
+	prepareTestEnv(t)
63
+	session := loginUser(t, "user2")
64
+	testRepoFork(t, session, "user2", "repo1", "user3", "repo1")
65
+
66
+	// Check that no more forking is allowed as user2 owns repository
67
+	//  and user3 organization that owner user2 is also now has forked this repository
68
+	req := NewRequest(t, "GET", "/user2/repo1")
69
+	resp := session.MakeRequest(t, req, http.StatusOK)
70
+	htmlDoc := NewHTMLParser(t, resp.Body)
71
+	_, exists := htmlDoc.doc.Find("a.ui.button[href^=\"/repo/fork/\"]").Attr("href")
72
+	assert.False(t, exists, "Forking should not be allowed anymore")
52 73
 }

+ 19 - 0
models/repo.go

@@ -651,6 +651,25 @@ func (repo *Repository) CanBeForked() bool {
651 651
 	return !repo.IsBare && repo.UnitEnabled(UnitTypeCode)
652 652
 }
653 653
 
654
+// CanUserFork returns true if specified user can fork repository.
655
+func (repo *Repository) CanUserFork(user *User) (bool, error) {
656
+	if user == nil {
657
+		return false, nil
658
+	}
659
+	if repo.OwnerID != user.ID && !user.HasForkedRepo(repo.ID) {
660
+		return true, nil
661
+	}
662
+	if err := user.GetOwnedOrganizations(); err != nil {
663
+		return false, err
664
+	}
665
+	for _, org := range user.OwnedOrgs {
666
+		if repo.OwnerID != org.ID && !org.HasForkedRepo(repo.ID) {
667
+			return true, nil
668
+		}
669
+	}
670
+	return false, nil
671
+}
672
+
654 673
 // CanEnablePulls returns true if repository meets the requirements of accepting pulls.
655 674
 func (repo *Repository) CanEnablePulls() bool {
656 675
 	return !repo.IsMirror && !repo.IsBare

+ 5 - 0
modules/context/repo.go

@@ -337,6 +337,11 @@ func RepoAssignment() macaron.Handler {
337 337
 		ctx.Data["IsRepositoryAdmin"] = ctx.Repo.IsAdmin()
338 338
 		ctx.Data["IsRepositoryWriter"] = ctx.Repo.IsWriter()
339 339
 
340
+		if ctx.Data["CanSignedUserFork"], err = ctx.Repo.Repository.CanUserFork(ctx.User); err != nil {
341
+			ctx.Handle(500, "CanUserFork", err)
342
+			return
343
+		}
344
+
340 345
 		ctx.Data["DisableSSH"] = setting.SSH.Disabled
341 346
 		ctx.Data["ExposeAnonSSH"] = setting.SSH.ExposeAnonymous
342 347
 		ctx.Data["DisableHTTP"] = setting.Repository.DisableHTTPGit

+ 20 - 6
routers/repo/pull.go

@@ -61,6 +61,8 @@ func getForkRepository(ctx *context.Context) *models.Repository {
61 61
 	ctx.Data["repo_name"] = forkRepo.Name
62 62
 	ctx.Data["description"] = forkRepo.Description
63 63
 	ctx.Data["IsPrivate"] = forkRepo.IsPrivate
64
+	canForkToUser := forkRepo.OwnerID != ctx.User.ID && !ctx.User.HasForkedRepo(forkRepo.ID)
65
+	ctx.Data["CanForkToUser"] = canForkToUser
64 66
 
65 67
 	if err = forkRepo.GetOwner(); err != nil {
66 68
 		ctx.Handle(500, "GetOwner", err)
@@ -69,11 +71,23 @@ func getForkRepository(ctx *context.Context) *models.Repository {
69 71
 	ctx.Data["ForkFrom"] = forkRepo.Owner.Name + "/" + forkRepo.Name
70 72
 	ctx.Data["ForkFromOwnerID"] = forkRepo.Owner.ID
71 73
 
72
-	if err := ctx.User.GetOrganizations(true); err != nil {
73
-		ctx.Handle(500, "GetOrganizations", err)
74
+	if err := ctx.User.GetOwnedOrganizations(); err != nil {
75
+		ctx.Handle(500, "GetOwnedOrganizations", err)
74 76
 		return nil
75 77
 	}
76
-	ctx.Data["Orgs"] = ctx.User.Orgs
78
+	var orgs []*models.User
79
+	for _, org := range ctx.User.OwnedOrgs {
80
+		if forkRepo.OwnerID != org.ID && !org.HasForkedRepo(forkRepo.ID) {
81
+			orgs = append(orgs, org)
82
+		}
83
+	}
84
+	ctx.Data["Orgs"] = orgs
85
+
86
+	if canForkToUser {
87
+		ctx.Data["ContextUser"] = ctx.User
88
+	} else if len(orgs) > 0 {
89
+		ctx.Data["ContextUser"] = orgs[0]
90
+	}
77 91
 
78 92
 	return forkRepo
79 93
 }
@@ -87,7 +101,6 @@ func Fork(ctx *context.Context) {
87 101
 		return
88 102
 	}
89 103
 
90
-	ctx.Data["ContextUser"] = ctx.User
91 104
 	ctx.HTML(200, tplFork)
92 105
 }
93 106
 
@@ -95,15 +108,16 @@ func Fork(ctx *context.Context) {
95 108
 func ForkPost(ctx *context.Context, form auth.CreateRepoForm) {
96 109
 	ctx.Data["Title"] = ctx.Tr("new_fork")
97 110
 
98
-	forkRepo := getForkRepository(ctx)
111
+	ctxUser := checkContextUser(ctx, form.UID)
99 112
 	if ctx.Written() {
100 113
 		return
101 114
 	}
102 115
 
103
-	ctxUser := checkContextUser(ctx, form.UID)
116
+	forkRepo := getForkRepository(ctx)
104 117
 	if ctx.Written() {
105 118
 		return
106 119
 	}
120
+
107 121
 	ctx.Data["ContextUser"] = ctxUser
108 122
 
109 123
 	if ctx.HasError() {

+ 1 - 1
templates/repo/header.tmpl

@@ -32,7 +32,7 @@
32 32
 						</div>
33 33
 						{{if .CanBeForked}}
34 34
 							<div class="ui compact labeled button" tabindex="0">
35
-								<a class="ui compact button {{if eq .OwnerID $.SignedUserID}}poping up{{end}}" {{if not (eq .OwnerID $.SignedUserID)}}href="{{AppSubUrl}}/repo/fork/{{.ID}}"{{else}} data-content="{{$.i18n.Tr "repo.fork_from_self"}}" data-position="top center" data-variation="tiny"{{end}}>
35
+								<a class="ui compact button {{if not $.CanSignedUserFork}}poping up{{end}}" {{if $.CanSignedUserFork}}href="{{AppSubUrl}}/repo/fork/{{.ID}}"{{else}} data-content="{{$.i18n.Tr "repo.fork_from_self"}}" data-position="top center" data-variation="tiny"{{end}}>
36 36
 									<i class="octicon octicon-repo-forked"></i>{{$.i18n.Tr "repo.fork"}}
37 37
 								</a>
38 38
 								<a class="ui basic label" href="{{.Link}}/forks">

+ 10 - 10
templates/repo/pulls/fork.tmpl

@@ -19,17 +19,17 @@
19 19
 							</span>
20 20
 							<i class="dropdown icon"></i>
21 21
 							<div class="menu">
22
-								<div class="item" data-value="{{.SignedUser.ID}}">
23
-									<img class="ui mini image" src="{{.SignedUser.RelAvatarLink}}">
24
-									{{.SignedUser.ShortName 20}}
25
-								</div>
22
+								{{if .CanForkToUser}}
23
+									<div class="item" data-value="{{.SignedUser.ID}}">
24
+										<img class="ui mini image" src="{{.SignedUser.RelAvatarLink}}">
25
+										{{.SignedUser.ShortName 20}}
26
+									</div>
27
+								{{end}}
26 28
 								{{range .Orgs}}
27
-									{{if and (.IsOwnedBy $.SignedUser.ID) (ne .ID $.ForkFromOwnerID)}}
28
-										<div class="item" data-value="{{.ID}}">
29
-											<img class="ui mini image" src="{{.RelAvatarLink}}">
30
-											{{.ShortName 20}}
31
-										</div>
32
-									{{end}}
29
+									<div class="item" data-value="{{.ID}}">
30
+										<img class="ui mini image" src="{{.RelAvatarLink}}">
31
+										{{.ShortName 20}}
32
+									</div>
33 33
 								{{end}}
34 34
 							</div>
35 35
 						</div>