fix(sharing): validate JWT expiration and share existence on stream endpoint (#5426)

* fix(sharing): validate JWT expiration and share existence on stream endpoint

The public stream endpoint (/public/s/{token}) was using
TokenAuth.Decode() which only verifies the JWT signature but skips
exp claim validation. This allowed expired share stream URLs to remain
functional indefinitely. Additionally, deleting a share did not revoke
previously issued stream tokens since the handler never performed a
server-side share lookup.

Fixed by switching decodeStreamInfo() to use auth.Validate() which
properly checks the exp claim, and by embedding the share ID ("sid")
in stream tokens so the handler can verify the share still exists.
Old tokens without the sid claim remain backward compatible but still
benefit from expiration validation.

* fix(sharing): check share expiration on stream requests

Replace the lightweight Exists() check with Get() + expiration
validation, so that shares whose ExpiresAt was updated to an earlier
time after token issuance are also rejected (410 Gone). Reuses the
existing checkShareError handler for consistent error responses.
This commit is contained in:
Deluan Quintão
2026-04-27 19:36:57 -04:00
committed by GitHub
parent 0fe08bfa74
commit 5c4f0298a6
6 changed files with 244 additions and 5 deletions
+8
View File
@@ -21,6 +21,7 @@ type Claims struct {
ID string // "id" - artwork/mediafile ID
Format string // "f" - audio format
BitRate int // "b" - audio bitrate
ShareID string // "sid" - share ID for share stream tokens
}
// ToMap converts Claims to a map[string]any for use with TokenAuth.Encode().
@@ -54,6 +55,9 @@ func (c Claims) ToMap() map[string]any {
if c.BitRate != 0 {
m["b"] = c.BitRate
}
if c.ShareID != "" {
m["sid"] = c.ShareID
}
return m
}
@@ -92,5 +96,9 @@ func ClaimsFromToken(token jwt.Token) Claims {
c.BitRate = int(bf)
}
}
var sid string
if err := token.Get("sid", &sid); err == nil {
c.ShareID = sid
}
return c
}
+9
View File
@@ -28,6 +28,7 @@ var _ = Describe("Claims", func() {
Expect(m).NotTo(HaveKey("id"))
Expect(m).NotTo(HaveKey("f"))
Expect(m).NotTo(HaveKey("b"))
Expect(m).NotTo(HaveKey("sid"))
})
It("includes expiration and issued-at when set", func() {
@@ -52,6 +53,12 @@ var _ = Describe("Claims", func() {
Expect(m).To(HaveKeyWithValue("f", "mp3"))
Expect(m).To(HaveKeyWithValue("b", 192))
})
It("includes share ID claim when set", func() {
c := auth.Claims{ShareID: "abc1234567"}
m := c.ToMap()
Expect(m).To(HaveKeyWithValue("sid", "abc1234567"))
})
})
Describe("ClaimsFromToken", func() {
@@ -84,6 +91,7 @@ var _ = Describe("Claims", func() {
ID: "al-456",
Format: "opus",
BitRate: 128,
ShareID: "abc1234567",
}
token, _, err := tokenAuth.Encode(original.ToMap())
Expect(err).NotTo(HaveOccurred())
@@ -91,6 +99,7 @@ var _ = Describe("Claims", func() {
c := auth.ClaimsFromToken(token)
Expect(c.Issuer).To(Equal("ND"))
Expect(c.ID).To(Equal("al-456"))
Expect(c.ShareID).To(Equal("abc1234567"))
Expect(c.Format).To(Equal("opus"))
Expect(c.BitRate).To(Equal(128))
})
+1
View File
@@ -102,6 +102,7 @@ func encodeMediafileShare(s model.Share, id string) string {
ID: id,
Format: s.Format,
BitRate: s.MaxBitRate,
ShareID: s.ID,
}
token, _ := auth.CreateExpiringPublicToken(V(s.ExpiresAt), claims)
return token
+17 -5
View File
@@ -4,11 +4,13 @@ import (
"errors"
"net/http"
"strconv"
"time"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/core/stream"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
. "github.com/navidrome/navidrome/utils/gg"
"github.com/navidrome/navidrome/utils/req"
)
@@ -23,6 +25,18 @@ func (pub *Router) handleStream(w http.ResponseWriter, r *http.Request) {
return
}
if info.shareID != "" {
share, err := pub.ds.Share(ctx).Get(info.shareID)
if err != nil {
checkShareError(ctx, w, err, info.shareID)
return
}
if expiresAt := V(share.ExpiresAt); !expiresAt.IsZero() && expiresAt.Before(time.Now()) {
checkShareError(ctx, w, model.ErrExpired, info.shareID)
return
}
}
mf, err := pub.ds.MediaFile(ctx).Get(info.id)
if err != nil {
if errors.Is(err, model.ErrNotFound) {
@@ -63,17 +77,14 @@ type shareTrackInfo struct {
id string
format string
bitrate int
shareID string
}
func decodeStreamInfo(tokenString string) (shareTrackInfo, error) {
token, err := auth.TokenAuth.Decode(tokenString)
c, err := auth.Validate(tokenString)
if err != nil {
return shareTrackInfo{}, err
}
if token == nil {
return shareTrackInfo{}, errors.New("unauthorized")
}
c := auth.ClaimsFromToken(token)
if c.ID == "" {
return shareTrackInfo{}, errors.New("required claim \"id\" not found")
}
@@ -81,5 +92,6 @@ func decodeStreamInfo(tokenString string) (shareTrackInfo, error) {
id: c.ID,
format: c.Format,
bitrate: c.BitRate,
shareID: c.ShareID,
}, nil
}
+196
View File
@@ -0,0 +1,196 @@
package public
import (
"context"
"errors"
"net/http"
"net/http/httptest"
"time"
"github.com/go-chi/jwtauth/v5"
"github.com/navidrome/navidrome/core/auth"
"github.com/navidrome/navidrome/core/stream"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/tests"
. "github.com/navidrome/navidrome/utils/gg"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
type mockStreamer struct {
req stream.Request
called bool
}
func (m *mockStreamer) NewStream(_ context.Context, _ *model.MediaFile, r stream.Request) (*stream.Stream, error) {
m.called = true
m.req = r
return nil, errors.New("mock: not implemented")
}
var _ = Describe("decodeStreamInfo", func() {
BeforeEach(func() {
auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil)
})
It("decodes a valid token with all fields", func() {
claims := auth.Claims{ID: "mf-123", Format: "mp3", BitRate: 192, ShareID: "share123"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
info, err := decodeStreamInfo(token)
Expect(err).NotTo(HaveOccurred())
Expect(info.id).To(Equal("mf-123"))
Expect(info.format).To(Equal("mp3"))
Expect(info.bitrate).To(Equal(192))
Expect(info.shareID).To(Equal("share123"))
})
It("rejects an expired token", func() {
claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(-time.Hour), claims)
_, err := decodeStreamInfo(token)
Expect(err).To(HaveOccurred())
})
It("accepts a token without exp (non-expiring share)", func() {
claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
token, _ := auth.CreatePublicToken(claims)
info, err := decodeStreamInfo(token)
Expect(err).NotTo(HaveOccurred())
Expect(info.id).To(Equal("mf-123"))
Expect(info.shareID).To(Equal("share123"))
})
It("rejects a token without an id claim", func() {
claims := auth.Claims{ShareID: "share123"}
token, _ := auth.CreatePublicToken(claims)
_, err := decodeStreamInfo(token)
Expect(err).To(HaveOccurred())
})
It("rejects an invalid token string", func() {
_, err := decodeStreamInfo("not-a-valid-token")
Expect(err).To(HaveOccurred())
})
It("handles tokens without shareID (backward compat)", func() {
claims := auth.Claims{ID: "mf-123", Format: "opus"}
token, _ := auth.CreatePublicToken(claims)
info, err := decodeStreamInfo(token)
Expect(err).NotTo(HaveOccurred())
Expect(info.id).To(Equal("mf-123"))
Expect(info.format).To(Equal("opus"))
Expect(info.shareID).To(BeEmpty())
})
})
var _ = Describe("encodeMediafileShare", func() {
BeforeEach(func() {
auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil)
})
It("includes the share ID in the token", func() {
exp := P(time.Now().Add(time.Hour))
s := model.Share{ID: "shareABC", Format: "mp3", MaxBitRate: 320, ExpiresAt: exp}
token := encodeMediafileShare(s, "mf-999")
info, err := decodeStreamInfo(token)
Expect(err).NotTo(HaveOccurred())
Expect(info.shareID).To(Equal("shareABC"))
Expect(info.id).To(Equal("mf-999"))
Expect(info.format).To(Equal("mp3"))
Expect(info.bitrate).To(Equal(320))
})
It("creates a non-expiring token when share has no expiry", func() {
s := model.Share{ID: "shareXYZ", ExpiresAt: nil}
token := encodeMediafileShare(s, "mf-111")
info, err := decodeStreamInfo(token)
Expect(err).NotTo(HaveOccurred())
Expect(info.shareID).To(Equal("shareXYZ"))
Expect(info.id).To(Equal("mf-111"))
})
})
var _ = Describe("handleStream", func() {
var ds *tests.MockDataStore
var shareRepo *tests.MockShareRepo
var streamer *mockStreamer
var pub *Router
BeforeEach(func() {
auth.TokenAuth = jwtauth.New("HS256", []byte("test-secret"), nil)
ds = &tests.MockDataStore{}
shareRepo = &tests.MockShareRepo{}
ds.MockedShare = shareRepo
streamer = &mockStreamer{}
pub = &Router{ds: ds, streamer: streamer}
})
makeRequest := func(token string) *httptest.ResponseRecorder {
r := httptest.NewRequest("GET", "/public/s/token?%3Aid="+token, nil)
w := httptest.NewRecorder()
pub.handleStream(w, r)
return w
}
It("passes all validation and reaches the streamer for a valid token", func() {
shareRepo.ID = "share123"
mfRepo := tests.CreateMockMediaFileRepo()
mfRepo.SetData(model.MediaFiles{{ID: "mf-123", Title: "Test Song"}})
ds.MockedMediaFile = mfRepo
claims := auth.Claims{ID: "mf-123", Format: "mp3", BitRate: 192, ShareID: "share123"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
makeRequest(token)
Expect(streamer.called).To(BeTrue())
Expect(streamer.req.Format).To(Equal("mp3"))
Expect(streamer.req.BitRate).To(Equal(192))
})
It("returns 400 for an expired token", func() {
claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(-time.Hour), claims)
w := makeRequest(token)
Expect(w.Code).To(Equal(http.StatusBadRequest))
})
It("returns 404 when share has been deleted", func() {
shareRepo.ID = "other-share"
claims := auth.Claims{ID: "mf-123", ShareID: "deleted-share"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
w := makeRequest(token)
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("returns 410 when share has been set to expired", func() {
shareRepo.ID = "share123"
expired := time.Now().Add(-time.Hour)
shareRepo.Entity = &model.Share{ID: "share123", ExpiresAt: &expired}
claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
token, _ := auth.CreatePublicToken(claims)
w := makeRequest(token)
Expect(w.Code).To(Equal(http.StatusGone))
})
It("returns 500 when share lookup fails", func() {
shareRepo.Error = errors.New("db error")
claims := auth.Claims{ID: "mf-123", ShareID: "share123"}
token, _ := auth.CreateExpiringPublicToken(time.Now().Add(time.Hour), claims)
w := makeRequest(token)
Expect(w.Code).To(Equal(http.StatusInternalServerError))
})
It("skips share check for tokens without shareID (backward compat)", func() {
claims := auth.Claims{ID: "mf-123"}
token, _ := auth.CreatePublicToken(claims)
w := makeRequest(token)
// Should get past share check, then fail on media file lookup (no mock data)
Expect(w.Code).To(Equal(http.StatusNotFound))
})
It("returns 400 for an invalid token", func() {
w := makeRequest("not-a-valid-token")
Expect(w.Code).To(Equal(http.StatusBadRequest))
})
})
+13
View File
@@ -44,3 +44,16 @@ func (m *MockShareRepo) Exists(id string) (bool, error) {
}
return id == m.ID, nil
}
func (m *MockShareRepo) Get(id string) (*model.Share, error) {
if m.Error != nil {
return nil, m.Error
}
if id != m.ID {
return nil, model.ErrNotFound
}
if s, ok := m.Entity.(*model.Share); ok {
return s, nil
}
return &model.Share{ID: id}, nil
}