| @@ -0,0 +1,345 @@ |
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later |
| 2 | + | |
| 3 | + | package web |
| 4 | + | |
| 5 | + | import ( |
| 6 | + | "fmt" |
| 7 | + | "net/http" |
| 8 | + | "net/http/httptest" |
| 9 | + | "net/url" |
| 10 | + | "strings" |
| 11 | + | "testing" |
| 12 | + | "time" |
| 13 | + | |
| 14 | + | "github.com/stretchr/testify/assert" |
| 15 | + | "github.com/stretchr/testify/mock" |
| 16 | + | "github.com/stretchr/testify/require" |
| 17 | + | |
| 18 | + | "bnema.dev/ermite/internal/domain" |
| 19 | + | "bnema.dev/ermite/internal/ports" |
| 20 | + | "bnema.dev/ermite/internal/ports/mocks" |
| 21 | + | ) |
| 22 | + | |
| 23 | + | type formRig struct { |
| 24 | + | *issueRig |
| 25 | + | form *mocks.IssueSubmissionService |
| 26 | + | } |
| 27 | + | |
| 28 | + | func newFormRig(t *testing.T, mode domain.IssuesMode) *formRig { |
| 29 | + | t.Helper() |
| 30 | + | r := &formRig{issueRig: &issueRig{issues: mocks.NewIssueService(t), mrs: mocks.NewMergeRequestService(t)}, form: mocks.NewIssueSubmissionService(t)} |
| 31 | + | browse := mocks.NewBrowseService(t) |
| 32 | + | browse.EXPECT().ListRefs(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(ports.RefPage{}, nil).Maybe() |
| 33 | + | r.mrs.EXPECT().CountOpen(mock.Anything, mock.Anything, mock.Anything).Return(0, nil).Maybe() |
| 34 | + | r.srv = newTestServer(t, Config{ |
| 35 | + | Repos: reposServing(t, issueRepoView(t, mode)), Browse: browse, MergeRequests: r.mrs, Issues: r.issues, |
| 36 | + | IssueForm: r.form, Sessions: newSessions(t, "tok"), Limiter: &countingLimiter{}, |
| 37 | + | }) |
| 38 | + | return r |
| 39 | + | } |
| 40 | + | |
| 41 | + | func challenge(n int) ports.IssueChallenge { |
| 42 | + | return ports.IssueChallenge{ |
| 43 | + | Token: fmt.Sprintf("tok-%d.sig", n), Nonce: "AAECAwQFBgcICQoLDA0ODw", Difficulty: 18, |
| 44 | + | FormToken: fmt.Sprintf("form-%d", n), |
| 45 | + | IssuedAt: time.UnixMilli(1_700_000_000_000), MinSubmitAt: time.UnixMilli(1_700_000_003_000), |
| 46 | + | ExpiresAt: time.UnixMilli(1_700_000_600_000), |
| 47 | + | } |
| 48 | + | } |
| 49 | + | |
| 50 | + | // post sends a submission as a browser of this site would. |
| 51 | + | func (r *formRig) post(path string, hdr map[string]string, body string) *httptest.ResponseRecorder { |
| 52 | + | req := httptest.NewRequest(http.MethodPost, path, strings.NewReader(body)) |
| 53 | + | req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| 54 | + | for k, v := range hdr { |
| 55 | + | req.Header.Set(k, v) |
| 56 | + | } |
| 57 | + | rec := httptest.NewRecorder() |
| 58 | + | r.srv.ServeHTTP(rec, req) |
| 59 | + | return rec |
| 60 | + | } |
| 61 | + | |
| 62 | + | var fromSite = map[string]string{"Origin": testPublicURL} |
| 63 | + | |
| 64 | + | func goodForm() url.Values { |
| 65 | + | return url.Values{"title": {"It crashes"}, "body": {"line1\r\nline2"}, "author": {"Alex"}, "token": {"tok-1.sig"}, "form_token": {"form-1"}, "solution": {"42"}, "website": {""}} |
| 66 | + | } |
| 67 | + | |
| 68 | + | func TestIssueFormGet(t *testing.T) { |
| 69 | + | t.Parallel() |
| 70 | + | r := newFormRig(t, domain.IssuesPublic) |
| 71 | + | r.form.EXPECT().Challenge(mock.Anything, repoName(t, "proj"), mock.Anything).Return(challenge(1), nil).Once() |
| 72 | + | |
| 73 | + | rec := r.do("GET", "/proj/issues/new", false, nil) |
| 74 | + | require.Equal(t, 200, rec.Code, rec.Body.String()) |
| 75 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 76 | + | body := rec.Body.String() |
| 77 | + | for _, want := range []string{ |
| 78 | + | `<noscript>`, "This form needs JavaScript: it runs a short proof of work so that bots cannot flood the tracker.", |
| 79 | + | `action="/proj/issues/new"`, ` hidden`, `data-issue-form`, `data-nonce="AAECAwQFBgcICQoLDA0ODw"`, |
| 80 | + | `data-difficulty="18"`, `data-min-submit-at="1700000003000"`, `data-worker="/-/static/js/pow-worker.`, |
| 81 | + | `name="token" value="tok-1.sig"`, `name="form_token" value="form-1"`, `name="solution" value=""`, |
| 82 | + | `name="title" type="text" required`, `<output data-for="issue-body" data-max="10000"`, `<output data-for="issue-author" data-max="40"`, |
| 83 | + | `<div class="hp" aria-hidden="true">`, `name="website" type="text" tabindex="-1" autocomplete="off"`, |
| 84 | + | `<output data-for="issue-title" data-max="120"`, `0 / 10000`, `Checking your browser… (this takes a few seconds)`, |
| 85 | + | `/-/static/js/issue-form.`, |
| 86 | + | } { |
| 87 | + | assert.Contains(t, body, want) |
| 88 | + | } |
| 89 | + | assert.NotContains(t, body, `maxlength`, "the limits count code points, not UTF-16 units") |
| 90 | + | assert.NotContains(t, body, `name="website" type="hidden"`) |
| 91 | + | assert.NotContains(t, body, `type="hidden" name="website"`) |
| 92 | + | noscript := body[strings.Index(body, "<noscript>"):strings.Index(body, "</noscript>")] |
| 93 | + | assert.NotContains(t, noscript, "<form", "no usable form without JavaScript") |
| 94 | + | assert.NotContains(t, body, "style=", "no inline style") |
| 95 | + | for _, tag := range strings.Split(body, "<script")[1:] { |
| 96 | + | head, _, _ := strings.Cut(tag, ">") |
| 97 | + | assert.Contains(t, head, `src="`, "no inline script") |
| 98 | + | } |
| 99 | + | } |
| 100 | + | |
| 101 | + | func TestIssueFormIsNotThereUnlessPublic(t *testing.T) { |
| 102 | + | t.Parallel() |
| 103 | + | want := "" |
| 104 | + | for i, tc := range []struct { |
| 105 | + | name string |
| 106 | + | mode domain.IssuesMode |
| 107 | + | err error |
| 108 | + | }{{"unknown", domain.IssuesPublic, domain.ErrNotFound}, {"owner mode", domain.IssuesOwner, domain.ErrNotFound}, {"off", domain.IssuesOff, domain.ErrNotFound}} { |
| 109 | + | r := newFormRig(t, tc.mode) |
| 110 | + | r.form.EXPECT().Challenge(mock.Anything, mock.Anything, mock.Anything).Return(ports.IssueChallenge{}, tc.err).Maybe() |
| 111 | + | path := "/proj/issues/new" |
| 112 | + | if tc.name == "unknown" { |
| 113 | + | path = "/nope/issues/new" |
| 114 | + | } |
| 115 | + | rec := r.do("GET", path, false, nil) |
| 116 | + | assert.Equal(t, 404, rec.Code, tc.name) |
| 117 | + | if i == 0 { |
| 118 | + | want = rec.Body.String() |
| 119 | + | continue |
| 120 | + | } |
| 121 | + | assert.Equal(t, want, rec.Body.String(), tc.name+": the same page") |
| 122 | + | } |
| 123 | + | } |
| 124 | + | |
| 125 | + | func TestIssueFormOwnerKeepsHisForm(t *testing.T) { |
| 126 | + | t.Parallel() |
| 127 | + | r := newFormRig(t, domain.IssuesPublic) // no Challenge expected |
| 128 | + | rec := r.do("GET", "/proj/issues/new", true, nil) |
| 129 | + | require.Equal(t, 200, rec.Code) |
| 130 | + | assert.Contains(t, rec.Body.String(), `action="/proj/issues"`) |
| 131 | + | assert.NotContains(t, rec.Body.String(), "data-issue-form") |
| 132 | + | } |
| 133 | + | |
| 134 | + | func TestIssueFormGetPausedAndLimited(t *testing.T) { |
| 135 | + | t.Parallel() |
| 136 | + | r := newFormRig(t, domain.IssuesPublic) |
| 137 | + | r.form.EXPECT().Challenge(mock.Anything, mock.Anything, mock.Anything).Return(ports.IssueChallenge{}, domain.ErrSubmissionsPaused).Once() |
| 138 | + | rec := r.do("GET", "/proj/issues/new", false, nil) |
| 139 | + | assert.Equal(t, 503, rec.Code) |
| 140 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 141 | + | assert.Contains(t, rec.Body.String(), "Submissions are paused, try later.") |
| 142 | + | } |
| 143 | + | |
| 144 | + | func TestIssueFormPostOutcomes(t *testing.T) { |
| 145 | + | t.Parallel() |
| 146 | + | generic := "The submission could not be verified. Reload the page and try again." |
| 147 | + | tests := []struct { |
| 148 | + | name string |
| 149 | + | err error |
| 150 | + | status int |
| 151 | + | body string |
| 152 | + | hdr map[string]string |
| 153 | + | }{ |
| 154 | + | {"not found", domain.ErrNotFound, 404, "no such page", nil}, |
| 155 | + | {"rate limited", &domain.SubmissionRetryError{After: 25 * time.Minute}, 429, "Too many submissions from your network, try again in 25 minutes.", map[string]string{"Retry-After": "1500"}}, |
| 156 | + | {"rate limited, rounded up", &domain.SubmissionRetryError{After: 90 * time.Second}, 429, "try again in 2 minutes.", map[string]string{"Retry-After": "90"}}, |
| 157 | + | {"paused", domain.ErrSubmissionsPaused, 503, "Submissions are paused, try later.", nil}, |
| 158 | + | {"invalid", domain.ErrSubmissionInvalid, 400, generic, nil}, |
| 159 | + | {"wrapped invalid", fmt.Errorf("%w: x", domain.ErrSubmissionInvalid), 400, generic, nil}, |
| 160 | + | } |
| 161 | + | for _, tc := range tests { |
| 162 | + | t.Run(tc.name, func(t *testing.T) { |
| 163 | + | t.Parallel() |
| 164 | + | r := newFormRig(t, domain.IssuesPublic) |
| 165 | + | r.form.EXPECT().Submit(mock.Anything, repoName(t, "proj"), mock.Anything, mock.Anything).Return(tc.err).Once() |
| 166 | + | rec := r.post("/proj/issues/new", fromSite, goodForm().Encode()) |
| 167 | + | assert.Equal(t, tc.status, rec.Code, rec.Body.String()) |
| 168 | + | assert.Contains(t, rec.Body.String(), tc.body) |
| 169 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 170 | + | for k, v := range tc.hdr { |
| 171 | + | assert.Equal(t, v, rec.Header().Get(k)) |
| 172 | + | } |
| 173 | + | assert.NotContains(t, rec.Body.String(), "tok-1.sig", "the token is never echoed") |
| 174 | + | for _, leak := range []string{"honeypot", "proof", "too fast", "replay", "banned"} { |
| 175 | + | assert.NotContains(t, rec.Body.String(), leak, "no detail") |
| 176 | + | } |
| 177 | + | }) |
| 178 | + | } |
| 179 | + | } |
| 180 | + | |
| 181 | + | // TestIssueFormPostInvalidTokenLooksTheSame: the use case answers an invalid |
| 182 | + | // submission the same whatever the client's ban, limit or the queue; the page |
| 183 | + | // must not differ either. |
| 184 | + | func TestIssueFormPostInvalidTokenLooksTheSame(t *testing.T) { |
| 185 | + | t.Parallel() |
| 186 | + | var first *httptest.ResponseRecorder |
| 187 | + | for _, state := range []string{"banned", "rate limited", "queue full"} { |
| 188 | + | r := newFormRig(t, domain.IssuesPublic) |
| 189 | + | r.form.EXPECT().Submit(mock.Anything, repoName(t, "proj"), mock.Anything, mock.Anything).Return(domain.ErrSubmissionInvalid).Once() |
| 190 | + | f := goodForm() |
| 191 | + | f.Set("token", "garbage") |
| 192 | + | rec := r.post("/proj/issues/new", fromSite, f.Encode()) |
| 193 | + | assert.Equal(t, 400, rec.Code, state) |
| 194 | + | if first == nil { |
| 195 | + | first = rec |
| 196 | + | continue |
| 197 | + | } |
| 198 | + | assert.Equal(t, first.Body.String(), rec.Body.String(), state) |
| 199 | + | assert.Equal(t, first.Header().Get("Retry-After"), rec.Header().Get("Retry-After"), state) |
| 200 | + | } |
| 201 | + | } |
| 202 | + | |
| 203 | + | func TestIssueFormPostSuccess(t *testing.T) { |
| 204 | + | t.Parallel() |
| 205 | + | r := newFormRig(t, domain.IssuesPublic) |
| 206 | + | r.form.EXPECT().Submit(mock.Anything, repoName(t, "proj"), mock.Anything, ports.IssueSubmission{ |
| 207 | + | Token: "tok-1.sig", Solution: "42", FormToken: "form-1", Title: "It crashes", Body: "line1\nline2", Author: "Alex", |
| 208 | + | }).Return(nil).Once() |
| 209 | + | rec := r.post("/proj/issues/new", fromSite, goodForm().Encode()) |
| 210 | + | assert.Equal(t, 303, rec.Code) |
| 211 | + | assert.Equal(t, "/proj/issues/submitted", rec.Header().Get("Location")) |
| 212 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 213 | + | |
| 214 | + | thanks := r.do("GET", "/proj/issues/submitted", false, nil) |
| 215 | + | assert.Equal(t, 200, thanks.Code) |
| 216 | + | assert.Equal(t, "no-store", thanks.Header().Get("Cache-Control")) |
| 217 | + | assert.Contains(t, thanks.Body.String(), "Thanks, your issue is waiting for moderation.") |
| 218 | + | } |
| 219 | + | |
| 220 | + | func TestIssueSubmittedPageOnlyForPublic(t *testing.T) { |
| 221 | + | t.Parallel() |
| 222 | + | r := newFormRig(t, domain.IssuesOwner) |
| 223 | + | assert.Equal(t, 404, r.do("GET", "/proj/issues/submitted", false, nil).Code) |
| 224 | + | } |
| 225 | + | |
| 226 | + | func TestIssueFormTextErrorShowsTheFormAgain(t *testing.T) { |
| 227 | + | t.Parallel() |
| 228 | + | r := newFormRig(t, domain.IssuesPublic) |
| 229 | + | r.form.EXPECT().Submit(mock.Anything, mock.Anything, mock.Anything, mock.Anything). |
| 230 | + | Return(fmt.Errorf("title: %w", &domain.TextError{Reason: "is empty"})).Once() |
| 231 | + | r.form.EXPECT().Challenge(mock.Anything, mock.Anything, mock.Anything).Return(challenge(2), nil).Once() |
| 232 | + | f := goodForm() |
| 233 | + | f.Set("title", "<b>mine</b>") |
| 234 | + | rec := r.post("/proj/issues/new", fromSite, f.Encode()) |
| 235 | + | require.Equal(t, 422, rec.Code, rec.Body.String()) |
| 236 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 237 | + | body := rec.Body.String() |
| 238 | + | assert.Contains(t, body, "<b>mine</b>", "the title is kept") |
| 239 | + | assert.Contains(t, body, "line1\nline2", "the body is kept") |
| 240 | + | assert.Contains(t, body, `value="Alex"`) |
| 241 | + | assert.Contains(t, body, "The title is empty.") |
| 242 | + | assert.Contains(t, body, `name="token" value="tok-2.sig"`, "a new challenge") |
| 243 | + | assert.NotContains(t, body, "tok-1.sig", "the spent token is not echoed") |
| 244 | + | } |
| 245 | + | |
| 246 | + | func TestIssueFormPostLimits(t *testing.T) { |
| 247 | + | t.Parallel() |
| 248 | + | t.Run("oversize", func(t *testing.T) { |
| 249 | + | t.Parallel() |
| 250 | + | r := newFormRig(t, domain.IssuesPublic) // Submit is never called |
| 251 | + | big := url.Values{"title": {"x"}, "body": {strings.Repeat("a", maxIssueFormBytes+10)}} |
| 252 | + | rec := r.post("/proj/issues/new", fromSite, big.Encode()) |
| 253 | + | assert.Equal(t, 413, rec.Code) |
| 254 | + | assert.Equal(t, "no-store", rec.Header().Get("Cache-Control")) |
| 255 | + | }) |
| 256 | + | t.Run("duplicated fields", func(t *testing.T) { |
| 257 | + | t.Parallel() |
| 258 | + | for _, f := range issueFormFields { |
| 259 | + | r := newFormRig(t, domain.IssuesPublic) |
| 260 | + | rec := r.post("/proj/issues/new", fromSite, goodForm().Encode()+"&"+f+"=again") |
| 261 | + | assert.Equal(t, 400, rec.Code, f) |
| 262 | + | assert.Contains(t, rec.Body.String(), "could not be verified", f) |
| 263 | + | } |
| 264 | + | }) |
| 265 | + | } |
| 266 | + | |
| 267 | + | // The gate still demands the Origin of this site for the anonymous POST. |
| 268 | + | func TestIssueFormPostNeedsTheOrigin(t *testing.T) { |
| 269 | + | t.Parallel() |
| 270 | + | r := newFormRig(t, domain.IssuesPublic) // Submit is never called |
| 271 | + | for name, hdr := range map[string]map[string]string{ |
| 272 | + | "missing": nil, |
| 273 | + | "cross origin": {"Origin": "https://evil.example"}, |
| 274 | + | "null": {"Origin": "null"}, |
| 275 | + | "same host, other scheme": {"Origin": "http://forge.example"}, |
| 276 | + | } { |
| 277 | + | rec := r.post("/proj/issues/new", hdr, goodForm().Encode()) |
| 278 | + | assert.Equal(t, 403, rec.Code, name) |
| 279 | + | } |
| 280 | + | } |
| 281 | + | |
| 282 | + | func TestIssueFormPostOnOwnerModeIsNotFound(t *testing.T) { |
| 283 | + | t.Parallel() |
| 284 | + | r := newFormRig(t, domain.IssuesOwner) |
| 285 | + | r.form.EXPECT().Submit(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(domain.ErrNotFound).Once() |
| 286 | + | rec := r.post("/proj/issues/new", fromSite, goodForm().Encode()) |
| 287 | + | assert.Equal(t, 404, rec.Code) |
| 288 | + | } |
| 289 | + | |
| 290 | + | // anonymousPost opens exactly /<repo>/issues/new and nothing else. |
| 291 | + | func TestAnonymousPostOpensOnlyTheIssueForm(t *testing.T) { |
| 292 | + | t.Parallel() |
| 293 | + | r := newFormRig(t, domain.IssuesPublic) |
| 294 | + | for _, p := range []string{ |
| 295 | + | "/x/issues/new/extra", "/x/issues/123", "/x/issues", "/x/issues/new/", "/x/issues/123/close", "/x/issues/new/close", |
| 296 | + | "/x%2Fissues/new", "/x%2Fy/issues/new", "/%2Fissues/new", "/issues/new", "//issues/new", "/X/issues/new", |
| 297 | + | "/a/b/issues/new", "/x.git/issues/new", "/../issues/new", "/-/inbox/pending/x/y/approve", "/x/merge-requests", |
| 298 | + | "/x/issues%2Fnew", "/x/issues/new%2Fextra", "/x/issues/%6Eew", |
| 299 | + | } { |
| 300 | + | u := &url.URL{Path: p} |
| 301 | + | if dec, err := url.PathUnescape(p); err == nil { |
| 302 | + | u = &url.URL{Path: dec, RawPath: p} |
| 303 | + | } |
| 304 | + | req := &http.Request{Method: http.MethodPost, URL: u} |
| 305 | + | got := r.srv.anonymousPost(req) |
| 306 | + | want := false |
| 307 | + | assert.Equal(t, want, got, p) |
| 308 | + | } |
| 309 | + | for _, p := range []string{"/x/issues/new", "/proj/issues/new", "/a.b_c-d/issues/new"} { |
| 310 | + | req := &http.Request{Method: http.MethodPost, URL: &url.URL{Path: p}} |
| 311 | + | assert.True(t, r.srv.anonymousPost(req), p) |
| 312 | + | req.Method = http.MethodPut |
| 313 | + | assert.False(t, r.srv.anonymousPost(req), p+" PUT") |
| 314 | + | } |
| 315 | + | // Without the use case the path is not opened at all. |
| 316 | + | plain := newIssueRig(t, domain.IssuesPublic, false) |
| 317 | + | req := &http.Request{Method: http.MethodPost, URL: &url.URL{Path: "/proj/issues/new"}} |
| 318 | + | assert.False(t, plain.srv.anonymousPost(req)) |
| 319 | + | } |
| 320 | + | |
| 321 | + | func TestIssueListShowsNewIssueToVisitorsOfPublicRepos(t *testing.T) { |
| 322 | + | t.Parallel() |
| 323 | + | for mode, want := range map[domain.IssuesMode]bool{domain.IssuesPublic: true, domain.IssuesOwner: false} { |
| 324 | + | r := newFormRig(t, mode) |
| 325 | + | r.issues.EXPECT().List(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(ports.IssuePage{}, nil) |
| 326 | + | rec := r.do("GET", "/proj/issues", false, nil) |
| 327 | + | require.Equal(t, 200, rec.Code) |
| 328 | + | assert.Equal(t, want, strings.Contains(rec.Body.String(), `href="/proj/issues/new"`), string(mode)) |
| 329 | + | } |
| 330 | + | } |
| 331 | + | |
| 332 | + | func TestExternalIssueIsRenderedUntrusted(t *testing.T) { |
| 333 | + | t.Parallel() |
| 334 | + | r := newFormRig(t, domain.IssuesPublic) |
| 335 | + | i := testIssue(t, 5, domain.IssueStateOpen, "bug", "[a](javascript:alert(1)) [b](https://example.org/) <script>x</script>") |
| 336 | + | i.External, i.Author = true, "Eve <b>" |
| 337 | + | r.issues.EXPECT().Get(mock.Anything, mock.Anything, mock.Anything, int64(5)).Return(ports.IssueDetail{Issue: i}, nil) |
| 338 | + | rec := r.do("GET", "/proj/issues/5", false, nil) |
| 339 | + | require.Equal(t, 200, rec.Code) |
| 340 | + | body := rec.Body.String() |
| 341 | + | assert.NotContains(t, body, "javascript:") |
| 342 | + | assert.NotContains(t, body, "<script>x") |
| 343 | + | assert.Contains(t, body, `rel="nofollow noopener noreferrer ugc"`) |
| 344 | + | assert.Contains(t, body, "Eve <b>") |
| 345 | + | } |