| 1 | package docs_test |
| 2 | |
| 3 | import ( |
| 4 | "net/http" |
| 5 | "strings" |
| 6 | "testing" |
| 7 | |
| 8 | "github.com/carlosframework/docs/internal/docs" |
| 9 | ) |
| 10 | |
| 11 | // members builds an app whose front door is a real allowlist, so the |
| 12 | // difference between a configured member and somebody admitted only by a |
| 13 | // grant actually exists. NewForTest's empty allowlist means "admit |
| 14 | // everyone" in development, which is why none of this was reachable before. |
| 15 | func members(t *testing.T, addresses string) *docs.App { |
| 16 | t.Helper() |
| 17 | app, err := docs.NewForTestWith(t, docs.Config{AllowedAddresses: addresses}) |
| 18 | if err != nil { |
| 19 | t.Fatalf("NewForTestWith: %v", err) |
| 20 | } |
| 21 | return app |
| 22 | } |
| 23 | |
| 24 | // A grant admits its holder as a GUEST, not as a member. |
| 25 | // |
| 26 | // Suite §1 ruling 1: "Sharing is membership; an outside guest is a guest |
| 27 | // member." Docs §8.2: "Outside guests are granted a specific document and |
| 28 | // become guest members." The branch had no guest kind at all, so a single |
| 29 | // reader grant was an unbounded, transitive key to the instance's front |
| 30 | // door: Bob is admitted by Alice's grant, creates his own documents, shares |
| 31 | // them to Mallory, who is now admitted and can repeat it. Alice cannot |
| 32 | // undo any of it, and $DOCS_ALLOWED_ADDRESSES silently stopped being the |
| 33 | // membership list the operator configured. (Branch review, finding 1.) |
| 34 | // |
| 35 | // A guest reads and writes what they were granted, and nothing else. |
| 36 | func TestAGuestCannotCreateOrShare(t *testing.T) { |
| 37 | app := members(t, "alice@example.com") |
| 38 | alice := signInCookies(t, app, "alice@example.com") |
| 39 | sqid := createDoc(t, app, alice, "Minutes") |
| 40 | post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer") |
| 41 | |
| 42 | // Bob gets in — the invitation is redeemable, which is the whole |
| 43 | // reason grant-admission exists. |
| 44 | bob := signInCookies(t, app, "bob@example.com") |
| 45 | if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK { |
| 46 | t.Fatalf("the guest cannot read the document he was granted: %d", rec.Code) |
| 47 | } |
| 48 | |
| 49 | // But he is a guest. He cannot mint documents of his own — 403 |
| 50 | // exactly, so a 500 cannot pass for a refusal. |
| 51 | if rec := post(t, app, bob, "/documents", "title=Bob%27s+own"); rec.Code != http.StatusForbidden { |
| 52 | t.Errorf("guest create: status %d, want 403", rec.Code) |
| 53 | } |
| 54 | // ...and he cannot admit anybody else, which is the transitive half |
| 55 | // and the one that makes the first unbounded. |
| 56 | // |
| 57 | // Asserting 403 specifically, not merely "not 303". The first version |
| 58 | // of this test accepted any non-303, which the OWNER check already |
| 59 | // produces for Bob on Alice's document — so deleting the member guard |
| 60 | // from sharePost left it green. The re-review caught that by mutation: |
| 61 | // three of the four guarded routes had never been exercised at all. |
| 62 | for _, c := range []struct{ what, path, body string }{ |
| 63 | {"share", "/d/" + sqid + "/share", "address=mallory@example.com&role=writer"}, |
| 64 | {"revoke", "/d/" + sqid + "/share/revoke", "address=alice@example.com"}, |
| 65 | {"publish", "/d/" + sqid + "/share/link", "on=1"}, |
| 66 | } { |
| 67 | if rec := post(t, app, bob, c.path, c.body); rec.Code != http.StatusForbidden { |
| 68 | t.Errorf("guest %s: status %d, want 403 from the member guard", c.what, rec.Code) |
| 69 | } |
| 70 | } |
| 71 | } |
| 72 | |
| 73 | // A guest is not shown a control they cannot use. Two of them: the create |
| 74 | // form in the list header, and File → New document in the editor's menu |
| 75 | // bar. Both post /documents, which answers a guest 403 with a bare text |
| 76 | // page. (Re-review, finding 3.) |
| 77 | func TestAGuestIsNotShownCreateControls(t *testing.T) { |
| 78 | app := members(t, "alice@example.com") |
| 79 | alice := signInCookies(t, app, "alice@example.com") |
| 80 | sqid := createDoc(t, app, alice, "Minutes") |
| 81 | post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer") |
| 82 | bob := signInCookies(t, app, "bob@example.com") |
| 83 | |
| 84 | for _, page := range []string{"/", "/d/" + sqid} { |
| 85 | rec := get(t, app, bob, page) |
| 86 | // The status first: an absent control and an unrendered page look |
| 87 | // identical to a substring check, so without this the test passes |
| 88 | // on a 404 — which is the opposite of what it claims to show. |
| 89 | if rec.Code != http.StatusOK { |
| 90 | t.Fatalf("the guest's %s answered %d, want 200", page, rec.Code) |
| 91 | } |
| 92 | if strings.Contains(rec.Body.String(), `action="/documents"`) { |
| 93 | t.Errorf("the guest's %s offers a create control they are refused", page) |
| 94 | } |
| 95 | } |
| 96 | |
| 97 | // Alice keeps both, or the check above passes on an app that offers |
| 98 | // nobody a way to create a document. |
| 99 | for _, page := range []string{"/", "/d/" + sqid} { |
| 100 | if body := get(t, app, alice, page).Body.String(); !strings.Contains(body, `action="/documents"`) { |
| 101 | t.Errorf("the member's %s lost its create control", page) |
| 102 | } |
| 103 | } |
| 104 | } |
| 105 | |
| 106 | // A configured member keeps everything. Without this the rule above passes |
| 107 | // on an app where nobody can create anything. |
| 108 | func TestAMemberStillCreatesAndShares(t *testing.T) { |
| 109 | app := members(t, "alice@example.com,carol@example.com") |
| 110 | carol := signInCookies(t, app, "carol@example.com") |
| 111 | |
| 112 | sqid := createDoc(t, app, carol, "Carol's") |
| 113 | if rec := post(t, app, carol, "/d/"+sqid+"/share", "address=dave@example.com&role=reader"); rec.Code != http.StatusSeeOther { |
| 114 | t.Errorf("a configured member could not share: status %d", rec.Code) |
| 115 | } |
| 116 | } |
| 117 | |
| 118 | // Revoking the last grant ends admission, not just the document. |
| 119 | // |
| 120 | // Before this, `allows()` ran only at sign-in, so a revoked guest kept a |
| 121 | // valid session for up to 30 days — and, per the finding above, kept |
| 122 | // creating and sharing with it. "Stop sharing" was complete for the |
| 123 | // document and said nothing about the instance. (Branch review, finding 3.) |
| 124 | func TestRevokingTheLastGrantEndsAdmission(t *testing.T) { |
| 125 | app := members(t, "alice@example.com") |
| 126 | alice := signInCookies(t, app, "alice@example.com") |
| 127 | sqid := createDoc(t, app, alice, "Minutes") |
| 128 | post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader") |
| 129 | |
| 130 | bob := signInCookies(t, app, "bob@example.com") |
| 131 | if rec := get(t, app, bob, "/d/"+sqid); rec.Code != http.StatusOK { |
| 132 | t.Fatalf("precondition: the guest cannot read (%d)", rec.Code) |
| 133 | } |
| 134 | |
| 135 | post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com") |
| 136 | |
| 137 | // The same cookies, still cryptographically valid, now reach nothing. |
| 138 | // 303 exactly: "not 200" would let a 500 pass for a refusal. |
| 139 | for _, path := range []string{"/", "/d/" + sqid} { |
| 140 | if rec := get(t, app, bob, path); rec.Code != http.StatusSeeOther { |
| 141 | t.Errorf("a revoked guest's GET %s answered %d, want 303", path, rec.Code) |
| 142 | } |
| 143 | } |
| 144 | |
| 145 | // A member is not touched by any of this: the recheck must key on |
| 146 | // admission, not on holding a grant. |
| 147 | if rec := get(t, app, alice, "/"); rec.Code != http.StatusOK { |
| 148 | t.Errorf("the owner lost access to her own list: %d", rec.Code) |
| 149 | } |
| 150 | } |
| 151 | |
| 152 | // A revoked guest's writes are REFUSED, not redirected. |
| 153 | // |
| 154 | // The worst defect on this branch, and I introduced it while fixing the |
| 155 | // last one. requireAdmission answered every method with 303 → /signin. |
| 156 | // fetch() follows redirects by default, so the editor's POST became a GET |
| 157 | // of the sign-in page, came back 200 with HTML, and save-queue.mjs read |
| 158 | // res.ok and reported the batch saved. Alice revokes Bob mid-sentence and |
| 159 | // Bob types for an hour into a page that says everything is fine. |
| 160 | // |
| 161 | // That is strictly worse than the reader-editor bug this branch already |
| 162 | // fixed: that one at least lit the "save failed" indicator. rastrillo's |
| 163 | // own refuseSession redirects GET/HEAD and 403s everything else, for |
| 164 | // exactly this reason, and requireAdmission now does the same. |
| 165 | // |
| 166 | // A redirect is a fine answer to "show me a page". It is never an answer |
| 167 | // to "store this". (Re-review, finding 1.) |
| 168 | func TestARevokedGuestsWritesAreRefusedNotRedirected(t *testing.T) { |
| 169 | app := members(t, "alice@example.com") |
| 170 | alice := signInCookies(t, app, "alice@example.com") |
| 171 | sqid := createDoc(t, app, alice, "Minutes") |
| 172 | post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=writer") |
| 173 | bob := signInCookies(t, app, "bob@example.com") |
| 174 | |
| 175 | // He really can write, first — or the assertion below passes on an |
| 176 | // app where nobody can. |
| 177 | if rec := postBytes(t, app, bob, "/d/"+sqid+"/updates", docs.ReadFragmentFixture(t, "hello")); rec.Code != http.StatusNoContent { |
| 178 | t.Fatalf("precondition: the guest writer cannot write (%d)", rec.Code) |
| 179 | } |
| 180 | |
| 181 | post(t, app, alice, "/d/"+sqid+"/share/revoke", "address=bob@example.com") |
| 182 | |
| 183 | // Now every write must be refused with a status the browser reports |
| 184 | // as a failure. A 3xx here is the bug: fetch follows it and the save |
| 185 | // queue reads the sign-in page's 200 as success. |
| 186 | rec := postBytes(t, app, bob, "/d/"+sqid+"/updates", docs.ReadFragmentFixture(t, "hello")) |
| 187 | if rec.Code >= 300 && rec.Code < 400 { |
| 188 | t.Errorf("a revoked guest's write was redirected (%d → %q); fetch follows it and the save reports ok", |
| 189 | rec.Code, rec.Header().Get("Location")) |
| 190 | } |
| 191 | if rec.Code != http.StatusForbidden { |
| 192 | t.Errorf("a revoked guest's write answered %d; want 403", rec.Code) |
| 193 | } |
| 194 | |
| 195 | // GETs still redirect — that is what a redirect is for, and a person |
| 196 | // following a link deserves the sign-in page rather than a bare 403. |
| 197 | if rec := get(t, app, bob, "/"); rec.Code != http.StatusSeeOther { |
| 198 | t.Errorf("a revoked guest's GET / answered %d, want 303 to sign-in", rec.Code) |
| 199 | } |
| 200 | } |
| 201 | |
| 202 | // The wiring, end to end through the real sign-in, rather than allows() |
| 203 | // and HasGrant each tested against a stand-in for the other. |
| 204 | func TestAGrantAdmitsThroughTheRealSignIn(t *testing.T) { |
| 205 | app := members(t, "alice@example.com") |
| 206 | alice := signInCookies(t, app, "alice@example.com") |
| 207 | sqid := createDoc(t, app, alice, "Minutes") |
| 208 | |
| 209 | // Before any grant, Bob is refused — and refused invisibly, since |
| 210 | // gateBegin answers a refused address exactly as it answers a real |
| 211 | // one so the endpoint is not a membership oracle. |
| 212 | if _, err := app.TestSignIn(t, "bob@example.com"); err == nil { |
| 213 | t.Fatal("an address on no list and holding no grant was mailed a link") |
| 214 | } |
| 215 | |
| 216 | post(t, app, alice, "/d/"+sqid+"/share", "address=bob@example.com&role=reader") |
| 217 | |
| 218 | link, err := app.TestSignIn(t, "bob@example.com") |
| 219 | if err != nil { |
| 220 | t.Fatalf("a granted address was not mailed a link: %v", err) |
| 221 | } |
| 222 | if !strings.Contains(link, "/auth/verify") { |
| 223 | t.Fatalf("not a verify link: %q", link) |
| 224 | } |
| 225 | } |
| 226 | |
amadan