Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Fold admin work into repository settings and a Users page

9579c1f976254fdeb2ec73d4016db5b25c7921c1 · Keenan Nemetz · 14d ago · parent 381d6aac 

Fold admin work into repository settings and a Users page

Remove the combined /admin tabs. Per-repository access rules now live on the repository's Settings page, visible to instance admins while repository details and the danger zone stay with repository admins. User management moves to /users, linked from the top navigation only for instance admins, and repository creation moves to /repos/new. Align the invite, danger-zone, and settings forms with the shared card-and-actions layout, and make settings section headings consistent.

server/admin_web_test.go +54−37

@@ -8,7 +8,7 @@ import (
8 8 "testing"
9 9 )
10 10
11 func TestAdminAccessEditor(t *testing.T) {
11 func TestRepositoryAccessSettings(t *testing.T) {
12 12 t.Parallel()
13 13 f := browserFixture(t)
14 14 if _, err := f.srv.Identity.SetAdmin(f.ownerID, true, "test"); err != nil {
@@ -17,27 +17,18 @@ func TestAdminAccessEditor(t *testing.T) {
17 17 f.addWriter(t)
18 18 sess, cookie := f.browserSession(t, f.ownerID)
19 19
20 resp, body := f.browserGet(t, "/", cookie)
21 if resp.StatusCode != 200 || !strings.Contains(body, `href="/admin"`) {
22 t.Fatalf("instance admin does not see the Admin link: %d", resp.StatusCode)
23 }
24 resp, body = f.browserGet(t, "/admin", cookie)
25 if resp.StatusCode != 200 || !strings.Contains(body, f.repo) || !strings.Contains(body, "/access") {
26 t.Fatalf("admin page = %d:\n%s", resp.StatusCode, body)
27 }
28
29 // The repository is private and the admin has no ACL entry beyond the owner
30 // role; the admin route must still open it.
31 resp, body = f.browserGet(t, "/admin/repos/"+f.repo+"/access", cookie)
20 // Access rules are instance-admin settings, so they live on the repository's
21 // own Settings page rather than in a separate admin surface.
22 resp, body := f.browserGet(t, "/r/"+f.repo+"/settings", cookie)
32 23 if resp.StatusCode != 200 {
33 t.Fatalf("access page = %d:\n%s", resp.StatusCode, body)
24 t.Fatalf("settings page = %d:\n%s", resp.StatusCode, body)
34 25 }
35 if !strings.Contains(body, `class="admin-tabs"`) || !strings.Contains(body, `href="/admin" aria-current="page"`) {
36 t.Fatalf("access page is missing the active admin tab:\n%s", body)
26 if !strings.Contains(body, `id="access"`) {
27 t.Fatalf("settings page has no access section:\n%s", body)
37 28 }
38 29 for _, want := range []string{"Default role", "Approvals required", "wilbur", "Runner"} {
39 30 if !strings.Contains(body, want) {
40 t.Fatalf("access page does not show %q:\n%s", want, body)
31 t.Fatalf("access section does not show %q:\n%s", want, body)
41 32 }
42 33 }
43 34
@@ -50,10 +41,13 @@ func TestAdminAccessEditor(t *testing.T) {
50 41 "role:" + f.writerID: {"maintain"},
51 42 "runner:" + f.writerID: {"on"},
52 43 }
53 resp, _ = f.browserPost(t, "/admin/repos/"+f.repo+"/access", cookie, form)
54 if resp.StatusCode != 303 || resp.Header.Get("Location") != "/admin/repos/"+f.repo+"/access?saved=1" {
44 resp, _ = f.browserPost(t, "/r/"+f.repo+"/settings/access", cookie, form)
45 if resp.StatusCode != 303 || resp.Header.Get("Location") != "/r/"+f.repo+"/settings?saved=access#access" {
55 46 t.Fatalf("save = %d %q", resp.StatusCode, resp.Header.Get("Location"))
56 47 }
48 if resp, body := f.browserGet(t, "/r/"+f.repo+"/settings?saved=access", cookie); resp.StatusCode != 200 || !strings.Contains(body, "Access rules saved") {
49 t.Fatalf("saved access page = %d:\n%s", resp.StatusCode, body)
50 }
57 51
58 52 m, err := f.srv.Repos.Open(f.repo)
59 53 if err != nil {
@@ -104,7 +98,7 @@ func TestAdminAccessEditor(t *testing.T) {
104 98 form.Set("checks", "on")
105 99 form.Set("approvals", "0")
106 100 form.Set("role:"+f.writerID, "none")
107 resp, _ = f.browserPost(t, "/admin/repos/"+f.repo+"/access", cookie, form)
101 resp, _ = f.browserPost(t, "/r/"+f.repo+"/settings/access", cookie, form)
108 102 if resp.StatusCode != 303 {
109 103 t.Fatalf("second save = %d", resp.StatusCode)
110 104 }
@@ -153,51 +147,74 @@ func TestInstanceAdminCanBrowsePrivateRepoWithoutACLEntry(t *testing.T) {
153 147 }
154 148 }
155 149
156 func TestAdminAccessRequiresInstanceAdmin(t *testing.T) {
150 func TestRepositoryAccessRequiresInstanceAdmin(t *testing.T) {
157 151 t.Parallel()
158 152 f := browserFixture(t)
159 153 f.addWriter(t)
160 154
161 155 // Anonymous is sent to login; an authenticated non-admin gets the same 404
162 156 // as a missing page.
163 resp, _ := f.browserGet(t, "/admin", "")
157 resp, _ := f.browserGet(t, "/users", "")
164 158 if resp.StatusCode != 303 || !strings.Contains(resp.Header.Get("Location"), "/login") {
165 t.Fatalf("anonymous admin = %d %q", resp.StatusCode, resp.Header.Get("Location"))
159 t.Fatalf("anonymous users page = %d %q", resp.StatusCode, resp.Header.Get("Location"))
166 160 }
167 161 writerSess, writerCookie := f.browserSession(t, f.writerID)
168 if resp, _ := f.browserGet(t, "/admin", writerCookie); resp.StatusCode != 404 {
169 t.Fatalf("non-admin admin page = %d, want 404", resp.StatusCode)
162 if resp, _ := f.browserGet(t, "/users", writerCookie); resp.StatusCode != 404 {
163 t.Fatalf("non-admin users page = %d, want 404", resp.StatusCode)
170 164 }
171 if resp, _ := f.browserGet(t, "/admin/repos/"+f.repo+"/access", writerCookie); resp.StatusCode != 404 {
172 t.Fatalf("non-admin access page = %d, want 404", resp.StatusCode)
165 if resp, _ := f.browserGet(t, "/r/"+f.repo+"/settings", writerCookie); resp.StatusCode != 404 {
166 t.Fatalf("maintainer settings = %d, want 404", resp.StatusCode)
173 167 }
174 if _, body := f.browserGet(t, "/", writerCookie); strings.Contains(body, `href="/admin"`) {
175 t.Fatal("non-admin sees the Admin link")
168 if _, body := f.browserGet(t, "/", writerCookie); strings.Contains(body, `href="/users"`) {
169 t.Fatal("non-admin sees the Users link")
176 170 }
177 resp, _ = f.browserPost(t, "/admin/repos/"+f.repo+"/access", writerCookie, url.Values{
171 resp, _ = f.browserPost(t, "/r/"+f.repo+"/settings/access", writerCookie, url.Values{
178 172 "csrf": {writerSess.CSRF}, "default": {"read"}, "approvals": {"0"},
179 173 })
180 174 if resp.StatusCode != 404 {
181 t.Fatalf("non-admin save = %d, want 404", resp.StatusCode)
175 t.Fatalf("non-admin access save = %d, want 404", resp.StatusCode)
182 176 }
183 177
184 if _, err := f.srv.Identity.SetAdmin(f.ownerID, true, "test"); err != nil {
178 // Promote the writer to instance admin. They still have no repository-admin
179 // role, so the page shows only the access section and never the danger zone.
180 if _, err := f.srv.Identity.SetAdmin(f.writerID, true, "test"); err != nil {
185 181 t.Fatal(err)
186 182 }
187 sess, cookie := f.browserSession(t, f.ownerID)
183 resp, body := f.browserGet(t, "/r/"+f.repo+"/settings", writerCookie)
184 if resp.StatusCode != 200 || !strings.Contains(body, `id="access"`) {
185 t.Fatalf("instance-admin settings = %d:\n%s", resp.StatusCode, body)
186 }
187 for _, unwanted := range []string{"Danger zone", "Default branch"} {
188 if strings.Contains(body, unwanted) {
189 t.Fatalf("instance admin without a repository role sees %q:\n%s", unwanted, body)
190 }
191 }
192 if _, home := f.browserGet(t, "/", writerCookie); !strings.Contains(home, `href="/users"`) {
193 t.Fatal("instance admin does not see the Users link")
194 }
195
196 // Repository details remain the repository admin's, even for an instance
197 // admin, and the handlers enforce that rather than trusting the template.
198 resp, _ = f.browserPost(t, "/r/"+f.repo+"/settings", writerCookie, url.Values{
199 "csrf": {writerSess.CSRF}, "description": {"hijack"}, "default_branch": {"main"}, "visibility": {"public"},
200 })
201 if resp.StatusCode != 404 {
202 t.Fatalf("instance-admin repository save = %d, want 404", resp.StatusCode)
203 }
204
188 205 form := url.Values{"default": {"read"}, "approvals": {"0"}}
189 resp, _ = f.browserPost(t, "/admin/repos/"+f.repo+"/access", cookie, form)
206 resp, _ = f.browserPost(t, "/r/"+f.repo+"/settings/access", writerCookie, form)
190 207 if resp.StatusCode != 403 {
191 208 t.Fatalf("missing CSRF = %d, want 403", resp.StatusCode)
192 209 }
193 form.Set("csrf", sess.CSRF)
210 form.Set("csrf", writerSess.CSRF)
194 211 form.Set("approvals", "-1")
195 if resp, body := f.browserPost(t, "/admin/repos/"+f.repo+"/access", cookie, form); resp.StatusCode != 400 || !strings.Contains(body, "non-negative") {
212 if resp, body := f.browserPost(t, "/r/"+f.repo+"/settings/access", writerCookie, form); resp.StatusCode != 400 || !strings.Contains(body, "non-negative") {
196 213 t.Fatalf("negative approvals = %d:\n%s", resp.StatusCode, body)
197 214 }
198 215 form.Set("approvals", "1")
199 216 form.Set("default", "wizard")
200 if resp, body := f.browserPost(t, "/admin/repos/"+f.repo+"/access", cookie, form); resp.StatusCode != 400 || !strings.Contains(body, "unknown role") {
217 if resp, body := f.browserPost(t, "/r/"+f.repo+"/settings/access", writerCookie, form); resp.StatusCode != 400 || !strings.Contains(body, "unknown role") {
201 218 t.Fatalf("unknown role = %d:\n%s", resp.StatusCode, body)
202 219 }
203 220 m, err := f.srv.Repos.Open(f.repo)