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

diff --git a/integrations/pull_create_test.go b/integrations/pull_create_test.go
index 7db1ce1ecfd013fe8ba625dd44ea8b230c4b0aaa..a62144c613fba3aed64417a0765856cb335ef545 100644 (file)
--- a/integrations/pull_create_test.go
+++ b/integrations/pull_create_test.go
@@ -46,7 +46,7 @@ func testPullCreate(t *testing.T, session *TestSession, user, repo, branch strin
 func TestPullCreate(t *testing.T) {
        prepareTestEnv(t)
        session := loginUser(t, "user1")
-       testRepoFork(t, session)
+       testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
        testEditFile(t, session, "user1", "repo1", "master", "README.md")
        testPullCreate(t, session, "user1", "repo1", "master")
 }

diff --git a/integrations/pull_merge_test.go b/integrations/pull_merge_test.go
index ddb18f80248d2bee8828159c283a8278cbf8506b..100298b0832937cd03eae2651fc533faf3dac124 100644 (file)
--- a/integrations/pull_merge_test.go
+++ b/integrations/pull_merge_test.go
@@ -48,7 +48,7 @@ func testPullCleanUp(t *testing.T, session *TestSession, user, repo, pullnum str
 func TestPullMerge(t *testing.T) {
        prepareTestEnv(t)
        session := loginUser(t, "user1")
-       testRepoFork(t, session)
+       testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
        testEditFile(t, session, "user1", "repo1", "master", "README.md")
 
        resp := testPullCreate(t, session, "user1", "repo1", "master")
@@ -61,7 +61,7 @@ func TestPullMerge(t *testing.T) {
 func TestPullCleanUpAfterMerge(t *testing.T) {
        prepareTestEnv(t)
        session := loginUser(t, "user1")
-       testRepoFork(t, session)
+       testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
        testEditFileToNewBranch(t, session, "user1", "repo1", "master", "feature/test", "README.md")
 
        resp := testPullCreate(t, session, "user1", "repo1", "feature/test")

diff --git a/integrations/repo_fork_test.go b/integrations/repo_fork_test.go
index bd6ec7915512a172c35fbef864fa9821d30afd5e..f8cc2e3f3b4d33389e2df67ec7cdb47b3377b87b 100644 (file)
--- a/integrations/repo_fork_test.go
+++ b/integrations/repo_fork_test.go
@@ -5,19 +5,24 @@
 package integrations
 
 import (
+       "fmt"
        "net/http"
        "testing"
 
+       "code.gitea.io/gitea/models"
+
        "github.com/stretchr/testify/assert"
 )
 
-func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
+func testRepoFork(t *testing.T, session *TestSession, ownerName, repoName, forkOwnerName, forkRepoName string) *TestResponse {
+       forkOwner := models.AssertExistsAndLoadBean(t, &models.User{Name: forkOwnerName}).(*models.User)
+
        // Step0: check the existence of the to-fork repo
-       req := NewRequest(t, "GET", "/user1/repo1")
+       req := NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
        resp := session.MakeRequest(t, req, http.StatusNotFound)
 
        // Step1: go to the main page of repo
-       req = NewRequest(t, "GET", "/user2/repo1")
+       req = NewRequestf(t, "GET", "/%s/%s", ownerName, repoName)
        resp = session.MakeRequest(t, req, http.StatusOK)
 
        // Step2: click the fork button
@@ -31,15 +36,17 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
        htmlDoc = NewHTMLParser(t, resp.Body)
        link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/fork/\"]").Attr("action")
        assert.True(t, exists, "The template has changed")
+       _, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", forkOwner.ID)).Attr("data-value")
+       assert.True(t, exists, fmt.Sprintf("Fork owner '%s' is not present in select box", forkOwnerName))
        req = NewRequestWithValues(t, "POST", link, map[string]string{
                "_csrf":     htmlDoc.GetCSRF(),
-               "uid":       "1",
-               "repo_name": "repo1",
+               "uid":       fmt.Sprintf("%d", forkOwner.ID),
+               "repo_name": forkRepoName,
        })
        resp = session.MakeRequest(t, req, http.StatusFound)
 
        // Step4: check the existence of the forked repo
-       req = NewRequest(t, "GET", "/user1/repo1")
+       req = NewRequestf(t, "GET", "/%s/%s", forkOwnerName, forkRepoName)
        resp = session.MakeRequest(t, req, http.StatusOK)
 
        return resp
@@ -48,5 +55,19 @@ func testRepoFork(t *testing.T, session *TestSession) *TestResponse {
 func TestRepoFork(t *testing.T) {
        prepareTestEnv(t)
        session := loginUser(t, "user1")
-       testRepoFork(t, session)
+       testRepoFork(t, session, "user2", "repo1", "user1", "repo1")
+}
+
+func TestRepoForkToOrg(t *testing.T) {
+       prepareTestEnv(t)
+       session := loginUser(t, "user2")
+       testRepoFork(t, session, "user2", "repo1", "user3", "repo1")
+
+       // Check that no more forking is allowed as user2 owns repository
+       //  and user3 organization that owner user2 is also now has forked this repository
+       req := NewRequest(t, "GET", "/user2/repo1")
+       resp := session.MakeRequest(t, req, http.StatusOK)
+       htmlDoc := NewHTMLParser(t, resp.Body)
+       _, exists := htmlDoc.doc.Find("a.ui.button[href^=\"/repo/fork/\"]").Attr("href")
+       assert.False(t, exists, "Forking should not be allowed anymore")
 }

diff --git a/models/repo.go b/models/repo.go
index a6817f6514f252da0804a8e1bdd7b35b45c8f7b2..9819d4b77a1207f8c958dfdf199407cf26bb6b4a 100644 (file)
--- a/models/repo.go
+++ b/models/repo.go
@@ -651,6 +651,25 @@ func (repo *Repository) CanBeForked() bool {
        return !repo.IsBare && repo.UnitEnabled(UnitTypeCode)
 }
 
+// CanUserFork returns true if specified user can fork repository.
+func (repo *Repository) CanUserFork(user *User) (bool, error) {
+       if user == nil {
+               return false, nil
+       }
+       if repo.OwnerID != user.ID && !user.HasForkedRepo(repo.ID) {
+               return true, nil
+       }
+       if err := user.GetOwnedOrganizations(); err != nil {
+               return false, err
+       }
+       for _, org := range user.OwnedOrgs {
+               if repo.OwnerID != org.ID && !org.HasForkedRepo(repo.ID) {
+                       return true, nil
+               }
+       }
+       return false, nil
+}
+
 // CanEnablePulls returns true if repository meets the requirements of accepting pulls.
 func (repo *Repository) CanEnablePulls() bool {
        return !repo.IsMirror && !repo.IsBare

diff --git a/modules/context/repo.go b/modules/context/repo.go
index 788588f8e4a375dbd5496c4b6e2fc9e11ed6193c..88208922c84e54ad2584926a98802e5c597e6882 100644 (file)
--- a/modules/context/repo.go
+++ b/modules/context/repo.go
@@ -337,6 +337,11 @@ func RepoAssignment() macaron.Handler {
                ctx.Data["IsRepositoryAdmin"] = ctx.Repo.IsAdmin()
                ctx.Data["IsRepositoryWriter"] = ctx.Repo.IsWriter()
 
+               if ctx.Data["CanSignedUserFork"], err = ctx.Repo.Repository.CanUserFork(ctx.User); err != nil {
+                       ctx.Handle(500, "CanUserFork", err)
+                       return
+               }
+
                ctx.Data["DisableSSH"] = setting.SSH.Disabled
                ctx.Data["ExposeAnonSSH"] = setting.SSH.ExposeAnonymous
                ctx.Data["DisableHTTP"] = setting.Repository.DisableHTTPGit

diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index 48e17665dac3af26ff4b25211a36f355e3c3551e..f80e8cb1e532b783f25d4cabaa5485b81c8078e9 100644 (file)
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -61,6 +61,8 @@ func getForkRepository(ctx *context.Context) *models.Repository {
        ctx.Data["repo_name"] = forkRepo.Name
        ctx.Data["description"] = forkRepo.Description
        ctx.Data["IsPrivate"] = forkRepo.IsPrivate
+       canForkToUser := forkRepo.OwnerID != ctx.User.ID && !ctx.User.HasForkedRepo(forkRepo.ID)
+       ctx.Data["CanForkToUser"] = canForkToUser
 
        if err = forkRepo.GetOwner(); err != nil {
                ctx.Handle(500, "GetOwner", err)
@@ -69,11 +71,23 @@ func getForkRepository(ctx *context.Context) *models.Repository {
        ctx.Data["ForkFrom"] = forkRepo.Owner.Name + "/" + forkRepo.Name
        ctx.Data["ForkFromOwnerID"] = forkRepo.Owner.ID
 
-       if err := ctx.User.GetOrganizations(true); err != nil {
-               ctx.Handle(500, "GetOrganizations", err)
+       if err := ctx.User.GetOwnedOrganizations(); err != nil {
+               ctx.Handle(500, "GetOwnedOrganizations", err)
                return nil
        }
-       ctx.Data["Orgs"] = ctx.User.Orgs
+       var orgs []*models.User
+       for _, org := range ctx.User.OwnedOrgs {
+               if forkRepo.OwnerID != org.ID && !org.HasForkedRepo(forkRepo.ID) {
+                       orgs = append(orgs, org)
+               }
+       }
+       ctx.Data["Orgs"] = orgs
+
+       if canForkToUser {
+               ctx.Data["ContextUser"] = ctx.User
+       } else if len(orgs) > 0 {
+               ctx.Data["ContextUser"] = orgs[0]
+       }
 
        return forkRepo
 }
@@ -87,7 +101,6 @@ func Fork(ctx *context.Context) {
                return
        }
 
-       ctx.Data["ContextUser"] = ctx.User
        ctx.HTML(200, tplFork)
 }
 
@@ -95,15 +108,16 @@ func Fork(ctx *context.Context) {
 func ForkPost(ctx *context.Context, form auth.CreateRepoForm) {
        ctx.Data["Title"] = ctx.Tr("new_fork")
 
-       forkRepo := getForkRepository(ctx)
+       ctxUser := checkContextUser(ctx, form.UID)
        if ctx.Written() {
                return
        }
 
-       ctxUser := checkContextUser(ctx, form.UID)
+       forkRepo := getForkRepository(ctx)
        if ctx.Written() {
                return
        }
+
        ctx.Data["ContextUser"] = ctxUser
 
        if ctx.HasError() {

diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl
index 6baacd537b455aafe796b5fc7db3f9639f632942..ee6fcb388d03b05604000d2b34cc854da8a81a8b 100644 (file)
--- a/templates/repo/header.tmpl
+++ b/templates/repo/header.tmpl
@@ -32,7 +32,7 @@
                                                </div>
                                                {{if .CanBeForked}}
                                                        <div class="ui compact labeled button" tabindex="0">
-                                                               <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}}>
+                                                               <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}}>
                                                                        <i class="octicon octicon-repo-forked"></i>{{$.i18n.Tr "repo.fork"}}
                                                                </a>
                                                                <a class="ui basic label" href="{{.Link}}/forks">

diff --git a/templates/repo/pulls/fork.tmpl b/templates/repo/pulls/fork.tmpl
index 133cc92f7cb08de611ce4144f9b225e646810761..e9fbfdf2b61a574ce19644c86fc9a02b8674f02d 100644 (file)
--- a/templates/repo/pulls/fork.tmpl
+++ b/templates/repo/pulls/fork.tmpl
@@ -19,17 +19,17 @@
                                                        </span>
                                                        <i class="dropdown icon"></i>
                                                        <div class="menu">
-                                                               <div class="item" data-value="{{.SignedUser.ID}}">
-                                                                       <img class="ui mini image" src="{{.SignedUser.RelAvatarLink}}">
-                                                                       {{.SignedUser.ShortName 20}}
-                                                               </div>
+                                                               {{if .CanForkToUser}}
+                                                                       <div class="item" data-value="{{.SignedUser.ID}}">
+                                                                               <img class="ui mini image" src="{{.SignedUser.RelAvatarLink}}">
+                                                                               {{.SignedUser.ShortName 20}}
+                                                                       </div>
+                                                               {{end}}
                                                                {{range .Orgs}}
-                                                                       {{if and (.IsOwnedBy $.SignedUser.ID) (ne .ID $.ForkFromOwnerID)}}
-                                                                               <div class="item" data-value="{{.ID}}">
-                                                                                       <img class="ui mini image" src="{{.RelAvatarLink}}">
-                                                                                       {{.ShortName 20}}
-                                                                               </div>
-                                                                       {{end}}
+                                                                       <div class="item" data-value="{{.ID}}">
+                                                                               <img class="ui mini image" src="{{.RelAvatarLink}}">
+                                                                               {{.ShortName 20}}
+                                                                       </div>
                                                                {{end}}
                                                        </div>
                                                </div>