fix(auth): build password-reset links from an operator-set public address, never the Host header (M462 #4981)
test-go / test (push) Successful in 1m29s
test-web / test (push) Successful in 1m37s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
test-go / integration (push) Canceled after 2m45s
release / Build signed APK (releases and dev) (push) Canceled after 3m40s
test-go / test (push) Successful in 1m29s
test-web / test (push) Successful in 1m37s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
test-go / integration (push) Canceled after 2m45s
release / Build signed APK (releases and dev) (push) Canceled after 3m40s
buildResetURL used r.Host and r.TLS, so a forgot-password request with a forged Host emailed the victim a real reset token on a link to the attacker's server. Links now come only from network_settings.public_url (migration 0062), and no reset email is sent while it is empty; the response stays the same opaque 200 and the log says why. The address is set on a new "Public address" card under Admin → Integrations, which offers the page's own origin and warns while unset. PUT /api/admin/network-settings takes either field alone, so the proxy card and this one can't overwrite each other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -23,10 +23,16 @@ type networkSettingsResp struct {
|
|||||||
// arrived and count them rather than guess.
|
// arrived and count them rather than guess.
|
||||||
ForwardedChain string `json:"forwarded_chain"`
|
ForwardedChain string `json:"forwarded_chain"`
|
||||||
RemoteAddr string `json:"remote_addr"`
|
RemoteAddr string `json:"remote_addr"`
|
||||||
|
// PublicURL is where users reach Minstrel; reset emails link to it and
|
||||||
|
// are not sent while it is empty.
|
||||||
|
PublicURL string `json:"public_url"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Both fields are optional so the proxy card and the public-address card can
|
||||||
|
// each save their own value without overwriting the other's.
|
||||||
type updateNetworkSettingsReq struct {
|
type updateNetworkSettingsReq struct {
|
||||||
TrustedProxyHops int `json:"trusted_proxy_hops"`
|
TrustedProxyHops *int `json:"trusted_proxy_hops"`
|
||||||
|
PublicURL *string `json:"public_url"`
|
||||||
}
|
}
|
||||||
|
|
||||||
func (h *handlers) handleGetNetworkSettings(w http.ResponseWriter, r *http.Request) {
|
func (h *handlers) handleGetNetworkSettings(w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -39,13 +45,37 @@ func (h *handlers) handleUpdateNetworkSettings(w http.ResponseWriter, r *http.Re
|
|||||||
writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON"))
|
writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON"))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := h.netSettings.SetHops(r.Context(), req.TrustedProxyHops); err != nil {
|
if req.TrustedProxyHops == nil && req.PublicURL == nil {
|
||||||
if errors.Is(err, netsettings.ErrHopsOutOfRange) {
|
writeErr(w, apierror.BadRequest("invalid_body", "nothing to update"))
|
||||||
writeErr(w, apierror.BadRequest("invalid_hops", err.Error()))
|
return
|
||||||
|
}
|
||||||
|
// Validate everything before writing anything, so a bad public URL can't
|
||||||
|
// leave the hops half-saved.
|
||||||
|
if req.TrustedProxyHops != nil && (*req.TrustedProxyHops < 0 || *req.TrustedProxyHops > netsettings.MaxTrustedProxyHops) {
|
||||||
|
writeErr(w, apierror.BadRequest("invalid_hops", netsettings.ErrHopsOutOfRange.Error()))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if req.PublicURL != nil {
|
||||||
|
if _, err := netsettings.NormalizePublicURL(*req.PublicURL); err != nil {
|
||||||
|
writeErr(w, apierror.BadRequest("invalid_public_url", err.Error()))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if req.TrustedProxyHops != nil {
|
||||||
|
if err := h.netSettings.SetHops(r.Context(), *req.TrustedProxyHops); err != nil {
|
||||||
|
if errors.Is(err, netsettings.ErrHopsOutOfRange) {
|
||||||
|
writeErr(w, apierror.BadRequest("invalid_hops", err.Error()))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
writeErrWithLog(w, h.logger, "admin network: update failed", apierror.Internal(err))
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if req.PublicURL != nil {
|
||||||
|
if err := h.netSettings.SetPublicURL(r.Context(), *req.PublicURL); err != nil {
|
||||||
|
writeErrWithLog(w, h.logger, "admin network: public URL update failed", apierror.Internal(err))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
writeErrWithLog(w, h.logger, "admin network: update failed", apierror.Internal(err))
|
|
||||||
return
|
|
||||||
}
|
}
|
||||||
// Echo the payload recomputed under the NEW value, so the card can show
|
// Echo the payload recomputed under the NEW value, so the card can show
|
||||||
// immediately what the change did to this request's own address rather
|
// immediately what the change did to this request's own address rather
|
||||||
@@ -61,5 +91,6 @@ func (h *handlers) networkSettingsPayload(r *http.Request) networkSettingsResp {
|
|||||||
DetectedClientIP: auth.ClientIP(r, hops),
|
DetectedClientIP: auth.ClientIP(r, hops),
|
||||||
ForwardedChain: r.Header.Get("X-Forwarded-For"),
|
ForwardedChain: r.Header.Get("X-Forwarded-For"),
|
||||||
RemoteAddr: r.RemoteAddr,
|
RemoteAddr: r.RemoteAddr,
|
||||||
|
PublicURL: h.netSettings.PublicURL(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+19
-12
@@ -69,7 +69,7 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request)
|
|||||||
if err == nil && user.Email != nil && *user.Email != "" {
|
if err == nil && user.Email != nil && *user.Email != "" {
|
||||||
matched = true
|
matched = true
|
||||||
auditTarget = user.ID
|
auditTarget = user.ID
|
||||||
if sendErr := h.sendResetEmail(r.Context(), r, user); sendErr != nil {
|
if sendErr := h.sendResetEmail(r.Context(), user); sendErr != nil {
|
||||||
h.logger.Warn("forgot-password: send failed",
|
h.logger.Warn("forgot-password: send failed",
|
||||||
"email", email, "err", sendErr)
|
"email", email, "err", sendErr)
|
||||||
// fall through; response is still 200
|
// fall through; response is still 200
|
||||||
@@ -89,7 +89,7 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request)
|
|||||||
// sendResetEmail generates a token, persists it, renders + sends the
|
// sendResetEmail generates a token, persists it, renders + sends the
|
||||||
// email. Returns the underlying error (caller logs it but doesn't
|
// email. Returns the underlying error (caller logs it but doesn't
|
||||||
// surface to the HTTP response).
|
// surface to the HTTP response).
|
||||||
func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq.User) error {
|
func (h *handlers) sendResetEmail(ctx context.Context, user dbq.User) error {
|
||||||
if h.mailer == nil {
|
if h.mailer == nil {
|
||||||
return errors.New("forgot-password: no mailer configured")
|
return errors.New("forgot-password: no mailer configured")
|
||||||
}
|
}
|
||||||
@@ -109,7 +109,10 @@ func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
resetURL := buildResetURL(r, token)
|
resetURL, err := buildResetURL(h.netSettings.PublicURL(), token)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
textBody, htmlBody, err := mailer.RenderResetEmail(mailer.ResetEmailVars{
|
textBody, htmlBody, err := mailer.RenderResetEmail(mailer.ResetEmailVars{
|
||||||
Username: user.Username,
|
Username: user.Username,
|
||||||
ResetURL: resetURL,
|
ResetURL: resetURL,
|
||||||
@@ -127,14 +130,18 @@ func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq
|
|||||||
return h.mailer.Send(ctx, *user.Email, mailer.ResetEmailSubject, textBody, htmlBody)
|
return h.mailer.Send(ctx, *user.Email, mailer.ResetEmailSubject, textBody, htmlBody)
|
||||||
}
|
}
|
||||||
|
|
||||||
func buildResetURL(r *http.Request, token string) string {
|
// errNoPublicURL stops a reset email from going out before the operator has
|
||||||
scheme := "http"
|
// said where Minstrel lives. It surfaces in the log, not the response, which
|
||||||
if r.TLS != nil {
|
// stays the same opaque 200 either way.
|
||||||
scheme = "https"
|
var errNoPublicURL = errors.New("forgot-password: no public URL set (Admin → Integrations → Public address); reset email not sent")
|
||||||
|
|
||||||
|
// buildResetURL builds the emailed link from the operator-set public URL.
|
||||||
|
// It deliberately ignores the request: the Host header is whatever the
|
||||||
|
// requester sent, and building from it let a forged Host plant a real reset
|
||||||
|
// token on a link to someone else's server.
|
||||||
|
func buildResetURL(publicURL, token string) (string, error) {
|
||||||
|
if publicURL == "" {
|
||||||
|
return "", errNoPublicURL
|
||||||
}
|
}
|
||||||
host := r.Host
|
return publicURL + "/reset-password/" + token, nil
|
||||||
if host == "" {
|
|
||||||
host = "localhost"
|
|
||||||
}
|
|
||||||
return scheme + "://" + host + "/reset-password/" + token
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,11 +7,59 @@ import (
|
|||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"os"
|
"os"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
|
"github.com/jackc/pgx/v5/pgxpool"
|
||||||
|
|
||||||
"git.fabledsword.com/bvandeusen/minstrel/internal/mailer"
|
"git.fabledsword.com/bvandeusen/minstrel/internal/mailer"
|
||||||
|
"git.fabledsword.com/bvandeusen/minstrel/internal/netsettings"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// withPublicURL gives h a network-settings service holding url, restoring an
|
||||||
|
// empty value afterwards so other tests see the default.
|
||||||
|
func withPublicURL(t *testing.T, h *handlers, pool *pgxpool.Pool, url string) {
|
||||||
|
t.Helper()
|
||||||
|
ns, err := netsettings.New(context.Background(), pool, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("netsettings: %v", err)
|
||||||
|
}
|
||||||
|
if err := ns.SetPublicURL(context.Background(), url); err != nil {
|
||||||
|
t.Fatalf("set public url: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _ = ns.SetPublicURL(context.Background(), "") })
|
||||||
|
h.netSettings = ns
|
||||||
|
}
|
||||||
|
|
||||||
|
// With no public URL set, a reset email is not sent at all, and the response
|
||||||
|
// is still the same opaque 200.
|
||||||
|
func TestForgotPassword_NoPublicURL_SendsNothing(t *testing.T) {
|
||||||
|
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||||
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||||
|
}
|
||||||
|
h, pool := testHandlers(t)
|
||||||
|
fake := &mailer.FakeSender{}
|
||||||
|
h.mailer = fake
|
||||||
|
withPublicURL(t, h, pool, "")
|
||||||
|
|
||||||
|
user := seedUser(t, pool, "nourl", "pw", false)
|
||||||
|
if _, err := pool.Exec(context.Background(),
|
||||||
|
"UPDATE users SET email = 'nourl@example.com' WHERE id = $1", user.ID); err != nil {
|
||||||
|
t.Fatalf("seed email: %v", err)
|
||||||
|
}
|
||||||
|
req := httptest.NewRequest(http.MethodPost, "/api/auth/forgot-password",
|
||||||
|
bytes.NewReader([]byte(`{"email":"nourl@example.com"}`)))
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.handleForgotPassword(rec, req)
|
||||||
|
|
||||||
|
if rec.Code != http.StatusOK {
|
||||||
|
t.Errorf("status = %d, want 200", rec.Code)
|
||||||
|
}
|
||||||
|
if len(fake.Sent) != 0 {
|
||||||
|
t.Errorf("sent %d emails with no public URL set, want 0", len(fake.Sent))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) {
|
func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) {
|
||||||
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" {
|
||||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||||
@@ -19,6 +67,7 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) {
|
|||||||
h, pool := testHandlers(t)
|
h, pool := testHandlers(t)
|
||||||
fake := &mailer.FakeSender{}
|
fake := &mailer.FakeSender{}
|
||||||
h.mailer = fake
|
h.mailer = fake
|
||||||
|
withPublicURL(t, h, pool, "https://music.example.com")
|
||||||
|
|
||||||
user := seedUser(t, pool, "forgotuser", "pw", false)
|
user := seedUser(t, pool, "forgotuser", "pw", false)
|
||||||
if _, err := pool.Exec(context.Background(),
|
if _, err := pool.Exec(context.Background(),
|
||||||
@@ -29,7 +78,8 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) {
|
|||||||
body := `{"email":"forgot@example.com"}`
|
body := `{"email":"forgot@example.com"}`
|
||||||
req := httptest.NewRequest(http.MethodPost, "/api/auth/forgot-password",
|
req := httptest.NewRequest(http.MethodPost, "/api/auth/forgot-password",
|
||||||
bytes.NewReader([]byte(body)))
|
bytes.NewReader([]byte(body)))
|
||||||
req.Host = "minstrel.example.com"
|
// A forged Host must not reach the link: it comes from the public URL.
|
||||||
|
req.Host = "attacker.example.net"
|
||||||
rec := httptest.NewRecorder()
|
rec := httptest.NewRecorder()
|
||||||
h.handleForgotPassword(rec, req)
|
h.handleForgotPassword(rec, req)
|
||||||
|
|
||||||
@@ -43,6 +93,12 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) {
|
|||||||
if got.To != "forgot@example.com" {
|
if got.To != "forgot@example.com" {
|
||||||
t.Errorf("To = %q, want forgot@example.com", got.To)
|
t.Errorf("To = %q, want forgot@example.com", got.To)
|
||||||
}
|
}
|
||||||
|
if !strings.Contains(got.TextBody, "https://music.example.com/reset-password/") {
|
||||||
|
t.Errorf("reset link not built from the public URL:\n%s", got.TextBody)
|
||||||
|
}
|
||||||
|
if strings.Contains(got.TextBody+got.HTMLBody, "attacker.example.net") {
|
||||||
|
t.Error("the request's Host header reached the emailed link")
|
||||||
|
}
|
||||||
// Token row was inserted.
|
// Token row was inserted.
|
||||||
var tokenCount int
|
var tokenCount int
|
||||||
if err := pool.QueryRow(context.Background(),
|
if err := pool.QueryRow(context.Background(),
|
||||||
|
|||||||
@@ -430,6 +430,7 @@ type MissingReacquisition struct {
|
|||||||
type NetworkSetting struct {
|
type NetworkSetting struct {
|
||||||
ID bool
|
ID bool
|
||||||
TrustedProxyHops int32
|
TrustedProxyHops int32
|
||||||
|
PublicUrl string
|
||||||
}
|
}
|
||||||
|
|
||||||
type PasswordReset struct {
|
type PasswordReset struct {
|
||||||
|
|||||||
@@ -10,23 +10,34 @@ import (
|
|||||||
)
|
)
|
||||||
|
|
||||||
const getNetworkSettings = `-- name: GetNetworkSettings :one
|
const getNetworkSettings = `-- name: GetNetworkSettings :one
|
||||||
SELECT id, trusted_proxy_hops FROM network_settings WHERE id = true
|
SELECT id, trusted_proxy_hops, public_url FROM network_settings WHERE id = true
|
||||||
`
|
`
|
||||||
|
|
||||||
func (q *Queries) GetNetworkSettings(ctx context.Context) (NetworkSetting, error) {
|
func (q *Queries) GetNetworkSettings(ctx context.Context) (NetworkSetting, error) {
|
||||||
row := q.db.QueryRow(ctx, getNetworkSettings)
|
row := q.db.QueryRow(ctx, getNetworkSettings)
|
||||||
var i NetworkSetting
|
var i NetworkSetting
|
||||||
err := row.Scan(&i.ID, &i.TrustedProxyHops)
|
err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl)
|
||||||
|
return i, err
|
||||||
|
}
|
||||||
|
|
||||||
|
const updatePublicURL = `-- name: UpdatePublicURL :one
|
||||||
|
UPDATE network_settings SET public_url = $1 WHERE id = true RETURNING id, trusted_proxy_hops, public_url
|
||||||
|
`
|
||||||
|
|
||||||
|
func (q *Queries) UpdatePublicURL(ctx context.Context, publicUrl string) (NetworkSetting, error) {
|
||||||
|
row := q.db.QueryRow(ctx, updatePublicURL, publicUrl)
|
||||||
|
var i NetworkSetting
|
||||||
|
err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl)
|
||||||
return i, err
|
return i, err
|
||||||
}
|
}
|
||||||
|
|
||||||
const updateTrustedProxyHops = `-- name: UpdateTrustedProxyHops :one
|
const updateTrustedProxyHops = `-- name: UpdateTrustedProxyHops :one
|
||||||
UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING id, trusted_proxy_hops
|
UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING id, trusted_proxy_hops, public_url
|
||||||
`
|
`
|
||||||
|
|
||||||
func (q *Queries) UpdateTrustedProxyHops(ctx context.Context, trustedProxyHops int32) (NetworkSetting, error) {
|
func (q *Queries) UpdateTrustedProxyHops(ctx context.Context, trustedProxyHops int32) (NetworkSetting, error) {
|
||||||
row := q.db.QueryRow(ctx, updateTrustedProxyHops, trustedProxyHops)
|
row := q.db.QueryRow(ctx, updateTrustedProxyHops, trustedProxyHops)
|
||||||
var i NetworkSetting
|
var i NetworkSetting
|
||||||
err := row.Scan(&i.ID, &i.TrustedProxyHops)
|
err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl)
|
||||||
return i, err
|
return i, err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1 @@
|
|||||||
|
ALTER TABLE network_settings DROP COLUMN IF EXISTS public_url;
|
||||||
@@ -0,0 +1,8 @@
|
|||||||
|
-- The address users reach Minstrel at, e.g. https://music.example.com.
|
||||||
|
--
|
||||||
|
-- Password-reset links used to be built from the request's Host header,
|
||||||
|
-- which the requester controls: a forgot-password call with a forged Host
|
||||||
|
-- would email the victim a real reset token on a link to the attacker's
|
||||||
|
-- server. Links are now built only from this operator-set value, and none
|
||||||
|
-- are sent while it is empty (M462 #4981).
|
||||||
|
ALTER TABLE network_settings ADD COLUMN public_url text NOT NULL DEFAULT '';
|
||||||
@@ -3,3 +3,6 @@ SELECT * FROM network_settings WHERE id = true;
|
|||||||
|
|
||||||
-- name: UpdateTrustedProxyHops :one
|
-- name: UpdateTrustedProxyHops :one
|
||||||
UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING *;
|
UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING *;
|
||||||
|
|
||||||
|
-- name: UpdatePublicURL :one
|
||||||
|
UPDATE network_settings SET public_url = $1 WHERE id = true RETURNING *;
|
||||||
|
|||||||
@@ -0,0 +1,34 @@
|
|||||||
|
package netsettings
|
||||||
|
|
||||||
|
import "testing"
|
||||||
|
|
||||||
|
func TestNormalizePublicURL(t *testing.T) {
|
||||||
|
ok := map[string]string{
|
||||||
|
"": "",
|
||||||
|
" ": "",
|
||||||
|
"https://music.example.com": "https://music.example.com",
|
||||||
|
"https://music.example.com/": "https://music.example.com",
|
||||||
|
"http://192.168.1.10:4533": "http://192.168.1.10:4533",
|
||||||
|
" https://Music.Example.com/ ": "https://Music.Example.com",
|
||||||
|
}
|
||||||
|
for in, want := range ok {
|
||||||
|
got, err := NormalizePublicURL(in)
|
||||||
|
if err != nil || got != want {
|
||||||
|
t.Errorf("NormalizePublicURL(%q) = %q, %v; want %q", in, got, err, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, in := range []string{
|
||||||
|
"music.example.com", // no scheme
|
||||||
|
"ftp://music.example.com", // wrong scheme
|
||||||
|
"https://", // no host
|
||||||
|
"https://music.example.com/app", // path
|
||||||
|
"https://music.example.com/?x=1", // query
|
||||||
|
"https://music.example.com/#frag", // fragment
|
||||||
|
"https://user:pw@music.example.com",
|
||||||
|
"javascript:alert(1)",
|
||||||
|
} {
|
||||||
|
if _, err := NormalizePublicURL(in); err == nil {
|
||||||
|
t.Errorf("NormalizePublicURL(%q) accepted, want ErrInvalidPublicURL", in)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -12,6 +12,8 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"net/url"
|
||||||
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
|
|
||||||
"github.com/jackc/pgx/v5/pgxpool"
|
"github.com/jackc/pgx/v5/pgxpool"
|
||||||
@@ -32,13 +34,18 @@ const (
|
|||||||
// so the API layer can answer 400 instead of surfacing a constraint violation.
|
// so the API layer can answer 400 instead of surfacing a constraint violation.
|
||||||
var ErrHopsOutOfRange = errors.New("trusted proxy hops must be between 0 and 10")
|
var ErrHopsOutOfRange = errors.New("trusted proxy hops must be between 0 and 10")
|
||||||
|
|
||||||
|
// ErrInvalidPublicURL is returned by SetPublicURL for anything that isn't a
|
||||||
|
// bare http(s) origin, so the API layer can answer 400.
|
||||||
|
var ErrInvalidPublicURL = errors.New("public URL must be an http:// or https:// address with a host and no path, query or fragment")
|
||||||
|
|
||||||
// Service caches the network settings and owns their persistence.
|
// Service caches the network settings and owns their persistence.
|
||||||
type Service struct {
|
type Service struct {
|
||||||
pool *pgxpool.Pool
|
pool *pgxpool.Pool
|
||||||
logger *slog.Logger
|
logger *slog.Logger
|
||||||
|
|
||||||
mu sync.RWMutex
|
mu sync.RWMutex
|
||||||
hops int
|
hops int
|
||||||
|
publicURL string
|
||||||
}
|
}
|
||||||
|
|
||||||
// New loads the settings once and caches them.
|
// New loads the settings once and caches them.
|
||||||
@@ -58,6 +65,7 @@ func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service
|
|||||||
return s, err
|
return s, err
|
||||||
}
|
}
|
||||||
s.hops = int(row.TrustedProxyHops)
|
s.hops = int(row.TrustedProxyHops)
|
||||||
|
s.publicURL = row.PublicUrl
|
||||||
return s, nil
|
return s, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -106,3 +114,53 @@ func (s *Service) SetHops(ctx context.Context, hops int) error {
|
|||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// PublicURL returns the operator-set address users reach Minstrel at, with no
|
||||||
|
// trailing slash, or "" when it hasn't been set. Links that leave the app (a
|
||||||
|
// password-reset email) are built from this and never from the request's
|
||||||
|
// Host header, which the requester controls. Nil-safe like Hops.
|
||||||
|
func (s *Service) PublicURL() string {
|
||||||
|
if s == nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
|
s.mu.RLock()
|
||||||
|
defer s.mu.RUnlock()
|
||||||
|
return s.publicURL
|
||||||
|
}
|
||||||
|
|
||||||
|
// NormalizePublicURL validates raw as a bare http(s) origin and returns it
|
||||||
|
// without a trailing slash. "" is valid and means unset.
|
||||||
|
func NormalizePublicURL(raw string) (string, error) {
|
||||||
|
raw = strings.TrimSpace(raw)
|
||||||
|
if raw == "" {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
u, err := url.Parse(raw)
|
||||||
|
if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" ||
|
||||||
|
u.User != nil || (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.Fragment != "" {
|
||||||
|
return "", ErrInvalidPublicURL
|
||||||
|
}
|
||||||
|
return u.Scheme + "://" + u.Host, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// SetPublicURL validates, persists and caches the public URL. "" clears it.
|
||||||
|
func (s *Service) SetPublicURL(ctx context.Context, raw string) error {
|
||||||
|
normalized, err := NormalizePublicURL(raw)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
if s == nil || s.pool == nil {
|
||||||
|
return errors.New("network settings unavailable")
|
||||||
|
}
|
||||||
|
row, err := dbq.New(s.pool).UpdatePublicURL(ctx, normalized)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
s.mu.Lock()
|
||||||
|
s.publicURL = row.PublicUrl
|
||||||
|
s.mu.Unlock()
|
||||||
|
if s.logger != nil {
|
||||||
|
s.logger.Info("netsettings: public URL updated", "public_url", normalized)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -708,6 +708,9 @@ export type NetworkSettings = {
|
|||||||
detected_client_ip: string;
|
detected_client_ip: string;
|
||||||
forwarded_chain: string;
|
forwarded_chain: string;
|
||||||
remote_addr: string;
|
remote_addr: string;
|
||||||
|
// Where users reach Minstrel. Password-reset emails link here and are not
|
||||||
|
// sent while it is empty.
|
||||||
|
public_url: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
export async function getNetworkSettings(): Promise<NetworkSettings> {
|
export async function getNetworkSettings(): Promise<NetworkSettings> {
|
||||||
@@ -722,6 +725,13 @@ export async function updateNetworkSettings(hops: number): Promise<NetworkSettin
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Saves only the public address; the proxy depth is left as it is.
|
||||||
|
export async function updatePublicUrl(publicUrl: string): Promise<NetworkSettings> {
|
||||||
|
return api.put<NetworkSettings>('/api/admin/network-settings', {
|
||||||
|
public_url: publicUrl
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// Duplicates report (#3912) -------------------------------------------------
|
// Duplicates report (#3912) -------------------------------------------------
|
||||||
|
|
||||||
export async function listDuplicates(
|
export async function listDuplicates(
|
||||||
|
|||||||
@@ -19,7 +19,9 @@ const DETAIL_CODES: ReadonlySet<string> = new Set([
|
|||||||
'library_not_writable',
|
'library_not_writable',
|
||||||
'file_delete_failed',
|
'file_delete_failed',
|
||||||
// The server names the field and its range (#3913).
|
// The server names the field and its range (#3913).
|
||||||
'invalid_setting'
|
'invalid_setting',
|
||||||
|
// The server says what shape of address it wants (#4981).
|
||||||
|
'invalid_public_url'
|
||||||
]);
|
]);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
<script lang="ts">
|
||||||
|
import { onMount } from 'svelte';
|
||||||
|
import { Save, TriangleAlert } from 'lucide-svelte';
|
||||||
|
import { getNetworkSettings, updatePublicUrl } from '$lib/api/admin';
|
||||||
|
import { errMessage } from '$lib/api/errors';
|
||||||
|
import { pushToast } from '$lib/stores/toast.svelte';
|
||||||
|
|
||||||
|
let saved = $state<string | null>(null);
|
||||||
|
let value = $state('');
|
||||||
|
let saving = $state(false);
|
||||||
|
let loadError = $state(false);
|
||||||
|
|
||||||
|
const dirty = $derived(saved !== null && value.trim() !== saved);
|
||||||
|
// The address this page was loaded from is almost always the right answer,
|
||||||
|
// so offer it rather than making the operator type it.
|
||||||
|
const here = typeof window !== 'undefined' ? window.location.origin : '';
|
||||||
|
|
||||||
|
async function load() {
|
||||||
|
try {
|
||||||
|
const s = await getNetworkSettings();
|
||||||
|
saved = s.public_url;
|
||||||
|
value = s.public_url;
|
||||||
|
loadError = false;
|
||||||
|
} catch {
|
||||||
|
loadError = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
onMount(load);
|
||||||
|
|
||||||
|
async function save() {
|
||||||
|
saving = true;
|
||||||
|
try {
|
||||||
|
const s = await updatePublicUrl(value.trim());
|
||||||
|
saved = s.public_url;
|
||||||
|
value = s.public_url;
|
||||||
|
pushToast(saved ? 'Public address saved.' : 'Public address cleared.');
|
||||||
|
} catch (e) {
|
||||||
|
pushToast(errMessage(e, "Couldn't save the public address."), 'error');
|
||||||
|
} finally {
|
||||||
|
saving = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
</script>
|
||||||
|
|
||||||
|
<section class="space-y-4 rounded-xl border border-border bg-surface p-5">
|
||||||
|
<div>
|
||||||
|
<h3 class="font-display text-lg font-medium text-text-primary">Public address</h3>
|
||||||
|
<p class="mt-1 text-sm text-text-secondary">
|
||||||
|
The address people use to reach Minstrel. Password-reset emails link here.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
{#if loadError}
|
||||||
|
<p class="text-sm text-action-destructive">
|
||||||
|
Couldn't load network settings.
|
||||||
|
<button type="button" class="underline hover:no-underline" onclick={load}>Try again</button>
|
||||||
|
</p>
|
||||||
|
{:else if saved === null}
|
||||||
|
<p class="text-sm text-text-secondary">Loading…</p>
|
||||||
|
{:else}
|
||||||
|
<div class="flex flex-wrap items-end gap-3">
|
||||||
|
<label class="flex min-w-0 flex-1 flex-col gap-1">
|
||||||
|
<span class="text-sm text-text-secondary">Address</span>
|
||||||
|
<input
|
||||||
|
type="url"
|
||||||
|
inputmode="url"
|
||||||
|
placeholder="https://music.example.com"
|
||||||
|
bind:value
|
||||||
|
class="w-full rounded border border-border bg-background px-2 py-1 font-mono
|
||||||
|
focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent"
|
||||||
|
/>
|
||||||
|
</label>
|
||||||
|
{#if here && value.trim() !== here}
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
class="rounded-md border border-border px-3 py-1.5 text-sm hover:bg-surface-hover
|
||||||
|
focus-visible:ring-2 focus-visible:ring-accent"
|
||||||
|
onclick={() => (value = here)}
|
||||||
|
>
|
||||||
|
Use {here}
|
||||||
|
</button>
|
||||||
|
{/if}
|
||||||
|
<button
|
||||||
|
type="button"
|
||||||
|
class="inline-flex items-center gap-1.5 rounded-md border border-border px-3 py-1.5
|
||||||
|
text-sm hover:bg-surface-hover focus-visible:ring-2 focus-visible:ring-accent
|
||||||
|
disabled:opacity-50"
|
||||||
|
disabled={saving || !dirty}
|
||||||
|
onclick={save}
|
||||||
|
>
|
||||||
|
<Save size={14} aria-hidden="true" />
|
||||||
|
{saving ? 'Saving…' : 'Save'}
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
{#if !saved}
|
||||||
|
<p class="flex items-start gap-2 text-sm text-text-secondary" role="status">
|
||||||
|
<TriangleAlert size={14} class="mt-0.5 flex-shrink-0 text-action-destructive" aria-hidden="true" />
|
||||||
|
<span>
|
||||||
|
Not set, so password-reset emails are not being sent. Minstrel won't build the link from
|
||||||
|
whatever address a request claims, because anyone can claim any address.
|
||||||
|
</span>
|
||||||
|
</p>
|
||||||
|
{/if}
|
||||||
|
{/if}
|
||||||
|
</section>
|
||||||
@@ -0,0 +1,75 @@
|
|||||||
|
import { describe, expect, test, vi, beforeEach } from 'vitest';
|
||||||
|
import { render, screen, fireEvent, waitFor } from '@testing-library/svelte';
|
||||||
|
import PublicAddressCard from './PublicAddressCard.svelte';
|
||||||
|
|
||||||
|
const getNetworkSettings = vi.fn();
|
||||||
|
const updatePublicUrl = vi.fn();
|
||||||
|
|
||||||
|
vi.mock('$lib/api/admin', () => ({
|
||||||
|
getNetworkSettings: () => getNetworkSettings(),
|
||||||
|
updatePublicUrl: (url: string) => updatePublicUrl(url)
|
||||||
|
}));
|
||||||
|
|
||||||
|
const pushToast = vi.fn();
|
||||||
|
vi.mock('$lib/stores/toast.svelte', () => ({
|
||||||
|
pushToast: (...args: unknown[]) => pushToast(...args)
|
||||||
|
}));
|
||||||
|
|
||||||
|
function settings(publicUrl: string) {
|
||||||
|
return {
|
||||||
|
trusted_proxy_hops: 1,
|
||||||
|
max_hops: 10,
|
||||||
|
detected_client_ip: '198.51.100.7',
|
||||||
|
forwarded_chain: '198.51.100.7',
|
||||||
|
remote_addr: '172.18.0.1:40000',
|
||||||
|
public_url: publicUrl
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('PublicAddressCard', () => {
|
||||||
|
test('warns that reset emails are not sent while the address is unset', async () => {
|
||||||
|
getNetworkSettings.mockResolvedValue(settings(''));
|
||||||
|
render(PublicAddressCard);
|
||||||
|
expect(await screen.findByText(/password-reset emails are not being sent/i)).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('no warning once an address is saved', async () => {
|
||||||
|
getNetworkSettings.mockResolvedValue(settings('https://music.example.com'));
|
||||||
|
render(PublicAddressCard);
|
||||||
|
await screen.findByDisplayValue('https://music.example.com');
|
||||||
|
expect(screen.queryByText(/not being sent/i)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('offers this page’s own origin and saves it trimmed', async () => {
|
||||||
|
getNetworkSettings.mockResolvedValue(settings(''));
|
||||||
|
updatePublicUrl.mockResolvedValue(settings(window.location.origin));
|
||||||
|
render(PublicAddressCard);
|
||||||
|
|
||||||
|
await fireEvent.click(await screen.findByRole('button', { name: /^Use / }));
|
||||||
|
await fireEvent.click(screen.getByRole('button', { name: /save/i }));
|
||||||
|
|
||||||
|
await waitFor(() => expect(updatePublicUrl).toHaveBeenCalledWith(window.location.origin));
|
||||||
|
expect(pushToast).toHaveBeenCalledWith('Public address saved.');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('shows the server’s reason when an address is refused', async () => {
|
||||||
|
getNetworkSettings.mockResolvedValue(settings(''));
|
||||||
|
updatePublicUrl.mockRejectedValue({
|
||||||
|
code: 'invalid_public_url',
|
||||||
|
message: 'public URL must be an http:// or https:// address',
|
||||||
|
status: 400
|
||||||
|
});
|
||||||
|
render(PublicAddressCard);
|
||||||
|
|
||||||
|
const input = await screen.findByPlaceholderText('https://music.example.com');
|
||||||
|
await fireEvent.input(input, { target: { value: 'music.example.com' } });
|
||||||
|
await fireEvent.click(screen.getByRole('button', { name: /save/i }));
|
||||||
|
|
||||||
|
await waitFor(() => expect(pushToast).toHaveBeenCalled());
|
||||||
|
expect(pushToast.mock.calls[0][1]).toBe('error');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -47,6 +47,7 @@
|
|||||||
"duplicate_group_not_pending": "That group has already been resolved.",
|
"duplicate_group_not_pending": "That group has already been resolved.",
|
||||||
"survivor_not_in_group": "That copy isn't part of this group any more.",
|
"survivor_not_in_group": "That copy isn't part of this group any more.",
|
||||||
"invalid_setting": "That setting is out of range.",
|
"invalid_setting": "That setting is out of range.",
|
||||||
|
"invalid_public_url": "That address isn't valid.",
|
||||||
"album_not_found": "That album no longer exists.",
|
"album_not_found": "That album no longer exists.",
|
||||||
"artist_not_found": "That artist no longer exists.",
|
"artist_not_found": "That artist no longer exists.",
|
||||||
"playlist_not_found": "That playlist no longer exists.",
|
"playlist_not_found": "That playlist no longer exists.",
|
||||||
|
|||||||
@@ -28,6 +28,7 @@
|
|||||||
import { pushToast } from '$lib/stores/toast.svelte';
|
import { pushToast } from '$lib/stores/toast.svelte';
|
||||||
import Modal from '$lib/components/Modal.svelte';
|
import Modal from '$lib/components/Modal.svelte';
|
||||||
import NetworkSettingsCard from '$lib/components/NetworkSettingsCard.svelte';
|
import NetworkSettingsCard from '$lib/components/NetworkSettingsCard.svelte';
|
||||||
|
import PublicAddressCard from '$lib/components/PublicAddressCard.svelte';
|
||||||
import type { LidarrConfig, LidarrTestResult } from '$lib/api/types';
|
import type { LidarrConfig, LidarrTestResult } from '$lib/api/types';
|
||||||
|
|
||||||
// Lidarr connection panel. The "saved api key" is masked as "***" on GET —
|
// Lidarr connection panel. The "saved api key" is masked as "***" on GET —
|
||||||
@@ -827,6 +828,7 @@
|
|||||||
other card on this page, and it's an operator-wide setting rather than
|
other card on this page, and it's an operator-wide setting rather than
|
||||||
a per-user preference. -->
|
a per-user preference. -->
|
||||||
<NetworkSettingsCard />
|
<NetworkSettingsCard />
|
||||||
|
<PublicAddressCard />
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<Modal
|
<Modal
|
||||||
|
|||||||
Reference in New Issue
Block a user