roundcube / roundcubemail

The Roundcube Webmail suite
https://roundcube.net
GNU General Public License v3.0
5.79k stars 1.62k forks source link

new_user_identity: Empty password from$rcube->get_user_password() prevents LDAP connection #7667

Closed f466162 closed 3 years ago

f466162 commented 3 years ago

Hi,

I want to make use of the new_user_identity plugin in my RC installation as using the username is not sufficient to derive the final mail address for the identity. I have setup all connections, but new_user_plugin cannot properly bind to my LDAP server and it forbids binding anonymously. After the login I can connect to my LDAP addressbook. LDAP binds are configured as user-specific.

I investigated and found out that in program/lib/Roundcube/rcube_ldap.php:304 the password cannot be retrieved from $rcube->get_user_password(). Then I had another look to the session object and when it's created and found out $rcube->storage_init() is called after the new_user_plugin. Thus, the bind password for LDAP is not available for the new_user_plugin.

When I replace $bind_pw = $rcube->get_user_password() with the hardwired password, the new_user_identity plugin is working properly. Furthermore, even when not replacing the call to get_user_password(), I can access the address book.

This seems to be a "timing problem" as the $rcube->storage_init() call is done after the new_user_identity plugin runs.

If you need additional information, please give me a hint.

PS: I'm using the roundcube/roundcubemail Docker image.

alecpl commented 3 years ago

So, it looks like setting $this->password before the rcube_user::create() call in rcmail::login() should do the trick? I would also reset it back to null after the call.

alecpl commented 3 years ago

Fixed.

drybjed commented 3 years ago

@alecpl Is there a chance that this fix can be backported to the release-1.4 branch?

drybjed commented 3 years ago

Just a heads up: after backporting the changes by hand to the release-1.4 branch, I tested this on my existing installation. Completely new user tried to login with an empty user field (LDAP server saw a bind attempt at uid=,ou=People,dc=example,dc=com), but after logging out and back in it seems that the plugin worked fine again. @alecpl, did you test this on a new user account by chance?

alecpl commented 3 years ago

I didn't really test it ;) Could you show your configuration? Is the user logging with full email address, or just the local part? Did you backport all parts of the patch, e.g. the change in rcube.php. Maybe this part needs to be first in the get_user_email() method.

drybjed commented 3 years ago

I backported all of the changes in relevant files. User is logging using the local part, but Roundcube's username_domain is set to the local domain so it should be added automatically.

The LDAP address book configuration from config/config.inc.php:

$config['ldap_public']['People'] = array(
    'name' => 'Example Org',
    'hosts' => array(
        'ldapserver4.example.org',
        'ldapserver3.example.org',
    ),
    'port' => '389',
    'use_tls' => true,
    'ldap_version' => 3,
    'user_specific' => true,
    'base_dn' => 'ou=People,dc=gumed,dc=pl',
    'bind_dn' => 'uid=%u,ou=People,dc=example,dc=org',
    'bind_pass' => '',
    'filter' => '(objectClass=inetOrgPerson)',
    'scope' => 'sub',
    'searchonly' => true,
    'vlv' => false,
    'sort' => 'sn',
    'search_fields' => array(
        'sn',
        'cn',
        'mail',
        'telephoneNumber',
    ),
    'hidden' => true,
    'writable' => false,
    'groups' => array(
        'base_dn' => 'ou=Groups,dc=example,dc=org',
        'filter' => '(objectClass=groupOfNames)',
        'object_classes' => array(
            'groupOfNames',
        ),
    ),
    'fieldmap' => array(
        'name' => 'cn',
        'firstname' => 'givenName',
        'surname' => 'sn',
        'jobtitle' => 'title',
        'email' => 'mailAddress:*',
        'phone:home' => 'homePhone',
        'phone:work' => 'telephoneNumber',
        'phone:mobile' => 'mobile',
        'phone:pager' => 'pager',
        'phone:workfax' => 'facsimileTelephoneNumber',
        'street' => 'street',
        'zipcode' => 'postalCode',
        'region' => 'st',
        'locality' => 'l',
        'website' => 'eduOrgHomePageURI',
        'notes' => 'description:*',
        'photo' => 'jpegPhoto',
    ),
);

I'm not sure if in the bind_dn parameter I should use %u or something else, regular address book searches work with this setup. I also switched to using a separate, global uid=roundcube bind user with its own password, and that made new user account work correctly the first time.

Plugin configuration:

$config['new_user_identity_addressbook'] = 'People';
$config['new_user_identity_match'] = 'mail';
$config['new_user_identity_onlogin'] = true;

I chose mail to allow matching via multiple e-mail addresses. It would be interesting to allow matching via multiple attributes, say uid and mail, but that's probably a bit more involved. Also, I assume that the identity match is for the LDAP directory 'mail' attribute and not the Roundube's 'email' field used in the address book?

alecpl commented 3 years ago

I think fdd52a5312c should fix the issue.

drybjed commented 3 years ago

@alecpl Yup, works as advertised, thanks! So, how about that backport? :-)

alecpl commented 3 years ago

The change has a (small, but still) potential to break things. So, I will not backport it.