[Bug]: Passwordless authtokens should remain passwordless

mdoggydog commented 2 years ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

On (one of) my system(s), this bug is manifesting itself in two related ways:

When logged in to the web UI, via SAML SSO (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.)
Applications using app tokens (e.g., desktop sync client, or Android app) are logged out and the app tokens cease working. After logging back in, they are logged out again within about an hour. (Super annoying and defeats the whole purpose of having app tokens.)

What appears to be happening is:

Something is calling 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 a null password field) such that they are no longer password-less tokens.
This causes a cascade of problems later on when a formerly password-less token is used/checked again. For example:
- A client tries to use its app token, and some NC app calls Session::getUser().
- Session::getUser() calls Session::validateSession().
- Session::validateSession() calls Session::validateToken().
- Session::validateToken() calls Session::checkTokenCredentials().
- Session::checkTokenCredentials() asks the PublicKeyTokenProvider for the token's password (i.e., getPassword()).
- Ooops... the formerly password-less token now has a token! Session::checkTokenCredentials() calls Manager::checkPassword() to check that password!
- Manager::checkPassword() calls Manager::checkPasswordNoLogging().
- I'm not even sure what the password is that is being checked at this point (since there shouldn't even be one), but Manager::checkPasswordNoLogging() ends up calling User_LDAP::checkPassword() which calls User_LDAP::getLDAPUserByLoginName()... which throws an exception No user available for the given login name... because the $loginName being provided is the login_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; } to PublicKeyTokenProvider::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?

[X] Default user-backend (database)
[X] LDAP/ Active Directory
[X] SSO - SAML
[ ] Other

Configuration report

{
    "system": {
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "localhost",
            "XXXXX.org"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "pgsql",
        "version": "22.2.3.0",
        "overwrite.cli.url": "https:\/\/XXXXX.org",
        "htaccess.RewriteBase": "\/",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "default_phone_region": "US",
        "installed": true,
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "mail_from_address": "***REMOVED SENSITIVE VALUE***",
        "mail_smtpmode": "sendmail",
        "mail_sendmailmode": "smtp",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "ldapIgnoreNamingRules": false,
        "ldapProviderFactory": "OCA\\User_LDAP\\LDAPProviderFactory",
        "maintenance": false,
        "enabledPreviewProviders": [
            "OC\\Preview\\PNG",
            "OC\\Preview\\JPEG",
            "OC\\Preview\\GIF",
            "OC\\Preview\\HEIC",
            "OC\\Preview\\BMP",
            "OC\\Preview\\XBitmap",
            "OC\\Preview\\MP3",
            "OC\\Preview\\TXT",
            "OC\\Preview\\MarkDown",
            "OC\\Preview\\OpenDocument",
            "OC\\Preview\\Krita",
            "OC\\Preview\\Movie",
            "OC\\Preview\\PDF",
            "OC\\Preview\\Postscript"
        ],
        "theme": "",
        "loglevel": 0,
        "app_install_overwrite": [
            "files_texteditor"
        ]
    }
}

List of activated Apps

Enabled:
  - accessibility: 1.8.0
  - activity: 2.15.0
  - announcementcenter: 6.1.1
  - apporder: 0.14.0
  - calendar: 3.0.5
  - circles: 22.1.1
  - cloud_federation_api: 1.5.0
  - comments: 1.12.0
  - contacts: 4.0.7
  - contactsinteraction: 1.3.0
  - dashboard: 7.2.0
  - dav: 1.19.0
  - external: 3.9.0
  - federatedfilesharing: 1.12.0
  - federation: 1.12.0
  - files: 1.17.0
  - files_markdown: 2.3.5
  - files_pdfviewer: 2.3.1
  - files_rightclick: 1.1.0
  - files_sharing: 1.14.0
  - files_texteditor: 2.14.0
  - files_trashbin: 1.12.0
  - files_versions: 1.15.0
  - files_videoplayer: 1.11.0
  - firstrunwizard: 2.11.0
  - groupfolders: 10.0.2
  - logreader: 2.7.0
  - lookup_server_connector: 1.10.0
  - maps: 0.1.10
  - metadata: 0.15.0
  - nextcloud_announcements: 1.11.0
  - notifications: 2.10.1
  - notify_push: 0.3.0
  - oauth2: 1.10.0
  - password_policy: 1.12.0
  - photos: 1.4.0
  - privacy: 1.6.0
  - provisioning_api: 1.12.0
  - quota_warning: 1.13.0
  - recommendations: 1.1.0
  - richdocuments: 4.2.3
  - serverinfo: 1.12.0
  - settings: 1.4.0
  - sharebymail: 1.12.0
  - spreed: 12.2.2
  - support: 1.5.0
  - survey_client: 1.10.0
  - systemtags: 1.12.0
  - tasks: 0.14.2
  - theming: 1.13.0
  - twofactor_backupcodes: 1.11.0
  - updatenotification: 1.12.0
  - user_ldap: 1.12.1
  - user_saml: 4.1.1
  - user_status: 1.2.0
  - viewer: 1.6.0
  - weather_status: 1.2.0
  - workflowengine: 2.4.0
Disabled:
  - admin_audit
  - encryption
  - files_external
  - ldap_write_support
  - ldapcontacts
  - mail
  - notes
  - text

Nextcloud Signing status

No errors have been found.

Nextcloud Logs

{"reqId":"IBYIC7RBOfDmXlaHGmH3","level":0,"time":"2022-01-27T06:11:09+00:00","remoteAddr":"X.X.X.X","user":"LDAP-ENTRY-UUID-GOES-HERE","app":"user_ldap","method":"PROPFIND","url":"/remote.php/dav/files/LDAP-ENTRY-UUID-GOES-HERE/","message":"No user available for the given login name on localhost:389","userAgent":"Mozilla/5.0 (Linux) mirall/3.3.5-1 (Nextcloud, debian-5.15.0-3-amd64 ClientArchitecture: x86_64 OsArchitecture: x86_64)","version":"22.2.3.0","exception":{"Exception":"OCA\\User_LDAP\\Exceptions\\NotOnLDAP","Message":"No user available for the given login name on localhost:389","Code":0,"Trace":[{"file":"/srv/XXXX.org/nextcloud/apps/user_ldap/lib/User_LDAP.php","line":176,"function":"getLDAPUserByLoginName","class":"OCA\\User_LDAP\\User_LDAP","type":"->"},{"function":"checkPassword","class":"OCA\\User_LDAP\\User_LDAP","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/srv/XXXX.org/nextcloud/apps/user_ldap/lib/User_Proxy.php","line":81,"function":"call_user_func_array"},{"file":"/srv/XXXX.org/nextcloud/apps/user_ldap/lib/Proxy.php","line":171,"function":"walkBackends","class":"OCA\\User_LDAP\\User_Proxy","type":"->"},{"file":"/srv/XXXX.org/nextcloud/apps/user_ldap/lib/User_Proxy.php","line":224,"function":"handleRequest","class":"OCA\\User_LDAP\\Proxy","type":"->"},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Manager.php","line":251,"function":"checkPassword","class":"OCA\\User_LDAP\\User_Proxy","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Manager.php","line":228,"function":"checkPasswordNoLogging","class":"OC\\User\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Session.php","line":764,"function":"checkPassword","class":"OC\\User\\Manager","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Session.php","line":804,"function":"checkTokenCredentials","class":"OC\\User\\Session","type":"->"},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Session.php","line":268,"function":"validateToken","class":"OC\\User\\Session","type":"->","args":["*** sensitive parameters replaced ***"]},{"file":"/srv/XXXX.org/nextcloud/lib/private/User/Session.php","line":243,"function":"validateSession","class":"OC\\User\\Session","type":"->"},{"file":"/srv/XXXX.org/nextcloud/apps/user_saml/appinfo/app.php","line":108,"function":"getUser","class":"OC\\User\\Session","type":"->"},{"file":"/srv/XXXX.org/nextcloud/lib/private/legacy/OC_App.php","line":303,"args":["/srv/XXXX.org/nextcloud/apps/user_saml/appinfo/app.php"],"function":"require_once"},{"file":"/srv/XXXX.org/nextcloud/lib/private/legacy/OC_App.php","line":185,"function":"requireAppFile","class":"OC_App","type":"::"},{"file":"/srv/XXXX.org/nextcloud/lib/private/legacy/OC_App.php","line":139,"function":"loadApp","class":"OC_App","type":"::"},{"file":"/srv/XXXX.org/nextcloud/remote.php","line":150,"function":"loadApps","class":"OC_App","type":"::"}],"File":"/srv/XXXX.org/nextcloud/apps/user_ldap/lib/User_LDAP.php","Line":161,"CustomMessage":"No user available for the given login name on localhost:389"}}

Additional info

See above.

mdoggydog commented 2 years ago

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).

chris246 commented 2 years ago

I think I can confirm this observation:

@mdoggydog: Something is calling PublicKeyTokenProvider::updatePasswords(). (I'm not sure what code flow is leading to this; maybe a CALDAV client logging in?)

I disabled DAV Access (not using App Passwords) to my account yesterday and I stay logged in now.

ikogan commented 2 years ago

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.

ChristophWurst commented 2 years ago

I'm not even sure what the password is that is being checked at this point

It is the password of another login. But in your case this is suspicious. Is SAML the only authentication mechanism?

because the $loginName being provided is the login_name from the token (Nextcloud's internal user-id), not the username that a user types-in to login

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.

szaimen commented 1 year ago

Hi, please update to 24.0.9 or better 25.0.3 and report back if it fixes the issue. Thank you!

My goal is to add a label like e.g. 25-feedback to this ticket of an up-to-date major Nextcloud version where the bug could be reproduced. However this is not going to work without your help. So thanks for all your effort!

If you don't manage to reproduce the issue in time and the issue gets closed but you can reproduce the issue afterwards, feel free to create a new bug report with up-to-date information by following this link: https://github.com/nextcloud/server/issues/new?assignees=&labels=bug%2C0.+Needs+triage&template=BUG_REPORT.yml&title=%5BBug%5D%3A+

davidmehren commented 1 year ago

I can still reproduce this problem with 25.0.3. I was also able to fix it via the patch from https://github.com/nextcloud/server/issues/26502#issuecomment-821983093

szaimen commented 1 year ago

cc @ChristophWurst @juliushaertl

nextcloud / server