search

nextcloud / user_external

👥 External user authentication methods like IMAP, SMB and FTP
https://apps.nextcloud.com/apps/user_external
108 stars 64 forks source link

Address issue #58 #82

Closed alejandroliu closed 5 years ago

alejandroliu commented 5 years ago

Signed-off-by: Alejandro Liu alejandro_liu@hotmail.com

Fixes #58

Changes proposed in this pull request:

This PR introduces an additional request in the BasicAuth external backend to check if the provided URL is performing authentication.

The performance hit of the additional request is quite minor, as it only happens during session login. And doesn't happen again until the session logs out.

nerdmaennchen commented 5 years ago

Sorry for my late review. From what I (now) know: You are absolutely correct. Testing for that header should prevent arbitrary user/pw combinations from logging in when authenticating agains a misconfigured URL.

Im not sure if a second request is actually necessary though. Correct me if I'm wrong, but wouldn't it be enough to test the response of the authentication request for the presence of the www-authenticate header? If so, a single request would suffice and the performance penalty (I agree with you here, I'd say it is miniscule) would be even further reduced.

Anyways, thank you very much for you contribution!

All the best, Lutz

violoncelloCH commented 5 years ago

@alejandroliu thank you for your contribution! @nerdmaennchen @alejandroliu I just have done a little research on this myself (and also some tests)... It seems basicauth is designed to not send anything on a correct authentication, so it looks like it's not possible to only do one request... so this implementation is probably the best we can do, if we want a check (performance losses or not)...

violoncelloCH commented 5 years ago

@nerdmaennchen feel free to merge, if you agree with this :)

alejandroliu commented 5 years ago

Are there any pending issues?