From 83cae67aa3fe8f9eb732f86020e58b9ea4d8b5ec Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 13 Sep 2023 00:53:03 +0200 Subject: [PATCH] [SHARED] 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. (cherry picked from commit bf679b24dd23c9ed586b9439e293bbd27cc89232) (cherry picked from commit 1963085dd9d1521b7a4aa8558d409bd1a9f2e1da) (cherry picked from commit fb94095d1992c3e47f03e0fccc98a90707a5271b) (cherry picked from commit e1d1e46afee6891becdb6ccd027fc66843b56db9) (cherry picked from commit 93993029e4ec8a20a8bc38d80bb4b801e52ee1b7) (cherry picked from commit df3b058179d8f3e06cc6fb335b287c72c8952821) (cherry picked from commit 8ccc6b9cba46a736665e4b25523da0baf1679702) (cherry picked from commit 9fbe28fca35e3d02c23521e063679775ec0792f8) (cherry picked from commit 4ef2be6dc705c693735e024b28fd7dac3de39d47) https://codeberg.org/forgejo/forgejo/pulls/1873 Moved test from repo_test.go to forgejo_confirmation_repo_test.go to avoid conflicts. --- routers/web/repo/setting/setting.go | 10 +- templates/repo/settings/options.tmpl | 10 +- .../forgejo_confirmation_repo_test.go | 151 ++++++++++++++++++ tests/integration/pull_create_test.go | 3 +- 4 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 tests/integration/forgejo_confirmation_repo_test.go diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 0864b1c911..69cbcdbf8d 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 85ff82fda7..2a989d3bda 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/forgejo_confirmation_repo_test.go b/tests/integration/forgejo_confirmation_repo_test.go new file mode 100644 index 0000000000..7e1629ebda --- /dev/null +++ b/tests/integration/forgejo_confirmation_repo_test.go @@ -0,0 +1,151 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "net/http/httptest" + "testing" + + gitea_context "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/tests" + + "github.com/stretchr/testify/assert" +) + +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.") + }) + }) +} 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) }