-
-
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
[Bug]: Passwordless authtokens should remain passwordless #30894
Comments
…rds()` Fixes nextcloud#30894 (at least, it is supposed to). Signed-off-by: Matt Marjanovic <[email protected]>
I just submitted a tiny PR with the fix which I mentioned at the end of my bug description above. I also forgot to mention that this issue is probably related to #26502 and its cousins (e.g., pulsejet/nextcloud-oidc-login#148). |
…rds()` Fixes nextcloud#30894 (at least, it is supposed to). Signed-off-by: Matt Marjanovic <[email protected]>
I think I can confirm this observation:
I disabled DAV Access (not using App Passwords) to my account yesterday and I stay logged in now. |
I think I'm seeing something similar, it tends to happen every time my LDAP server dies for some reason. What generally happens is that the LDAP server, for some reason, starts to fail to authenticate Nextcloud. When a user logs in, this is flagged as a bad authentication by the user and all tokens are revoked. This includes SAML sessions as well as app passwords and means that users have to not only log into every single app again, but they also have to regenerate every app token. I can create a different ticket if this isn't related but it seems to be. |
It is the password of another login. But in your case this is suspicious. Is SAML the only authentication mechanism?
That smells like a bug. The token's login name should be what the user entered during login. This can, but doesn't have to be, the internal user ID. |
This comment was marked as resolved.
This comment was marked as resolved.
I can still reproduce this problem with 25.0.3. |
Bug description
On (one of) my system(s), this bug is manifesting itself in two related ways:
user_saml
), after some shorter-than-usual timeout (< 1 hour), the web UI appears to have logged out and will go through the SAML auth handshake again. (Since the user still has valid credentials with the SAML IdP, this is pretty automatic, but the user will still lose any work in progress, etc.)What appears to be happening is:
PublicKeyTokenProvider::updatePasswords()
. (I'm not sure what code flow is leading to this; maybe a CALDAV client logging in?)updatePasswords()
scans all of the user's authtokens and updates all of them, including password-less tokens (those with anull
password field) such that they are no longer password-less tokens.Session::getUser()
.Session::getUser()
callsSession::validateSession()
.Session::validateSession()
callsSession::validateToken()
.Session::validateToken()
callsSession::checkTokenCredentials()
.Session::checkTokenCredentials()
asks thePublicKeyTokenProvider
for the token's password (i.e.,getPassword()
).Session::checkTokenCredentials()
callsManager::checkPassword()
to check that password!Manager::checkPassword()
callsManager::checkPasswordNoLogging()
.Manager::checkPasswordNoLogging()
ends up callingUser_LDAP::checkPassword()
which callsUser_LDAP::getLDAPUserByLoginName()
... which throws an exceptionNo user available for the given login name...
because the$loginName
being provided is thelogin_name
from the token (Nextcloud's internal user-id), not the username that a user types-in to login.Manager::checkPassword()
fails, and the client fails to authenticate with its app token.It is possible that, if a system is using LDAP and is using users' public LDAP user-id's as the NC internal uid (e.g., instead of using the LDAP entry UUID as the NC internal uid) that this problem would not show itself (because
User_LDAP::checkPassword()
could succeed, even though it should never have been called to begin with).However, I believe the underlying problem is that password-less app tokens are being unintentionally transformed into password-bearing tokens.
Indeed, I added a simple
if (is_null($t->getPassword())) { continue; }
toPublicKeyTokenProvider::updatePasswords()
and that appears to have fixed the problems that I was seeing.Steps to reproduce
See above.
Expected behavior
See above.
Installation method
Manual installation
Operating system
Debian/Ubuntu
PHP engine version
PHP 7.4
Web server
Apache (supported)
Database engine version
PostgreSQL
Is this bug present after an update or on a fresh install?
Updated from a minor version (ex. 22.2.3 to 22.2.4)
Are you using the Nextcloud Server Encryption module?
Encryption is Disabled
What user-backends are you using?
Configuration report
List of activated Apps
Nextcloud Signing status
Nextcloud Logs
Additional info
See above.
The text was updated successfully, but these errors were encountered: