search

sentriz / gonic

music streaming server / free-software subsonic server API implementation
ircs://irc.libera.chat/#gonic
GNU General Public License v3.0
1.58k stars 112 forks source link

LDAP Authentication #346

Open betapictoris opened 1 year ago

betapictoris commented 1 year ago

Limitations

Fixes: #44.

sentriz commented 1 year ago

thanks for the effort here :+1:

it also may take some time for me to review since i don't know anything about LDAP. and i don't think i should blindy accept something i don't know about (no offense :grin:) i will do some reading or get a second pair of eyeballs on it

there seems to be some lint errors btw

betapictoris commented 1 year ago

thanks for the effort here 👍

it also may take some time for me to review since i don't know anything about LDAP. and i don't think i should blindy accept something i don't know about (no offense 😁) i will do some reading or get a second pair of eyeballs on it

there seems to be some lint errors btw

I totally understand about randomly merging a pull request, specifically when it comes to authentication. Thanks for the heads up!

betapictoris commented 11 months ago

As a heads up a99d63ab12838a1b16771abd6cf200d82b2c0b51 removed not null from the database model for User.Password, so this pull request isn't exactly ready yet - as that is going to require a migration.

sentriz commented 11 months ago

thank you @nitnelave and @BetaPictoris for the updates and review :+1: @BetaPictoris i will check it out shortly. would you mind rebasing in the meantime? thanks!

nitnelave commented 11 months ago

@BetaPictoris there seems to be more conflicts that crept up in the meantime. Can you have a look?

betapictoris commented 10 months ago

Hey y'all, I've fixed conflicts, sorry that it took a while!

betapictoris commented 10 months ago

hey sorry this look so long, only having a look now 🫢

All good, thanks for the comments!

it looks like for every single question to the server, we go though the withUser middleware and create a new connection to the LDAP server, do the bind, search, etc stuff. seems like a lot of work. is there no token exchange or something so that the next time the user visists they can skip all that?

There is no token exchange (at least not that I know of in standard LDAP) but we could connect and bind at the server start, or at the first use of LDAP. This would remove that extra workload, but at the cost of having an open LDAP connection sitting around.

it would be nice if i could use LDAP from the get go. right now it seems like an admin must first visit the dashboard via normal means, then setup LDAP, future visits? seems like it defaults the purpose for single-user servers

There shouldn't be any future visits to the LDAP page, unless the configuration changes. These fields could be moved/copied into an environment variables if it would better fit the rest of the setup, in your regards.

whats the reason the LDAP users cant have dashboards? how would they change their transcode settings or add internet ratio stations for example? or setup scrobbling?

This could be added, I just haven't gotten around to it, but at the moment withUser and ServeLoginDo are entirely separate. I could likely move the LDAP feature out of withUser and both withUser and ServeLoginDo could point to a single function.

looks also like go-ldap/ldap is being used but the latest version is /v3

I'll update go-ldap.

bubylou commented 10 months ago

Is there a container build for this? If not I can build it myself. It's just annoying to then get it into my K3S cluster. I'd love to test it out, having LDAP for Gonic is a huge plus for me.

betapictoris commented 7 months ago

@sentriz

whats the reason the LDAP users cant have dashboards? how would they change their transcode settings or add internet ratio stations for example? or setup scrobbling?

LDAP users now have dashboards (f6ecd6d39b0a46bd2f8ae590021533ccb0071277).

sentriz commented 7 months ago

sorry for the delay! and thanks for updating, it looks a lot better now i think. have one question though,

when using this over the subsonic API, we are using the password param to do an BindRequest. how does that work, and which clients support this?

nitnelave commented 7 months ago

Unless I'm mistaken, it's invisible to the client. They send their credentials to the server, and the server decides how to check the password: it can be built-in user management, or forwarding the credentials to an LDAP server for validation: a successful bind means that the credentials are valid. The client API doesn't change.

sentriz commented 7 months ago

@nitnelave ah OK so they are sending their LDAP password in place of their subsonic one?

nitnelave commented 7 months ago

Yes. The point of having an LDAP server is to share the identity. Every service works with the same user/password, and it's checked centrally in the LDAP server. Changing it there changes it for every service, since they all just ask the LDAP server if the password is correct.

nitnelave commented 7 months ago

Actually, they wouldn't even have a subsonic password: you can dynamically create a subsonic user after a successful bind. "I didn't know about this user, but the LDAP server tells me they exist and have access to subsonic, so I'll create an account for them"

Usually, the flow goes like:

When querying the LDAP server (usually as the admin to save one query), you can also fetch some more details from LDAP, such as the user's email and/or display name.

sentriz commented 7 months ago

Actually, they wouldn't even have a subsonic password: you can dynamically create a subsonic user after a successful bind. "I didn't know about this user, but the LDAP server tells me they exist and have access to subsonic, so I'll create an account for them"

Usually, the flow goes like:

* user sends their credentials to subsonic

* subsonic connects to the LDAP server as admin, and checks that the user is allowed to access subsonic (that can be either "user exists" or often "user belongs to the subsonic_users group").

* if the user is allowed access, subsonic connects to the LDAP server using the user's credentials.

* if that succeeds, the user is logged in. If necessary, create a subsonic account to store preferences and so on.

When querying the LDAP server (usually as the admin to save one query), you can also fetch some more details from LDAP, such as the user's email and/or display name.

thanks for the info :+1: also, and from gonic's perspective would it be standard practice to hit the LDAP for every http request is receives? seems like a lot of overhead, esp considering the case of browsing a list of albums from a browser. that could be 100s of request to gonic like getCoverArt.view?id=x&p=xxx, which is in turning hitting the LDAP server for each request

nitnelave commented 7 months ago

No, logging a user in can be an expensive operation (think multiple rounds of cryptographic hashing if you want something secure). Repeated attempts to log in can fairly easily bring an authentication server to its knees if it's not protected.

Usually you'd call the LDAP server once when logging in, then remember the login. Either through a session token, some JWT or similar, you'd get a short-to-medium term authorization (e.g. 1h, 24h, until 20min of inactivity, until explicit logout, etc). Note that the user's access to the server (gonic) can change at any time and you won't be notified of that; you'll only see that when they next try to log in. As a result, keeping the session until the user explicitly logs out is not recommended (but could be offered as an option for those who want it, along with a way to forcefully invalidate the user's active sessions).

betapictoris commented 7 months ago

No, logging a user in can be an expensive operation (think multiple rounds of cryptographic hashing if you want something secure). Repeated attempts to log in can fairly easily bring an authentication server to its knees if it's not protected.

The best why I can think of doing this is syncing passwords from the LDAP server (ie. checking local accounts, querying the LDAP server if it fails, then create the user if it doesn't exist, update the password if it's out-of-date). This does have a few caveats (as for every failed login there would be a request to the LDAP server, which could be avoided with some sort of rate limit -- but that leads to out of date credentials). Other than this, I'm not sure if there is any amazing way of implementing this, as far as I know (standard) LDAP doesn't have any token exchange, nor does OpenSubsonic.

nitnelave commented 7 months ago

Getting the password (hashes) from LDAP is not recommended, both in terms of security (you don't want to spread them everywhere and increase the attack surface) and compatibility (not every server hashes them in a compatible way). For instance, LLDAP doesn't expose the password hashes because it's doing a zero-knowledge proof that basically can't be replicated in gonic.

How does normal auth work in gonic? The credentials are sent with every request? You can implement a server-side auth caching method, i.e. "these credentials were authorized by the LDAP server at time T, so they'll be valid until T+24h before we recheck them"

betapictoris commented 7 months ago

Getting the password (hashes) from LDAP is not recommended, both in terms of security (you don't want to spread them everywhere and increase the attack surface) and compatibility (not every server hashes them in a compatible way). For instance, LLDAP doesn't expose the password hashes because it's doing a zero-knowledge proof that basically can't be replicated in gonic.

I totally agree on the security thing, it was a concern I had with the idea too, but my thought was that you could just hash the password after the request (considering gonic would already know it). I still don't think it's a good idea though.

How does normal auth work in gonic? The credentials are sent with every request?

Yeah, every request includes URL parameters.

You can implement a server-side auth caching method, i.e. "these credentials were authorized by the LDAP server at time T, so they'll be valid until T+24h before we recheck them"

I was thinking about this, and the more I do the more logical it seems. I don't know how @sentriz would feel, but the password reset button (in the web UI) could be replaced with an "LDAP refresh" button for LDAP users/in LDAP mode to clear the cache for that user.

betapictoris commented 7 months ago

@sentriz, on implementing a cache for LDAP credentials, currently local user passwords are stored in plain text - which I think we can all agree is not a safe long-term solution. Although, this raises the question as to how securing the cache should be approached, I can think of a few ways of doing this (but wanted to run them by you before doing anything that might not fit in with the project) including implementing:

Also, if you don't want to add hashing yourself to the database, I might be able to add it later (although I don't have the resources to work on opening another pull request at the moment).

sentriz commented 7 months ago

to my knowledge, storing is plaintext is a requirement from the subsonic protocol. this is to check credentials using the salt+token method

sentriz commented 7 months ago

there is some more discussion here https://github.com/opensubsonic/open-subsonic-api/discussions/25

betapictoris commented 6 months ago

to my knowledge, storing is plaintext is a requirement from the subsonic protocol. this is to check credentials using the salt+token method

Sorry for the late response, but I hadn't thought of that, thanks for letting me know.

betapictoris commented 6 months ago

You can implement a server-side auth caching method.

@nitnelave, this has been done in fccf01642f0553fd6d22336e14c8bb06d591c478.