diff options
Diffstat (limited to 'access_test.go')
| -rw-r--r-- | access_test.go | 364 |
1 files changed, 364 insertions, 0 deletions
diff --git a/access_test.go b/access_test.go new file mode 100644 index 0000000..d7ec662 --- /dev/null +++ b/access_test.go @@ -0,0 +1,364 @@ +package main + +import ( + "bytes" + "fmt" + "log" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" +) + +// The rules are unit tested on Viewer. This proves the wiring: that every +// route actually asks, and asks the right question. +func TestAccessThroughTheRouter(t *testing.T) { + f := testForge(t) + for _, name := range []string{"owner", "writer", "reader", "stranger"} { + addUser(t, f, name, "hunter2hunter2") + } + // owner administers both repositories, and changing a grant asks for a code, + // so this is the viewer that holds an authenticator. + addTOTP(t, f, "owner") + grants := map[string]string{"owner": grantAdmin, "writer": grantWrite, "reader": grantRead} + addRepo(t, f, "pub", Repo{ + Desc: "public", Visibility: visPublic, Issues: featUsers, Wiki: featUsers, + Labels: []string{"bug"}, Grants: grants, + }) + addRepo(t, f, "priv", Repo{ + Desc: "private", Visibility: visPrivate, Issues: featMembers, Wiki: featMembers, + Labels: []string{"bug"}, Grants: grants, + }) + if _, err := os.Stat(cgitBinary()); err != nil { + t.Skip("cgit is not installed, and some of these rows are cgit's") + } + h := f.routes() + + // Signing in is deliberately expensive, so it happens once per viewer. + sessions := map[string][]*http.Cookie{"": nil} + sessions["owner"] = codeSession(t, f, h, "owner", "hunter2hunter2") + for _, name := range []string{"writer", "reader", "stranger"} { + sessions[name] = signIn(t, f, h, name) + } + + // Something to read in each. + token := f.csrf(withCookies("/", sessions["owner"])) + for _, repo := range []string{"pub", "priv"} { + post(h, "/"+repo+"/issues", url.Values{"csrf": {token}, "title": {"one"}, "body": {"b"}}, sessions["owner"]) + post(h, "/"+repo+"/wiki/", url.Values{ + "csrf": {token}, "base": {""}, "body": {"# page"}, "action": {"save"}}, sessions["owner"]) + } + + const ( + ok = http.StatusOK + gone = http.StatusNotFound + refused = http.StatusForbidden + redirect = http.StatusSeeOther + ) + // What each viewer should get. Anonymous is the empty name, and a path + // holding %s gets the viewer's name, so one row's writes cannot decide the + // next row's answer. + cases := []struct { + method, path string + anon, stranger int + reader, writer, owner int + }{ + {"GET", "/pub/issues", ok, ok, ok, ok, ok}, + {"GET", "/pub/issues/1", ok, ok, ok, ok, ok}, + {"GET", "/pub/wiki", ok, ok, ok, ok, ok}, + // Public and readable, so refusing to administer it is a 403 and not a + // pretence that it is not there. + {"GET", "/pub/settings", refused, refused, refused, refused, ok}, + {"GET", "/pub/", ok, ok, ok, ok, ok}, + {"GET", "/priv/", gone, gone, ok, ok, ok}, + {"GET", "/priv/issues", gone, gone, ok, ok, ok}, + {"GET", "/priv/issues/1", gone, gone, ok, ok, ok}, + {"GET", "/priv/wiki", gone, gone, ok, ok, ok}, + {"GET", "/priv/settings", gone, gone, refused, refused, ok}, + {"GET", "/priv.issues/", gone, gone, ok, ok, ok}, + {"GET", "/priv.wiki/", gone, gone, ok, ok, ok}, + + // issues is users on pub, so any account may write, but not a visitor. + {"POST", "/pub/issues", refused, redirect, redirect, redirect, redirect}, + // issues is members on priv, so a grant is needed. + {"POST", "/priv/issues", gone, gone, redirect, redirect, redirect}, + {"POST", "/priv/issues/1/comment", gone, gone, redirect, redirect, redirect}, + // closing is triage: the author, or whoever the repo trusts with code. + {"POST", "/priv/issues/1/state", gone, gone, refused, redirect, redirect}, + {"POST", "/priv/issues/1/labels", gone, gone, refused, redirect, redirect}, + // the wiki knob, not the write grant, governs the wiki. + {"POST", "/pub/wiki/%s", refused, redirect, redirect, redirect, redirect}, + {"POST", "/priv/wiki/%s", gone, gone, redirect, redirect, redirect}, + // A repository's own settings follow the admin grant. Deleting it is + // the global flag's alone, so even the repository's admin is refused + // before the typed confirmation is ever looked at. + {"POST", "/priv/settings/grant", gone, gone, refused, refused, redirect}, + {"POST", "/priv/settings/delete", gone, gone, refused, refused, refused}, + // the global admin pages are neither. + {"GET", "/admin", redirect, refused, refused, refused, refused}, + } + + for _, c := range cases { + for _, who := range []struct { + name string + want int + }{ + {"", c.anon}, {"stranger", c.stranger}, {"reader", c.reader}, + {"writer", c.writer}, {"owner", c.owner}, + } { + label := who.name + if label == "" { + label = "anonymous" + } + session := sessions[who.name] + path := c.path + if strings.Contains(path, "%s") { + path = fmt.Sprintf(path, label) + } + if got := request(t, f, h, c.method, path, session); got != who.want { + t.Errorf("%s %s as %s: %d, wanted %d", c.method, path, label, got, who.want) + } + } + } +} + +// request drives one route with a body good enough to get past validation, so +// that what is being measured is the permission check and not the form. +func request(t *testing.T, f *Forge, h http.Handler, method, path string, session []*http.Cookie) int { + t.Helper() + if method == http.MethodGet { + return get(h, path, session).Code + } + form := url.Values{"csrf": {f.csrf(withCookies("/", session))}} + form.Set("title", "t") + form.Set("body", "b") + form.Set("account", "reader") + form.Set("level", grantRead) + form.Set("label", "bug") + form.Set("confirm", "wrong-on-purpose") // deletion must refuse before it acts + form.Set("base", "") + form.Set("action", "save") + form.Set("code", freshCode(t, f, "owner", knownSecret)) // for the rows that confirm + return post(h, path, form, session).Code +} + +func signIn(t *testing.T, f *Forge, h http.Handler, name string) []*http.Cookie { + t.Helper() + w := post(h, "/login", url.Values{"name": {name}, "password": {"hunter2hunter2"}}, nil) + if w.Code != http.StatusSeeOther { + t.Fatalf("signing %s in: status %d", name, w.Code) + } + return w.Result().Cookies() +} + +// What neither CGI will take. A control byte cannot go in an environment +// variable at all, and git-http-backend refuses a relative or empty component +// with a 500 of its own. Percent encoded, these get past the mux's cleaning. +func TestUnservablePathsAreRefused(t *testing.T) { + f := testForge(t) + addRepo(t, f, "notes", Repo{Visibility: visPublic, Issues: featUsers, Wiki: featUsers}) + if _, err := os.Stat(cgitBinary()); err != nil { + t.Skip("cgit is not installed, and the rows that used to answer 500 are cgit's") + } + h := f.routes() + for _, path := range []string{ + "/notes/tree/%00", + "/notes/plain/a%00b", + "/notes.git/%00/info/refs", + "/notes.git/info/refs%00", + "/notes/tree/%0aforged", + "/notes/%7f", + "/notes.git/%2e%2e/%2e%2e/etc/passwd", + "/notes.git/%2e/info/refs", + "/notes.git%2f%2finfo/refs", + "/notes.git/%2f/info/refs", + "/notes/%2e%2e/log/", + } { + if code := get(h, path, nil).Code; code != http.StatusNotFound { + t.Errorf("%s answered %d", path, code) + } + } + // And what the guard must let through, including the trailing slash cgit + // emits and expects back. + for _, path := range []string{"/notes/issues", "/notes/", "/notes/log/", "/notes.git/info/refs?service=git-upload-pack"} { + if code := get(h, path, nil).Code; code != http.StatusOK { + t.Errorf("an ordinary path %s answered %d", path, code) + } + } +} + +// The path arrives percent-decoded, so a newline in it would end the log line +// and let anybody write the next one. +func TestLogLineCannotBeForged(t *testing.T) { + f := testForge(t) + addRepo(t, f, "notes", Repo{Visibility: visPublic, Issues: featUsers, Wiki: featDisabled}) + h := f.routes() + + var buf bytes.Buffer + old := log.Writer() + log.SetOutput(&buf) + defer log.SetOutput(old) + + // A path that closes the real line and opens a plausible fake one. + get(h, "/notes/issues%0a2026-08-15%2000:00:00%20203.0.113.9%20POST%20/admin/user/delete%20303%200%201ms", nil) + + out := strings.TrimRight(buf.String(), "\n") + if lines := strings.Split(out, "\n"); len(lines) != 1 { + t.Errorf("one request wrote %d log lines:\n%s", len(lines), out) + } + if !strings.Contains(out, `\n`) { + t.Errorf("the newline in the path was not escaped: %s", out) + } +} + +// A private repository's page must not sit in a shared cache. cgit says the +// opposite on its own pages, dating them five minutes ahead, so the policy has +// to be stated here and not left to what the CGI happens to send. +func TestNoPageIsStored(t *testing.T) { + f := testForge(t) + addUser(t, f, "alice", "hunter2hunter2") + addRepo(t, f, "priv", Repo{ + Visibility: visPrivate, Issues: featMembers, Wiki: featMembers, + Grants: map[string]string{"alice": grantRead}, + }) + h := f.routes() + session := signIn(t, f, h, "alice") + for _, path := range []string{"/", "/settings", "/priv/issues", "/priv/wiki", "/priv/"} { + w := get(h, path, session) + if w.Code != http.StatusOK { + t.Fatalf("%s answered %d", path, w.Code) + } + if got := w.Header().Get("Cache-Control"); got != "no-store" { + t.Errorf("%s says Cache-Control: %q", path, got) + } + } + // And what is the same for everybody keeps its own. + for _, path := range []string{"/cgit.css", "/favicon.ico", "/avatar/alice"} { + if got := get(h, path, session).Header().Get("Cache-Control"); !strings.HasPrefix(got, "max-age=") { + t.Errorf("%s says Cache-Control: %q", path, got) + } + } +} + +// Markdown renders an image with whatever src it was handed, so a body written +// by anyone who may file an issue would have the reader fetch from wherever the +// author chose. On a private repository that names who is reading it. The +// policy is what stops the fetch, so it has to say so on the page that carries +// one. +func TestARemoteImageIsRefusedByThePolicy(t *testing.T) { + f := testForge(t) + addUser(t, f, "alice", "hunter2hunter2") + addRepo(t, f, "priv", Repo{ + Visibility: visPrivate, Issues: featMembers, Wiki: featDisabled, + Grants: map[string]string{"alice": grantRead}, + }) + h := f.routes() + session := signIn(t, f, h, "alice") + form := url.Values{ + "csrf": {f.csrf(withCookies("/", session))}, "title": {"one"}, + "body": {""}, + } + if w := post(h, "/priv/issues", form, session); w.Code != http.StatusSeeOther { + t.Fatalf("opening the issue: status %d", w.Code) + } + + w := get(h, "/priv/issues/1", session) + if !strings.Contains(w.Body.String(), `src="https://tracker.example/p.png"`) { + t.Fatal("markdown no longer renders a remote image, and this measures the header that stops it") + } + if policy := w.Header().Get("Content-Security-Policy"); !strings.Contains(policy, "img-src 'self'") { + t.Errorf("the page carries a third party image under %q", policy) + } +} + +// A HEAD is a GET whose body is dropped on the way out. The mux answers one +// for every route it holds a pattern for, so the pages under a repository, +// which read the method themselves, have to as well. +func TestHeadIsAnsweredWhereverGetIs(t *testing.T) { + f := testForge(t) + addRepo(t, f, "notes", Repo{Visibility: visPublic, Issues: featUsers, Wiki: featUsers}) + h := f.routes() + if code := get(h, "/notes/issues", nil).Code; code != http.StatusOK { + t.Fatalf("the route this is measured against answered %d", code) + } + for _, path := range []string{ + "/", "/login", "/healthz", "/notes/issues", "/notes/issues/new", + "/notes/issues/1", "/notes/wiki", "/notes/wiki/page", "/notes/settings", + } { + want := get(h, path, nil).Code + r := httptest.NewRequest("HEAD", path, nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != want { + t.Errorf("HEAD %s answered %d, where GET answers %d", path, w.Code, want) + } + } + // And a method nothing serves still says so. + r := httptest.NewRequest("PUT", "/notes/issues", nil) + w := httptest.NewRecorder() + h.ServeHTTP(w, r) + if w.Code != http.StatusNotFound { + t.Errorf("PUT answered %d", w.Code) + } +} + +// A repository that has been deleted must stop answering everywhere at once. +func TestDeletedRepoIsGone(t *testing.T) { + f := testForge(t) + addRepo(t, f, "notes", Repo{Visibility: visPublic, Issues: featUsers, Wiki: featUsers}) + h := f.routes() + if code := get(h, "/notes/issues", nil).Code; code != http.StatusOK { + t.Fatalf("before the delete, the repository did not answer: %d", code) + } + if err := f.store.DeleteRepo("notes", f.store.forgeAuthor()); err != nil { + t.Fatal(err) + } + for _, path := range []string{"/notes/issues", "/notes/wiki", "/notes/settings"} { + if code := get(h, path, nil).Code; code != http.StatusNotFound { + t.Errorf("%s answered %d after the repository was deleted", path, code) + } + } + // git is challenged rather than told, so the answer cannot say whether the + // name was ever taken. + for _, path := range []string{ + "/notes.git/info/refs?service=git-upload-pack", + "/notes.issues.git/info/refs?service=git-upload-pack", + } { + if code := get(h, path, nil).Code; code != http.StatusUnauthorized { + t.Errorf("%s answered %d after the repository was deleted", path, code) + } + } + if body := get(h, "/", nil).Body.String(); strings.Contains(body, "notes") { + t.Error("the index still lists a deleted repository") + } +} + +// The name goes into an attribute of an SVG this server writes, so what the +// route accepts is what keeps that safe. +func TestAvatarIsDrawnFromTheNameAlone(t *testing.T) { + h := testForge(t).routes() + + one := avatarSVG("alice") + if !bytes.Equal(one, avatarSVG("alice")) { + t.Error("one account was drawn two different ways") + } + if bytes.Equal(one, avatarSVG("bob")) { + t.Error("two accounts were drawn the same way") + } + + for _, name := range []string{`a"><script>x</script>`, "../../etc/passwd", "Alice", "a b", "a/b"} { + if code := get(h, "/avatar/"+url.PathEscape(name), nil).Code; code != http.StatusNotFound { + t.Errorf("/avatar/%q answered %d, and only an account name may be drawn", name, code) + } + } + + w := get(h, "/avatar/sso:carol", nil) + if w.Code != http.StatusOK || w.Header().Get("Content-Type") != "image/svg+xml" { + t.Errorf("a usable name answered %d as %q", w.Code, w.Header().Get("Content-Type")) + } + if body := w.Body.String(); strings.Contains(body, "<script") || !strings.Contains(body, "<svg") { + t.Errorf("the drawing is not a drawing: %q", body) + } +} |