diff --git a/internal/api/pgrepo.go b/internal/api/pgrepo.go index 5a7064d..582abac 100644 --- a/internal/api/pgrepo.go +++ b/internal/api/pgrepo.go @@ -2000,18 +2000,23 @@ func (p *PGRepo) UserDetail(ctx context.Context, userID string) (*UserDetail, er linkRows, err := p.db.QueryContext(ctx, `SELECT mc_uuid::text, COALESCE(auth_source, 'mojang'), verified_at FROM account_links WHERE user_id = $1 ORDER BY verified_at`, userID) + // A failed read is the call's failure: an empty list would tell the admin the + // user has no Minecraft account linked. if err != nil { - return &d, nil // best-effort; linked accounts are informational + return nil, err } defer linkRows.Close() for linkRows.Next() { var a LinkedAccount if err := linkRows.Scan(&a.MCUUID, &a.AuthSource, &a.VerifiedAt); err != nil { - return &d, nil + return nil, err } d.LinkedAccounts = append(d.LinkedAccounts, a) } - return &d, linkRows.Err() + if err := linkRows.Err(); err != nil { + return nil, err + } + return &d, nil } // CreateUser mints a new user row. A username conflict → ErrConflict. diff --git a/internal/pgint/users_test.go b/internal/pgint/users_test.go new file mode 100644 index 0000000..e9fae0b --- /dev/null +++ b/internal/pgint/users_test.go @@ -0,0 +1,58 @@ +//go:build pgint + +package pgint + +import ( + "context" + "net/url" + "os" + "testing" + + "felis.lolicon.best/internal/api" + "felis.lolicon.best/internal/store" +) + +// ---- user admin ---------------------------------------------------------------- + +// A user's detail whose linked accounts cannot be read is an error. It used to come +// back with an empty list, which tells the admin the user has linked nothing. +func TestUserDetailLinkReadFailure(t *testing.T) { + ctx := context.Background() + u := newUser(t, "user", "udl") + mc := testUUID(t) + mustExec(t, `INSERT INTO account_links (user_id, mc_uuid) VALUES ($1, $2)`, u.ID, mc) + + // A role that reads users and servers and nothing else. + const role = "pgint_nolinks" + drop := func() { + for _, stmt := range []string{"DROP OWNED BY " + role, "DROP ROLE " + role} { + if _, err := db.ExecContext(ctx, stmt); err != nil { + t.Errorf("%s: %v", stmt, err) + } + } + } + mustExec(t, "CREATE ROLE "+role+" LOGIN") + t.Cleanup(drop) + mustExec(t, "GRANT USAGE ON SCHEMA public TO "+role) + mustExec(t, "GRANT SELECT ON users, servers TO "+role) + dsn, err := url.Parse(os.Getenv("FELIS_TEST_PG_URL")) + if err != nil { + t.Fatal(err) + } + dsn.User = url.User(role) + drv, err := store.Open(ctx, dsn.String()) + if err != nil { + t.Fatalf("open as %s: %v", role, err) + } + defer drv.Close() + limited := api.NewPGRepo(drv.DB()) + + if d, err := limited.UserDetail(ctx, u.ID); err == nil { + t.Fatalf("UserDetail with account_links unreadable = %+v, nil; want its error", d) + } + mustExec(t, "GRANT SELECT ON account_links TO "+role) + d, err := limited.UserDetail(ctx, u.ID) + if err != nil || d.Username != u.Username || len(d.LinkedAccounts) != 1 || d.LinkedAccounts[0].MCUUID != mc { + t.Fatalf("UserDetail with account_links readable = %+v, %v; want %s with %s linked", d, err, u.Username, mc) + } +}