-
Notifications
You must be signed in to change notification settings - Fork 232
Add separate password for Subsonic API #1544
Conversation
I'm not seeing how this PR addresses any issues posted in #69 at all. In fact, I think this is a step backwards because it now provides an additional attack vector (a REST token), in addition to the passwords that can be compromised. The original passwords are still not stored securely. And now a new cred (the REST token) isn't stored securely either, which leads to a double fail. Everything is stored openly. #69's explicit issue (it's in the title itself) is that credentials are stored insecurely (in the db). How is this PR addressing that? The original creds are still insecure, the new creds are also now insecure, further compounding the problem. You may just issue another account and simulate the effects of this PR without the need for any code. In addition, third party creds remain openly stored.
Hmm, the bulk of the other PR (airsonic-advanced/airsonic-advanced#63) is exactly so it won't "break" things and retain backwards-compatibility (and thus does not merit a major version change like 11.0). I switch back and forth between the branches frequently without needing db resets, which is the point of it. A clean break (necessitating a major version change) would've made for a much smaller changeset, but I thought it better to have people on the 10.x versions secured too. Moreover, the other PR actually stores things securely, thus addressing the pivotal focus of #69. |
Rather than add another password, maybe we should just integrate with open id connect instead. That way we can move to not storing anything. If there's interest in this I can see about a POC. |
And lose every person who doesn't have access to the internet or doesn't actually want to use or create an account with an online OAuth provider. This includes internal network or LAN use cases. I think OAuth/Open ID is an additional auth method you can provide, but making it the sole mechanism of auth is going too far. |
That's seems a rather odd suggestion to make when you have (had?) a project specifically about removing dependence on external services from Airsonic. It's annoying enough when I can't use my Plex server normally due to "cloud issues". |
I strongly disagree with adding an additional external dependency to Airsonic AND making it a mandatory requirement. |
I would... say the same thing, especially since this would mean I would have to install and maintain an OpenID provider on my local network (because I sometimes use Airsonic offline). I'm putting this on the back burner to be fair, so let's close this. Anybody who wants to do something about this issue feel free to pick this or the implementation in airsonic-advanced/airsonic-advanced#63 and merge it! |
This series of commits adds a single separate password for the Subsonic API:
This password ("REST token" in the code for now, should we use "Subsonic API password" instead?) is empty by default, but can be regenerated or cleared in the "Personal Settings" page.
By default, existing users will see no changes:
Following #69, the t+s login using the user password can then be removed as a breaking change in 11.0 alongside the introduction of hashed user passwords. After this point, users with apps using the t+s login will need to change their password to use the REST token.
Note that another, more complex proposal was discussed in #69 and implemented in another PR in another fork. I think the issues pointed to by #69 need at least a minimal solution in place for 10.6 before we start breaking things in 11.0, hence this PR.