mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-08-09 20:26:13 +00:00
fix(admin): surface silent Management API auth failures, add link-state health checks (#585)
## Summary - #269's stuck Spotify presets traced to the account link never completing, invisible because the admin UI silently swallowed 401s from `/api/mgmt/*` instead of prompting for a retry. The browser's own Basic Auth caching already works correctly here (verified live); the bug was purely missing feedback. - Adds two Health-tab checks: Spotify configured but no account linked, and Management API credentials still at the published default. Refs #269, #419. ## Test plan - [x] `make check` (fmt, vet, unit tests) clean - [x] `make lint` clean - [x] Live-tested end to end with a headless Chrome (chromedp) against a local build: confirmed the new failure-path messages render correctly for `fetchSpotifyStatus`, `fetchAccountList`, and `linkSpotify` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
a0f8b86922
commit
bb9bce440e
@@ -196,6 +196,21 @@ func NewServer(ds *datastore.DataStore, sm *setup.Manager, serverURL string, red
|
||||
},
|
||||
s.GetDNSRunning,
|
||||
)
|
||||
health.RegisterSpotifyAccountLinkedCheck(
|
||||
s.healthRegistry,
|
||||
func() bool { return s.spotifyService != nil },
|
||||
func() int {
|
||||
if s.spotifyService == nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
return len(s.spotifyService.GetAccounts())
|
||||
},
|
||||
)
|
||||
health.RegisterMgmtDefaultCredentialsCheck(
|
||||
s.healthRegistry,
|
||||
func() (string, string) { return s.mgmtUsername, s.mgmtPassword },
|
||||
)
|
||||
|
||||
// Health QuickFix executor for the empty-margeAccountUUID
|
||||
// finding from RegisterSpeakerInfoReachable. Lives here (not in
|
||||
|
||||
@@ -112,6 +112,8 @@ async function fetchSpotifyStatus() {
|
||||
const settingsResponse = await fetch("/api/setup/settings");
|
||||
const settings = await settingsResponse.json();
|
||||
const header = document.getElementById("spotify-status-header");
|
||||
const nameEl = document.getElementById("spotify-account-name");
|
||||
const linkBtn = document.getElementById("link-spotify-btn");
|
||||
|
||||
if (!settings.spotify_configured) {
|
||||
if (header) header.style.display = "none";
|
||||
@@ -121,10 +123,22 @@ async function fetchSpotifyStatus() {
|
||||
if (header) header.style.display = "flex";
|
||||
|
||||
const response = await fetch("/api/mgmt/spotify/accounts");
|
||||
if (!response.ok) return;
|
||||
if (!response.ok) {
|
||||
// Don't fail silently: a stale/hidden Spotify status here is exactly
|
||||
// what made #269 look like a Spotify bug instead of a Management API
|
||||
// login that never completed.
|
||||
if (header) {
|
||||
header.style.background = "#f8d7da";
|
||||
header.style.border = "1px solid #dc3545";
|
||||
}
|
||||
if (nameEl) {
|
||||
nameEl.innerText = response.status === 401
|
||||
? "Unable to check (Management login required — retry, or reload the page)"
|
||||
: `Unable to check (HTTP ${response.status})`;
|
||||
}
|
||||
return;
|
||||
}
|
||||
const data = await response.json();
|
||||
const nameEl = document.getElementById("spotify-account-name");
|
||||
const linkBtn = document.getElementById("link-spotify-btn");
|
||||
|
||||
if (data.accounts && data.accounts.length > 0) {
|
||||
header.style.background = "#e6ffed";
|
||||
@@ -162,8 +176,12 @@ async function linkSpotify() {
|
||||
try {
|
||||
const response = await fetch("/api/mgmt/spotify/init", {method: "POST"});
|
||||
if (!response.ok) {
|
||||
const err = await response.text();
|
||||
alert("Failed to initialize Spotify link: " + err);
|
||||
if (response.status === 401) {
|
||||
alert("Management login failed or was cancelled. Please try again — your browser should prompt for the Management API username/password.");
|
||||
} else {
|
||||
const err = await response.text();
|
||||
alert("Failed to initialize Spotify link: " + err);
|
||||
}
|
||||
return;
|
||||
}
|
||||
const data = await response.json();
|
||||
@@ -837,7 +855,15 @@ async function fetchVersion() {
|
||||
async function fetchAccountList() {
|
||||
try {
|
||||
const response = await fetch("/api/mgmt/accounts");
|
||||
if (!response.ok) return;
|
||||
if (!response.ok) {
|
||||
const selector = document.getElementById("account-selector");
|
||||
if (selector) {
|
||||
selector.innerHTML = response.status === 401
|
||||
? `<option value="">Management login required — retry or reload the page</option>`
|
||||
: `<option value="">Unable to load accounts (HTTP ${response.status})</option>`;
|
||||
}
|
||||
return;
|
||||
}
|
||||
const data = await response.json();
|
||||
const selector = document.getElementById("account-selector");
|
||||
if (selector) {
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
package health
|
||||
|
||||
// CheckIDMgmtDefaultCredentials is the registry id of the
|
||||
// management-credentials check. It fires when the Management API
|
||||
// (Basic Auth in front of /api/mgmt/*, guarding Spotify/Amazon
|
||||
// account linking and Local Accounts) is still using the published
|
||||
// default username/password. Those defaults are documented publicly,
|
||||
// so anyone who has read the docs can use them.
|
||||
//
|
||||
// This is a visibility-only nudge (see issue #419): it does not gate
|
||||
// anything and does not change default behavior. A future release is
|
||||
// expected to require an explicit choice (set a password, or opt out)
|
||||
// before /admin is reachable; until then this just surfaces the gap.
|
||||
const CheckIDMgmtDefaultCredentials = "mgmt_default_credentials"
|
||||
|
||||
// DefaultMgmtUsername and DefaultMgmtPassword are the published
|
||||
// defaults (cmd/soundtouch-service flags --mgmt-username/--mgmt-password).
|
||||
// Duplicated here (rather than imported) to keep this package's
|
||||
// dependency surface small; keep in sync if the CLI defaults change.
|
||||
const (
|
||||
DefaultMgmtUsername = "admin"
|
||||
DefaultMgmtPassword = "change_me!"
|
||||
)
|
||||
|
||||
// RegisterMgmtDefaultCredentialsCheck registers the check.
|
||||
// getMgmtCredentials returns the currently-configured management
|
||||
// username and password (typically Server's mgmtUsername/mgmtPassword).
|
||||
func RegisterMgmtDefaultCredentialsCheck(r *Registry, getMgmtCredentials func() (username, password string)) {
|
||||
r.Register(Check{
|
||||
ID: CheckIDMgmtDefaultCredentials,
|
||||
Title: "Management API credentials have been changed from the published default",
|
||||
Run: func() []Finding {
|
||||
username, password := getMgmtCredentials()
|
||||
return runMgmtDefaultCredentialsCheck(username, password)
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func runMgmtDefaultCredentialsCheck(username, password string) []Finding {
|
||||
if username != DefaultMgmtUsername || password != DefaultMgmtPassword {
|
||||
return nil
|
||||
}
|
||||
|
||||
return []Finding{{
|
||||
Severity: SeverityInfo,
|
||||
Message: "The Management API (Spotify/Amazon account linking, Local Accounts) is still using " +
|
||||
"the published default credentials (admin / change_me!). Anyone who has read the docs can use them.",
|
||||
Details: "Set MGMT_USERNAME and MGMT_PASSWORD (env vars or the matching CLI flags) to your own " +
|
||||
"values and restart the service.",
|
||||
}}
|
||||
}
|
||||
@@ -0,0 +1,27 @@
|
||||
package health
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestMgmtDefaultCredentialsCheck_NoFindingWhenChanged(t *testing.T) {
|
||||
for _, tc := range []struct{ username, password string }{
|
||||
{"admin", "somethingElse"},
|
||||
{"someoneElse", "change_me!"},
|
||||
{"customadmin", "customsecret"},
|
||||
} {
|
||||
got := runMgmtDefaultCredentialsCheck(tc.username, tc.password)
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected no findings for %+v, got %+v", tc, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestMgmtDefaultCredentialsCheck_WarnsWhenStillDefault(t *testing.T) {
|
||||
got := runMgmtDefaultCredentialsCheck(DefaultMgmtUsername, DefaultMgmtPassword)
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected one finding for unchanged default credentials, got %+v", got)
|
||||
}
|
||||
|
||||
if got[0].Severity != SeverityInfo {
|
||||
t.Errorf("expected SeverityInfo (visibility-only nudge, not a gate), got %v", got[0].Severity)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,48 @@
|
||||
package health
|
||||
|
||||
// CheckIDSpotifyAccountLinked is the registry id of the Spotify
|
||||
// account-link check. It fires when a Spotify developer app is
|
||||
// configured (client_id/secret set) but no account has completed the
|
||||
// OAuth link — the state in which every speaker's Spotify token
|
||||
// request 503s ("No Spotify accounts linked"), and Spotify presets
|
||||
// only appear to work right after a manual Spotify Connect session
|
||||
// (which bypasses AfterTouch's token proxy entirely and doesn't
|
||||
// survive a reboot).
|
||||
//
|
||||
// See issue #269: this state is otherwise invisible outside the
|
||||
// service log / a diagnostic export, so operators mistake it for a
|
||||
// per-speaker Spotify limitation instead of a link they never
|
||||
// finished.
|
||||
const CheckIDSpotifyAccountLinked = "spotify_account_linked"
|
||||
|
||||
// RegisterSpotifyAccountLinkedCheck registers the check.
|
||||
// getSpotifyConfigured reports whether a Spotify client_id/secret is
|
||||
// set (typically Server's spotifyService != nil); getLinkedAccountCount
|
||||
// returns how many Spotify accounts have completed the OAuth link
|
||||
// (typically len(spotifyService.GetAccounts())).
|
||||
func RegisterSpotifyAccountLinkedCheck(r *Registry, getSpotifyConfigured func() bool, getLinkedAccountCount func() int) {
|
||||
r.Register(Check{
|
||||
ID: CheckIDSpotifyAccountLinked,
|
||||
Title: "Spotify account is linked",
|
||||
Run: func() []Finding {
|
||||
return runSpotifyAccountLinkedCheck(getSpotifyConfigured(), getLinkedAccountCount())
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
func runSpotifyAccountLinkedCheck(configured bool, linkedCount int) []Finding {
|
||||
if !configured || linkedCount > 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
return []Finding{{
|
||||
Severity: SeverityWarning,
|
||||
Message: "Spotify is configured (client ID/secret set) but no account is linked yet. " +
|
||||
"Every speaker's Spotify token request will fail (503 \"No Spotify accounts linked\"), " +
|
||||
"so stored Spotify presets stay dead until you manually Spotify Connect from a phone — " +
|
||||
"and that only primes the one speaker you connected to, only until its next reboot.",
|
||||
Details: "Finish the link on the Admin page: Settings → Spotify → \"Link Spotify Account\", " +
|
||||
"then complete the consent screen. Once linked, AfterTouch re-primes each speaker " +
|
||||
"automatically on power-on.",
|
||||
}}
|
||||
}
|
||||
@@ -0,0 +1,28 @@
|
||||
package health
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestSpotifyAccountLinkedCheck_NoFindingWhenNotConfigured(t *testing.T) {
|
||||
got := runSpotifyAccountLinkedCheck(false, 0)
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected no findings when Spotify isn't configured, got %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpotifyAccountLinkedCheck_NoFindingWhenLinked(t *testing.T) {
|
||||
got := runSpotifyAccountLinkedCheck(true, 1)
|
||||
if len(got) != 0 {
|
||||
t.Errorf("expected no findings when an account is linked, got %+v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSpotifyAccountLinkedCheck_WarnsWhenConfiguredButUnlinked(t *testing.T) {
|
||||
got := runSpotifyAccountLinkedCheck(true, 0)
|
||||
if len(got) != 1 {
|
||||
t.Fatalf("expected one finding for configured-but-unlinked, got %+v", got)
|
||||
}
|
||||
|
||||
if got[0].Severity != SeverityWarning {
|
||||
t.Errorf("expected SeverityWarning, got %v", got[0].Severity)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user