From bf679b24dd23c9ed586b9439e293bbd27cc89232 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 13 Sep 2023 00:53:03 +0200 Subject: [PATCH] [GITEA] Make confirmation clearer for dangerous actions - Currently the confirmation for dangerous actions such as transferring the repository or deleting it only requires the user to ~~copy paste~~ type the repository name. - This can be problematic when the user has a fork or another repository with the same name as an organization's repository, and the confirmation doesn't make clear that it could be deleting the wrong repository. While it's mentioned in the dialog, it's better to be on the safe side and also add the owner's name to be an element that has to be typed for these dangerous actions. - Added integration tests. --- routers/web/repo/setting/setting.go | 10 +- templates/repo/settings/options.tmpl | 10 +- tests/integration/pull_create_test.go | 3 +- tests/integration/repo_test.go | 138 ++++++++++++++++++++++++++ 4 files changed, 150 insertions(+), 11 deletions(-) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 68943586ef..095d86ac3c 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -681,7 +681,7 @@ func SettingsPost(ctx *context.Context) { ctx.Error(http.StatusNotFound) return } - if repo.Name != form.RepoName { + if repo.FullName() != form.RepoName { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil) return } @@ -712,7 +712,7 @@ func SettingsPost(ctx *context.Context) { ctx.ServerError("Convert Fork", err) return } - if repo.Name != form.RepoName { + if repo.FullName() != form.RepoName { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil) return } @@ -745,7 +745,7 @@ func SettingsPost(ctx *context.Context) { ctx.Error(http.StatusNotFound) return } - if repo.Name != form.RepoName { + if repo.FullName() != form.RepoName { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil) return } @@ -827,7 +827,7 @@ func SettingsPost(ctx *context.Context) { ctx.Error(http.StatusNotFound) return } - if repo.Name != form.RepoName { + if repo.FullName() != form.RepoName { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil) return } @@ -851,7 +851,7 @@ func SettingsPost(ctx *context.Context) { ctx.Error(http.StatusNotFound) return } - if repo.Name != form.RepoName { + if repo.FullName() != form.RepoName { ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil) return } diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 78700e05b7..9481b63606 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -816,7 +816,7 @@
@@ -847,7 +847,7 @@
@@ -879,7 +879,7 @@
@@ -917,7 +917,7 @@
@@ -949,7 +949,7 @@
diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index a6ee0d9dfa..f843ec2a24 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -4,6 +4,7 @@ package integration import ( + "fmt" "net/http" "net/http/httptest" "net/url" @@ -129,7 +130,7 @@ func testDeleteRepository(t *testing.T, session *TestSession, ownerName, repoNam req = NewRequestWithValues(t, "POST", relURL+"?action=delete", map[string]string{ "_csrf": htmlDoc.GetCSRF(), - "repo_name": repoName, + "repo_name": fmt.Sprintf("%s/%s", ownerName, repoName), }) session.MakeRequest(t, req, http.StatusSeeOther) } diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 99a27d6a7f..39ddf3cec0 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -6,12 +6,15 @@ package integration import ( "fmt" "net/http" + "net/http/httptest" "path" "strings" "testing" "time" + gitea_context "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/translation" "code.gitea.io/gitea/tests" "github.com/PuerkitoBio/goquery" @@ -444,3 +447,138 @@ func TestGeneratedSourceLink(t *testing.T) { assert.Equal(t, "/user27/repo49/src/commit/aacbdfe9e1c4b47f60abe81849045fa4e96f1d75/test/test.txt", dataURL) }) } + +func TestDangerZoneConfirmation(t *testing.T) { + defer tests.PrepareTestEnv(t)() + mustInvalidRepoName := func(resp *httptest.ResponseRecorder) { + t.Helper() + + htmlDoc := NewHTMLParser(t, resp.Body) + assert.Contains(t, + htmlDoc.doc.Find(".ui.negative.message").Text(), + translation.NewLocale("en-US").Tr("form.enterred_invalid_repo_name"), + ) + } + + t.Run("Transfer ownership", func(t *testing.T) { + session := loginUser(t, "user2") + + t.Run("Fail", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "transfer", + "repo_name": "repo1", + "new_owner_name": "user1", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + mustInvalidRepoName(resp) + }) + t.Run("Pass", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "transfer", + "repo_name": "user2/repo1", + "new_owner_name": "user1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.EqualValues(t, flashCookie.Value, "success%3DThis%2Brepository%2Bhas%2Bbeen%2Bmarked%2Bfor%2Btransfer%2Band%2Bawaits%2Bconfirmation%2Bfrom%2B%2522User%2BOne%2522") + }) + }) + + t.Run("Convert fork", func(t *testing.T) { + session := loginUser(t, "user20") + + t.Run("Fail", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user20/big_test_public_fork_7/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user20/big_test_public_fork_7/settings"), + "action": "convert_fork", + "repo_name": "big_test_public_fork_7", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + mustInvalidRepoName(resp) + }) + t.Run("Pass", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user20/big_test_public_fork_7/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user20/big_test_public_fork_7/settings"), + "action": "convert_fork", + "repo_name": "user20/big_test_public_fork_7", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Bfork%2Bhas%2Bbeen%2Bconverted%2Binto%2Ba%2Bregular%2Brepository.") + }) + }) + + t.Run("Delete wiki", func(t *testing.T) { + session := loginUser(t, "user2") + + t.Run("Fail", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "delete-wiki", + "repo_name": "repo1", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + mustInvalidRepoName(resp) + }) + t.Run("Pass", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "delete-wiki", + "repo_name": "user2/repo1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Brepository%2Bwiki%2Bdata%2Bhas%2Bbeen%2Bdeleted.") + }) + }) + + t.Run("Delete", func(t *testing.T) { + session := loginUser(t, "user2") + + t.Run("Fail", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "delete", + "repo_name": "repo1", + }) + resp := session.MakeRequest(t, req, http.StatusOK) + mustInvalidRepoName(resp) + }) + t.Run("Pass", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{ + "_csrf": GetCSRF(t, session, "/user2/repo1/settings"), + "action": "delete", + "repo_name": "user2/repo1", + }) + session.MakeRequest(t, req, http.StatusSeeOther) + + flashCookie := session.GetCookie(gitea_context.CookieNameFlash) + assert.NotNil(t, flashCookie) + assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Brepository%2Bhas%2Bbeen%2Bdeleted.") + }) + }) +}