feat(auth): store Subsonic API keys hashed; a new key is shown once (M462 #4983)
test-web / test (push) Successful in 2m4s
test-go / test (push) Successful in 2m23s
test-go / integration (push) Successful in 5m28s
release / Build signed APK (releases and dev) (push) Successful in 6m29s
release / Build + push container image (push) Successful in 29s
release / Verify release artifacts (tag releases only) (push) Skipped
test-web / test (push) Successful in 2m4s
test-go / test (push) Successful in 2m23s
test-go / integration (push) Successful in 5m28s
release / Build signed APK (releases and dev) (push) Successful in 6m29s
release / Build + push container image (push) Successful in 29s
release / Verify release artifacts (tag releases only) (push) Skipped
users.api_token held each user's apiKey in plaintext and was looked up by equality, so a leaked row or backup handed out working keys. Migration 0063 replaces it with api_token_hash (sha256, hex), computed in place from the existing keys so every Subsonic client keeps working. The key can no longer be read back: GET /api/me/api-token is gone, and POST returns the new key once. Settings shows it right after Regenerate with a copy button and a "won't be shown again" note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -169,7 +169,7 @@ func (h *handlers) handleAdminCreateUser(w http.ResponseWriter, r *http.Request)
|
||||
user, err := q.CreateUserAdmin(r.Context(), dbq.CreateUserAdminParams{
|
||||
Username: req.Username,
|
||||
PasswordHash: string(hash),
|
||||
ApiToken: apiToken,
|
||||
ApiTokenHash: auth.HashAPIToken(apiToken),
|
||||
IsAdmin: req.IsAdmin,
|
||||
DisplayName: req.DisplayName,
|
||||
})
|
||||
|
||||
@@ -108,7 +108,6 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev
|
||||
authed.Put("/me/password", h.handleChangePassword)
|
||||
authed.Put("/me/profile", h.handleUpdateMyProfile)
|
||||
authed.Put("/me/timezone", h.handlePutTimezone)
|
||||
authed.Get("/me/api-token", h.handleGetMyAPIToken)
|
||||
authed.Post("/me/api-token", h.handleRegenerateMyAPIToken)
|
||||
authed.Get("/me/sessions", h.handleListMySessions)
|
||||
authed.Delete("/me/sessions/{id}", h.handleRevokeMySession)
|
||||
|
||||
@@ -161,7 +161,7 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) {
|
||||
user, err := q.CreateUserFirstAdminRace(r.Context(), dbq.CreateUserFirstAdminRaceParams{
|
||||
Username: req.Username,
|
||||
PasswordHash: string(hash),
|
||||
ApiToken: apiToken,
|
||||
ApiTokenHash: auth.HashAPIToken(apiToken),
|
||||
DisplayName: req.DisplayName,
|
||||
})
|
||||
if err != nil {
|
||||
|
||||
@@ -91,7 +91,7 @@ func seedUser(t *testing.T, pool *pgxpool.Pool, username, password string, isAdm
|
||||
u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{
|
||||
Username: prefixed,
|
||||
PasswordHash: string(hash),
|
||||
ApiToken: "test-api-token-" + prefixed,
|
||||
ApiTokenHash: "test-api-token-" + prefixed,
|
||||
IsAdmin: isAdmin,
|
||||
})
|
||||
if err != nil {
|
||||
|
||||
@@ -13,23 +13,11 @@ type apiTokenResp struct {
|
||||
APIToken string `json:"api_token"`
|
||||
}
|
||||
|
||||
// handleGetMyAPIToken implements GET /api/me/api-token.
|
||||
func (h *handlers) handleGetMyAPIToken(w http.ResponseWriter, r *http.Request) {
|
||||
user, ok := requireUser(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
q := dbq.New(h.pool)
|
||||
current, err := q.GetUserByID(r.Context(), user.ID)
|
||||
if err != nil {
|
||||
h.logger.Error("get api token: lookup failed", "err", err)
|
||||
writeErr(w, apierror.Internal(err))
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, apiTokenResp{APIToken: current.ApiToken})
|
||||
}
|
||||
|
||||
// handleRegenerateMyAPIToken implements POST /api/me/api-token.
|
||||
//
|
||||
// Only the key's hash is stored, so this response is the one time the key
|
||||
// can be read. There is deliberately no GET: a key that has been shown and
|
||||
// lost is replaced, not looked up.
|
||||
func (h *handlers) handleRegenerateMyAPIToken(w http.ResponseWriter, r *http.Request) {
|
||||
user, ok := requireUser(w, r)
|
||||
if !ok {
|
||||
@@ -42,11 +30,10 @@ func (h *handlers) handleRegenerateMyAPIToken(w http.ResponseWriter, r *http.Req
|
||||
return
|
||||
}
|
||||
q := dbq.New(h.pool)
|
||||
updated, err := q.RegenerateApiToken(r.Context(), dbq.RegenerateApiTokenParams{
|
||||
ID: user.ID,
|
||||
ApiToken: newToken,
|
||||
})
|
||||
if err != nil {
|
||||
if err := q.RegenerateApiToken(r.Context(), dbq.RegenerateApiTokenParams{
|
||||
ID: user.ID,
|
||||
ApiTokenHash: auth.HashAPIToken(newToken),
|
||||
}); err != nil {
|
||||
h.logger.Error("regenerate api token: update failed", "err", err)
|
||||
writeErr(w, apierror.Internal(err))
|
||||
return
|
||||
@@ -54,5 +41,5 @@ func (h *handlers) handleRegenerateMyAPIToken(w http.ResponseWriter, r *http.Req
|
||||
|
||||
audit.WriteOrLog(r.Context(), h.pool, h.logger, user.ID, user.ID, audit.ActionTokenRegenerate, nil)
|
||||
|
||||
writeJSON(w, http.StatusOK, apiTokenResp{APIToken: updated.ApiToken})
|
||||
writeJSON(w, http.StatusOK, apiTokenResp{APIToken: newToken})
|
||||
}
|
||||
|
||||
@@ -8,47 +8,23 @@ import (
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/auth"
|
||||
"github.com/go-chi/chi/v5"
|
||||
)
|
||||
|
||||
func newMeTokenRouter(h *handlers) chi.Router {
|
||||
r := chi.NewRouter()
|
||||
r.Get("/api/me/api-token", h.handleGetMyAPIToken)
|
||||
r.Post("/api/me/api-token", h.handleRegenerateMyAPIToken)
|
||||
return r
|
||||
}
|
||||
|
||||
func TestGetAPIToken_ReturnsCurrentToken(t *testing.T) {
|
||||
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
h, pool := testHandlers(t)
|
||||
user := seedUser(t, pool, "tok1", "pw", false)
|
||||
|
||||
req := httptest.NewRequest(http.MethodGet, "/api/me/api-token", nil)
|
||||
req = withUser(req, user)
|
||||
rec := httptest.NewRecorder()
|
||||
newMeTokenRouter(h).ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200; body=%s", rec.Code, rec.Body.String())
|
||||
}
|
||||
var resp apiTokenResp
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("decode: %v", err)
|
||||
}
|
||||
if resp.APIToken != user.ApiToken {
|
||||
t.Errorf("api_token = %q, want %q", resp.APIToken, user.ApiToken)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRegenerateAPIToken_IssuesNewToken(t *testing.T) {
|
||||
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
h, pool := testHandlers(t)
|
||||
user := seedUser(t, pool, "tok2", "pw", false)
|
||||
oldToken := user.ApiToken
|
||||
oldHash := user.ApiTokenHash
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/me/api-token", nil)
|
||||
req = withUser(req, user)
|
||||
@@ -65,20 +41,20 @@ func TestRegenerateAPIToken_IssuesNewToken(t *testing.T) {
|
||||
if resp.APIToken == "" {
|
||||
t.Fatalf("api_token is empty")
|
||||
}
|
||||
if resp.APIToken == oldToken {
|
||||
t.Errorf("api_token unchanged after regenerate")
|
||||
}
|
||||
|
||||
// Verify DB row has the new token and old token no longer matches.
|
||||
var dbToken string
|
||||
// The DB holds the new key's hash, never the key, and the old key no
|
||||
// longer matches.
|
||||
var dbHash string
|
||||
if err := pool.QueryRow(context.Background(),
|
||||
"SELECT api_token FROM users WHERE id = $1", user.ID).Scan(&dbToken); err != nil {
|
||||
t.Fatalf("read token: %v", err)
|
||||
"SELECT api_token_hash FROM users WHERE id = $1", user.ID).Scan(&dbHash); err != nil {
|
||||
t.Fatalf("read token hash: %v", err)
|
||||
}
|
||||
if dbToken != resp.APIToken {
|
||||
t.Errorf("DB api_token = %q, want %q", dbToken, resp.APIToken)
|
||||
if dbHash != auth.HashAPIToken(resp.APIToken) {
|
||||
t.Errorf("DB api_token_hash = %q, want hash of the returned key", dbHash)
|
||||
}
|
||||
if dbToken == oldToken {
|
||||
t.Errorf("old token still in DB after regenerate")
|
||||
if dbHash == oldHash {
|
||||
t.Errorf("old key hash still in DB after regenerate")
|
||||
}
|
||||
if dbHash == resp.APIToken {
|
||||
t.Errorf("DB holds the raw key")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user