throw exception upon auth

src/Access/Authentication.cpp

@@ -254,8 +254,26 @@ bool Authentication::areCredentialsValid(
     if ([[maybe_unused]] const auto * always_allow_credentials = typeid_cast<const AlwaysAllowCredentials *>(&credentials))
         return true;
 
-    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "TODO arthur, list possible types");
+
-//    throw Exception(ErrorCodes::NOT_IMPLEMENTED, "areCredentialsValid(): authentication type {} not supported", toString(auth_data.getType()));
+    // below code sucks, but works for now I guess.
+    std::string possible_authentication_types;
+    bool first = true;
+
+    for (const auto & authentication_method : authentication_methods)
+    {
+        if (first)
+        {
+            possible_authentication_types += ", ";
+            first = false;
+        }
+        possible_authentication_types += toString(authentication_method.getType());
+    }
+
+    throw Exception(
+        ErrorCodes::NOT_IMPLEMENTED,
+        "areCredentialsValid(): Invalid credentials provided, available authentication methods are {}",
+        possible_authentication_types);
+
 }
 
 }

src/Access/IAccessStorage.cpp

@@ -514,8 +514,8 @@ std::optional<AuthResult> IAccessStorage::authenticateImpl(
     const Poco::Net::IPAddress & address,
     const ExternalAuthenticators & external_authenticators,
     bool throw_if_user_not_exists,
-    bool ,
+    bool allow_no_password,
-    bool ) const
+    bool allow_plaintext_password) const
 {
     if (auto id = find<User>(credentials.getUserName()))
     {
@@ -526,10 +526,16 @@ std::optional<AuthResult> IAccessStorage::authenticateImpl(
                 throwAddressNotAllowed(address);
 
             // todo arthur
-//            auto auth_type = user->auth_data.getType();
+            // for now, just throw exception in case a user exists with invalid auth method
-//            if (((auth_type == AuthenticationType::NO_PASSWORD) && !allow_no_password) ||
+            // back in the day, it would also throw an exception. There might be a smarter alternative
-//                ((auth_type == AuthenticationType::PLAINTEXT_PASSWORD) && !allow_plaintext_password))
+            // like a user scan during startup.
-//                throwAuthenticationTypeNotAllowed(auth_type);
+            for (const auto & auth_method : user->authentication_methods)
+            {
+                auto auth_type = auth_method.getType();
+                if (((auth_type == AuthenticationType::NO_PASSWORD) && !allow_no_password) ||
+                    ((auth_type == AuthenticationType::PLAINTEXT_PASSWORD) && !allow_plaintext_password))
+                    throwAuthenticationTypeNotAllowed(auth_type);
+            }
 
             if (!areCredentialsValid(*user, credentials, external_authenticators, auth_result.settings))
                 throwInvalidCredentials();

src/Interpreters/Access/InterpreterCreateUserQuery.cpp

@@ -72,8 +72,8 @@ namespace
 
         if (auth_data || !query.alter)
         {
-            // todo arthur
+            // I suppose it is guaranteed a user will always have at least one authentication method
-            auto auth_type = user.authentication_methods[0].getType();
+            auto auth_type = user.authentication_methods.back().getType();
             if (((auth_type == AuthenticationType::NO_PASSWORD) && !allow_no_password) ||
                 ((auth_type == AuthenticationType::PLAINTEXT_PASSWORD)  && !allow_plaintext_password))
             {

src/Interpreters/Access/InterpreterShowCreateAccessEntityQuery.cpp

@@ -65,6 +65,8 @@ namespace
         }
 
         // todo arthur
+        // to fix this, I'll need to turn `query->auth_data` into a list
+        // that also means creating a user with multiple authentication methods should be allowed
         if (user.authentication_methods[0].getType() != AuthenticationType::NO_PASSWORD)
             query->auth_data = user.authentication_methods[0].toAST();