bvandeusen
b970b87343
Server-side fix for the drift audit finding (Scribe #578, parent #552). Mirrored on Android (and Flutter) but the root cause and the smallest blast-radius fix both live here. The bug: - Android MeApi.getProfile() calls GET /api/me and deserializes into MyProfileWire which has nullable display_name + email. - Server's handleGetMe was emitting the narrower UserView shape (id, username, is_admin only). - Android always saw displayName=null, email=null. The Settings → Profile screen rendered BLANK form fields for users with stored values. - Saving from the blank state submitted empty strings to PUT /api/me/profile, which interprets empty as "clear to NULL" (me_profile.go:53-65) — DESTROYING the user's saved profile. - Flutter (flutter_client/lib/api/endpoints/settings.dart:9-12) has the identical bug pattern. The fix: - handleGetMe now emits profileViewFromUser(user) — the same shape PUT /api/me/profile already returns (meProfileResp: id, username, display_name, email, is_admin). - auth.UserFromContext already returns a full dbq.User row, so no extra DB lookup needed. - Web's User TypeScript type is narrower than this response but doesn't care about the extra fields (TS structural typing). - LoginResp.User still uses UserView; login response unchanged. New test asserts the regression directly: a user with stored display_name + email sees them in /api/me. Old test updated to decode into meProfileResp and assert the nullable fields are correctly null for an unset profile. Android side needs no change — the existing wire shape already expected display_name + email; this just delivers them.
93 lines
2.8 KiB
Go
93 lines
2.8 KiB
Go
package api
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
|
)
|
|
|
|
func TestHandleGetMe_ReturnsAuthenticatedUser(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
user := seedUser(t, pool, "alice", "hunter2", true)
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/me", nil)
|
|
req = withUser(req, user)
|
|
w := httptest.NewRecorder()
|
|
h.handleGetMe(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d body = %s", w.Code, w.Body.String())
|
|
}
|
|
var got meProfileResp
|
|
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
if got.Username != "test-alice" || !got.IsAdmin {
|
|
t.Errorf("user = %+v, want test-alice/admin", got)
|
|
}
|
|
if got.DisplayName != nil {
|
|
t.Errorf("DisplayName = %v, want nil for a user with no profile set", *got.DisplayName)
|
|
}
|
|
if got.Email != nil {
|
|
t.Errorf("Email = %v, want nil for a user with no profile set", *got.Email)
|
|
}
|
|
}
|
|
|
|
// Drift #578 regression guard: a user whose profile fields ARE set
|
|
// must see them in the /api/me response. Previously this handler
|
|
// emitted the narrower UserView so Android Settings → Profile saw
|
|
// display_name=null and email=null on every read, wiped the form
|
|
// fields, and submitting from that blank state destroyed the user's
|
|
// stored values via the "empty string clears to NULL" semantics of
|
|
// PUT /api/me/profile.
|
|
func TestHandleGetMe_IncludesDisplayNameAndEmail(t *testing.T) {
|
|
h, pool := testHandlers(t)
|
|
user := seedUser(t, pool, "alice", "hunter2", false)
|
|
|
|
displayName := "Alice Liddell"
|
|
email := "alice@example.com"
|
|
updated, err := dbq.New(pool).UpdateUserProfile(context.Background(), dbq.UpdateUserProfileParams{
|
|
ID: user.ID,
|
|
DisplayName: &displayName,
|
|
Email: &email,
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("UpdateUserProfile: %v", err)
|
|
}
|
|
|
|
req := httptest.NewRequest(http.MethodGet, "/api/me", nil)
|
|
req = withUser(req, updated)
|
|
w := httptest.NewRecorder()
|
|
h.handleGetMe(w, req)
|
|
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("status = %d body = %s", w.Code, w.Body.String())
|
|
}
|
|
var got meProfileResp
|
|
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
|
|
t.Fatalf("decode: %v", err)
|
|
}
|
|
if got.DisplayName == nil || *got.DisplayName != displayName {
|
|
t.Errorf("DisplayName = %v, want %q", got.DisplayName, displayName)
|
|
}
|
|
if got.Email == nil || *got.Email != email {
|
|
t.Errorf("Email = %v, want %q", got.Email, email)
|
|
}
|
|
}
|
|
|
|
func TestHandleGetMe_MissingContextReturns500(t *testing.T) {
|
|
h, _ := testHandlers(t)
|
|
req := httptest.NewRequest(http.MethodGet, "/api/me", nil)
|
|
w := httptest.NewRecorder()
|
|
h.handleGetMe(w, req)
|
|
|
|
if w.Code != http.StatusInternalServerError {
|
|
t.Errorf("status = %d, want 500 (context should be populated by RequireUser)", w.Code)
|
|
}
|
|
_ = dbq.User{} // keep the import used
|
|
}
|