From 7be23896c218806061a37ebf5970011e9c2d6635 Mon Sep 17 00:00:00 2001 From: Matej Ferencevic Date: Fri, 22 Feb 2019 13:20:54 +0100 Subject: [PATCH] Make auth library case insensitive Reviewers: teon.banek Reviewed By: teon.banek Subscribers: pullbot Differential Revision: https://phabricator.memgraph.io/D1881 --- src/auth/auth.cpp | 27 +++- src/auth/models.cpp | 11 +- tests/integration/auth/runner.py | 18 +-- tests/unit/auth.cpp | 220 +++++++++++++++++++++++++++++++ 4 files changed, 256 insertions(+), 20 deletions(-) diff --git a/src/auth/auth.cpp b/src/auth/auth.cpp index 15f50da29..e492daa4a 100644 --- a/src/auth/auth.cpp +++ b/src/auth/auth.cpp @@ -1,6 +1,7 @@ #include "auth/auth.hpp" #include "auth/exceptions.hpp" +#include "utils/string.hpp" namespace auth { @@ -36,7 +37,9 @@ std::experimental::optional Auth::Authenticate( return user; } -std::experimental::optional Auth::GetUser(const std::string &username) { +std::experimental::optional Auth::GetUser( + const std::string &username_orig) { + auto username = utils::ToLowerCase(username_orig); auto existing_user = storage_.Get(kUserPrefix + username); if (!existing_user) return std::experimental::nullopt; @@ -88,7 +91,8 @@ std::experimental::optional Auth::AddUser( return new_user; } -bool Auth::RemoveUser(const std::string &username) { +bool Auth::RemoveUser(const std::string &username_orig) { + auto username = utils::ToLowerCase(username_orig); if (!storage_.Get(kUserPrefix + username)) return false; std::vector keys( {kLinkPrefix + username, kUserPrefix + username}); @@ -102,7 +106,9 @@ std::vector Auth::AllUsers() { std::vector ret; for (auto it = storage_.begin(kUserPrefix); it != storage_.end(kUserPrefix); ++it) { - auto user = GetUser(it->first.substr(kUserPrefix.size())); + auto username = it->first.substr(kUserPrefix.size()); + if (username != utils::ToLowerCase(username)) continue; + auto user = GetUser(username); if (user) { ret.push_back(*user); } @@ -114,7 +120,9 @@ bool Auth::HasUsers() { return storage_.begin(kUserPrefix) != storage_.end(kUserPrefix); } -std::experimental::optional Auth::GetRole(const std::string &rolename) { +std::experimental::optional Auth::GetRole( + const std::string &rolename_orig) { + auto rolename = utils::ToLowerCase(rolename_orig); auto existing_role = storage_.Get(kRolePrefix + rolename); if (!existing_role) return std::experimental::nullopt; @@ -144,12 +152,13 @@ std::experimental::optional Auth::AddRole(const std::string &rolename) { return new_role; } -bool Auth::RemoveRole(const std::string &rolename) { +bool Auth::RemoveRole(const std::string &rolename_orig) { + auto rolename = utils::ToLowerCase(rolename_orig); if (!storage_.Get(kRolePrefix + rolename)) return false; std::vector keys; for (auto it = storage_.begin(kLinkPrefix); it != storage_.end(kLinkPrefix); ++it) { - if (it->second == rolename) { + if (utils::ToLowerCase(it->second) == rolename) { keys.push_back(it->first); } } @@ -165,6 +174,7 @@ std::vector Auth::AllRoles() { for (auto it = storage_.begin(kRolePrefix); it != storage_.end(kRolePrefix); ++it) { auto rolename = it->first.substr(kRolePrefix.size()); + if (rolename != utils::ToLowerCase(rolename)) continue; auto role = GetRole(rolename); if (role) { ret.push_back(*role); @@ -175,11 +185,14 @@ std::vector Auth::AllRoles() { return ret; } -std::vector Auth::AllUsersForRole(const std::string &rolename) { +std::vector Auth::AllUsersForRole(const std::string &rolename_orig) { + auto rolename = utils::ToLowerCase(rolename_orig); std::vector ret; for (auto it = storage_.begin(kLinkPrefix); it != storage_.end(kLinkPrefix); ++it) { auto username = it->first.substr(kLinkPrefix.size()); + if (username != utils::ToLowerCase(username)) continue; + if (it->second != utils::ToLowerCase(it->second)) continue; if (it->second == rolename) { auto user = GetUser(username); if (user) { diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 7810e1b4f..268b5d25c 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -7,6 +7,7 @@ #include "auth/crypto.hpp" #include "auth/exceptions.hpp" #include "utils/cast.hpp" +#include "utils/string.hpp" DEFINE_bool(auth_password_permit_null, true, "Set to false to disable null passwords."); @@ -140,10 +141,11 @@ bool operator!=(const Permissions &first, const Permissions &second) { return !(first == second); } -Role::Role(const std::string &rolename) : rolename_(rolename) {} +Role::Role(const std::string &rolename) + : rolename_(utils::ToLowerCase(rolename)) {} Role::Role(const std::string &rolename, const Permissions &permissions) - : rolename_(rolename), permissions_(permissions) {} + : rolename_(utils::ToLowerCase(rolename)), permissions_(permissions) {} const std::string &Role::rolename() const { return rolename_; } const Permissions &Role::permissions() const { return permissions_; } @@ -172,11 +174,12 @@ bool operator==(const Role &first, const Role &second) { first.permissions_ == second.permissions_; } -User::User(const std::string &username) : username_(username) {} +User::User(const std::string &username) + : username_(utils::ToLowerCase(username)) {} User::User(const std::string &username, const std::string &password_hash, const Permissions &permissions) - : username_(username), + : username_(utils::ToLowerCase(username)), password_hash_(password_hash), permissions_(permissions) {} diff --git a/tests/integration/auth/runner.py b/tests/integration/auth/runner.py index 1e6e0a63c..6a22b60bc 100755 --- a/tests/integration/auth/runner.py +++ b/tests/integration/auth/runner.py @@ -251,9 +251,9 @@ def execute_test(memgraph_binary, tester_binary, checker_binary): # Prepare all users execute_admin_queries([ - "CREATE USER admin IDENTIFIED BY 'admin'", - "GRANT ALL PRIVILEGES TO admin", - "CREATE USER user IDENTIFIED BY 'user'" + "CREATE USER ADmin IDENTIFIED BY 'admin'", + "GRANT ALL PRIVILEGES TO admIN", + "CREATE USER usEr IDENTIFIED BY 'user'" ]) # Find all existing permissions @@ -268,10 +268,10 @@ def execute_test(memgraph_binary, tester_binary, checker_binary): user_perms = get_permissions(permissions, mask) print("\033[1;34m~~ Checking queries with privileges: ", ", ".join(user_perms), " ~~\033[0m") - admin_queries = ["REVOKE ALL PRIVILEGES FROM user"] + admin_queries = ["REVOKE ALL PRIVILEGES FROM uSer"] if len(user_perms) > 0: admin_queries.append( - "GRANT {} TO user".format(", ".join(user_perms))) + "GRANT {} TO User".format(", ".join(user_perms))) execute_admin_queries(admin_queries) authorized, unauthorized = [], [] for query, query_perms in QUERIES: @@ -288,8 +288,8 @@ def execute_test(memgraph_binary, tester_binary, checker_binary): # Run the user/role permissions test print("\033[1;36m~~ Starting permissions test ~~\033[0m") execute_admin_queries([ - "CREATE ROLE role", - "REVOKE ALL PRIVILEGES FROM user", + "CREATE ROLE roLe", + "REVOKE ALL PRIVILEGES FROM uSeR", ]) execute_checker(checker_binary, []) for user_perm in ["GRANT", "DENY", "REVOKE"]: @@ -299,14 +299,14 @@ def execute_test(memgraph_binary, tester_binary, checker_binary): user_perm, ", role ", role_perm, "user mapped to role:", mapped, " ~~\033[0m") if mapped: - execute_admin_queries(["SET ROLE FOR user TO role"]) + execute_admin_queries(["SET ROLE FOR USER TO roLE"]) else: execute_admin_queries(["CLEAR ROLE FOR user"]) user_prep = "FROM" if user_perm == "REVOKE" else "TO" role_prep = "FROM" if role_perm == "REVOKE" else "TO" execute_admin_queries([ "{} MATCH {} user".format(user_perm, user_prep), - "{} MATCH {} role".format(role_perm, role_prep) + "{} MATCH {} rOLe".format(role_perm, role_prep) ]) expected = [] perms = [user_perm, role_perm] if mapped else [user_perm] diff --git a/tests/unit/auth.cpp b/tests/unit/auth.cpp index dd06c6363..5e5a4b428 100644 --- a/tests/unit/auth.cpp +++ b/tests/unit/auth.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -459,6 +460,225 @@ TEST_F(AuthWithStorage, UserRoleUniqueName) { ASSERT_FALSE(auth.AddUser("role")); } +TEST(AuthWithoutStorage, CaseInsensitivity) { + { + auto user1 = User("test"); + auto user2 = User("Test"); + ASSERT_EQ(user1, user2); + ASSERT_EQ(user1.username(), user2.username()); + ASSERT_EQ(user1.username(), "test"); + ASSERT_EQ(user2.username(), "test"); + } + { + auto perms = Permissions(); + auto user1 = User("test", "pw", perms); + auto user2 = User("Test", "pw", perms); + ASSERT_EQ(user1, user2); + ASSERT_EQ(user1.username(), user2.username()); + ASSERT_EQ(user1.username(), "test"); + ASSERT_EQ(user2.username(), "test"); + } + { + auto role1 = Role("role"); + auto role2 = Role("Role"); + ASSERT_EQ(role1, role2); + ASSERT_EQ(role1.rolename(), role2.rolename()); + ASSERT_EQ(role1.rolename(), "role"); + ASSERT_EQ(role2.rolename(), "role"); + } + { + auto perms = Permissions(); + auto role1 = Role("role", perms); + auto role2 = Role("Role", perms); + ASSERT_EQ(role1, role2); + ASSERT_EQ(role1.rolename(), role2.rolename()); + ASSERT_EQ(role1.rolename(), "role"); + ASSERT_EQ(role2.rolename(), "role"); + } +} + +TEST_F(AuthWithStorage, CaseInsensitivity) { + // AddUser + { + auto user = auth.AddUser("Alice", "alice"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + ASSERT_FALSE(auth.AddUser("alice")); + ASSERT_FALSE(auth.AddUser("alicE")); + } + { + auto user = auth.AddUser("BoB", "bob"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "bob"); + ASSERT_FALSE(auth.AddUser("bob")); + ASSERT_FALSE(auth.AddUser("bOb")); + } + + // Authenticate + { + auto user = auth.Authenticate("alice", "alice"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + } + { + auto user = auth.Authenticate("alICe", "alice"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + } + + // GetUser + { + auto user = auth.GetUser("alice"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + } + { + auto user = auth.GetUser("aLicE"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + } + ASSERT_FALSE(auth.GetUser("carol")); + + // RemoveUser + { + auto user = auth.AddUser("caRol", "carol"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "carol"); + ASSERT_TRUE(auth.RemoveUser("cAROl")); + ASSERT_FALSE(auth.RemoveUser("carol")); + ASSERT_FALSE(auth.GetUser("CAROL")); + } + + // AllUsers + { + auto users = auth.AllUsers(); + ASSERT_EQ(users.size(), 2); + std::sort(users.begin(), users.end(), [](const auto &a, const auto &b) { + return a.username() < b.username(); + }); + ASSERT_EQ(users[0].username(), "alice"); + ASSERT_EQ(users[1].username(), "bob"); + } + + // AddRole + { + auto role = auth.AddRole("Moderator"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + ASSERT_FALSE(auth.AddRole("moderator")); + ASSERT_FALSE(auth.AddRole("MODERATOR")); + } + { + auto role = auth.AddRole("adMIN"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "admin"); + ASSERT_FALSE(auth.AddRole("Admin")); + ASSERT_FALSE(auth.AddRole("ADMIn")); + } + ASSERT_FALSE(auth.AddRole("ALICE")); + ASSERT_FALSE(auth.AddUser("ModeRAtor")); + + // GetRole + { + auto role = auth.GetRole("moderator"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + } + { + auto role = auth.GetRole("MoDERATOR"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + } + ASSERT_FALSE(auth.GetRole("root")); + + // RemoveRole + { + auto role = auth.AddRole("RooT"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "root"); + ASSERT_TRUE(auth.RemoveRole("rOOt")); + ASSERT_FALSE(auth.RemoveRole("RoOt")); + ASSERT_FALSE(auth.GetRole("RoOt")); + } + + // AllRoles + { + auto roles = auth.AllRoles(); + ASSERT_EQ(roles.size(), 2); + std::sort(roles.begin(), roles.end(), [](const auto &a, const auto &b) { + return a.rolename() < b.rolename(); + }); + ASSERT_EQ(roles[0].rolename(), "admin"); + ASSERT_EQ(roles[1].rolename(), "moderator"); + } + + // SaveRole + { + auto role = auth.GetRole("MODErator"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + role->permissions().Grant(auth::Permission::MATCH); + auth.SaveRole(*role); + } + { + auto role = auth.GetRole("modeRATOR"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + ASSERT_EQ(role->permissions().Has(auth::Permission::MATCH), + auth::PermissionLevel::GRANT); + } + + // SaveUser + { + auto user = auth.GetUser("aLice"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + auto role = auth.GetRole("moderAtor"); + ASSERT_TRUE(role); + ASSERT_EQ(role->rolename(), "moderator"); + user->SetRole(*role); + auth.SaveUser(*user); + } + { + auto user = auth.GetUser("aLIce"); + ASSERT_TRUE(user); + ASSERT_EQ(user->username(), "alice"); + ASSERT_TRUE(user->role()); + ASSERT_EQ(user->role()->rolename(), "moderator"); + } + + // AllUsersForRole + { + auto carol = auth.AddUser("caROl"); + ASSERT_TRUE(carol); + ASSERT_EQ(carol->username(), "carol"); + auto dave = auth.AddUser("daVe"); + ASSERT_TRUE(dave); + ASSERT_EQ(dave->username(), "dave"); + auto admin = auth.GetRole("aDMin"); + ASSERT_TRUE(admin); + ASSERT_EQ(admin->rolename(), "admin"); + carol->SetRole(*admin); + auth.SaveUser(*carol); + dave->SetRole(*admin); + auth.SaveUser(*dave); + } + { + auto users = auth.AllUsersForRole("modeRAtoR"); + ASSERT_EQ(users.size(), 1); + ASSERT_EQ(users[0].username(), "alice"); + } + { + auto users = auth.AllUsersForRole("AdmiN"); + ASSERT_EQ(users.size(), 2); + std::sort(users.begin(), users.end(), [](const auto &a, const auto &b) { + return a.username() < b.username(); + }); + ASSERT_EQ(users[0].username(), "carol"); + ASSERT_EQ(users[1].username(), "dave"); + } +} + TEST(AuthWithoutStorage, Crypto) { auto hash = EncryptPassword("hello"); ASSERT_TRUE(VerifyPassword("hello", hash));