nextcloud / nextcloud/server

[Bug]: user_ldap crashes during login, password policy needs missing null check(s)

Open
#35,175 10 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

0. Needs triage 26-feedback bug feature: ldap needs review
Dominant language
PHP
Stars
36.9k
Forks
5.2k
Avg merge
2d 3h
Merged PRs (30d)
713

Description

⚠️ This issue respects the following points: ⚠️
  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.
Bug description

function⋅handlePasswordExpiry($params) in apps/user_ldap/lib/User/User.php fail to handle multiple corener cases:

  • First is after this (line 651](https://github.com/nextcloud/server/blob/baf74f0aa1b8947272b10532a3c3a1cff16b6085/apps/user_ldap/lib/User/User.php#L651)
    This returns nothing if user never changed password (and he isn't on a grace time/reset)!
  • I presume (although I did NOT try it) this is wrong too (although it is null checked
    line 668
    Existing password policy doesn't mean we must have these attributes - pwdPolicy MAY have these attributes
    Although I - and most others - will have all MAY attributes set, I actually don't use these, as I think forcing users to change their passwords often leads to passwords more likely to be written down.
    But pwdCheckQuality and pwdMinLength is not zero on my server, and on other setups, these could be the only attributes set!
    So these could be neither in cache, nor in search result!
Steps to reproduce
  1. Setup LDAP server, with a password policy that only has pwdCheckQuality and pwdMinLength attributes set.
  2. Connect that user backend to Nextcloud
  3. Create a new LDAP user. Set their password during creation, not in a separate operation
  4. Try to login with the new user
Expected behavior

Newly created user should be able to log in!
If no pwdChangedTime attribute does not exist for a given user, it is not necessarily a grace login.
It it doesn't exists for a user

  1. If no pwdMaxAge and pwdExpireWarning is set in policy, then nothing to check
    LDAP User could be created with throwaway token based registration, where he gets to choose his password during user creation! Maybe they are just asked at a helpdesk PC to type in a password, I know, it's horror....
  2. If these policy attrs are set, and pwdChangedTime is empty, use createTimeStamp

Furthermore, don't blindly override ppolicy cache with a search result that doesn't return every attribute

Installation method

Community Manual installation with Archive

Operating system

Other

PHP engine version

PHP 8.0

Web server

Other

Database engine version

PostgreSQL

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

What user-backends are you using?
  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other
Configuration report
Irrelevant
List of activated Apps
Irrelevant
Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in apps/user_ldap/lib/User/User.php at handlePasswordExpiry($params), especially the referenced lines around 651 and 668. Reproduce the LDAP setup with only pwdCheckQuality and pwdMinLength, then trace how policy and user attributes are obtained. Done means newly created users can log in and missing password-policy attributes do not cause a crash or incorrectly replace cached values.

Written by the indexing model from the issue text.

Assessment

Tech stack
php
Domain
authentication
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.