Closes #27. Guild membership is now the whole gate. `discordCallback` checks membership (and `DISCORD_REQUIRED_ROLE` when set), then `Store.EnsureReader` creates the Reader on first sight and returns the same row on every later login. The refusal returns before `EnsureReader`, so a turned-away sign-in leaves no row behind. `OWNER_DISCORD_ID` still seeds the owner, but only as the administrator — it no longer gates login. The cutover grace path goes with it: `API_TOKEN`, `API_TOKEN_GRACE_UNTIL` and the legacy branch in `httpmw.ResolveReader` are deleted, so a credential authenticates exactly one Reader or nothing. `userscript.Handler` drops its re-derivation too — the resolved path segment is already the credential. New surfaces: an empty library offers both install links (behind the tab-specific empty states, so "No favourites yet" still wins), and the owner alone gets a Readers panel with `POST /readers/{id}/revoke`. The owner's own row is not revocable — 404, not a self-logout. Isolation is asserted from both directions for read, modify and delete, and the shared-series invariant is pinned: two Readers on one series produce one series row, two independent progresses, one poll per due cycle, and one Reader's delete leaves the other's bookmark and the poll intact. Verified: `go test ./...` green; live smoke against a throwaway Postgres — empty-library state in both colour branches, roster rendering, a real revoke through the panel (target 401s next request, owner untouched), owner self-revoke refused 404, per-Reader `/u/<cred>` and bearer auth both 200 with 404 for an unknown credential. Reviewed-on: #36 Co-authored-by: Sulthan Zaki <sultankiki05@gmail.com> Co-committed-by: Sulthan Zaki <sultankiki05@gmail.com>
This commit was merged in pull request #36.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -10,31 +9,19 @@ import (
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"bookmarkmanager/backend/internal/store"
|
||||
"bookmarkmanager/backend/internal/token"
|
||||
|
||||
_ "github.com/jackc/pgx/v5/stdlib"
|
||||
)
|
||||
|
||||
// insertReader creates an extra reader row (registration is closed, so the
|
||||
// store has no path for this — tests reach past it) and returns its id. The
|
||||
// credential is derived the same way the owner's is, so it authenticates
|
||||
// through the real router.
|
||||
func insertReader(t *testing.T, dbURL, discordID string) int64 {
|
||||
// registerReader creates an extra Reader the way a first login does and
|
||||
// returns its id. The credential is derived the same way the owner's is, so it
|
||||
// authenticates through the real router.
|
||||
func registerReader(t *testing.T, s *store.Store, discordID string) int64 {
|
||||
t.Helper()
|
||||
db, err := sql.Open("pgx", dbURL)
|
||||
id, err := s.EnsureReader(discordID, token.Hash(readerCredential(discordID)))
|
||||
if err != nil {
|
||||
t.Fatalf("open db: %v", err)
|
||||
}
|
||||
defer db.Close()
|
||||
hash := token.Hash(token.Token([]byte(testTokenKey), discordID, 0))
|
||||
var id int64
|
||||
if err := db.QueryRow(
|
||||
`INSERT INTO readers (discord_id, token_sha256) VALUES ($1, $2) RETURNING id`,
|
||||
discordID, hash[:]).Scan(&id); err != nil {
|
||||
t.Fatalf("insert reader: %v", err)
|
||||
t.Fatalf("register reader %q: %v", discordID, err)
|
||||
}
|
||||
return id
|
||||
}
|
||||
@@ -59,34 +46,29 @@ func withBody(req *http.Request, body string) *http.Request {
|
||||
return req
|
||||
}
|
||||
|
||||
// The retired global token resolves to the owner Reader only while the grace
|
||||
// deadline is in the future — testConfig sets it, so the acceptance path is
|
||||
// the existing auth() tests; this pins the other side of the window.
|
||||
func TestLegacyTokenDeadAfterGrace(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
cfg := testConfig()
|
||||
cfg.GraceUntil = time.Now().Add(-time.Hour)
|
||||
srv := newRouter(s, cfg)
|
||||
// A refused credential is refused however plausible it looks: only a hash the
|
||||
// readers table holds authenticates anything.
|
||||
func TestUnknownCredentialRejected(t *testing.T) {
|
||||
srv := newRouter(newTestStore(t), testConfig())
|
||||
|
||||
rr := httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", testToken))
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", readerCredential("never-registered")))
|
||||
if rr.Code != http.StatusUnauthorized {
|
||||
t.Fatalf("legacy token after grace: status = %d, want 401", rr.Code)
|
||||
t.Fatalf("unregistered Reader's credential: status = %d, want 401", rr.Code)
|
||||
}
|
||||
|
||||
// The owner's own derived credential is unaffected by the window closing.
|
||||
rr = httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", ownerCredential()))
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("derived token after grace: status = %d, want 200", rr.Code)
|
||||
t.Fatalf("owner's derived credential: status = %d, want 200", rr.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// A Reader's credential authenticates exactly that Reader: rows written under
|
||||
// one credential are invisible to the other, on the same key.
|
||||
func TestPerReaderIsolation(t *testing.T) {
|
||||
s, dbURL := newTestStoreURL(t)
|
||||
insertReader(t, dbURL, "other-reader")
|
||||
s := newTestStore(t)
|
||||
registerReader(t, s, "other-reader")
|
||||
srv := newRouter(s, testConfig())
|
||||
|
||||
ownerKey := "asura:solo"
|
||||
@@ -137,24 +119,73 @@ func TestPerReaderIsolation(t *testing.T) {
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &owners); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(owners) != 1 || owners[0].Title != "Solo Leveling" {
|
||||
t.Fatalf("owner list = %+v, want their own row", owners)
|
||||
if len(owners) != 1 || owners[0].Title != "Solo Leveling" || owners[0].LastChapterNum != 0 {
|
||||
t.Fatalf("owner list = %+v, want their own row at their own progress", owners)
|
||||
}
|
||||
|
||||
// One Reader's credential cannot delete the other's row.
|
||||
req = credRequest(http.MethodDelete, "/bookmarks/"+ownerKey, readerCredential("other-reader"))
|
||||
// The mirror: the owner's write does not move the other Reader's progress
|
||||
// either. Without it, isolation is only asserted in one direction.
|
||||
req = credRequest(http.MethodPut, "/bookmarks/"+ownerKey, ownerCredential())
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
rr = httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, req)
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("other reader delete: status = %d, want 204", rr.Code)
|
||||
srv.ServeHTTP(rr, withBody(req, `{"key":"asura:solo","site":"asura","series_id":"solo","title":"Solo Leveling","last_chapter_num":9}`))
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("owner put: status = %d, want 200", rr.Code)
|
||||
}
|
||||
rr = httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", ownerCredential()))
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &owners); err != nil {
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", readerCredential("other-reader")))
|
||||
var theirs3 []store.Bookmark
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &theirs3); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if len(owners) != 1 {
|
||||
t.Fatalf("owner's row was deletable by another Reader: list = %+v", owners)
|
||||
if len(theirs3) != 1 || theirs3[0].LastChapterNum != 3 {
|
||||
t.Fatalf("other reader list = %+v, want progress 3 after the owner's write", theirs3)
|
||||
}
|
||||
|
||||
// DELETE is scoped to its caller too, asserted in both directions: each
|
||||
// Reader's delete on the shared key takes only their own row.
|
||||
list := func(cred string) []store.Bookmark {
|
||||
t.Helper()
|
||||
rr := httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodGet, "/bookmarks", cred))
|
||||
var got []store.Bookmark
|
||||
if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
return got
|
||||
}
|
||||
del := func(cred string) {
|
||||
t.Helper()
|
||||
rr := httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, credRequest(http.MethodDelete, "/bookmarks/"+ownerKey, cred))
|
||||
if rr.Code != http.StatusNoContent {
|
||||
t.Fatalf("delete: status = %d, want 204", rr.Code)
|
||||
}
|
||||
}
|
||||
|
||||
del(readerCredential("other-reader"))
|
||||
if got := list(ownerCredential()); len(got) != 1 {
|
||||
t.Fatalf("owner's row was deletable by the other Reader: %+v", got)
|
||||
}
|
||||
if got := list(readerCredential("other-reader")); len(got) != 0 {
|
||||
t.Fatalf("other Reader's own delete left %+v behind", got)
|
||||
}
|
||||
|
||||
// The mirror: the other Reader takes the key again, the owner deletes
|
||||
// theirs, and the other's row is untouched.
|
||||
req = credRequest(http.MethodPut, "/bookmarks/"+ownerKey, readerCredential("other-reader"))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
rr = httptest.NewRecorder()
|
||||
srv.ServeHTTP(rr, withBody(req, body))
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("other reader re-put: status = %d, want 200", rr.Code)
|
||||
}
|
||||
del(ownerCredential())
|
||||
if got := list(readerCredential("other-reader")); len(got) != 1 {
|
||||
t.Fatalf("other Reader's row was deletable by the owner: %+v", got)
|
||||
}
|
||||
if got := list(ownerCredential()); len(got) != 0 {
|
||||
t.Fatalf("owner's own delete left %+v behind", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -162,7 +193,7 @@ func TestPerReaderIsolation(t *testing.T) {
|
||||
// with the Reader's credential inside: the credential never appears in the
|
||||
// address bar, the page markup, or any Location header.
|
||||
func TestInstallServesScriptWithCredential(t *testing.T) {
|
||||
cfg := webConfig()
|
||||
cfg := testConfig()
|
||||
dir := t.TempDir()
|
||||
path := filepath.Join(dir, "manga-bookmark.user.js")
|
||||
novelPath := filepath.Join(dir, "novel-bookmark.user.js")
|
||||
@@ -225,43 +256,6 @@ func TestInstallServesScriptWithCredential(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The retired global token also keeps the script download path working during
|
||||
// the grace window — that is how already-installed scripts auto-update across
|
||||
// the cutover — and dies with it. The copy served on the legacy path embeds
|
||||
// the Reader's derived credential, so the next update poll migrates the
|
||||
// device onto its per-Reader path: the window empties itself.
|
||||
func TestLegacyTokenUserscriptPathDuringGrace(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "manga-bookmark.user.js")
|
||||
if err := os.WriteFile(path, []byte("const API_TOKEN = \"__API_TOKEN__\";\n"), 0o644); err != nil {
|
||||
t.Fatalf("write script: %v", err)
|
||||
}
|
||||
|
||||
s := newTestStore(t)
|
||||
cfg := testConfig()
|
||||
cfg.UserscriptPath = path
|
||||
|
||||
// Within the window the legacy URL serves the script, but with the
|
||||
// owner's derived credential substituted — not the legacy one.
|
||||
rr := httptest.NewRecorder()
|
||||
newRouter(s, cfg).ServeHTTP(rr, httptest.NewRequest(http.MethodGet,
|
||||
"/u/"+testToken+"/manga-bookmark.user.js", nil))
|
||||
if rr.Code != http.StatusOK {
|
||||
t.Fatalf("legacy path during grace: status = %d, want 200", rr.Code)
|
||||
}
|
||||
if got := rr.Body.String(); !strings.Contains(got, `API_TOKEN = "`+ownerCredential()+`"`) {
|
||||
t.Fatalf("legacy-path script does not carry the derived credential:\n%s", got)
|
||||
}
|
||||
|
||||
// After the deadline the same URL is a 404 like any unknown credential.
|
||||
cfg.GraceUntil = time.Now().Add(-time.Hour)
|
||||
rr = httptest.NewRecorder()
|
||||
newRouter(s, cfg).ServeHTTP(rr, httptest.NewRequest(http.MethodGet,
|
||||
"/u/"+testToken+"/manga-bookmark.user.js", nil))
|
||||
if rr.Code != http.StatusNotFound {
|
||||
t.Fatalf("legacy path after grace: status = %d, want 404", rr.Code)
|
||||
}
|
||||
}
|
||||
|
||||
// Rotation through the web UI invalidates the old credential immediately,
|
||||
// mints one that authenticates the API and the script path, and warns that
|
||||
// every device must reinstall.
|
||||
@@ -271,7 +265,7 @@ func TestRotateCredentialViaWebUI(t *testing.T) {
|
||||
if err := os.WriteFile(path, []byte("const API_TOKEN = \"__API_TOKEN__\";\n"), 0o644); err != nil {
|
||||
t.Fatalf("write script: %v", err)
|
||||
}
|
||||
cfg := webConfig()
|
||||
cfg := testConfig()
|
||||
cfg.UserscriptPath = path
|
||||
srv := newRouter(s, cfg)
|
||||
|
||||
@@ -338,7 +332,7 @@ func TestRotateCredentialViaWebUI(t *testing.T) {
|
||||
// The app page offers the install links; the credential never appears in its
|
||||
// markup.
|
||||
func TestIndexShowsSetupPanelWithoutCredential(t *testing.T) {
|
||||
srv, st := newWebTestServer(t, webConfig())
|
||||
srv, st := newWebTestServer(t, testConfig())
|
||||
req := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
req.AddCookie(sessionCookie(t, st))
|
||||
rr := httptest.NewRecorder()
|
||||
|
||||
Reference in New Issue
Block a user