From e5ef453ebe553f48e0eb3814e5c547e998108358 Mon Sep 17 00:00:00 2001 From: zkvvoob Date: Wed, 20 May 2026 17:52:43 +0300 Subject: [PATCH] fix(server): address review: log DB error in app-password auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer note: The original error from GetActiveForUser is being swallowed and replaced with model.ErrInvalidAuth. If a database error occurs here, it will be difficult to diagnose because it will be logged as a simple invalid login. It's better to return the actual error so the caller can log it appropriately. Proposed implementation: If we just return err from validateAppPasswordCredentials, a DB failure gets logged as WARN "Invalid login" — same diagnostic hole, different error string. Instead, log the DB error at Error level inside validateAppPasswordCredentials before returning ErrInvalidAuth, so it shows up regardless of how the caller treats the return value. --- server/subsonic/middlewares.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index 8f2d33382..ee03fcc76 100644 --- a/server/subsonic/middlewares.go +++ b/server/subsonic/middlewares.go @@ -174,7 +174,11 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler { // committed by the time the UPDATE returns. func validateAppPasswordCredentials(ctx context.Context, ds model.DataStore, user *model.User, pass, token, salt string) error { aps, err := ds.AppPassword(ctx).GetActiveForUser(ctx, user.ID) - if err != nil || len(aps) == 0 { + if err != nil { + log.Error(ctx, "Failed to load app passwords during auth", "userId", user.ID, err) + return model.ErrInvalidAuth + } + if len(aps) == 0 { return model.ErrInvalidAuth }