auth: Fix userdb auth cache with username changes

The problem was for example when userdb lookup uses only the username part
of the username@domain lookup. Then:

 * "username" lookup caches the results for "username". Since the username
   didn't change, it doesn't store in the cache the "user" field.
 * "username@domain" lookup looks up "username" from cache. Since there is
   no "user" field, the code didn't think the username had changed.

Fix this by saving the "user" field to auth cache, regardless of whether
it's the same as the current username.

diff --git a/src/auth/auth-request-fields.c b/src/auth/auth-request-fields.c
index 3a8dbfb56ca4cd82d0c65715598f65dacf8d49be..dca7a92ceb0a9552b116d8c73df71682972b345a 100644 (file)
--- a/src/auth/auth-request-fields.c
+++ b/src/auth/auth-request-fields.c
@@ -395,7 +395,7 @@ bool auth_request_set_username(struct auth_request *request,
                event_add_str(request->event, "translated_user",
                              request->fields.translated_username);
        }
-       request->user_changed_by_lookup = TRUE;
+       request->user_returned_by_lookup = TRUE;
 
        if (login_username != NULL) {
                if (!auth_request_set_login_username(request,

diff --git a/src/auth/auth-request.c b/src/auth/auth-request.c
index 6e618444856a5aba2d500ed6faf0a56cdaafdbc6..effa1b46af7d4992975fbae59ba800cb1fa41011 100644 (file)
--- a/src/auth/auth-request.c
+++ b/src/auth/auth-request.c
@@ -1128,7 +1128,7 @@ void auth_request_verify_plain(struct auth_request *request,
                request->mech_password = p_strdup(request->pool, password);
        else
                i_assert(request->mech_password == password);
-       request->user_changed_by_lookup = FALSE;
+       request->user_returned_by_lookup = FALSE;
 
        if (request->policy_processed ||
            !request->set->policy_check_before_auth) {
@@ -1319,7 +1319,7 @@ void auth_request_lookup_credentials(struct auth_request *request,
        if (request->wanted_credentials_scheme == NULL)
                request->wanted_credentials_scheme =
                        p_strdup(request->pool, scheme);
-       request->user_changed_by_lookup = FALSE;
+       request->user_returned_by_lookup = FALSE;
 
        if (request->policy_processed ||
            !request->set->policy_check_before_auth) {
@@ -1445,7 +1445,7 @@ auth_request_userdb_save_cache(struct auth_request *request,
                auth_fields_append(request->fields.userdb_reply, str,
                                   AUTH_FIELD_FLAG_CHANGED,
                                   AUTH_FIELD_FLAG_CHANGED, FALSE);
-               if (request->user_changed_by_lookup) {
+               if (request->user_returned_by_lookup) {
                        /* username was changed by passdb or userdb */
                        if (str_len(str) > 0)
                                str_append_c(str, '\t');
@@ -1592,7 +1592,7 @@ void auth_request_userdb_callback(enum userdb_result result,
                if (result == USERDB_RESULT_INTERNAL_FAILURE)
                        request->userdbs_seen_internal_failure = TRUE;
 
-               request->user_changed_by_lookup = FALSE;
+               request->user_returned_by_lookup = FALSE;
 
                request->userdb = next_userdb;
                auth_request_lookup_user(request,
@@ -1666,7 +1666,7 @@ void auth_request_lookup_user(struct auth_request *request,
        const char *cache_key, *error;
 
        request->private_callback.userdb = callback;
-       request->user_changed_by_lookup = FALSE;
+       request->user_returned_by_lookup = FALSE;
        request->userdb_lookup = TRUE;
        request->userdb_cache_result = AUTH_REQUEST_CACHE_NONE;
        if (request->fields.userdb_reply == NULL)
@@ -1833,8 +1833,8 @@ auth_request_try_update_username(struct auth_request *request,
                        "username changed %s -> %s",
                        request->fields.user, new_value);
                auth_request_set_username_forced(request, new_value);
-               request->user_changed_by_lookup = TRUE;
        }
+       request->user_returned_by_lookup = TRUE;
        return TRUE;
 }
 

diff --git a/src/auth/auth-request.h b/src/auth/auth-request.h
index 9cb7d8440b94fac975ce38e421d5d871d7fd51bf..6c06a4c962a29cdb4803ea98bd002c66d2537b9f 100644 (file)
--- a/src/auth/auth-request.h
+++ b/src/auth/auth-request.h
@@ -207,9 +207,9 @@ struct auth_request {
        bool in_delayed_failure_queue:1;
        bool removed_from_handler:1;
        bool snapshot_have_userdb_prefetch_set:1;
-       /* username was changed by this passdb/userdb lookup. Used by
-          auth-workers to determine whether to send back a changed username. */
-       bool user_changed_by_lookup:1;
+       /* The last passdb/userdb lookup returned a username field. This may
+          or may not have changed the current username. */
+       bool user_returned_by_lookup:1;
        /* each passdb lookup can update the current success-status using the
           result_* rules. the authentication succeeds only if this is TRUE
           at the end. mechanisms that don't require passdb, but do a passdb

diff --git a/src/auth/auth-worker-server.c b/src/auth/auth-worker-server.c
index ff0c6fff81808b978f789a7708cca54ba74e2ac7..9687c4865690b2ce671e4cb95ed3824d02f631f1 100644 (file)
--- a/src/auth/auth-worker-server.c
+++ b/src/auth/auth-worker-server.c
@@ -231,7 +231,7 @@ static void verify_plain_callback(enum passdb_result result,
        }
        if (result != PASSDB_RESULT_INTERNAL_FAILURE) {
                str_append_c(str, '\t');
-               if (request->user_changed_by_lookup)
+               if (request->user_returned_by_lookup)
                        str_append_tabescaped(str, request->fields.user);
                str_append_c(str, '\t');
                if (request->passdb_password != NULL)
@@ -391,7 +391,7 @@ lookup_credentials_callback(enum passdb_result result,
                        str_append(str, "NEXT\t");
                else
                        str_append(str, "OK\t");
-               if (request->user_changed_by_lookup)
+               if (request->user_returned_by_lookup)
                        str_append_tabescaped(str, request->fields.user);
                str_append_c(str, '\t');
                if (request->wanted_credentials_scheme[0] != '\0') {
@@ -528,7 +528,7 @@ lookup_user_callback(enum userdb_result result,
                break;
        case USERDB_RESULT_OK:
                str_append(str, "OK\t");
-               if (auth_request->user_changed_by_lookup)
+               if (auth_request->user_returned_by_lookup)
                        str_append_tabescaped(str, auth_request->fields.user);
                /* export only the fields changed by this lookup */
                auth_fields_append(auth_request->fields.userdb_reply, str,

diff --git a/src/auth/userdb-blocking.c b/src/auth/userdb-blocking.c
index c2871f0415267677d4df805ee535c62f9a4db77a..5ce9523e1b1f3dae635744e909414cb8a9308899 100644 (file)
--- a/src/auth/userdb-blocking.c
+++ b/src/auth/userdb-blocking.c
@@ -37,10 +37,10 @@ static bool user_callback(struct auth_worker_connection *conn ATTR_UNUSED,
                        args += 2;
                }
 
-               if (username[0] != '\0' &&
-                   strcmp(request->fields.user, username) != 0) {
-                       auth_request_set_username_forced(request, username);
-                       request->user_changed_by_lookup = TRUE;
+               if (username[0] != '\0') {
+                       if (strcmp(request->fields.user, username) != 0)
+                               auth_request_set_username_forced(request, username);
+                       request->user_returned_by_lookup = TRUE;
                }
        } else {
                result = USERDB_RESULT_INTERNAL_FAILURE;