Files
OwnCord/Server/service/profile_fields_test.go
T
J3vbandClaude Fable 5 8579cb5d91 fix: batch of 25 correctness fixes across server and client (#1370)
* chore(workflows): raise subagent effort tiers (sonnet/haiku to xhigh, prove opus to high)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(voice): 6 defect(s) (OC-0098, OC-0004, OC-0005, OC-0006, OC-0007, OC-0020)

* fix(db): 1 defect(s) (OC-0096)

* fix(admin): 1 defect(s) (OC-0097)

* fix(auth): 2 defect(s) (OC-0099, OC-0021)

* fix(voice): 1 defect(s) (OC-0018)

* fix(admin): 1 defect(s) (OC-0045)

* fix(api): 1 defect(s) (OC-0103)

* fix(client): 1 defect(s) (OC-0105)

* fix(client): 1 defect(s) (OC-0107)

* fix(api): 1 defect(s) (OC-0109)

* fix(api): 1 defect(s) (OC-0112)

* test(admin): compare restore bytes with bytes.Equal

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(voice): 2 defect(s) (OC-0095, OC-0014)

OC-0095: createRoom never called setE2EEEnabled(true), so the full ECDH/HKDF/AES-GCM key exchange completed but frames still reached the SFU in plaintext.

OC-0014: token refresh timer was 23h while the server mints LiveKit tokens with a 5-minute TTL, so any reconnect after minute 5 presented an expired token.

* fix(profile): 2 defect(s) (OC-0100, OC-0102)

* fix(service): 1 defect(s) (OC-0022)

Archived channels were only read-only for SendMessage/DeleteMessage. Edit, reaction, pin and purge sinks bypassed the check. Route every write sink through a shared requireChannelWritable gate.

* fix(api): 1 defect(s) (OC-0048)

* chore(workflows): correct stale model labels in bughunt-fix phase details

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(client): 1 defect(s) (OC-0015)

* fix(voice): 1 defect(s) (OC-0002)

* test: fix two CI-only failures in the batch-4 test suite

The delete-account broadcast test now observes member_ban on a second
client's socket: the hub broadcasts and then force-disconnects the target,
so on a slow runner the close could beat the target's own copy of the
frame. The observer is also the party the event exists for.

The voice e2e mock now echoes the real joined channel id on voice_leave
(it hardcoded channel_id 0, which the dispatcher's channel-matched
self-leave teardown correctly ignores), and the rejoin test waits for the
mock's delayed echoes to settle before clicking the row again — clicking
inside the echo window toggled a leave instead of a join.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-08-14 14:49:27 +02:00

315 lines
12 KiB
Go

package service
import (
"context"
"errors"
"strings"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/owncord/server/db"
"github.com/owncord/server/permissions"
)
// Phase 6 profile fields. The rules that matter here are the ones a handler
// could plausibly skip: sanitization, the length bounds, "omitted means
// unchanged", and "empty means cleared". They live in the service precisely so
// every transport gets them, so they are tested against the service.
func newUserSvc(t *testing.T) (*UserService, *db.DB) {
t.Helper()
database := newTestDB(t)
seedUser(t, database, &db.User{ID: 1, Username: "ada", PasswordHash: "h"})
return NewUserService(database), database
}
func TestUpdateProfile_SetsAndClearsDisplayNameAndAbout(t *testing.T) {
svc, _ := newUserSvc(t)
ctx := context.Background()
name, about := "Ada L.", "counts on it"
u, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", DisplayName: &name, About: &about})
if err != nil {
t.Fatalf("UpdateProfile: %v", err)
}
if u.DisplayName == nil || *u.DisplayName != name {
t.Fatalf("display_name = %v, want %q", u.DisplayName, name)
}
if u.About == nil || *u.About != about {
t.Fatalf("about = %v, want %q", u.About, about)
}
// A patch that mentions neither field must leave both standing — the
// update writes every column, so this is the merge doing its job.
u, err = svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada"})
if err != nil {
t.Fatalf("UpdateProfile (username only): %v", err)
}
if u.DisplayName == nil || u.About == nil {
t.Fatalf("a username-only patch cleared other fields: %v / %v", u.DisplayName, u.About)
}
// An explicit empty string clears.
empty := ""
u, err = svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", DisplayName: &empty, About: &empty})
if err != nil {
t.Fatalf("UpdateProfile (clear): %v", err)
}
if u.DisplayName != nil || u.About != nil {
t.Fatalf("expected cleared, got %v / %v", u.DisplayName, u.About)
}
}
// raceDetectingStore wraps a real Store and records whether two GetUserByID
// calls were ever in flight at once, to prove UpdateProfile's read-merge-
// write serializes per user rather than merely happening to avoid a race
// under a particular timing.
type raceDetectingStore struct {
Store
active int32
overlap int32
}
func (r *raceDetectingStore) GetUserByID(ctx context.Context, id int64) (*db.User, error) {
if atomic.AddInt32(&r.active, 1) > 1 {
atomic.AddInt32(&r.overlap, 1)
}
time.Sleep(5 * time.Millisecond) // widen the window a real race would need
defer atomic.AddInt32(&r.active, -1)
return r.Store.GetUserByID(ctx, id)
}
func TestUpdateProfile_ConcurrentUpdatesSerializePerUser(t *testing.T) {
database := newTestDB(t)
seedUser(t, database, &db.User{ID: 1, Username: "ada", PasswordHash: "h"})
rs := &raceDetectingStore{Store: database}
svc := NewUserService(rs)
ctx := context.Background()
// Simulates PATCH /users/me (sets display_name) racing
// POST /users/me/avatar (sets about, standing in for the avatar column;
// both calls pass the unrelated field's current value the way the real
// handlers do). Without serialization, whichever call's read lands
// between the other's read and write would revert that other call's
// change when it writes its own stale merge.
var wg sync.WaitGroup
wg.Add(2)
go func() {
defer wg.Done()
name := "Ada L."
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", DisplayName: &name}); err != nil {
t.Errorf("UpdateProfile (display_name): %v", err)
}
}()
go func() {
defer wg.Done()
about := "counts on it"
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", About: &about}); err != nil {
t.Errorf("UpdateProfile (about): %v", err)
}
}()
wg.Wait()
if got := atomic.LoadInt32(&rs.overlap); got != 0 {
t.Errorf("UpdateProfile's read-merge-write overlapped %d times, want 0 (must be serialized per user)", got)
}
u, err := database.GetUserByID(ctx, 1)
if err != nil {
t.Fatalf("GetUserByID: %v", err)
}
if u.DisplayName == nil || *u.DisplayName != "Ada L." {
t.Errorf("display_name = %v, want %q — must not be reverted by a concurrent update", u.DisplayName, "Ada L.")
}
if u.About == nil || *u.About != "counts on it" {
t.Errorf("about = %v, want %q — must not be reverted by a concurrent update", u.About, "counts on it")
}
}
// OC-0102: an avatar-only patch must not carry a username at all, so a stale
// pre-lock snapshot (handleUploadAvatar captures user.Username before the
// multipart parse / image decode / disk write, well before this function's
// per-user lock) can never overwrite a rename that commits in the meantime.
// UpdateProfile's read-merge-write already treats DisplayName/About this
// way (nil-vs-non-nil pointer); Username needs the same "unspecified means
// leave it alone" contract via its own zero value, since it is a plain
// string rather than a pointer.
func TestUpdateProfile_AvatarOnlyPatchDoesNotOverwriteUsername(t *testing.T) {
svc, database := newUserSvc(t)
ctx := context.Background()
// Models a concurrent PATCH /users/me rename that lands and commits
// first.
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "bob"}); err != nil {
t.Fatalf("rename UpdateProfile: %v", err)
}
// An avatar-only caller supplies no username intent at all — Username is
// left at its zero value, the way handleUploadAvatar's ProfilePatch must
// after the fix (it no longer fills Username from its stale snapshot).
avatarURL := "/api/v1/files/abc"
u, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Avatar: &avatarURL})
if err != nil {
t.Fatalf("avatar-only UpdateProfile: %v", err)
}
if u.Username != "bob" {
t.Fatalf("username = %q, want %q — an avatar-only patch must not revert a concurrent rename", u.Username, "bob")
}
if u.Avatar == nil || *u.Avatar != avatarURL {
t.Fatalf("avatar = %v, want %q — the avatar itself must still be applied", u.Avatar, avatarURL)
}
stored, err := database.GetUserByID(ctx, 1)
if err != nil {
t.Fatalf("GetUserByID: %v", err)
}
if stored.Username != "bob" {
t.Fatalf("stored username = %q, want %q", stored.Username, "bob")
}
}
func TestUpdateProfile_SanitizesAndTrims(t *testing.T) {
svc, _ := newUserSvc(t)
name := " <b>Ada</b> "
about := "<script>alert(1)</script>hello"
u, err := svc.UpdateProfile(context.Background(), 1, ProfilePatch{
Username: "ada", DisplayName: &name, About: &about,
})
if err != nil {
t.Fatalf("UpdateProfile: %v", err)
}
if u.DisplayName == nil || *u.DisplayName != "Ada" {
t.Errorf("display_name = %v, want %q", u.DisplayName, "Ada")
}
if u.About == nil || strings.Contains(*u.About, "<") {
t.Errorf("about = %v, want markup stripped", u.About)
}
}
func TestUpdateProfile_RejectsOverlongFields(t *testing.T) {
svc, _ := newUserSvc(t)
ctx := context.Background()
tooLongName := strings.Repeat("a", MaxDisplayNameLen+1)
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", DisplayName: &tooLongName}); !errors.Is(err, ErrBadRequest) {
t.Errorf("overlong display_name err = %v, want ErrBadRequest", err)
}
tooLongAbout := strings.Repeat("b", MaxAboutLen+1)
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", About: &tooLongAbout}); !errors.Is(err, ErrBadRequest) {
t.Errorf("overlong about err = %v, want ErrBadRequest", err)
}
// Exactly at the bound is accepted, and the bound counts runes rather than
// bytes — otherwise a name in a non-Latin script would be a third as long.
atBound := strings.Repeat("é", MaxDisplayNameLen)
if _, err := svc.UpdateProfile(ctx, 1, ProfilePatch{Username: "ada", DisplayName: &atBound}); err != nil {
t.Errorf("display_name at bound rejected: %v", err)
}
}
func TestSetCustomStatus_RoundTripClearAndBound(t *testing.T) {
svc, database := newUserSvc(t)
ctx := context.Background()
if err := svc.SetCustomStatus(ctx, 1, " <i>debugging</i> "); err != nil {
t.Fatalf("SetCustomStatus: %v", err)
}
u, _ := database.GetUserByID(ctx, 1)
if u.CustomStatus == nil || *u.CustomStatus != "debugging" {
t.Fatalf("custom_status = %v, want sanitized+trimmed %q", u.CustomStatus, "debugging")
}
if err := svc.SetCustomStatus(ctx, 1, " "); err != nil {
t.Fatalf("SetCustomStatus (blank): %v", err)
}
u, _ = database.GetUserByID(ctx, 1)
if u.CustomStatus != nil {
t.Fatalf("whitespace-only text should clear, got %q", *u.CustomStatus)
}
if err := svc.SetCustomStatus(ctx, 1, strings.Repeat("x", MaxCustomStatusLen+1)); !errors.Is(err, ErrBadRequest) {
t.Errorf("overlong custom_status err = %v, want ErrBadRequest", err)
}
}
func TestClearCustomStatus(t *testing.T) {
svc, database := newUserSvc(t)
ctx := context.Background()
if err := svc.SetCustomStatus(ctx, 1, "afk"); err != nil {
t.Fatalf("SetCustomStatus: %v", err)
}
if err := svc.ClearCustomStatus(ctx, 1); err != nil {
t.Fatalf("ClearCustomStatus: %v", err)
}
u, _ := database.GetUserByID(ctx, 1)
if u.CustomStatus != nil {
t.Fatalf("custom_status = %q, want nil", *u.CustomStatus)
}
}
func TestAvatarFileURL(t *testing.T) {
if got := AvatarFileURL("abc"); got != "/api/v1/files/abc" {
t.Errorf("AvatarFileURL = %q", got)
}
}
func TestHandlePresenceUpdate_AcceptsInvisibleAndCarriesCustomStatus(t *testing.T) {
database := newTestDB(t)
seedUser(t, database, &db.User{ID: 1, Username: "ada", PasswordHash: "h"})
svc := NewChannelService(database, NewPermissionService(database, permissions.NewChecker(database)))
ctx := context.Background()
text := " <b>away</b> "
got, err := svc.HandlePresenceUpdate(ctx, 1, db.StatusInvisible, &text, nil)
if err != nil {
t.Fatalf("HandlePresenceUpdate: %v", err)
}
if got == nil || *got != "away" {
t.Fatalf("returned custom status = %v, want sanitized %q", got, "away")
}
u, _ := database.GetUserByID(ctx, 1)
// Stored as chosen — the invisible -> offline collapse is a broadcast-time
// concern, and storing it collapsed would make the next connect unable to
// tell "appear offline" from "not connected".
if u.Status != db.StatusInvisible {
t.Fatalf("stored status = %q, want invisible", u.Status)
}
// A later status flip that carries no custom_status field must leave the
// text alone AND still put it on the wire, or the auto-idle timer would
// blank everyone else's copy several times an hour.
got, err = svc.HandlePresenceUpdate(ctx, 1, db.StatusIdle, nil, nil)
if err != nil {
t.Fatalf("HandlePresenceUpdate (bare): %v", err)
}
if got == nil || *got != "away" {
t.Fatalf("bare status flip returned %v, want the stored %q", got, "away")
}
u, _ = database.GetUserByID(ctx, 1)
if u.CustomStatus == nil || *u.CustomStatus != "away" {
t.Fatalf("bare status flip changed the stored text: %v", u.CustomStatus)
}
}
func TestHandlePresenceUpdate_RejectsUnknownStatusAndOverlongText(t *testing.T) {
database := newTestDB(t)
seedUser(t, database, &db.User{ID: 1, Username: "ada", PasswordHash: "h"})
svc := NewChannelService(database, NewPermissionService(database, permissions.NewChecker(database)))
ctx := context.Background()
if _, err := svc.HandlePresenceUpdate(ctx, 1, "afk", nil, nil); !errors.Is(err, ErrBadRequest) {
t.Errorf("unknown status err = %v, want ErrBadRequest", err)
}
long := strings.Repeat("x", MaxCustomStatusLen+1)
if _, err := svc.HandlePresenceUpdate(ctx, 1, db.StatusOnline, &long, nil); !errors.Is(err, ErrBadRequest) {
t.Errorf("overlong custom_status err = %v, want ErrBadRequest", err)
}
// The rejected call must not have committed the status either.
u, _ := database.GetUserByID(ctx, 1)
if u.Status == db.StatusOnline {
t.Error("a rejected presence_update must not commit the status")
}
}