[FIX] User Suggestions & Bug Fixes #663
Conversation
…stions-fixes-2.0.9
✅ Deploy Preview for dashy-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
This pull request is split into 5 parts for easier review. Changed files are located in these folders:
|
|
Remaining tasks:
|
|
If anyone has a couple minutes spare, would really appreciate a quick code review, before I merge |
| @@ -89,7 +89,7 @@ const store = new Vuex.Store({ | |||
| perms.allowSaveLocally = false; | |||
| } | |||
| // Disable saving changes to disk, only | |||
| if (appConfig.preventWriteToDisk || !isUserAdmin) { | |||
| if (appConfig.preventWriteToDisk || !isUserAdmin()) { | |||
There was a problem hiding this comment.
This will not fix issue #590 because if I understand correctly this is only a frontend check for admin privileges and therefore anyone can still realistically update the configuration file.
If you were visit the site as a guest, then proceed to add a "local" (not existing in conf.yml) user with admin role then continue to log in into that user then you can still very much update the config file however you like.
I'm suggesting a proper backend check for the already-known-good users/admins to permit unchecked file writes and command execution if there isn't one yet.
There was a problem hiding this comment.
Yeah, you're right, there really should be more backend checks.
I have explained the attack in more detail in the Auth docs, as this authentication method definitely shouldn't be used outside of your local network. Keycloak is supported for more secure authentication. But it should also be possible to protect the app with any self-hosted auth provider.
Potentially the best option, is to remove simple-auth from the app all together, and let users bring their own auth provider.
There was a problem hiding this comment.
I'm not really sure that removing the simple-auth would fix the issue, if there is no backend check. I mean it is nice that the password is handled by Keycloak for example, but if you don't need anything to activate a backend function, only the knowledge that where it is located and sending a direct request to it.
I for one say that the password hash being publicly accessible a non-issue, but in my case Dashy is running on a mini-pc and there it is a problem if anyone can start a rebuild of the software or they can fill the available storage with freely written configuration files.
Of course what you are saying that this should be used within a local network is totally reasonable, I just advocate that we place approriate warnings, that this should not be publically accessible because of possible "attack vectors".
There was a problem hiding this comment.
I think that either the lack of any real security should be addressed, or at the very least there should be a highly visible indication that there is in fact no "read-only access" for guests to prevent malicious access unbeknown to anyone running this project.
ps: I really love this project, keep the amazing work up!
|
The fix for this issue has now been released in 2.0.9 ✨ If you haven't done so already, please update your instance to Feel free to reach out if you need any more support. If you are enjoying Dashy, consider supporting the project. |
startingViewnot working #646-Write docs for sub-items
Code Quality Checklist (Please complete)