-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix/docker #163
base: master
Are you sure you want to change the base?
Fix/docker #163
Conversation
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? |
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 |
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. |
Sorry for the double comment: 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 |
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. |
What kind of change does this PR introduce? (check at least one)
The PR fulfills these requirements:
fix #xxx[,#xxx]
, where "xxx" is the issue number)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