-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
LDAP authentication assumes passwords are reusable #11113
Comments
This comment was marked as off-topic.
This comment was marked as off-topic.
It's not the LDAP backend, but our token system assumes that they are. There's no way around currently. |
Right now I'm looking at maintaining a one-off build, reapplying patches to each upstream release. If there's a preferred way out of this, please let me know and I'll start working on a merge request. If nobody has a better way, turning my |
Is this issue something that can be added as a goal for the main project? This timer causes any LDAP back end to be useless if configured with 2FA. I am attempting to integrate the Duo LDAP proxy but cannot deploy something that causes re-authentication every 5 minutes. |
@ChristophWurst your opinion? |
If that's how some (LDAP) user back-ends work it's of course a problem. I second #11113 (comment). We haven't considered this kind of setup so far, hence this does not work. However, some Nextcloud components generally assume that the password is available (that's why we actually store and periodically check it), so this is just one of many parts that break apart if passwords are only used one time. I have no solution for this off the top of my head. |
Isn't it essentially external storages when is in conjunction with login credentials? and at least server side encryption. |
For what it's worth, nothing in my configuration appears to need a reusable password: my patch is working just fine in my organization. I would be okay with a configuration item like "This authentication backend does not use reusable passwords" and a big caveat saying that enabling the item would preclude using certain extended functionality that needs reusable passwords. |
On a second thought, I remembered that we already support tokens/sessions without a password, hence the password is actually optional when a token is created: server/lib/private/Authentication/Token/IProvider.php Lines 35 to 53 in 0211e17
When these tokens are checked, the password check is omitted (obviously). |
Yes, and also features in external apps like automatic account setup in Mail. |
When we just provide a flag in the config.php to not save passwords with the token, would it just work (i.e. apps not throwing exceptions)? that would be a rather cheap solution. |
That could be a cheap hack, yes. There are, however, other changes in this regard at #11390. Not sure if they make things easier or harder for this case. |
This seems like an enterprise use case to me. You might want to look into a Nextcloud subscription. |
@rullzer can you explain to the people who are watching this bug how a Nextcloud subscription can help with one-time passwords in LDAP? I had offered to put in some serious time creating a patch that would be accepted upstream, and your request for money in response to that offer strikes me as offensive. I had already added a Nextcloud subscription to my FY19 budget, and now that funds are available I was starting the process of doing this (my company moves very slowly). But now I'm reconsidering. Please help me get back to wanting to do unpaid work for your company by explaining how your sales pitch in a bug report on a DFSG-Free codebase shouldn't make me so angry. |
@nealey It was not my intend to offend you. I missed that you were offering to create a patch that would solve the issue in a sustainable way. Blame that on me maybe reading a bit to quickly. My apologies. As for the general question. Issues our customers are having are of course higher prioritized to get solved. Which is one of the benefits of a subscription. If you are still willing to move forward with this I'd be happy to give some pointers in the right direction and brainstorm on possible solutions. |
Hi. I'm running into this issue as well using the Duo LDAP proxy as my auth backend (push request every 5 mins) and trying to figure out the best way to proceed. I can't really turn users loose on it the way it is. I'm thinking about trimming down
|
@nealey have you tried the patch above? |
This comment was marked as outdated.
This comment was marked as outdated.
Which patch? The one I provided in the original 2018 bug report, or the one that logs everybody out unconditionally after an hour? I kind of like my solution better... |
I ran into the same problem as I'm using a FreeIPA LDAP backend with OTP. So the password has an OTP token as suffix. Would it be possible to have a setting which disables this check? |
This comment was marked as outdated.
This comment was marked as outdated.
Hello, bot! This issue still exists, how can we (the people affected by it) clarify things for you to help? |
It's my bad, the bot will keep pinging until the issue have been validated or not :) |
I just ran into this issue again with a Duo implementation. Is there any hope of getting an official fix for this or is there a better way to implement one of the patches offered by myself or @nealey? Everyone is using MFA for everything now. If MFA isn't addressed in a newer version of Nextcloud (haven't looked yet) I would think it will need to be in the very near future. PS: I should clarify, in my patch above _ automatically logout everyone_ shouldn't be interpreted as every currently logged in user is logged out on the hour or something like that. Each user has an hour after login to complete their upload/download before they are logged out. A prompt of somekind would be a nice addition I suppose but not sure how I would implement that. |
This comment was marked as outdated.
This comment was marked as outdated.
Having the same problem using DUO Authentication Proxy. |
The solution of the patch is to store the volatile password but never check it again. That way, if they were to use external storage, the storage would get a wrong password and fail. I rather like #11113 (comment) so that we don't store the password at all, with the implication that external storage and similar will not work if they require a user password. The easiest implementation will be to let admins set this via config.php. A bit more sophisticated approach is to ask the user backend if the password is "stable". |
Unfortunately simply patching the |
For context: I think the purpose of the code in Session.php was to kick users out / disable them when their password has changed or their LDAP account gets disabled (which in some LDAP implementations requires setting a dummy password). I guess having a switch to disable that behavior would be fine to cover for use cases where it doesn't make sense. |
The solution where we don't store passwords can be found here: #32624 would anyone be willing to test it (on their test system) :) |
Just wondering on the status for this issue? I have just setup a nextcloud Server (Nextcloud Hub 3 -25.0.2), using my FreeIPA with built in OTP. Which works great if I only login to the website. But if I create a token for use on my android app, or in gnome, it will work but after a few minutes, the token no longer seems to work, and I'm being prompted to sign in again. Sounds like there are some interesting suggestions, just wondering if anything has been implemented for the next update? Thanks |
This was actually fixed with #33225 Put 'auth.storeCryptedPassword' => false, in your config.php and this should work |
@CarlSchwan is this configuration variable documented anywhere? I found it only by sheer luck in this issue |
Steps to reproduce
Expected behaviour
I should stay logged in longer than 5 minutes at a time.
Actual behaviour
I am logged out after 5 minutes when Nextcloud tries to reauthenticate with my (non-reusable) login password.
Workaround
The following patch will skip the 5-minute password check:
Sorry, I'm having a lot of trouble pasting a tab character in here. Hopefully this patch is simple enough to recreate by hand. This code section is present in Nextcloud 14 as well.
Server configuration
Operating system: Container Linux by CoreOS 1800.7.0 (Rhyolite)
Web server: Apache2 2.4.25-3+deb
Database: MariaDB
PHP version: 7.1.20
Nextcloud version: 13.0.4
Updated from an older Nextcloud/ownCloud or fresh install: Updated from an older Nextcloud
Where did you install Nextcloud from:
docker run nextcloud:13.0.4
Signing status:
Signing status
List of activated apps:
App list
Nextcloud configuration:
Config report
Are you using external storage, if yes which one: local
Are you using encryption: yes, with an haproxy front-end
Are you using an external user-backend, if yes which one: LDAP
LDAP configuration (delete this part if not used)
LDAP config
Client configuration
Browser: Chrome 68.0.3440.118
Operating system: ChromeOS 68.0.3440.118
Logs
Web server error log
Web server error log
Nextcloud log (data/nextcloud.log)
Nextcloud log
Browser log
Browser log
Not relevant
The text was updated successfully, but these errors were encountered: