Impossible to add new contacts using "Add to address book" icon in message header

[kcybulski]

When I try to add new contact to LDAP (I have not tested with SQL), I get the following message in GUI "Could not save the contact address."

in logs/errors I have

PHP Warning:  ldap_add(): Add: Object class violation in /var/www/round6/program/include/rcube_ldap.php on line 707

in logs/ldap I have

[07-Sep-2011 11:55:27 +0200]: C: Connect [ldap_server:389]
[07-Sep-2011 11:55:27 +0200]: S: OK
[07-Sep-2011 11:55:27 +0200]: C: Bind [dn: cn=my_email@my_domain,ou=private,ou=rcabook,dc=local] [pass: my_pass]
[07-Sep-2011 11:55:27 +0200]: S: OK
[07-Sep-2011 11:55:27 +0200]: C: Search [(&(objectClass=inetOrgPerson)(|(mail=email@to.add)))]
[07-Sep-2011 11:55:27 +0200]: S: 0 record(s)
[07-Sep-2011 11:55:27 +0200]: C: Add [dn: mail=email@to.add,cn=my_email@my_domain,ou=private,ou=rcabook,dc=local]: Array
(
    [objectClass] => Array
        (
            [0] => top
            [1] => inetOrgPerson
        )

    [cn] => Name
    [mail] => email@to.add
)

[07-Sep-2011 11:55:27 +0200]: S: Object class violation
[07-Sep-2011 11:55:27 +0200]: C: Close

I think problem is in sn: ldap field, it is not set.

[alec]

From where is the 'top' class? Show your config of ldap addressbook.

[kcybulski]

Replying to alec:

From where is the 'top' class? Show your config of ldap addressbook.

$rcmail_config['ldap_public']['private'] = array(
'name'          => 'private',
'hosts'         => array('my_server'),
'port'          => 389,
'use_tls'             => false,
'user_specific' => true,   // If true the base_dn, bind_dn and bind_pass default to the user's IMAP login.
'base_dn'       => 'cn=%fu,ou=private,ou=rcabook,dc=local',
'bind_dn'       => 'cn=%fu,ou=private,ou=rcabook,dc=local',
'bind_pass'     => '',
'search_base_dn' => '',
'search_filter'  => '',   // e.g. '(&(objectClass=posixAccount)(uid=%u))'
'writable'      => true,   // Indicates if we can write to the LDAP directory or not.

'LDAP_Object_Classes' => array('top', 'inetOrgPerson'),
'required_fields'     => array('cn', 'sn', 'mail'),
'LDAP_rdn'      => 'mail',

'ldap_version'  => 3,       // using LDAPv3
'search_fields' => array('mail', 'cn'),  // fields to search in

  'fieldmap' => array(
    // Roundcube  => LDAP
    'name'        => 'cn',
    'surname'     => 'sn',
    'firstname'   => 'givenName',
    'email'       => 'mail',
    'phone:home'  => 'homePhone',
    'phone:work'  => 'telephoneNumber',
    'phone:mobile' => 'mobile',
    'street'      => 'street',
    'zipcode'     => 'postalCode',
    'locality'    => 'l',
    'country'     => 'c',
    'organization' => 'o',
  ),

'sort'          => 'cn',    // The field to sort the listing by.
'scope'         => 'sub',   // search mode: sub|base|list
'filter'        => '(objectClass=inetOrgPerson)',      // used for basic listing (if not empty) and will be &'d with search queries. example: status=act
'fuzzy_search'  => true,    // server allows wildcard search
'sizelimit'     => '0',     // Enables you to limit the count of entries fetched. Setting this to 0 means no limit.
'timelimit'     => '0',     // Sets the number of seconds how long is spend on the search. Setting this to 0 means no limit.
'groups'        => array(
    'base_dn'   => '',
    'filter'    => '(objectClass=groupOfNames)',
    'object_classes' => array("top", "groupOfNames"),
    ),

);

[alec]

Try to modify LDAP_Object_Classes, I assume some of them are not suitable for your LDAP server configuration.

[kcybulski]

Replying to alec:

Try to modify LDAP_Object_Classes, I assume some of them are not suitable for your LDAP server configuration.

Configuration is OK, I can add contacts to this address book manually but I _NEED_ to specify Last Name, if I write only Display Name and email I get this same error.
I setup ldap using ​http://trac.roundcube.net/wiki/Howto_Ldap

I think there are two problems within roundcube,

1) the surname field need to be set,
2) roundcube not use 'required_fields' => array('cn', 'sn', 'mail') configuration option. If sn is not set there should be message informing about it.

Solution for 1) is this patch (I think)

--- ./addcontact.inc    2011-07-18 18:57:15.000000000 +0200
+++ ./changed/addcontact.inc    2011-09-07 13:01:22.000000000 +0200
@@ -42,7 +42,8 @@
   if (!empty($contact_arr[1]['mailto'])) {
     $contact = array(
       'email' => $contact_arr[1]['mailto'],
-      'name' => $contact_arr[1]['name']
+      'name' => $contact_arr[1]['name'],
+      'surname' => $contact_arr[1]['name']
     );

     // Validity checks
@@ -59,7 +60,8 @@

     $contact['email'] = rcube_idn_to_utf8($contact['email']);
     $contact['name'] = rcube_addressbook::compose_display_name($contact);
-
+    $contact['surname'] = rcube_addressbook::compose_display_name($contact);
+
     // check for existing contacts
     $existing = $CONTACTS->search('email', $contact['email'], true, false);

I don't know roundcube enough for 2)

[alec]

Strange is that ldap_add() is called while it shouldn't, see line 699 of rcube_ldap.php file.

[kcybulski]

Replying to alec:

Strange is that ldap_add() is called while it shouldn't, see line 699 of rcube_ldap.php file.

I changed $missing to be declared as array and this helped. Patch attached.

--- ./rcube_ldap.php    2011-09-05 10:49:04.000000000 +0200
+++ ./changed/rcube_ldap.php    2011-09-12 16:24:16.462332447 +0200
@@ -686,7 +686,7 @@

         // Verify that the required fields are set.
         foreach ($this->prop['required_fields'] as $fld) {
-            $missing = null;
+            $missing[] = null;
             if (!isset($newentry[$fld])) {
                 $missing[] = $fld;
             }

Edit: Patch is not working.

[kcybulski]

Replying to kcybulski:

Replying to alec:

Strange is that ldap_add() is called while it shouldn't, see line 699 of rcube_ldap.php file.

I changed $missing to be declared as array and this helped. Patch attached.

CUT ...

Edit: Patch is not working.

This patch is working now. The $missing was set to null in each foreach pass and returned error only if last required field was empty.

--- ./rcube_ldap.php    2011-09-05 10:49:04.000000000 +0200
+++ ./changed/rcube_ldap.php    2011-09-12 16:57:34.954318579 +0200
@@ -685,8 +685,8 @@
         } // end foreach

         // Verify that the required fields are set.
+        $missing[] = null;
         foreach ($this->prop['required_fields'] as $fld) {
-            $missing = null;
             if (!isset($newentry[$fld])) {
                 $missing[] = $fld;
             }

[alec]

Jeee, I was blind. Applied in [6f45fa56]. However, this still doesn't resolve initial issue.

[kcybulski]

Replying to alec:

Jeee, I was blind. Applied in [6f45fa56]. However, this still doesn't resolve initial issue.

Witch the patch from comment 4 it is.

[alec]

Replying to kcybulski:

With the patch from comment 4 it is.

The patch is not good. Consider addressbook where surname isn't required. The code should detect required fields and fill them when needed. BTW, why do you require surname?

[andudi]

just as information about LDAP in general:

• allmost every LDAP addressbook implementation uses inetOrgPerson as object class for the contacts
• inetOrgPerson is based on organizationalPerson and this is based on "person"
• "person" NEEDS at least "cn" and "sn" attribute, else the server do not accept the transaction.

person os defined in core.schema:

objectclass ( 2.5.6.6 NAME 'person'
DESC 'RFC2256: a person'
SUP top STRUCTURAL
MUST ( sn $ cn )
MAY ( userPassword $ telephoneNumber $ seeAlso $ description ) )

Andreas

[kcybulski]

Replying to alec:

Replying to kcybulski:

With the patch from comment 4 it is.

The patch is not good. Consider addressbook where surname isn't required. The code should detect required fields and fill them when needed. BTW, why do you require surname?

It is as Andreas stated. The field is required in inetOrgPerson and 99% of other schema will be based on it and requiring it as well. We could try to build in mechanism to split Given Name from Surname but this is headache.

[alec]

It doesn't matter it's one or more percent. I don't want surname = name when surname is not required. It definitely isn't required when using sql-based addressbook.

[andudi]

But what ist the now break to implement a solution where surname (or what ever fields are required by 'required_fields') is filled with name (or something to be configured) if it is empty in addcontact.inc?

ALEC, if you need help for that, tell me, I would like to do it!

On the other hand:
as I understand the code it is allready prepared for plugins that handle the more complicated pattern matching like givenName and so on... right?

Andreas

[andudi]

ALEC, I am not sure if this is a sollution in your sence, but the following patch solves it for me.
It works for adding emails of both forms like:

1) email@ domain.tld

2) "Real Name" <email@ domain.tld>

Without my patch, both emails are not accepted on my setup, in 1) "cn" and "sn" is missing, in 2) just "sn"
How is it with a SQL addressbook? Is 1) accepted? If not, the default config of my patch would help as well...

Please check it and give me feedback, thanks!

The sollution could be easily inproved with regular expressions in the config, but I think I check the acceptance first :-)

Andreas

--- devel-roundcube_r5214/config/main.inc.php.dist      2011-09-12 22:11:58.000000000 +0200
+++ roundcubedev/config/main.inc.php.dist       2011-09-14 22:20:28.000000000 +0200
@@ -617,6 +617,12 @@
 // available placeholders: {street}, {locality}, {zipcode}, {country}, {region}
 $rcmail_config['address_template'] = '{street}<br/>{locality} {zipcode}<br/>{country} {region}';
  
+// The addcontact feature provides at least the "email" field for plain emails,     
+// and as well the "name" field if the email was like: "name" <email>                               
+// Define here how contact fields should be filled if they was empty. This is usefull e.g.
+// for the LDAP field "sn" (surname) that is required for LDAP contacts based on the "inetOrgPerson" object class
+$rcmail_config['addcontact_fillers'] = array('name' => 'email');
+//$rcmail_config['addcontact_fillers'] = array('name' => 'email', 'surname' => 'name');
+
 // ----------------------------------
 // USER PREFERENCES
 // ----------------------------------

--- devel-roundcube_r5214/program/steps/mail/addcontact.inc     2011-07-26 23:40:55.000000000 +0200
+++ roundcubedev/program/steps/mail/addcontact.inc      2011-09-14 22:21:13.000000000 +0200
@@ -60,6 +60,14 @@
     $contact['email'] = rcube_idn_to_utf8($contact['email']);
     $contact['name'] = rcube_addressbook::compose_display_name($contact);

+    // fill empty fields required in the config
+    $fillers = $RCMAIL->config->get('addcontact_fillers');
+    foreach ($fillers as $field => $filler)
+    {
+        if (empty($contact[$field]))
+            $contact[$field] = $contact[$filler];
+    }
+
     // check for existing contacts
     $existing = $CONTACTS->search('email', $contact['email'], true, false);

[alec]

Exactly, sql-based addressbook requires only email address. I can also imagine use of more than one addressbook with different required_fields value. So, this must be done in per-addresbook-source basis, not globally. I'm thinking about extending rcube_ldap::insert() method with additional argument. It's enabling will make that required_fields will be filled with specified name (if provided).

[thomasb]

Simple automatic completion added in [39cafac3]

The best solution, however, would be to bring up a dialog and ask the user to complete the missing fields.