fix: enhance song file download process and CORS handling

- Updated the song download logic in SongController to proxy files directly from the backend, avoiding CORS issues.
- Implemented a new method in SongService to retrieve song files, including error handling for private songs and download restrictions.
- Refactored frontend download functions to align with the new backend logic, improving user experience and error management.
- Added CORS configuration in docker-compose for MinIO to allow necessary headers and methods.

Author: tomast1337

apps/backend/src/song/song.controller.ts

@@ -6,6 +6,7 @@ import {
   Delete,
   Get,
   Headers,
+  HttpException,
   HttpStatus,
   Logger,
   Param,
@@ -378,15 +379,37 @@ export class SongController {
   ): Promise<void> {
     user = validateUser(user);
 
-    // TODO: no longer used
-    res.set({
-      'Content-Disposition': 'attachment; filename="song.nbs"',
-      // Expose the Content-Disposition header to the client
-      'Access-Control-Expose-Headers': 'Content-Disposition',
-    });
+    try {
+      // Get file directly from S3/MinIO and proxy it to avoid CORS issues
+      // This bypasses presigned URLs and CORS entirely
+      const { buffer, filename } = await this.songService.getSongFileBuffer(
+        id,
+        user,
+        src,
+        false,
+      );
+
+      // Set headers and send file
+      res.set({
+        'Content-Type': 'application/octet-stream',
+        'Content-Disposition': `attachment; filename="${filename.replace(
+          /[/"]/g,
+          '_',
+        )}"`,
+        'Access-Control-Expose-Headers': 'Content-Disposition',
+      });
 
-    const url = await this.songService.getSongDownloadUrl(id, user, src, false);
-    res.redirect(HttpStatus.FOUND, url);
+      res.send(Buffer.from(buffer));
+    } catch (error) {
+      this.logger.error('Error downloading song file:', error);
+      if (error instanceof HttpException) {
+        throw error;
+      }
+      throw new HttpException(
+        'An error occurred while retrieving the song file',
+        HttpStatus.INTERNAL_SERVER_ERROR,
+      );
+    }
   }
 
   @Get('/:id/open')

apps/backend/src/song/song.service.ts

@@ -386,6 +386,57 @@ export class SongService {
     }
   }
 
+  public async getSongFileBuffer(
+    publicId: string,
+    user: UserDocument | null,
+    src?: string,
+    packed: boolean = false,
+  ): Promise<{ buffer: ArrayBuffer; filename: string }> {
+    const foundSong = await this.songModel.findOne({ publicId: publicId });
+
+    if (!foundSong) {
+      throw new HttpException('Song not found with ID', HttpStatus.NOT_FOUND);
+    }
+
+    if (foundSong.visibility !== 'public') {
+      if (!user || foundSong.uploader.toString() !== user._id.toString()) {
+        throw new HttpException(
+          'This song is private',
+          HttpStatus.UNAUTHORIZED,
+        );
+      }
+    }
+
+    if (!packed && !foundSong.allowDownload) {
+      throw new HttpException(
+        'The uploader has disabled downloads of this song',
+        HttpStatus.UNAUTHORIZED,
+      );
+    }
+
+    const fileKey = packed ? foundSong.packedSongUrl : foundSong.nbsFileUrl;
+    const fileExt = packed ? '.zip' : '.nbs';
+    const fileName = `${foundSong.title}${fileExt}`;
+
+    try {
+      const fileBuffer = await this.fileService.getSongFile(fileKey);
+
+      // increment download count
+      if (!packed && src === 'downloadButton') {
+        foundSong.downloadCount++;
+        await foundSong.save();
+      }
+
+      return { buffer: fileBuffer, filename: fileName };
+    } catch (e) {
+      this.logger.error('Error getting song file', e);
+      throw new HttpException(
+        'An error occurred while retrieving the song file',
+        HttpStatus.INTERNAL_SERVER_ERROR,
+      );
+    }
+  }
+
   public async getMySongsPage({
     query,
     user,

apps/frontend/src/modules/song-edit/components/client/context/EditSong.context.tsx

@@ -21,6 +21,8 @@ import {
   editSongFormSchema,
 } from '@web/modules/song/components/client/SongForm.zod';
 
+// TODO: THIS FORM IS CURRENTLY BROKEN, WE NEED TO FIX IT
+
 export type useEditSongProviderType = {
   formMethods: UseFormReturn<EditSongForm>;
   submitSong: () => void;
@@ -237,51 +239,16 @@ export const EditSongProvider = ({
       const token = getTokenLocal();
 
       try {
-        // The backend redirects (302) to an S3 signed URL
-        // Get the redirect URL first, then fetch from S3 directly
-        let s3Url: string | null = null;
-
-        try {
-          // Make request without following redirects to get the S3 URL
-          await axiosInstance.get(`/song/${id}/download`, {
-            params: {
-              src: 'edit',
-            },
-            headers: { authorization: `Bearer ${token}` },
-            maxRedirects: 0, // Don't follow redirects
-            validateStatus: () => false, // Don't throw on any status
-          });
-        } catch (redirectError: any) {
-          // Axios throws an error when maxRedirects is 0 and a redirect occurs
-          // Extract the Location header from the redirect response
-          if (
-            redirectError.response?.status >= 300 &&
-            redirectError.response?.status < 400
-          ) {
-            s3Url = redirectError.response.headers.location;
-            if (!s3Url) {
-              throw new Error('Redirect received but no Location header found');
-            }
-          } else {
-            // Not a redirect error, re-throw it
-            throw redirectError;
-          }
-        }
-
-        if (!s3Url) {
-          throw new Error('Failed to get song download URL from redirect');
-        }
-
-        // Fetch the file directly from S3 using native fetch (handles CORS better)
-        const s3Response = await fetch(s3Url);
-        if (!s3Response.ok) {
-          throw new Error(
-            `Failed to fetch song from storage: ${s3Response.status} ${s3Response.statusText}`,
-          );
-        }
-
-        const arrayBuffer = await s3Response.arrayBuffer();
-        const songFile = arrayBuffer;
+        // Backend now proxies the file directly, avoiding CORS issues
+        const response = await axiosInstance.get(`/song/${id}/download`, {
+          params: {
+            src: 'edit',
+          },
+          headers: { authorization: `Bearer ${token}` },
+          responseType: 'arraybuffer', // Get as ArrayBuffer for parsing
+        });
+
+        const songFile = response.data;
 
         // convert to song
         const song = await parseSongFromBuffer(songFile);

apps/frontend/src/modules/song/util/downloadSong.ts

@@ -11,30 +11,32 @@ export const downloadSongFile = async (song: {
 }) => {
   const token = getTokenLocal();
 
-  axios
-    .get(`/song/${song.publicId}/download`, {
+  try {
+    // Backend now proxies the file directly, avoiding CORS issues
+    const response = await axios.get(`/song/${song.publicId}/download`, {
       params: {
         src: 'downloadButton',
       },
       headers: {
         authorization: `Bearer ${token}`,
       },
       responseType: 'blob',
-      withCredentials: true,
-    })
-    .then((res) => {
-      const url = window.URL.createObjectURL(res.data);
-      const link = document.createElement('a');
-      link.href = url;
+    });
 
-      link.setAttribute('download', `${song.title}.nbs`);
-      document.body.appendChild(link);
-      link.click();
+    const url = window.URL.createObjectURL(response.data);
+    const link = document.createElement('a');
+    link.href = url;
+    link.setAttribute('download', `${song.title}.nbs`);
+    document.body.appendChild(link);
+    link.click();
 
-      // Clean up
-      link.remove();
-      window.URL.revokeObjectURL(url);
-    });
+    // Clean up
+    link.remove();
+    window.URL.revokeObjectURL(url);
+  } catch (error) {
+    console.error('Error downloading song:', error);
+    toast.error('Failed to download song');
+  }
 };
 
 export const openSongInNBS = async (song: { publicId: string }) => {

docker-compose.yml

@@ -53,10 +53,12 @@ services:
         echo "Waiting for MinIO to be available..."
         sleep 2
       done &&
-      mc mb minio/noteblockworld-songs &&
-      mc mb minio/noteblockworld-thumbs &&
+      mc mb minio/noteblockworld-songs || true &&
+      mc mb minio/noteblockworld-thumbs || true &&
       mc policy set public minio/noteblockworld-songs &&
-      mc policy set public minio/noteblockworld-thumbs
+      mc policy set public minio/noteblockworld-thumbs &&
+      mc admin config set minio api "cors_allow_origin=* cors_allow_methods=GET,PUT,POST,DELETE,HEAD,OPTIONS cors_allow_headers=*" || echo "CORS config may already be set" &&
+      echo "CORS configuration applied successfully"
       '
 
 volumes: