@@ -699,11 +699,12 @@ XError Session_impl::authenticate(const char *user, const char *pass,
699699 if (error) return error;
700700
701701 bool has_sha256_memory = false ;
702- XError auth_error ;
703- XError reported_error =
704- XError{ER_ACCESS_DENIED_ERROR, " Invalid user or password " };
702+ bool fatal_error_received = false ;
703+ XError reported_error;
704+
705705 for (const auto &auth_method : optional_auth_methods.second ) {
706706 const bool is_last = &auth_method == &optional_auth_methods.second .back ();
707+
707708 if (auth_method == " PLAIN" && !is_secure_connection) {
708709 // If this is not the last authentication mechanism then do not report
709710 // error but try those other methods instead.
@@ -712,42 +713,70 @@ XError Session_impl::authenticate(const char *user, const char *pass,
712713 CR_X_INVALID_AUTH_METHOD,
713714 " Invalid authentication method: PLAIN over unsecure channel" };
714715 }
715- } else {
716- auth_error = protocol.execute_authenticate (
717- details::value_or_empty_string (user),
718- details::value_or_empty_string (pass),
719- details::value_or_empty_string (schema), auth_method);
720-
721- const auto current_error_code = auth_error.error ();
722-
723- // In case of 'broken pipe', 'peer disconnected' and timeouts we should
724- // break the authentication sequence and return an error.
725- if (current_error_code == CR_SERVER_GONE_ERROR ||
726- current_error_code == CR_X_WRITE_TIMEOUT ||
727- current_error_code == CR_X_READ_TIMEOUT ||
728- current_error_code == CR_UNKNOWN_ERROR)
729- return auth_error;
730-
731- if (current_error_code != ER_ACCESS_DENIED_ERROR ||
732- reported_error.error () == ER_ACCESS_DENIED_ERROR) {
733- reported_error = auth_error;
734- }
735716
736- // Also we should stop the authentication sequence on any fatal error.
737- if (auth_error.is_fatal ()) return reported_error;
738-
739- if (auth_method == " SHA256_MEMORY" ) has_sha256_memory = true ;
717+ continue ;
740718 }
741719
720+ XError current_error = protocol.execute_authenticate (
721+ details::value_or_empty_string (user),
722+ details::value_or_empty_string (pass),
723+ details::value_or_empty_string (schema), auth_method);
724+
742725 // Authentication successful, otherwise try to use different auth method
743- if (!auth_error) return {};
726+ if (!current_error) return {};
727+
728+ const auto current_error_code = current_error.error ();
729+
730+ // In case of connection errors ('broken pipe', 'peer disconnected',
731+ // timeouts...) we should break the authentication sequence and return
732+ // an error.
733+ if (current_error_code == CR_SERVER_GONE_ERROR ||
734+ current_error_code == CR_X_WRITE_TIMEOUT ||
735+ current_error_code == CR_X_READ_TIMEOUT ||
736+ current_error_code == CR_UNKNOWN_ERROR) {
737+ // Expected disconnection
738+ if (fatal_error_received) return reported_error;
739+
740+ // Unexpected disconnection
741+ return current_error;
742+ }
743+
744+ // Try to choose most important error:
745+ //
746+ // |Priority |Action |
747+ // |---------|------------------------------|
748+ // |1 |No error was set |
749+ // |2 |Last other than access denied |
750+ // |3 |Last access denied |
751+ if (!reported_error || current_error_code != ER_ACCESS_DENIED_ERROR ||
752+ reported_error.error () == ER_ACCESS_DENIED_ERROR) {
753+ reported_error = current_error;
754+ }
755+
756+ // Also we should stop the authentication sequence on fatal error.
757+ // Still it would break compatibility with servers that wrongly mark
758+ // Mysqlx::Error message with fatal flag.
759+ //
760+ // To workaround the problem of backward compatibility, we should
761+ // remember that fatal error was received and try to continue the
762+ // sequence. After reception of fatal error, following connection
763+ // errors are expected (CR_SERVER_GONE_ERROR, CR_X_WRITE_TIMEOUT....)
764+ // and must be ignored.
765+ if (current_error.is_fatal ()) fatal_error_received = true ;
766+
767+ if (auth_method == " SHA256_MEMORY" ) has_sha256_memory = true ;
744768 }
745769
770+ // In case when SHA256_MEMORY was used and no PLAIN (because of not using
771+ // secure connection) and all errors where ER_ACCESS_DENIED, there is
772+ // possibility that password cache on the server side is empty.
773+ // We need overwrite the error to give the user a hint that
774+ // secure connection can be used.
746775 if (has_sha256_memory && !is_secure_connection &&
747776 reported_error.error () == ER_ACCESS_DENIED_ERROR) {
748777 reported_error = XError{CR_X_AUTH_PLUGIN_ERROR,
749- " Authentication failed, check username and \
750- password or try a secure connection" };
778+ " Authentication failed, check username and "
779+ " password or try a secure connection" };
751780 }
752781
753782 return reported_error;
0 commit comments