| 1 | package docs |
| 2 | |
| 3 | import ( |
| 4 | "context" |
| 5 | "errors" |
| 6 | "log/slog" |
| 7 | "net/http" |
| 8 | "net/http/httptest" |
| 9 | "testing" |
| 10 | ) |
| 11 | |
| 12 | // requireAdmission has to tell three answers apart: admitted, refused, and |
| 13 | // "could not tell". The third is the one that costs somebody their work if |
| 14 | // it is folded into the second. |
| 15 | // |
| 16 | // A guest's editor posts through fetch. save-queue.mjs classifies a 4xx as |
| 17 | // "the bytes are the problem" and drops the batch permanently — and |
| 18 | // because Yjs updates depend on their predecessors, everything after it is |
| 19 | // poisoned too. A 5xx it retries. So a database stall answered 403 costs a |
| 20 | // guest their paragraph, while the same stall answered 503 costs them |
| 21 | // nothing. (Re-review 3, finding 1.) |
| 22 | // |
| 23 | // Tested at the middleware rather than over HTTP: the app's own session |
| 24 | // layer refuses first when the database is genuinely broken, so an |
| 25 | // end-to-end version would assert on rastrillo's 403 and never reach this |
| 26 | // code at all. That is exactly the "passes for a reason other than the one |
| 27 | // it names" failure this branch has produced four times, and the honest |
| 28 | // fix is to test the unit whose contract this is. |
| 29 | func TestRequireAdmissionTellsRefusalFromFailure(t *testing.T) { |
| 30 | reached := false |
| 31 | next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| 32 | reached = true |
| 33 | w.WriteHeader(http.StatusNoContent) |
| 34 | }) |
| 35 | |
| 36 | cases := []struct { |
| 37 | name string |
| 38 | granted func(string) (bool, error) |
| 39 | method string |
| 40 | want int |
| 41 | passes bool |
| 42 | }{ |
| 43 | {"a check that fails is retryable, not a refusal", func(string) (bool, error) { |
| 44 | return false, errors.New("database is stalled") |
| 45 | }, http.MethodPost, http.StatusServiceUnavailable, false}, |
| 46 | |
| 47 | {"a failed check on a GET is also retryable", func(string) (bool, error) { |
| 48 | return false, errors.New("database is stalled") |
| 49 | }, http.MethodGet, http.StatusServiceUnavailable, false}, |
| 50 | |
| 51 | {"a definite refusal of a write is 403", func(string) (bool, error) { |
| 52 | return false, nil |
| 53 | }, http.MethodPost, http.StatusForbidden, false}, |
| 54 | |
| 55 | {"a definite refusal of a read is a redirect to sign in", func(string) (bool, error) { |
| 56 | return false, nil |
| 57 | }, http.MethodGet, http.StatusSeeOther, false}, |
| 58 | |
| 59 | {"a guest who still holds a grant passes through", func(string) (bool, error) { |
| 60 | return true, nil |
| 61 | }, http.MethodPost, http.StatusNoContent, true}, |
| 62 | } |
| 63 | |
| 64 | for _, c := range cases { |
| 65 | t.Run(c.name, func(t *testing.T) { |
| 66 | reached = false |
| 67 | list := allowlist{ |
| 68 | addrs: map[string]struct{}{"alice@example.com": {}}, |
| 69 | granted: c.granted, |
| 70 | } |
| 71 | h := requireAdmissionOf(list, "/signin", signedInAs("bob@example.com"))(next) |
| 72 | |
| 73 | req := httptest.NewRequest(c.method, "/d/abc/updates", nil) |
| 74 | rec := httptest.NewRecorder() |
| 75 | h.ServeHTTP(rec, req) |
| 76 | |
| 77 | if rec.Code != c.want { |
| 78 | t.Errorf("status %d, want %d", rec.Code, c.want) |
| 79 | } |
| 80 | if reached != c.passes { |
| 81 | t.Errorf("reached the handler = %v, want %v", reached, c.passes) |
| 82 | } |
| 83 | }) |
| 84 | } |
| 85 | } |
| 86 | |
| 87 | // A configured member never touches the grant check, so a broken database |
| 88 | // cannot eject them. Without this the 503 above could be the answer |
| 89 | // everybody gets. |
| 90 | func TestAMemberIsUnaffectedByAFailedGrantCheck(t *testing.T) { |
| 91 | called := false |
| 92 | list := allowlist{ |
| 93 | addrs: map[string]struct{}{"alice@example.com": {}}, |
| 94 | granted: func(string) (bool, error) { |
| 95 | called = true |
| 96 | return false, errors.New("database is stalled") |
| 97 | }, |
| 98 | } |
| 99 | |
| 100 | reached := false |
| 101 | h := requireAdmissionOf(list, "/signin", signedInAs("alice@example.com"))(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
| 102 | reached = true |
| 103 | })) |
| 104 | |
| 105 | h.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/documents", nil)) |
| 106 | |
| 107 | if !reached { |
| 108 | t.Error("a member was stopped by a grant check that is not about them") |
| 109 | } |
| 110 | if called { |
| 111 | t.Error("a member's request consulted the grant store; allows() must answer from the configured set first") |
| 112 | } |
| 113 | } |
| 114 | |
| 115 | // signedInAs is the identity lookup requireAdmissionOf takes, standing in |
| 116 | // for rauth.From, whose context key is unexported. |
| 117 | func signedInAs(address string) func(*http.Request) (string, bool) { |
| 118 | return func(*http.Request) (string, bool) { return address, true } |
| 119 | } |
| 120 | |
| 121 | // admits — the plain-predicate form rauth's Authorize hook takes — fails |
| 122 | // CLOSED. It cannot distinguish "no" from "could not tell", so it has to |
| 123 | // choose, and at sign-in the safe choice is refusal: there is no unsaved |
| 124 | // work to lose and a genuinely admitted person can try again. |
| 125 | // |
| 126 | // This is the opposite choice from requireAdmission's 503 above, made for |
| 127 | // the opposite reason, and neither is right in the other's place. It has |
| 128 | // its own test because a mutation that made admits() return true on an |
| 129 | // error survived the whole suite: nothing anywhere pinned the direction, |
| 130 | // so a database failure would have opened the front door to any address. |
| 131 | func TestAdmitsFailsClosed(t *testing.T) { |
| 132 | list := allowlist{ |
| 133 | addrs: map[string]struct{}{"alice@example.com": {}}, |
| 134 | granted: func(string) (bool, error) { |
| 135 | return false, errors.New("database is stalled") |
| 136 | }, |
| 137 | } |
| 138 | |
| 139 | if list.admits("mallory@example.com") { |
| 140 | t.Error("a failed grant check admitted a stranger; sign-in must fail closed") |
| 141 | } |
| 142 | // A member is still admitted: the failure must not lock everybody out |
| 143 | // either, and allows() answers them without asking the database. |
| 144 | if !list.admits("alice@example.com") { |
| 145 | t.Error("a configured member was refused because of a grant check that is not about them") |
| 146 | } |
| 147 | } |
| 148 | |
| 149 | // Delivery fails closed even when the grant store is unavailable. |
| 150 | func TestAdmissionMailerFailsClosed(t *testing.T) { |
| 151 | list := allowlist{ |
| 152 | addrs: map[string]struct{}{"alice@example.com": {}}, |
| 153 | granted: func(string) (bool, error) { return false, errors.New("database is stalled") }, |
| 154 | } |
| 155 | recorder := &recordingMailer{next: loggingMailerStub{}} |
| 156 | sender := newMailQueue(recorder, listAdmits(list), nil, slog.New(slog.DiscardHandler)) |
| 157 | t.Cleanup(sender.close) |
| 158 | for _, address := range []string{"mallory@example.com", "alice@example.com"} { |
| 159 | if err := sender.Send(t.Context(), address, "subject", "body"); err != nil { |
| 160 | t.Fatal(err) |
| 161 | } |
| 162 | } |
| 163 | sender.wait() |
| 164 | if recorder.sends != 1 || recorder.lastTo != "alice@example.com" { |
| 165 | t.Fatalf("delivery count=%d recipient=%q", recorder.sends, recorder.lastTo) |
| 166 | } |
| 167 | } |
| 168 | |
| 169 | // HasGrant reports its error rather than folding it into false, which is |
| 170 | // the whole reason the 503 above can exist. A mutation returning |
| 171 | // (false, nil) on a query failure passed the suite: the chain was tested |
| 172 | // from the stub downward and never from the store up. (Re-review 4, |
| 173 | // finding 2.) |
| 174 | func TestHasGrantReportsItsError(t *testing.T) { |
| 175 | app, err := NewForTest(t) |
| 176 | if err != nil { |
| 177 | t.Fatalf("NewForTest: %v", err) |
| 178 | } |
| 179 | |
| 180 | // A cancelled context is the cheapest real failure: the query cannot |
| 181 | // run, and the honest answer is "could not tell", never "no". |
| 182 | ctx, cancel := context.WithCancel(context.Background()) |
| 183 | cancel() |
| 184 | |
| 185 | held, err := app.Store().HasGrant(ctx, "bob@example.com") |
| 186 | if err == nil { |
| 187 | t.Error("a failed grant check reported no error; the caller cannot tell a refusal from an outage") |
| 188 | } |
| 189 | if held { |
| 190 | t.Error("a failed grant check answered true") |
| 191 | } |
| 192 | } |
| 193 | |
amadan