fix(api): let an artist download their own track with downloads off

api/dbv1/media_link.go

@@ -93,3 +93,27 @@ func generateSignature(data map[string]interface{}) (string, error) {
 
 	return hexutil.Encode(signature), nil
 }
+
+// DownloadCid is the blob a download serves: the artist's original upload when
+// the row kept one, otherwise the transcode. Empty when the row has neither,
+// which happens on uploads whose cid backfill never ran - signing an empty cid
+// only produces a content-node URL guaranteed to 404.
+func (t *GetTracksRow) DownloadCid() string {
+	if t.OrigFileCid.String != "" {
+		return t.OrigFileCid.String
+	}
+	return t.TrackCid.String
+}
+
+// SignDownloadLink signs a download URL for this track without consulting
+// is_downloadable or the download gates. Nothing here re-checks access, so the
+// caller must already have established that the requester may bypass them -
+// today only a wallet-verified owner or account manager does, see
+// ApiServer.ownerDownloadLink. Returns nil when the row has no cid to sign.
+func (t *Track) SignDownloadLink(userId int32) (*MediaLink, error) {
+	cid := t.DownloadCid()
+	if cid == "" {
+		return nil, nil
+	}
+	return mediaLink(cid, t.TrackID, userId, nil)
+}

api/dbv1/tracks.go

@@ -224,11 +224,7 @@ func (q *Queries) TracksKeyed(ctx context.Context, arg TracksParams) (map[int32]
 
 		var download *MediaLink
 		if isStreamable && rawTrack.IsDownloadable && access.Download {
-			cid := rawTrack.OrigFileCid.String
-			if cid == "" {
-				cid = rawTrack.TrackCid.String
-			}
-			if cid != "" {
+			if cid := rawTrack.DownloadCid(); cid != "" {
 				download, err = mediaLink(cid, rawTrack.TrackID, arg.MyID.(int32), nil)
 				if err != nil {
 					return nil, err

api/v1_track_download.go

@@ -1,16 +1,28 @@
 package api
 
 import (
+	"path"
+	"strings"
+
 	"api.audius.co/api/dbv1"
 	"github.com/gofiber/fiber/v2"
 )
 
 func createFilename(track *dbv1.Track) string {
-	filename := track.OrigFilename.String
-	if filename == "" && track.OrigFileCid.String == "" {
-		filename = track.Title.String + ".mp3"
+	// The original upload is what a download serves whenever the row kept one,
+	// so its own name is the right one.
+	if track.OrigFileCid.String != "" && track.OrigFilename.String != "" {
+		return track.OrigFilename.String
+	}
+
+	// Otherwise the bytes are the mp3 transcode, and the name must not promise
+	// the format the artist uploaded: a .wav name on mp3 bytes is a file most
+	// editors refuse to open. Only the recorded filename is stripped of its
+	// extension - a title is free text and "Vol. 2" has no extension to trim.
+	if name := track.OrigFilename.String; name != "" {
+		return strings.TrimSuffix(name, path.Ext(name)) + ".mp3"
 	}
-	return filename
+	return track.Title.String + ".mp3"
 }
 
 type trackDownloadParams struct {
@@ -49,15 +61,27 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error {
 		return fiber.NewError(fiber.StatusNotFound, "track not found")
 	}
 
-	if !track.Access.Download {
-		return fiber.NewError(fiber.StatusForbidden, "you are not allowed to download this track")
+	// track.Download is only populated for tracks the public may download, so
+	// an artist who left downloads off - about four in five tracks - could not
+	// get their own file back. The edit page's "Download File" button and the
+	// replace-file flow both come through here, and both were 404ing for the
+	// owner of the track, surfacing as a generic "something went wrong".
+	downloadLink := track.Download
+	if downloadLink == nil {
+		downloadLink, err = app.ownerDownloadLink(c, &track)
+		if err != nil {
+			return err
+		}
 	}
 
-	if track.Download == nil {
+	if downloadLink == nil {
+		if !track.Access.Download {
+			return fiber.NewError(fiber.StatusForbidden, "you are not allowed to download this track")
+		}
 		return fiber.NewError(fiber.StatusNotFound, "track is not downloadable")
 	}
 
-	downloadUrl := tryFindWorkingUrl(track.Download)
+	downloadUrl := tryFindWorkingUrl(downloadLink)
 
 	q := downloadUrl.Query()
 	q.Set("skip_play_count", "true")
@@ -70,3 +94,25 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error {
 
 	return c.Redirect(downloadUrl.String(), fiber.StatusFound)
 }
+
+// ownerDownloadLink signs a download link for a requester who has proven they
+// own the track, or manage the account that does. Ownership comes from the
+// wallet recovered from the request signature, not from the user_id query
+// param behind myId: user_id is the caller's own claim, and it is only
+// trustworthy here because this route sits off authMiddleware's advisory-
+// user_id allowlist. Handing out an artist's original master should not rest
+// on that list continuing to exclude this route. Returns nil - not an error -
+// for everyone else, which leaves the caller's 404 in place.
+func (app *ApiServer) ownerDownloadLink(c *fiber.Ctx, track *dbv1.Track) (*dbv1.MediaLink, error) {
+	wallet := app.tryGetAuthedWallet(c)
+	if wallet == "" {
+		return nil, nil
+	}
+
+	ownerId := track.GetTracksRow.UserID
+	if !app.isAuthorizedRequest(c.Context(), ownerId, wallet) {
+		return nil, nil
+	}
+
+	return track.SignDownloadLink(ownerId)
+}

api/v1_track_download_test.go

@@ -4,6 +4,7 @@ import (
 	"net/http/httptest"
 	"testing"
 
+	"api.audius.co/api/testdata"
 	"api.audius.co/database"
 	"api.audius.co/trashid"
 	"github.com/stretchr/testify/assert"
@@ -74,3 +75,126 @@ func TestGetTrackDownload_Original(t *testing.T) {
 	assert.Contains(t, res.Header.Get("Location"), "signature=%7B%22data%22%3A%22%7B%5C%22cid%5C%22%3A%5C%22QmX123%5C%22%2C%5C%22timestamp%5C%22%3")
 	assert.Contains(t, res.Header.Get("Location"), "filename=DharitRocks.wav")
 }
+
+const (
+	ownerWallet    = "0x7d273271690538cf855e5b3002a0dd8c154bb060"
+	strangerWallet = "0xc3d1d41e6872ffbd15c473d14fc3a9250be5b5e0"
+	managerWallet  = "0x4954d18926ba0ed9378938444731be4e622537b2"
+)
+
+// seedNonDownloadableTrack sets up the shape most tracks on the network have:
+// an artist who never turned downloads on, whose original upload is still
+// sitting on the content node.
+func seedNonDownloadableTrack(t *testing.T) *ApiServer {
+	app := emptyTestApp(t)
+	database.Seed(app.pool.Replicas[0], database.FixtureMap{
+		"tracks": []map[string]any{
+			{
+				"track_id":        1,
+				"owner_id":        1,
+				"title":           "Private Master",
+				"track_cid":       "QmTranscode",
+				"orig_file_cid":   "QmOriginal",
+				"orig_filename":   "PrivateMaster.wav",
+				"is_downloadable": false,
+			},
+		},
+		"users": []map[string]any{
+			{"user_id": 1, "handle": "artist", "wallet": ownerWallet},
+			{"user_id": 2, "handle": "stranger", "wallet": strangerWallet},
+			{"user_id": 3, "handle": "manager", "wallet": managerWallet},
+		},
+	})
+	return app
+}
+
+func downloadWithWallet(t *testing.T, app *ApiServer, path string, wallet string) (int, string) {
+	req := httptest.NewRequest("GET", path, nil)
+	if wallet != "" {
+		sigData := testdata.GetSignatureData(wallet)
+		req.Header.Set("Encoded-Data-Message", sigData.Message)
+		req.Header.Set("Encoded-Data-Signature", sigData.Signature)
+	}
+	res, err := app.Test(req, -1)
+	assert.NoError(t, err)
+	return res.StatusCode, res.Header.Get("Location")
+}
+
+// An artist must be able to get their own upload back even with downloads off
+// for everyone else - that is what the edit page's "Download File" button does.
+func TestGetTrackDownload_OwnerOfNonDownloadableTrack(t *testing.T) {
+	app := seedNonDownloadableTrack(t)
+	path := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download"
+
+	status, location := downloadWithWallet(t, app, path, ownerWallet)
+	assert.Equal(t, 302, status)
+	assert.Contains(t, location, "tracks/cidstream/QmOriginal")
+	assert.Contains(t, location, "filename=PrivateMaster.wav")
+}
+
+// A manager the artist granted access to acts for them here too.
+func TestGetTrackDownload_ManagerOfNonDownloadableTrack(t *testing.T) {
+	app := seedNonDownloadableTrack(t)
+	database.SeedTable(app.pool.Replicas[0], "grants", []map[string]any{
+		{
+			"user_id":         1,
+			"grantee_address": managerWallet,
+			"is_approved":     true,
+			"is_revoked":      false,
+		},
+	})
+
+	status, location := downloadWithWallet(t, app,
+		"/v1/tracks/"+trashid.MustEncodeHashID(1)+"/download", managerWallet)
+	assert.Equal(t, 302, status)
+	assert.Contains(t, location, "tracks/cidstream/QmOriginal")
+}
+
+// Everyone else still gets the 404 the artist asked for by leaving downloads
+// off. Claiming to be the owner through the user_id query param must not be
+// enough: the bypass keys on the recovered signature, and the auth middleware
+// separately refuses a user_id no signature backs (403 rather than 404).
+func TestGetTrackDownload_NonOwnerOfNonDownloadableTrack(t *testing.T) {
+	app := seedNonDownloadableTrack(t)
+	trackPath := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download"
+	asOwner := trackPath + "?user_id=" + trashid.MustEncodeHashID(1)
+
+	status, _ := downloadWithWallet(t, app, trackPath, "")
+	assert.Equal(t, 404, status, "anonymous")
+
+	status, _ = downloadWithWallet(t, app, trackPath, strangerWallet)
+	assert.Equal(t, 404, status, "signed by another wallet")
+
+	status, _ = downloadWithWallet(t, app, asOwner, "")
+	assert.Equal(t, 403, status, "user_id claiming to be the owner, unsigned")
+
+	status, _ = downloadWithWallet(t, app, asOwner, strangerWallet)
+	assert.Equal(t, 403, status, "user_id claiming to be the owner, signed by another wallet")
+}
+
+// With no original kept, the download serves the mp3 transcode, so the name it
+// is served under has to say mp3 rather than the format that was uploaded.
+func TestGetTrackDownload_FilenameFallsBackToMp3(t *testing.T) {
+	app := emptyTestApp(t)
+	database.Seed(app.pool.Replicas[0], database.FixtureMap{
+		"tracks": []map[string]any{
+			{
+				"track_id":        1,
+				"owner_id":        1,
+				"title":           "Vol. 2",
+				"track_cid":       "QmTranscode",
+				"orig_filename":   "Vol. 2.wav",
+				"is_downloadable": true,
+			},
+		},
+		"users": []map[string]any{
+			{"user_id": 1, "handle": "artist", "wallet": ownerWallet},
+		},
+	})
+
+	status, location := downloadWithWallet(t, app,
+		"/v1/tracks/"+trashid.MustEncodeHashID(1)+"/download", "")
+	assert.Equal(t, 302, status)
+	assert.Contains(t, location, "tracks/cidstream/QmTranscode")
+	assert.Contains(t, location, "filename=Vol.+2.mp3")
+}