swingmx / swingmusic

Swing Music is a beautiful, self-hosted music player for your local audio files. Like a cooler Spotify ... but bring your own music.
https://swingmx.com
MIT License
666 stars 41 forks source link

Fix/docker #163

Open disconn3ct opened 5 months ago

disconn3ct commented 5 months ago

What kind of change does this PR introduce? (check at least one)

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

Other information:

This breaks the docker build into task-specific containers and only includes the necessary artifacts in the final image. (No promises that the process is 100% correct, but it works for me.) I did not update the workflows.

It also coincidentally fixes ARM builds.

Tested with: docker buildx build --platform linux/amd64,linux/arm64/v8 . -t sekret-hub-path:test --push

cwilvx commented 5 months ago

Hello @disconn3ct

Thanks for taking the time to work on this PR. Quick question:

The reason the Dockerfile uses a prebuilt binary file is because API keys are encrypted in the binary file. That way, anyone can use them without having access to them.

How should I pass the missing environment variables for deployment without making them publicly accessible? I'm assuming that even if I pass them in the Github workflow, you can access them in the container. Right?

tcsenpai commented 5 months ago

Hello @disconn3ct

Thanks for taking the time to work on this PR. Quick question:

The reason the Dockerfile uses a prebuilt binary file is because API keys are encrypted in the binary file. That way, anyone can use them without having access to them.

How should I pass the missing environment variables for deployment without making them publicly accessible? I'm assuming that even if I pass them in the Github workflow, you can access them in the container. Right?

I have (still) no idea of the API keys generaton and encrypting process but i have a question: when you say that the API Keys are encrypted in the binary, do you mean that all the swing instances have the same encrypted API key?

If so, can I have more info (like where to look at ) on the parts of the code that manages the API calls and that need indeed the encrypted API keys?

cwilvx commented 5 months ago

@tcsenpai

API keys are used to fetch lyrics and download similar artist data from LAST FM.

During development, API keys are read from the environment. During build time, API keys are read and hard coded into a app/configs.py file which is compiled into the distributable. This distributable is executed by users directly on their computers or via Docker. (The Docker image is just the distributable running inside an Ubuntu container). See Dockerfile.

When the distributable is run, the API keys are loaded from the config file into the app. See the Keys class in app/settings.py.

Functions needing the API keys import the Keys class. For example: the function that downloads similar artists data. See app/requests/artists.py.

tcsenpai commented 5 months ago

@cwilvx

It may be out of the scope of this program but if you don't encrypt the keys during the request, they can theoretically be read. Of course is just a Last.FM key and is not a real security issue, but if they are for example your keys I would look for some kind of other solutions.

On top of my mind, scraping with bs4 may be even easier once you create a nice scraper class.

Ofc if this is a non-issue feel free to ignore my comment, the software is great and I am actively using it as a public music player.

tcsenpai commented 5 months ago

Sorry for the double comment:

https://kerve.last.fm/kerve/similarartists?artist=Lana+Del+Rey&autocorrect=1&tracks=1&image_size=large&limit=30&format=json

I don't know if this is what you are using but I just used the request they make within their website and at least here it does look as working unauthenticated

cwilvx commented 5 months ago

@tcsenpai

For LAST FM, they provide a public API so going for the scraper would have been going a little far, and you have to maintain it too in case they change their page structure or do something funny to the website. In the future I might scrape them because damn! they have good images and they don't give access to them via the API.

I agree with the encryption part, I haven't figured out how to secure them and give them to app to users. Asking users to use their own keys is not an option, at least not for the core functionality.

Thank you for the Lana Del Rey request url, I'll sub that in the app and see how it goes.