auth: Log a warning if ldap attribute has unexpectedly multiple values.

diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c
index 67907f169dd9f51cd4530f911072ce17f5f5a11c..aa8bc580f789ee993e06468e28869aee09675d5a 100644 (file)
--- a/src/auth/auth-request.c
+++ b/src/auth/auth-request.c
@@ -1308,10 +1308,7 @@ void auth_request_set_userdb_field_values(struct auth_request *request,
        if (*values == NULL)
                return;
 
-       if (strcmp(name, "uid") == 0) {
-               /* there can be only one. use the first one. */
-               auth_request_set_userdb_field(request, name, *values);
-       } else if (strcmp(name, "gid") == 0) {
+       if (strcmp(name, "gid") == 0) {
                /* convert gids to comma separated list */
                string_t *value;
                gid_t gid;
@@ -1332,6 +1329,11 @@ void auth_request_set_userdb_field_values(struct auth_request *request,
                                      str_c(value));
        } else {
                /* add only one */
+               if (values[1] != NULL) {
+                       auth_request_log_warning(request, "userdb",
+                               "Multiple values found for '%s', "
+                               "using value '%s'", name, *values);
+               }
                auth_request_set_userdb_field(request, name, *values);
        }
 }
@@ -1672,6 +1674,19 @@ void auth_request_log_info(struct auth_request *auth_request,
        va_end(va);
 }
 
+void auth_request_log_warning(struct auth_request *auth_request,
+                             const char *subsystem,
+                             const char *format, ...)
+{
+       va_list va;
+
+       va_start(va, format);
+       T_BEGIN {
+               i_warning("%s", get_log_str(auth_request, subsystem, format, va));
+       } T_END;
+       va_end(va);
+}
+
 void auth_request_log_error(struct auth_request *auth_request,
                            const char *subsystem,
                            const char *format, ...)

diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h
index 42cffd12aa72f2f840ab1568bfdc9b51f25e323e..081112ec411b9cdaad5d84eaf10e5a3f40785a53 100644 (file)
--- a/src/auth/auth-request.h
+++ b/src/auth/auth-request.h
@@ -192,6 +192,9 @@ void auth_request_log_debug(struct auth_request *auth_request,
 void auth_request_log_info(struct auth_request *auth_request,
                           const char *subsystem,
                           const char *format, ...) ATTR_FORMAT(3, 4);
+void auth_request_log_warning(struct auth_request *auth_request,
+                             const char *subsystem,
+                             const char *format, ...);
 void auth_request_log_error(struct auth_request *auth_request,
                            const char *subsystem,
                            const char *format, ...) ATTR_FORMAT(3, 4);

diff --git a/src/auth/db-ldap.c b/src/auth/db-ldap.c
index b9c250625647d37ca219abbe8b1a3de53b5add09..f628fe2a929515de360bfba3fcceee1fe1da3cc0 100644 (file)
--- a/src/auth/db-ldap.c
+++ b/src/auth/db-ldap.c
@@ -58,12 +58,11 @@ struct db_ldap_result_iterate_context {
        struct var_expand_table *var_table;
 
        char *attr, **vals;
-       const char *name, *value, *template, *val_1_arr[2];
+       const char *name, *template, *val_1_arr[2];
        const char *const *static_attrs;
        BerElement *ber;
 
        string_t *var, *debug;
-       unsigned int value_idx;
 };
 
 struct db_ldap_sasl_bind_context {
@@ -1111,6 +1110,8 @@ db_ldap_result_iterate_finish(struct db_ldap_result_iterate_context *ctx)
 static void
 db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx)
 {
+       i_assert(ctx->vals == NULL);
+
        ctx->name = hash_table_lookup(ctx->attr_map, ctx->attr);
        ctx->template = NULL;
 
@@ -1119,10 +1120,8 @@ db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx)
                            ctx->name != NULL ? ctx->name : "?unknown?");
        }
 
-       if (ctx->name == NULL || *ctx->name == '\0') {
-               ctx->value = NULL;
+       if (ctx->name == NULL || *ctx->name == '\0')
                return;
-       }
 
        if (strchr(ctx->name, '%') != NULL &&
            (ctx->template = strchr(ctx->name, '=')) != NULL) {
@@ -1138,31 +1137,35 @@ db_ldap_result_change_attr(struct db_ldap_result_iterate_context *ctx)
 
        ctx->vals = ldap_get_values(ctx->conn->ld, ctx->entry,
                                    ctx->attr);
-       ctx->value = ctx->vals[0];
-       ctx->value_idx = 0;
 }
 
 static void
 db_ldap_result_return_value(struct db_ldap_result_iterate_context *ctx)
 {
-       bool first = ctx->value_idx == 0;
+       unsigned int i;
 
        if (ctx->template != NULL) {
-               ctx->var_table[0].value = ctx->value;
+               if (ctx->vals[1] != NULL) {
+                       auth_request_log_warning(ctx->auth_request, "ldap",
+                               "Multiple values found for '%s', "
+                               "using value '%s'", ctx->name, ctx->vals[0]);
+               }
+               ctx->var_table[0].value = ctx->vals[0];
                str_truncate(ctx->var, 0);
                var_expand(ctx->var, ctx->template, ctx->var_table);
-               ctx->value = str_c(ctx->var);
+               ctx->val_1_arr[0] = str_c(ctx->var);
        }
 
-       if (ctx->debug != NULL) {
-               if (!first)
-                       str_append_c(ctx->debug, '/');
-               if (ctx->auth_request->set->debug_passwords ||
-                   (strcmp(ctx->name, "password") != 0 &&
-                    strcmp(ctx->name, "password_noscheme") != 0))
-                       str_append(ctx->debug, ctx->value);
-               else
-                       str_append(ctx->debug, PASSWORD_HIDDEN_STR);
+       if (ctx->debug == NULL) {
+               /* no debugging */
+       } else if (ctx->auth_request->set->debug_passwords ||
+                  (strcmp(ctx->name, "password") != 0 &&
+                   strcmp(ctx->name, "password_noscheme") != 0)) {
+               str_append(ctx->debug, ctx->vals[0]);
+               for (i = 1; ctx->vals[i] != NULL; i++)
+                       str_printfa(ctx->debug, ", %s", ctx->vals[i]);
+       } else {
+               str_append(ctx->debug, PASSWORD_HIDDEN_STR);
        }
 }
 
@@ -1171,16 +1174,10 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx)
        const char *p;
 
        while (ctx->attr != NULL) {
-               if (ctx->vals == NULL) {
-                       /* a new attribute */
-                       db_ldap_result_change_attr(ctx);
-               } else {
-                       /* continuing existing attribute */
-                       if (ctx->value != NULL)
-                               ctx->value = ctx->vals[++ctx->value_idx];
-               }
+               /* a new attribute */
+               db_ldap_result_change_attr(ctx);
 
-               if (ctx->value != NULL) {
+               if (ctx->vals != NULL) {
                        db_ldap_result_return_value(ctx);
                        return TRUE;
                }
@@ -1195,14 +1192,13 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx)
                p = strchr(*ctx->static_attrs, '=');
                if (p == NULL) {
                        ctx->name = *ctx->static_attrs;
-                       ctx->value = "";
+                       ctx->val_1_arr[0] = "";
                } else {
                        ctx->name = t_strdup_until(*ctx->static_attrs, p);
-                       ctx->value = p + 1;
+                       ctx->val_1_arr[0] = p + 1;
                }
-               /* make _next_all() return correct values */
+               /* make _next() return correct values */
                ctx->template = "";
-               ctx->val_1_arr[0] = ctx->value;
                ctx->static_attrs++;
                return TRUE;
        }
@@ -1212,31 +1208,18 @@ static bool db_ldap_result_int_next(struct db_ldap_result_iterate_context *ctx)
 }
 
 bool db_ldap_result_iterate_next(struct db_ldap_result_iterate_context *ctx,
-                                const char **name_r, const char **value_r)
-{
-       if (!db_ldap_result_int_next(ctx))
-               return FALSE;
-
-       *name_r = ctx->name;
-       *value_r = ctx->value;
-       return TRUE;
-}
-
-bool db_ldap_result_iterate_next_all(struct db_ldap_result_iterate_context *ctx,
-                                    const char **name_r,
-                                    const char *const **values_r)
+                                const char **name_r,
+                                const char *const **values_r)
 {
        if (!db_ldap_result_int_next(ctx))
                return FALSE;
 
        if (ctx->template != NULL) {
                /* we can use only one value with templates */
-               ctx->val_1_arr[0] = ctx->value;
                *values_r = ctx->val_1_arr;
        } else {
                *values_r = (const char *const *)ctx->vals;
        }
-       ctx->value = NULL;
        *name_r = ctx->name;
        return TRUE;
 }

diff --git a/src/auth/db-ldap.h b/src/auth/db-ldap.h
index bd18e82600adb583ce1b23211bcb8edb8b42bb1a..8e8d68c7d74efb6d984f419bf5108fb6e35aaba9 100644 (file)
--- a/src/auth/db-ldap.h
+++ b/src/auth/db-ldap.h
@@ -178,9 +178,7 @@ db_ldap_result_iterate_init(struct ldap_connection *conn, LDAPMessage *entry,
                            struct auth_request *auth_request,
                            struct hash_table *attr_map);
 bool db_ldap_result_iterate_next(struct db_ldap_result_iterate_context *ctx,
-                                const char **name_r, const char **value_r);
-bool db_ldap_result_iterate_next_all(struct db_ldap_result_iterate_context *ctx,
-                                    const char **name_r,
-                                    const char *const **values_r);
+                                const char **name_r,
+                                const char *const **values_r);
 
 #endif

diff --git a/src/auth/passdb-ldap.c b/src/auth/passdb-ldap.c
index b7d923391535ba6bc61899f0b0359ca1ecd9aed3..4601015643764f877decaf050ace74b63455604a 100644 (file)
--- a/src/auth/passdb-ldap.c
+++ b/src/auth/passdb-ldap.c
@@ -43,12 +43,17 @@ ldap_query_save_result(struct ldap_connection *conn,
                       LDAPMessage *entry, struct auth_request *auth_request)
 {
        struct db_ldap_result_iterate_context *ldap_iter;
-       const char *name, *value;
+       const char *name, *const *values;
 
        ldap_iter = db_ldap_result_iterate_init(conn, entry, auth_request,
                                                conn->pass_attr_map);
-       while (db_ldap_result_iterate_next(ldap_iter, &name, &value)) {
-               auth_request_set_field(auth_request, name, value,
+       while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
+               if (values[1] != NULL) {
+                       auth_request_log_warning(auth_request, "ldap",
+                               "Multiple values found for '%s', "
+                               "using value '%s'", name, values[0]);
+               }
+               auth_request_set_field(auth_request, name, values[0],
                                       conn->set.default_pass_scheme);
        }
 }

diff --git a/src/auth/userdb-ldap.c b/src/auth/userdb-ldap.c
index ec7c2659dff413c304b03bf93288667bc1bca434..b629f740e068283c76dbf7b7d79da87e3ced70f2 100644 (file)
--- a/src/auth/userdb-ldap.c
+++ b/src/auth/userdb-ldap.c
@@ -52,7 +52,7 @@ ldap_query_get_result(struct ldap_connection *conn, LDAPMessage *entry,
 
        ldap_iter = db_ldap_result_iterate_init(conn, entry, auth_request,
                                                conn->user_attr_map);
-       while (db_ldap_result_iterate_next_all(ldap_iter, &name, &values)) {
+       while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
                auth_request_set_userdb_field_values(auth_request,
                                                     name, values);
        }
@@ -168,7 +168,7 @@ static void userdb_ldap_iterate_callback(struct ldap_connection *conn,
        ldap_iter = db_ldap_result_iterate_init(conn, res,
                                                request->auth_request,
                                                conn->iterate_attr_map);
-       while (db_ldap_result_iterate_next_all(ldap_iter, &name, &values)) {
+       while (db_ldap_result_iterate_next(ldap_iter, &name, &values)) {
                if (strcmp(name, "user") != 0) {
                        i_warning("ldap: iterate: "
                                  "Ignoring field not named 'user': %s", name);