Skip to content

Commit fb80265

Browse files
lafrikslafriks
authored
Fix so that user can still fork his own repository to owned organizations (#2699) (#2707)
* 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
1 parent 6fae585 commit fb80265

File tree

8 files changed

+86
-27
lines changed

8 files changed

+86
-27
lines changed

‎integrations/pull_create_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch strin
4646
func TestPullCreate(t *testing.T) {
4747
prepareTestEnv(t)
4848
session := loginUser(t, "user1")
49-
testRepoFork(t, session)
49+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
5050
testEditFile(t, session, "user1", "repo1", "master", "README.md")
5151
testPullCreate(t, session, "user1", "repo1", "master")
5252
}

‎integrations/pull_merge_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
4848
func TestPullMerge(t *testing.T) {
4949
prepareTestEnv(t)
5050
session := loginUser(t, "user1")
51-
testRepoFork(t, session)
51+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
5252
testEditFile(t, session, "user1", "repo1", "master", "README.md")
5353

5454
resp := testPullCreate(t, session, "user1", "repo1", "master")
@@ -61,7 +61,7 @@ func TestPullMerge(t *testing.T) {
6161
func TestPullCleanUpAfterMerge(t *testing.T) {
6262
prepareTestEnv(t)
6363
session := loginUser(t, "user1")
64-
testRepoFork(t, session)
64+
testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
6565
testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md")
6666

6767
resp := testPullCreate(t, session, "user1", "repo1", "feature/test")

‎integrations/repo_fork_test.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,24 @@
55
package integrations
66

77
import (
8+
"fmt"
89
"net/http"
910
"testing"
1011

12+
"code.gitea.io/gitea/models"
13+
1114
"github.com/stretchr/testify/assert"
1215
)
1316

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+
1520
// 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)
1722
resp := session.MakeRequest(t, req, http.StatusNotFound)
1823

1924
// Step1: go to the main page of repo
20-
req = NewRequest(t, "GET", "/user2/repo1")
25+
req = NewRequestf(t, "GET", "/%s/%s", ownerName, repoName)
2126
resp = session.MakeRequest(t, req, http.StatusOK)
2227

2328
// Step2: click the fork button
@@ -31,15 +36,17 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
3136
htmlDoc = NewHTMLParser(t, resp.Body)
3237
link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/fork/\"]").Attr("action")
3338
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))
3441
req = NewRequestWithValues(t, "POST", link, map[string]string{
3542
"_csrf": htmlDoc.GetCSRF(),
36-
"uid": "1",
37-
"repo_name": "repo1",
43+
"uid": fmt.Sprintf("%d", forkOwner.ID),
44+
"repo_name": forkRepoName,
3845
})
3946
resp = session.MakeRequest(t, req, http.StatusFound)
4047

4148
// Step4: check the existence of the forked repo
42-
req = NewRequest(t, "GET", "/user1/repo1")
49+
req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
4350
resp = session.MakeRequest(t, req, http.StatusOK)
4451

4552
return resp
@@ -48,5 +55,19 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
4855
func TestRepoFork(t *testing.T) {
4956
prepareTestEnv(t)
5057
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")
5273
}

‎models/repo.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,25 @@ func (repo *Repository) CanBeForked() bool {
651651
return !repo.IsBare && repo.UnitEnabled(UnitTypeCode)
652652
}
653653

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+
654673
// CanEnablePulls returns true if repository meets the requirements of accepting pulls.
655674
func (repo *Repository) CanEnablePulls() bool {
656675
return !repo.IsMirror && !repo.IsBare

‎modules/context/repo.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,11 @@ func RepoAssignment() macaron.Handler {
364364
ctx.Data["IsRepositoryAdmin"] = ctx.Repo.IsAdmin()
365365
ctx.Data["IsRepositoryWriter"] = ctx.Repo.IsWriter()
366366

367+
if ctx.Data["CanSignedUserFork"], err = ctx.Repo.Repository.CanUserFork(ctx.User); err != nil {
368+
ctx.Handle(500, "CanUserFork", err)
369+
return
370+
}
371+
367372
ctx.Data["DisableSSH"] = setting.SSH.Disabled
368373
ctx.Data["ExposeAnonSSH"] = setting.SSH.ExposeAnonymous
369374
ctx.Data["DisableHTTP"] = setting.Repository.DisableHTTPGit

‎routers/repo/pull.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ func getForkRepository(ctx *context.Context) *models.Repository {
6161
ctx.Data["repo_name"] = forkRepo.Name
6262
ctx.Data["description"] = forkRepo.Description
6363
ctx.Data["IsPrivate"] = forkRepo.IsPrivate
64+
canForkToUser := forkRepo.OwnerID != ctx.User.ID && !ctx.User.HasForkedRepo(forkRepo.ID)
65+
ctx.Data["CanForkToUser"] = canForkToUser
6466

6567
if err = forkRepo.GetOwner(); err != nil {
6668
ctx.Handle(500, "GetOwner", err)
@@ -69,11 +71,23 @@ func getForkRepository(ctx *context.Context) *models.Repository {
6971
ctx.Data["ForkFrom"] = forkRepo.Owner.Name + "/" + forkRepo.Name
7072
ctx.Data["ForkFromOwnerID"] = forkRepo.Owner.ID
7173

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)
7476
return nil
7577
}
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+
}
7791

7892
return forkRepo
7993
}
@@ -87,23 +101,23 @@ func Fork(ctx *context.Context) {
87101
return
88102
}
89103

90-
ctx.Data["ContextUser"] = ctx.User
91104
ctx.HTML(200, tplFork)
92105
}
93106

94107
// ForkPost response for forking a repository
95108
func ForkPost(ctx *context.Context, form auth.CreateRepoForm) {
96109
ctx.Data["Title"] = ctx.Tr("new_fork")
97110

98-
forkRepo := getForkRepository(ctx)
111+
ctxUser := checkContextUser(ctx, form.UID)
99112
if ctx.Written() {
100113
return
101114
}
102115

103-
ctxUser := checkContextUser(ctx, form.UID)
116+
forkRepo := getForkRepository(ctx)
104117
if ctx.Written() {
105118
return
106119
}
120+
107121
ctx.Data["ContextUser"] = ctxUser
108122

109123
if ctx.HasError() {

‎templates/repo/header.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
</div>
3333
{{if .CanBeForked}}
3434
<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}}>
3636
<i class="octicon octicon-repo-forked"></i>{{$.i18n.Tr "repo.fork"}}
3737
</a>
3838
<a class="ui basic label" href="{{.Link}}/forks">

‎templates/repo/pulls/fork.tmpl

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,17 @@
1919
</span>
2020
<i class="dropdown icon"></i>
2121
<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}}
2628
{{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>
3333
{{end}}
3434
</div>
3535
</div>

0 commit comments

Comments
 (0)