Skip to content

Commit c2053dd

Browse files
Gustedearl-warren
Gusted
authored and
earl-warren
committed
[SHARED] make confirmation clearer for dangerous actions
[GITEA] Fix cancelled migration deletion modal - https://codeberg.org/forgejo/forgejo/pulls/1473 made that dangerous actions such as deletion also would need to type in the owner's name. This was apparently not reflected to the deletion modal for migrations that failed or were cancelled. (cherry picked from commit c38dbd6f889aeb52745eddb276225acd0153cba6) (cherry picked from commit 7c07592d01b086b612195367c6a13560e5539767) (cherry picked from commit 78637af2b6440ae307de5e21b284c08c02dd4d13) [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. (cherry picked from commit 83cae67aa3fe8f9eb732f86020e58b9ea4d8b5ec) (cherry picked from commit 447009ff568a542985f6b3a9bc7237b9de3e3c54) (cherry picked from commit 72c0a6150aee7c3a965c87e7348faa2b48c520de) (cherry picked from commit 8ee9c07) (cherry picked from commit 89aba06403be898adbfff6b2d7bb01aad239a87c) (cherry picked from commit 798407599f3d77bdd8bad5fa7abba81d417cd916) (cherry picked from commit 41c9a2606bd8d7036e7d54ad7ab35af06ec99a34) (cherry picked from commit a57b214e366435240c4a210115c6a3fda0d37f8b) (cherry picked from commit fd287a91349bc7844544f9b6ff88e46157d3dc80)
1 parent 8cbd005 commit c2053dd

File tree

5 files changed

+164
-12
lines changed

5 files changed

+164
-12
lines changed

‎routers/web/repo/setting/setting.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,7 @@ func SettingsPost(ctx *context.Context) {
681681
ctx.Error(http.StatusNotFound)
682682
return
683683
}
684-
if repo.Name != form.RepoName {
684+
if repo.FullName() != form.RepoName {
685685
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
686686
return
687687
}
@@ -712,7 +712,7 @@ func SettingsPost(ctx *context.Context) {
712712
ctx.ServerError("Convert Fork", err)
713713
return
714714
}
715-
if repo.Name != form.RepoName {
715+
if repo.FullName() != form.RepoName {
716716
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
717717
return
718718
}
@@ -745,7 +745,7 @@ func SettingsPost(ctx *context.Context) {
745745
ctx.Error(http.StatusNotFound)
746746
return
747747
}
748-
if repo.Name != form.RepoName {
748+
if repo.FullName() != form.RepoName {
749749
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
750750
return
751751
}
@@ -827,7 +827,7 @@ func SettingsPost(ctx *context.Context) {
827827
ctx.Error(http.StatusNotFound)
828828
return
829829
}
830-
if repo.Name != form.RepoName {
830+
if repo.FullName() != form.RepoName {
831831
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
832832
return
833833
}
@@ -851,7 +851,7 @@ func SettingsPost(ctx *context.Context) {
851851
ctx.Error(http.StatusNotFound)
852852
return
853853
}
854-
if repo.Name != form.RepoName {
854+
if repo.FullName() != form.RepoName {
855855
ctx.RenderWithErr(ctx.Tr("form.enterred_invalid_repo_name"), tplSettingsOptions, nil)
856856
return
857857
}

‎templates/repo/migrate/migrating.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
<div class="field">
7070
<label>
7171
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
72-
<span class="text red">{{.Repository.Name}}</span>
72+
<span class="text red">{{.Repository.FullName}}</span>
7373
</label>
7474
</div>
7575
<div class="required field">

‎templates/repo/settings/options.tmpl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@
833833
<div class="field">
834834
<label>
835835
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
836-
<span class="text red">{{.Repository.Name}}</span>
836+
<span class="text red">{{.Repository.FullName}}</span>
837837
</label>
838838
</div>
839839
<div class="required field">
@@ -864,7 +864,7 @@
864864
<div class="field">
865865
<label>
866866
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
867-
<span class="text red">{{.Repository.Name}}</span>
867+
<span class="text red">{{.Repository.FullName}}</span>
868868
</label>
869869
</div>
870870
<div class="required field">
@@ -896,7 +896,7 @@
896896
<div class="field">
897897
<label>
898898
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
899-
<span class="text red">{{.Repository.Name}}</span>
899+
<span class="text red">{{.Repository.FullName}}</span>
900900
</label>
901901
</div>
902902
<div class="required field">
@@ -934,7 +934,7 @@
934934
<div class="field">
935935
<label>
936936
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
937-
<span class="text red">{{.Repository.Name}}</span>
937+
<span class="text red">{{.Repository.FullName}}</span>
938938
</label>
939939
</div>
940940
<div class="required field">
@@ -966,7 +966,7 @@
966966
<div class="field">
967967
<label>
968968
{{ctx.Locale.Tr "repo.settings.transfer_form_title"}}
969-
<span class="text red">{{.Repository.Name}}</span>
969+
<span class="text red">{{.Repository.FullName}}</span>
970970
</label>
971971
</div>
972972
<div class="required field">

‎tests/integration/forgejo_confirmation_repo_test.go

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright 2017 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package integration
5+
6+
import (
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
gitea_context "code.gitea.io/gitea/modules/context"
12+
"code.gitea.io/gitea/modules/translation"
13+
"code.gitea.io/gitea/tests"
14+
15+
"github.com/stretchr/testify/assert"
16+
)
17+
18+
func TestDangerZoneConfirmation(t *testing.T) {
19+
defer tests.PrepareTestEnv(t)()
20+
mustInvalidRepoName := func(resp *httptest.ResponseRecorder) {
21+
t.Helper()
22+
23+
htmlDoc := NewHTMLParser(t, resp.Body)
24+
assert.Contains(t,
25+
htmlDoc.doc.Find(".ui.negative.message").Text(),
26+
translation.NewLocale("en-US").Tr("form.enterred_invalid_repo_name"),
27+
)
28+
}
29+
30+
t.Run("Transfer ownership", func(t *testing.T) {
31+
session := loginUser(t, "user2")
32+
33+
t.Run("Fail", func(t *testing.T) {
34+
defer tests.PrintCurrentTest(t)()
35+
36+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
37+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
38+
"action": "transfer",
39+
"repo_name": "repo1",
40+
"new_owner_name": "user1",
41+
})
42+
resp := session.MakeRequest(t, req, http.StatusOK)
43+
mustInvalidRepoName(resp)
44+
})
45+
t.Run("Pass", func(t *testing.T) {
46+
defer tests.PrintCurrentTest(t)()
47+
48+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
49+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
50+
"action": "transfer",
51+
"repo_name": "user2/repo1",
52+
"new_owner_name": "user1",
53+
})
54+
session.MakeRequest(t, req, http.StatusSeeOther)
55+
56+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
57+
assert.NotNil(t, flashCookie)
58+
assert.EqualValues(t, flashCookie.Value, "success%3DThis%2Brepository%2Bhas%2Bbeen%2Bmarked%2Bfor%2Btransfer%2Band%2Bawaits%2Bconfirmation%2Bfrom%2B%2522User%2BOne%2522")
59+
})
60+
})
61+
62+
t.Run("Convert fork", func(t *testing.T) {
63+
session := loginUser(t, "user20")
64+
65+
t.Run("Fail", func(t *testing.T) {
66+
defer tests.PrintCurrentTest(t)()
67+
68+
req := NewRequestWithValues(t, "POST", "/user20/big_test_public_fork_7/settings", map[string]string{
69+
"_csrf": GetCSRF(t, session, "/user20/big_test_public_fork_7/settings"),
70+
"action": "convert_fork",
71+
"repo_name": "big_test_public_fork_7",
72+
})
73+
resp := session.MakeRequest(t, req, http.StatusOK)
74+
mustInvalidRepoName(resp)
75+
})
76+
t.Run("Pass", func(t *testing.T) {
77+
defer tests.PrintCurrentTest(t)()
78+
79+
req := NewRequestWithValues(t, "POST", "/user20/big_test_public_fork_7/settings", map[string]string{
80+
"_csrf": GetCSRF(t, session, "/user20/big_test_public_fork_7/settings"),
81+
"action": "convert_fork",
82+
"repo_name": "user20/big_test_public_fork_7",
83+
})
84+
session.MakeRequest(t, req, http.StatusSeeOther)
85+
86+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
87+
assert.NotNil(t, flashCookie)
88+
assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Bfork%2Bhas%2Bbeen%2Bconverted%2Binto%2Ba%2Bregular%2Brepository.")
89+
})
90+
})
91+
92+
t.Run("Delete wiki", func(t *testing.T) {
93+
session := loginUser(t, "user2")
94+
95+
t.Run("Fail", func(t *testing.T) {
96+
defer tests.PrintCurrentTest(t)()
97+
98+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
99+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
100+
"action": "delete-wiki",
101+
"repo_name": "repo1",
102+
})
103+
resp := session.MakeRequest(t, req, http.StatusOK)
104+
mustInvalidRepoName(resp)
105+
})
106+
t.Run("Pass", func(t *testing.T) {
107+
defer tests.PrintCurrentTest(t)()
108+
109+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
110+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
111+
"action": "delete-wiki",
112+
"repo_name": "user2/repo1",
113+
})
114+
session.MakeRequest(t, req, http.StatusSeeOther)
115+
116+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
117+
assert.NotNil(t, flashCookie)
118+
assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Brepository%2Bwiki%2Bdata%2Bhas%2Bbeen%2Bdeleted.")
119+
})
120+
})
121+
122+
t.Run("Delete", func(t *testing.T) {
123+
session := loginUser(t, "user2")
124+
125+
t.Run("Fail", func(t *testing.T) {
126+
defer tests.PrintCurrentTest(t)()
127+
128+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
129+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
130+
"action": "delete",
131+
"repo_name": "repo1",
132+
})
133+
resp := session.MakeRequest(t, req, http.StatusOK)
134+
mustInvalidRepoName(resp)
135+
})
136+
t.Run("Pass", func(t *testing.T) {
137+
defer tests.PrintCurrentTest(t)()
138+
139+
req := NewRequestWithValues(t, "POST", "/user2/repo1/settings", map[string]string{
140+
"_csrf": GetCSRF(t, session, "/user2/repo1/settings"),
141+
"action": "delete",
142+
"repo_name": "user2/repo1",
143+
})
144+
session.MakeRequest(t, req, http.StatusSeeOther)
145+
146+
flashCookie := session.GetCookie(gitea_context.CookieNameFlash)
147+
assert.NotNil(t, flashCookie)
148+
assert.EqualValues(t, flashCookie.Value, "success%3DThe%2Brepository%2Bhas%2Bbeen%2Bdeleted.")
149+
})
150+
})
151+
}

‎tests/integration/pull_create_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package integration
55

66
import (
7+
"fmt"
78
"net/http"
89
"net/http/httptest"
910
"net/url"
@@ -142,7 +143,7 @@ func testDeleteRepository(t *testing.T, session *TestSession, ownerName, repoNam
142143

143144
req = NewRequestWithValues(t, "POST", relURL+"?action=delete", map[string]string{
144145
"_csrf": htmlDoc.GetCSRF(),
145-
"repo_name": repoName,
146+
"repo_name": fmt.Sprintf("%s/%s", ownerName, repoName),
146147
})
147148
session.MakeRequest(t, req, http.StatusSeeOther)
148149
}

0 commit comments

Comments
 (0)