From bfcb6e8d48f7379a5b59a67d981b4298783172b9 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Tue, 9 Aug 2022 07:38:51 +0200 Subject: [PATCH 01/12] Added enum for more granular access control; Expanded functionality of fine grained access checker; Propagated changes to Edit, Deny and Revoke permissions methods in interpreter --- src/auth/models.cpp | 64 +++++++++++++++++++++++++++++---------- src/auth/models.hpp | 42 ++++++++++++++++--------- src/memgraph.cpp | 59 ++++++++++++++++++++++-------------- src/query/interpreter.cpp | 3 +- 4 files changed, 114 insertions(+), 54 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 16ba771cd8..9ec5272d7b 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -16,6 +16,7 @@ #include "auth/exceptions.hpp" #include "utils/cast.hpp" #include "utils/license.hpp" +#include "utils/logging.hpp" #include "utils/settings.hpp" #include "utils/string.hpp" @@ -184,27 +185,41 @@ bool operator==(const Permissions &first, const Permissions &second) { bool operator!=(const Permissions &first, const Permissions &second) { return !(first == second); } const std::string ASTERISK = "*"; +const std::pair DENY_ALL = {ASTERISK, memgraph::auth::LabelPermission::READ}; +const std::pair GRANT_ALL = {ASTERISK, memgraph::auth::LabelPermission::CREATE_DELETE}; -FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_set &grants, - const std::unordered_set &denies) +FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_map &grants, + const std::unordered_map &denies) : grants_(grants), denies_(denies) {} -PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission) const { - if ((denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) || denies_.find(permission) != denies_.end()) { +PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, LabelPermission label_permission) { + if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { return PermissionLevel::DENY; } - if ((grants_.size() == 1 && grants_.find(ASTERISK) != grants_.end()) || grants_.find(permission) != denies_.end()) { + if ((grants_.size() == 1 && grants_.find(ASTERISK) != grants_.end())) { return PermissionLevel::GRANT; } - return PermissionLevel::NEUTRAL; + auto grants_permission = PermissionLevel::DENY; + auto denies_permission = PermissionLevel::GRANT; + + if (denies_.find(permission) != denies_.end() && denies_[permission] & label_permission) { + denies_permission = PermissionLevel::DENY; + } + + if (grants_.find(permission) != grants_.end() && grants_[permission] & label_permission) { + grants_permission = PermissionLevel::GRANT; + } + + return denies_permission >= grants_permission ? denies_permission : grants_permission; } -void FineGrainedAccessPermissions::Grant(const std::string &permission) { +void FineGrainedAccessPermissions::Grant(const std::string &permission, LabelPermission label_permission) { if (permission == ASTERISK) { + MG_ASSERT(label_permission == LabelPermission::CREATE_DELETE); grants_.clear(); - grants_.insert(permission); + grants_[permission] = LabelPermission::CREATE_DELETE | LabelPermission::EDIT | LabelPermission::READ; return; } @@ -220,7 +235,15 @@ void FineGrainedAccessPermissions::Grant(const std::string &permission) { } if (grants_.find(permission) == grants_.end()) { - grants_.insert(permission); + ushort perm = 0; + auto u_label_perm = static_cast(label_permission); + + while (u_label_perm != 0) { + perm |= u_label_perm; + u_label_perm >>= (ushort)1; + } + + grants_[permission] = perm; } } @@ -244,10 +267,11 @@ void FineGrainedAccessPermissions::Revoke(const std::string &permission) { } } -void FineGrainedAccessPermissions::Deny(const std::string &permission) { +void FineGrainedAccessPermissions::Deny(const std::string &permission, LabelPermission label_permission) { if (permission == ASTERISK) { + MG_ASSERT(label_permission == LabelPermission::READ); denies_.clear(); - denies_.insert(permission); + denies_[permission] = LabelPermission::CREATE_DELETE | LabelPermission::EDIT | LabelPermission::READ; return; } @@ -263,7 +287,15 @@ void FineGrainedAccessPermissions::Deny(const std::string &permission) { } if (denies_.find(permission) == denies_.end()) { - denies_.insert(permission); + ushort perm = 0; + auto u_label_perm = static_cast(label_permission); + + while (u_label_perm <= (ushort)4) { + perm |= u_label_perm; + u_label_perm <<= (ushort)1; + } + + denies_[permission] = perm; } } @@ -282,8 +314,8 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo return FineGrainedAccessPermissions(data["grants"], data["denies"]); } -const std::unordered_set &FineGrainedAccessPermissions::grants() const { return grants_; } -const std::unordered_set &FineGrainedAccessPermissions::denies() const { return denies_; } +const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } +const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { return first.grants() == second.grants() && first.denies() == second.denies(); @@ -393,14 +425,14 @@ Permissions User::GetPermissions() const { FineGrainedAccessPermissions User::GetFineGrainedAccessPermissions() const { if (role_) { - std::unordered_set resultGrants; + std::unordered_map resultGrants; std::set_union(fine_grained_access_permissions_.grants().begin(), fine_grained_access_permissions_.grants().end(), role_->fine_grained_access_permissions().grants().begin(), role_->fine_grained_access_permissions().grants().end(), std::inserter(resultGrants, resultGrants.begin())); - std::unordered_set resultDenies; + std::unordered_map resultDenies; std::set_union(fine_grained_access_permissions_.denies().begin(), fine_grained_access_permissions_.denies().end(), role_->fine_grained_access_permissions().denies().begin(), diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 9761832ba4..9385ebc2f7 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -10,7 +10,7 @@ #include #include -#include +#include #include @@ -43,15 +43,27 @@ enum class Permission : uint64_t { }; // clang-format on +// clang-format off +enum class LabelPermission : ushort { + READ = 1, + EDIT = 1U << 1U, + CREATE_DELETE = 1U << 2U +}; +// clang-format on + +inline ushort operator|(LabelPermission a, LabelPermission b) { + return static_cast(a) | static_cast(b); +} + +inline ushort operator|(ushort a, LabelPermission b) { return a | static_cast(b); } + +inline bool operator&(ushort a, LabelPermission b) { return (a & static_cast(b)) != (ushort)0; } + // Function that converts a permission to its string representation. std::string PermissionToString(Permission permission); // Class that indicates what permission level the user/role has. -enum class PermissionLevel { - GRANT, - NEUTRAL, - DENY, -}; +enum class PermissionLevel : short { DENY, GRANT, NEUTRAL }; // Function that converts a permission level to its string representation. std::string PermissionLevelToString(PermissionLevel level); @@ -91,28 +103,28 @@ bool operator!=(const Permissions &first, const Permissions &second); class FineGrainedAccessPermissions final { public: - explicit FineGrainedAccessPermissions(const std::unordered_set &grants = {}, - const std::unordered_set &denies = {}); + explicit FineGrainedAccessPermissions(const std::unordered_map &grants = {}, + const std::unordered_map &denies = {}); - PermissionLevel Has(const std::string &permission) const; + PermissionLevel Has(const std::string &permission, LabelPermission label_permission); - void Grant(const std::string &permission); + void Grant(const std::string &permission, LabelPermission label_permission); void Revoke(const std::string &permission); - void Deny(const std::string &permission); + void Deny(const std::string &permission, LabelPermission label_permission); nlohmann::json Serialize() const; /// @throw AuthException if unable to deserialize. static FineGrainedAccessPermissions Deserialize(const nlohmann::json &data); - const std::unordered_set &grants() const; - const std::unordered_set &denies() const; + const std::unordered_map &grants() const; + const std::unordered_map &denies() const; private: - std::unordered_set grants_{}; - std::unordered_set denies_{}; + std::unordered_map grants_{}; + std::unordered_map denies_{}; }; bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second); diff --git a/src/memgraph.cpp b/src/memgraph.cpp index dc72d07860..056a7521a1 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -32,6 +32,7 @@ #include #include +#include "auth/models.hpp" #include "communication/bolt/v1/constants.hpp" #include "communication/websocket/auth.hpp" #include "communication/websocket/server.hpp" @@ -772,41 +773,55 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { void GrantPrivilege(const std::string &user_or_role, const std::vector &privileges, const std::vector &labels) override { - EditPermissions(user_or_role, privileges, labels, [](auto *permissions, const auto &permission) { - // TODO (mferencevic): should we first check that the - // privilege is granted/denied/revoked before - // unconditionally granting/denying/revoking it? - permissions->Grant(permission); - }); + EditPermissions( + user_or_role, privileges, labels, + [](auto *permissions, const auto &permission) { + // TODO (mferencevic): should we first check that the + // privilege is granted/denied/revoked before + // unconditionally granting/denying/revoking it? + permissions->Grant(permission); + }, + [](auto *label_permissions, const auto &label) { + label_permissions->Grant(label, memgraph::auth::LabelPermission::READ); + }); } void DenyPrivilege(const std::string &user_or_role, const std::vector &privileges, const std::vector &labels) override { - EditPermissions(user_or_role, privileges, labels, [](auto *permissions, const auto &permission) { - // TODO (mferencevic): should we first check that the - // privilege is granted/denied/revoked before - // unconditionally granting/denying/revoking it? - permissions->Deny(permission); - }); + EditPermissions( + user_or_role, privileges, labels, + [](auto *permissions, const auto &permission) { + // TODO (mferencevic): should we first check that the + // privilege is granted/denied/revoked before + // unconditionally granting/denying/revoking it? + permissions->Deny(permission); + }, + [](auto *label_permissions, const auto &label) { + label_permissions->Deny(label, memgraph::auth::LabelPermission::READ); + }); } void RevokePrivilege(const std::string &user_or_role, const std::vector &privileges, const std::vector &labels) override { - EditPermissions(user_or_role, privileges, labels, [](auto *permissions, const auto &permission) { - // TODO (mferencevic): should we first check that the - // privilege is granted/denied/revoked before - // unconditionally granting/denying/revoking it? - permissions->Revoke(permission); - }); + EditPermissions( + user_or_role, privileges, labels, + [](auto *permissions, const auto &permission) { + // TODO (mferencevic): should we first check that the + // privilege is granted/denied/revoked before + // unconditionally granting/denying/revoking it? + permissions->Revoke(permission); + }, + [](auto *label_permissions, const auto &label) { label_permissions->Revoke(label); }); } private: - template + template void EditPermissions(const std::string &user_or_role, const std::vector &privileges, - const std::vector &labels, const TEditFun &edit_fun) { + const std::vector &labels, const TEditFun &edit_fun, + const TEditLabelPermisionsFun &edit_label_permisions_fun) { if (!std::regex_match(user_or_role, name_regex_)) { throw memgraph::query::QueryRuntimeException("Invalid user or role name."); } @@ -827,7 +842,7 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { edit_fun(&user->permissions(), permission); } for (const auto &label : labels) { - edit_fun(&user->fine_grained_access_permissions(), label); + edit_label_permisions_fun(&user->fine_grained_access_permissions(), label); } locked_auth->SaveUser(*user); } else { @@ -835,7 +850,7 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { edit_fun(&role->permissions(), permission); } for (const auto &label : labels) { - edit_fun(&user->fine_grained_access_permissions(), label); + edit_label_permisions_fun(&user->fine_grained_access_permissions(), label); } locked_auth->SaveRole(*role); diff --git a/src/query/interpreter.cpp b/src/query/interpreter.cpp index f892c3c8de..73b5aece0f 100644 --- a/src/query/interpreter.cpp +++ b/src/query/interpreter.cpp @@ -270,7 +270,8 @@ class FineGrainedAccessChecker final : public memgraph::query::FineGrainedAccess auto labelPermissions = user_->GetFineGrainedAccessPermissions(); return std::any_of(labels.begin(), labels.end(), [&labelPermissions, dba](const auto label) { - return labelPermissions.Has(dba->LabelToName(label)) == memgraph::auth::PermissionLevel::GRANT; + return labelPermissions.Has(dba->LabelToName(label), memgraph::auth::LabelPermission::READ) == + memgraph::auth::PermissionLevel::GRANT; }); } From fdf141dac682ca5a4de6b793ad9168f8e374933f Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Wed, 10 Aug 2022 17:25:24 +0200 Subject: [PATCH 02/12] Introduced Merge method for merging two colle with permissions --- src/auth/models.cpp | 66 ++++++++++++++++++++++++--------------------- src/auth/models.hpp | 22 +++++++-------- 2 files changed, 47 insertions(+), 41 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 9ec5272d7b..721c5aa6b4 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -8,6 +8,7 @@ #include "auth/models.hpp" +#include #include #include @@ -31,6 +32,10 @@ DEFINE_string(auth_password_strength_regex, default_password_regex.data(), namespace memgraph::auth { namespace { +const std::string ASTERISK = "*"; +const uint64_t LabelPermissionMax = static_cast(memgraph::auth::LabelPermission::CREATE_DELETE); +} // namespace +namespace { // Constant list of all available permissions. const std::vector kPermissionsAll = { Permission::MATCH, Permission::CREATE, Permission::MERGE, Permission::DELETE, @@ -99,6 +104,19 @@ std::string PermissionLevelToString(PermissionLevel level) { } } +std::unordered_map Merge(const std::unordered_map &first, + const std::unordered_map &second) { + std::unordered_map result{first}; + + for (const auto &it : second) { + if (result.find(it.first) != result.end()) { + result[it.first] |= it.second; + } + } + + return result; +} + Permissions::Permissions(uint64_t grants, uint64_t denies) { // The deny bitmask has higher priority than the grant bitmask. denies_ = denies; @@ -184,12 +202,8 @@ bool operator==(const Permissions &first, const Permissions &second) { bool operator!=(const Permissions &first, const Permissions &second) { return !(first == second); } -const std::string ASTERISK = "*"; -const std::pair DENY_ALL = {ASTERISK, memgraph::auth::LabelPermission::READ}; -const std::pair GRANT_ALL = {ASTERISK, memgraph::auth::LabelPermission::CREATE_DELETE}; - -FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_map &grants, - const std::unordered_map &denies) +FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_map &grants, + const std::unordered_map &denies) : grants_(grants), denies_(denies) {} PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, LabelPermission label_permission) { @@ -234,13 +248,15 @@ void FineGrainedAccessPermissions::Grant(const std::string &permission, LabelPer grants_.erase(ASTERISK); } + uint64_t shift = 1; + if (grants_.find(permission) == grants_.end()) { - ushort perm = 0; - auto u_label_perm = static_cast(label_permission); + uint64_t perm = 0; + auto u_label_perm = static_cast(label_permission); while (u_label_perm != 0) { perm |= u_label_perm; - u_label_perm >>= (ushort)1; + u_label_perm >>= shift; } grants_[permission] = perm; @@ -286,13 +302,15 @@ void FineGrainedAccessPermissions::Deny(const std::string &permission, LabelPerm denies_.erase(ASTERISK); } + uint64_t shift = 1; + if (denies_.find(permission) == denies_.end()) { - ushort perm = 0; - auto u_label_perm = static_cast(label_permission); + uint64_t perm = 0; + auto u_label_perm = static_cast(label_permission); - while (u_label_perm <= (ushort)4) { + while (u_label_perm <= LabelPermissionMax) { perm |= u_label_perm; - u_label_perm <<= (ushort)1; + u_label_perm <<= shift; } denies_[permission] = perm; @@ -314,8 +332,8 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo return FineGrainedAccessPermissions(data["grants"], data["denies"]); } -const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } -const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } +const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } +const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { return first.grants() == second.grants() && first.denies() == second.denies(); @@ -425,21 +443,9 @@ Permissions User::GetPermissions() const { FineGrainedAccessPermissions User::GetFineGrainedAccessPermissions() const { if (role_) { - std::unordered_map resultGrants; - - std::set_union(fine_grained_access_permissions_.grants().begin(), fine_grained_access_permissions_.grants().end(), - role_->fine_grained_access_permissions().grants().begin(), - role_->fine_grained_access_permissions().grants().end(), - std::inserter(resultGrants, resultGrants.begin())); - - std::unordered_map resultDenies; - - std::set_union(fine_grained_access_permissions_.denies().begin(), fine_grained_access_permissions_.denies().end(), - role_->fine_grained_access_permissions().denies().begin(), - role_->fine_grained_access_permissions().denies().end(), - std::inserter(resultDenies, resultDenies.begin())); - - return FineGrainedAccessPermissions(resultGrants, resultDenies); + return FineGrainedAccessPermissions( + Merge(fine_grained_access_permissions_.grants(), role_->fine_grained_access_permissions().grants()), + Merge(fine_grained_access_permissions_.denies(), role_->fine_grained_access_permissions().denies())); } return fine_grained_access_permissions_; } diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 9385ebc2f7..40c19a61db 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -44,20 +44,20 @@ enum class Permission : uint64_t { // clang-format on // clang-format off -enum class LabelPermission : ushort { +enum class LabelPermission : uint64_t { READ = 1, EDIT = 1U << 1U, CREATE_DELETE = 1U << 2U }; // clang-format on -inline ushort operator|(LabelPermission a, LabelPermission b) { - return static_cast(a) | static_cast(b); +inline uint64_t operator|(LabelPermission a, LabelPermission b) { + return static_cast(a) | static_cast(b); } -inline ushort operator|(ushort a, LabelPermission b) { return a | static_cast(b); } +inline uint64_t operator|(uint64_t a, LabelPermission b) { return a | static_cast(b); } -inline bool operator&(ushort a, LabelPermission b) { return (a & static_cast(b)) != (ushort)0; } +inline bool operator&(uint64_t a, LabelPermission b) { return (a & static_cast(b)) != 0; } // Function that converts a permission to its string representation. std::string PermissionToString(Permission permission); @@ -103,8 +103,8 @@ bool operator!=(const Permissions &first, const Permissions &second); class FineGrainedAccessPermissions final { public: - explicit FineGrainedAccessPermissions(const std::unordered_map &grants = {}, - const std::unordered_map &denies = {}); + explicit FineGrainedAccessPermissions(const std::unordered_map &grants = {}, + const std::unordered_map &denies = {}); PermissionLevel Has(const std::string &permission, LabelPermission label_permission); @@ -119,12 +119,12 @@ class FineGrainedAccessPermissions final { /// @throw AuthException if unable to deserialize. static FineGrainedAccessPermissions Deserialize(const nlohmann::json &data); - const std::unordered_map &grants() const; - const std::unordered_map &denies() const; + const std::unordered_map &grants() const; + const std::unordered_map &denies() const; private: - std::unordered_map grants_{}; - std::unordered_map denies_{}; + std::unordered_map grants_{}; + std::unordered_map denies_{}; }; bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second); From 664d6e7ea3e55ba86ac93898c0fad07e6fd8a923 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Thu, 11 Aug 2022 00:15:43 +0200 Subject: [PATCH 03/12] e2e tests implementation started --- tests/e2e/write_procedures/CMakeLists.txt | 1 + tests/e2e/write_procedures/conftest.py | 8 +++++ tests/e2e/write_procedures/lba_procedures.py | 31 +++++++++++++++++++ tests/e2e/write_procedures/procedures/read.py | 10 ++++-- tests/e2e/write_procedures/workloads.yaml | 19 ++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/write_procedures/lba_procedures.py diff --git a/tests/e2e/write_procedures/CMakeLists.txt b/tests/e2e/write_procedures/CMakeLists.txt index 72b640271a..9aeaad664c 100644 --- a/tests/e2e/write_procedures/CMakeLists.txt +++ b/tests/e2e/write_procedures/CMakeLists.txt @@ -5,5 +5,6 @@ endfunction() copy_write_procedures_e2e_python_files(common.py) copy_write_procedures_e2e_python_files(conftest.py) copy_write_procedures_e2e_python_files(simple_write.py) +copy_write_procedures_e2e_python_files(lba_procedures.py) add_subdirectory(procedures) diff --git a/tests/e2e/write_procedures/conftest.py b/tests/e2e/write_procedures/conftest.py index b559b17413..1d407bbe5f 100644 --- a/tests/e2e/write_procedures/conftest.py +++ b/tests/e2e/write_procedures/conftest.py @@ -20,3 +20,11 @@ def connection(): yield connection cursor = connection.cursor() execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n") + + +@pytest.fixture(autouse=True) +def connection_with_username(): + connection = connect(username="Boris", password="") + yield connection + cursor = connection.cursor() + execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n") diff --git a/tests/e2e/write_procedures/lba_procedures.py b/tests/e2e/write_procedures/lba_procedures.py new file mode 100644 index 0000000000..b87aafb71a --- /dev/null +++ b/tests/e2e/write_procedures/lba_procedures.py @@ -0,0 +1,31 @@ +# Copyright 2022 Memgraph Ltd. +# +# Use of this software is governed by the Business Source License +# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source +# License, and you may not use this file except in compliance with the Business Source License. +# +# As of the Change Date specified in that file, in accordance with +# the Business Source License, use of this software will be governed +# by the Apache License, Version 2.0, included in the file +# licenses/APL.txt. + +import sys +import pytest + + +def test_1(connection_with_username): + assert True + + +# def test_2(connection): +# connection.cursor() +# assert False + + +# def test_3(connection): +# connection.cursor() +# assert False + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__, "-rA"])) diff --git a/tests/e2e/write_procedures/procedures/read.py b/tests/e2e/write_procedures/procedures/read.py index 3f791a37a1..16f96908ea 100644 --- a/tests/e2e/write_procedures/procedures/read.py +++ b/tests/e2e/write_procedures/procedures/read.py @@ -11,13 +11,19 @@ import mgp +# import typing + @mgp.read_proc -def underlying_graph_is_mutable(ctx: mgp.ProcCtx, - object: mgp.Any) -> mgp.Record(mutable=bool): +def underlying_graph_is_mutable(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record(mutable=bool): return mgp.Record(mutable=object.underlying_graph_is_mutable()) @mgp.read_proc def graph_is_mutable(ctx: mgp.ProcCtx) -> mgp.Record(mutable=bool): return mgp.Record(mutable=ctx.graph.is_mutable()) + + +@mgp.read_proc +def number_of_visible_nodes(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record(mutable=bool): + return mgp.Record(mutable=True) diff --git a/tests/e2e/write_procedures/workloads.yaml b/tests/e2e/write_procedures/workloads.yaml index a6a57231f7..d68ca4b4f9 100644 --- a/tests/e2e/write_procedures/workloads.yaml +++ b/tests/e2e/write_procedures/workloads.yaml @@ -6,9 +6,28 @@ template_cluster: &template_cluster setup_queries: [] validation_queries: [] +template_cluster_lba: &template_cluster_lba + cluster: + main: + args: ["--bolt-port", "7687", "--log-level=TRACE"] + log_file: "lba-e2e.log" + setup_queries: [ + "CREATE (:Label1 {id: 1});", + "CREATE (:Label1 {id: 2});", + "CREATE (:Label2 {id: 1});", + "CREATE USER Boris;", + "GRANT LABELS :Label1 TO Boris;" + ] + validation_queries: [] + workloads: - name: "Write procedures simple" binary: "tests/e2e/pytest_runner.sh" proc: "tests/e2e/write_procedures/procedures/" args: ["write_procedures/simple_write.py"] <<: *template_cluster + - name: "Label-based auth" + binary: "tests/e2e/pytest_runner.sh" + proc: "tests/e2e/write_procedures/procedures/" + args: ["write_procedures/lba_procedures.py"] + <<: *template_cluster_lba From 71cde3ca6ead2bb73f4b1d30a9fa3b3ca7606179 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Thu, 11 Aug 2022 13:48:41 +0200 Subject: [PATCH 04/12] FineGrainedAccessChecker Grant and Deny methods reworked --- src/auth/models.cpp | 69 ++++++++++++++++++++++----------------------- src/auth/models.hpp | 3 ++ 2 files changed, 37 insertions(+), 35 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 721c5aa6b4..a112516b81 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -206,7 +206,8 @@ FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_ const std::unordered_map &denies) : grants_(grants), denies_(denies) {} -PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, LabelPermission label_permission) { +PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, + const LabelPermission label_permission) { if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { return PermissionLevel::DENY; } @@ -218,24 +219,20 @@ PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, auto grants_permission = PermissionLevel::DENY; auto denies_permission = PermissionLevel::GRANT; - if (denies_.find(permission) != denies_.end() && denies_[permission] & label_permission) { + if (denies_.find(permission) != denies_.end() && (denies_[permission] & label_permission)) { denies_permission = PermissionLevel::DENY; } - if (grants_.find(permission) != grants_.end() && grants_[permission] & label_permission) { + if (grants_.find(permission) != grants_.end() && (grants_[permission] & label_permission)) { grants_permission = PermissionLevel::GRANT; } return denies_permission >= grants_permission ? denies_permission : grants_permission; } -void FineGrainedAccessPermissions::Grant(const std::string &permission, LabelPermission label_permission) { +void FineGrainedAccessPermissions::Grant(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { - MG_ASSERT(label_permission == LabelPermission::CREATE_DELETE); grants_.clear(); - grants_[permission] = LabelPermission::CREATE_DELETE | LabelPermission::EDIT | LabelPermission::READ; - - return; } auto deniedPermissionIter = denies_.find(permission); @@ -248,18 +245,8 @@ void FineGrainedAccessPermissions::Grant(const std::string &permission, LabelPer grants_.erase(ASTERISK); } - uint64_t shift = 1; - if (grants_.find(permission) == grants_.end()) { - uint64_t perm = 0; - auto u_label_perm = static_cast(label_permission); - - while (u_label_perm != 0) { - perm |= u_label_perm; - u_label_perm >>= shift; - } - - grants_[permission] = perm; + grant_(permission, label_permission); } } @@ -283,13 +270,9 @@ void FineGrainedAccessPermissions::Revoke(const std::string &permission) { } } -void FineGrainedAccessPermissions::Deny(const std::string &permission, LabelPermission label_permission) { +void FineGrainedAccessPermissions::Deny(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { - MG_ASSERT(label_permission == LabelPermission::READ); denies_.clear(); - denies_[permission] = LabelPermission::CREATE_DELETE | LabelPermission::EDIT | LabelPermission::READ; - - return; } auto grantedPermissionIter = grants_.find(permission); @@ -302,18 +285,8 @@ void FineGrainedAccessPermissions::Deny(const std::string &permission, LabelPerm denies_.erase(ASTERISK); } - uint64_t shift = 1; - if (denies_.find(permission) == denies_.end()) { - uint64_t perm = 0; - auto u_label_perm = static_cast(label_permission); - - while (u_label_perm <= LabelPermissionMax) { - perm |= u_label_perm; - u_label_perm <<= shift; - } - - denies_[permission] = perm; + deny_(permission, label_permission); } } @@ -335,6 +308,32 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } +void FineGrainedAccessPermissions::grant_(const std::string &permission, LabelPermission label_permission) { + uint64_t shift = 1; + uint64_t perm = 0; + auto u_label_perm = static_cast(label_permission); + + while (u_label_perm != 0) { + perm |= u_label_perm; + u_label_perm >>= shift; + } + + grants_[permission] = perm; +} + +void FineGrainedAccessPermissions::deny_(const std::string &permission, LabelPermission label_permission) { + uint64_t shift = 1; + uint64_t perm = 0; + auto u_label_perm = static_cast(label_permission); + + while (u_label_perm <= LabelPermissionMax) { + perm |= u_label_perm; + u_label_perm <<= shift; + } + + denies_[permission] = perm; +} + bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { return first.grants() == second.grants() && first.denies() == second.denies(); } diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 40c19a61db..678098001f 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -125,6 +125,9 @@ class FineGrainedAccessPermissions final { private: std::unordered_map grants_{}; std::unordered_map denies_{}; + + void grant_(const std::string &permission, LabelPermission label_permission); + void deny_(const std::string &permission, LabelPermission label_permission); }; bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second); From 4c6c27247d9b683f221a20482943f1c762ef5eef Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Fri, 12 Aug 2022 09:53:25 +0200 Subject: [PATCH 05/12] removed faulty test addition --- tests/e2e/write_procedures/CMakeLists.txt | 1 - tests/e2e/write_procedures/conftest.py | 8 ----- tests/e2e/write_procedures/lba_procedures.py | 31 ------------------- tests/e2e/write_procedures/procedures/read.py | 7 ----- tests/e2e/write_procedures/workloads.yaml | 19 ------------ 5 files changed, 66 deletions(-) delete mode 100644 tests/e2e/write_procedures/lba_procedures.py diff --git a/tests/e2e/write_procedures/CMakeLists.txt b/tests/e2e/write_procedures/CMakeLists.txt index 9aeaad664c..72b640271a 100644 --- a/tests/e2e/write_procedures/CMakeLists.txt +++ b/tests/e2e/write_procedures/CMakeLists.txt @@ -5,6 +5,5 @@ endfunction() copy_write_procedures_e2e_python_files(common.py) copy_write_procedures_e2e_python_files(conftest.py) copy_write_procedures_e2e_python_files(simple_write.py) -copy_write_procedures_e2e_python_files(lba_procedures.py) add_subdirectory(procedures) diff --git a/tests/e2e/write_procedures/conftest.py b/tests/e2e/write_procedures/conftest.py index 1d407bbe5f..b559b17413 100644 --- a/tests/e2e/write_procedures/conftest.py +++ b/tests/e2e/write_procedures/conftest.py @@ -20,11 +20,3 @@ def connection(): yield connection cursor = connection.cursor() execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n") - - -@pytest.fixture(autouse=True) -def connection_with_username(): - connection = connect(username="Boris", password="") - yield connection - cursor = connection.cursor() - execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n") diff --git a/tests/e2e/write_procedures/lba_procedures.py b/tests/e2e/write_procedures/lba_procedures.py deleted file mode 100644 index b87aafb71a..0000000000 --- a/tests/e2e/write_procedures/lba_procedures.py +++ /dev/null @@ -1,31 +0,0 @@ -# Copyright 2022 Memgraph Ltd. -# -# Use of this software is governed by the Business Source License -# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source -# License, and you may not use this file except in compliance with the Business Source License. -# -# As of the Change Date specified in that file, in accordance with -# the Business Source License, use of this software will be governed -# by the Apache License, Version 2.0, included in the file -# licenses/APL.txt. - -import sys -import pytest - - -def test_1(connection_with_username): - assert True - - -# def test_2(connection): -# connection.cursor() -# assert False - - -# def test_3(connection): -# connection.cursor() -# assert False - - -if __name__ == "__main__": - sys.exit(pytest.main([__file__, "-rA"])) diff --git a/tests/e2e/write_procedures/procedures/read.py b/tests/e2e/write_procedures/procedures/read.py index 16f96908ea..0532fda1a0 100644 --- a/tests/e2e/write_procedures/procedures/read.py +++ b/tests/e2e/write_procedures/procedures/read.py @@ -11,8 +11,6 @@ import mgp -# import typing - @mgp.read_proc def underlying_graph_is_mutable(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record(mutable=bool): @@ -22,8 +20,3 @@ def underlying_graph_is_mutable(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record @mgp.read_proc def graph_is_mutable(ctx: mgp.ProcCtx) -> mgp.Record(mutable=bool): return mgp.Record(mutable=ctx.graph.is_mutable()) - - -@mgp.read_proc -def number_of_visible_nodes(ctx: mgp.ProcCtx, object: mgp.Any) -> mgp.Record(mutable=bool): - return mgp.Record(mutable=True) diff --git a/tests/e2e/write_procedures/workloads.yaml b/tests/e2e/write_procedures/workloads.yaml index d68ca4b4f9..a6a57231f7 100644 --- a/tests/e2e/write_procedures/workloads.yaml +++ b/tests/e2e/write_procedures/workloads.yaml @@ -6,28 +6,9 @@ template_cluster: &template_cluster setup_queries: [] validation_queries: [] -template_cluster_lba: &template_cluster_lba - cluster: - main: - args: ["--bolt-port", "7687", "--log-level=TRACE"] - log_file: "lba-e2e.log" - setup_queries: [ - "CREATE (:Label1 {id: 1});", - "CREATE (:Label1 {id: 2});", - "CREATE (:Label2 {id: 1});", - "CREATE USER Boris;", - "GRANT LABELS :Label1 TO Boris;" - ] - validation_queries: [] - workloads: - name: "Write procedures simple" binary: "tests/e2e/pytest_runner.sh" proc: "tests/e2e/write_procedures/procedures/" args: ["write_procedures/simple_write.py"] <<: *template_cluster - - name: "Label-based auth" - binary: "tests/e2e/pytest_runner.sh" - proc: "tests/e2e/write_procedures/procedures/" - args: ["write_procedures/lba_procedures.py"] - <<: *template_cluster_lba From 166df69178e7dbf8fc24be70074bc68663502ac3 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Sat, 13 Aug 2022 00:23:08 +0200 Subject: [PATCH 06/12] Naming fixes; FineGrainedAccessChecker unit tests introduced --- src/auth/models.cpp | 86 +++++++------- src/auth/models.hpp | 11 +- tests/unit/auth.cpp | 280 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 334 insertions(+), 43 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index a112516b81..cc9573ca77 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -208,22 +208,18 @@ FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_ PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, const LabelPermission label_permission) { - if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { - return PermissionLevel::DENY; - } - - if ((grants_.size() == 1 && grants_.find(ASTERISK) != grants_.end())) { - return PermissionLevel::GRANT; - } - auto grants_permission = PermissionLevel::DENY; auto denies_permission = PermissionLevel::GRANT; - if (denies_.find(permission) != denies_.end() && (denies_[permission] & label_permission)) { + const auto &deny_it = DeniesFind(permission); + + if (deny_it != denies_.end() && (deny_it->second & label_permission)) { denies_permission = PermissionLevel::DENY; } - if (grants_.find(permission) != grants_.end() && (grants_[permission] & label_permission)) { + const auto &grant_it = GrantsFind(permission); + + if (grant_it != grants_.end() && (grant_it->second & label_permission)) { grants_permission = PermissionLevel::GRANT; } @@ -233,21 +229,17 @@ PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, void FineGrainedAccessPermissions::Grant(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { grants_.clear(); - } - - auto deniedPermissionIter = denies_.find(permission); + } else if (grants_.size() == 1 && grants_.contains(ASTERISK)) { + grants_.erase(ASTERISK); - if (deniedPermissionIter != denies_.end()) { - denies_.erase(deniedPermissionIter); - } + auto denied_permission_iter = denies_.find(permission); - if (grants_.size() == 1 && grants_.find(ASTERISK) != grants_.end()) { - grants_.erase(ASTERISK); + if (denied_permission_iter != denies_.end()) { + denies_.erase(denied_permission_iter); + } } - if (grants_.find(permission) == grants_.end()) { - grant_(permission, label_permission); - } + DoGrant(permission, label_permission); } void FineGrainedAccessPermissions::Revoke(const std::string &permission) { @@ -258,36 +250,32 @@ void FineGrainedAccessPermissions::Revoke(const std::string &permission) { return; } - auto deniedPermissionIter = denies_.find(permission); - auto grantedPermissionIter = grants_.find(permission); + auto denied_permission_iter = denies_.find(permission); + auto granted_permission_iter = grants_.find(permission); - if (deniedPermissionIter != denies_.end()) { - denies_.erase(deniedPermissionIter); + if (denied_permission_iter != denies_.end()) { + denies_.erase(denied_permission_iter); } - if (grantedPermissionIter != grants_.end()) { - grants_.erase(grantedPermissionIter); + if (granted_permission_iter != grants_.end()) { + grants_.erase(granted_permission_iter); } } void FineGrainedAccessPermissions::Deny(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { denies_.clear(); - } - - auto grantedPermissionIter = grants_.find(permission); + } else if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { + denies_.erase(ASTERISK); - if (grantedPermissionIter != grants_.end()) { - grants_.erase(grantedPermissionIter); - } + auto granted_permission_iter = grants_.find(permission); - if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { - denies_.erase(ASTERISK); + if (granted_permission_iter != grants_.end()) { + grants_.erase(granted_permission_iter); + } } - if (denies_.find(permission) == denies_.end()) { - deny_(permission, label_permission); - } + DoDeny(permission, label_permission); } nlohmann::json FineGrainedAccessPermissions::Serialize() const { @@ -308,7 +296,7 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } -void FineGrainedAccessPermissions::grant_(const std::string &permission, LabelPermission label_permission) { +void FineGrainedAccessPermissions::DoGrant(const std::string &permission, LabelPermission label_permission) { uint64_t shift = 1; uint64_t perm = 0; auto u_label_perm = static_cast(label_permission); @@ -321,7 +309,7 @@ void FineGrainedAccessPermissions::grant_(const std::string &permission, LabelPe grants_[permission] = perm; } -void FineGrainedAccessPermissions::deny_(const std::string &permission, LabelPermission label_permission) { +void FineGrainedAccessPermissions::DoDeny(const std::string &permission, LabelPermission label_permission) { uint64_t shift = 1; uint64_t perm = 0; auto u_label_perm = static_cast(label_permission); @@ -334,6 +322,24 @@ void FineGrainedAccessPermissions::deny_(const std::string &permission, LabelPer denies_[permission] = perm; } +std::unordered_map::const_iterator FineGrainedAccessPermissions::Find(const std::string &key, + bool in_denies) const { + const auto &collection = in_denies ? denies_ : grants_; + const auto &it = collection.find("*"); + + return it != collection.end() ? it : collection.find(key); +} + +std::unordered_map::const_iterator FineGrainedAccessPermissions::GrantsFind( + const std::string &key) const { + return Find(key, false); +} + +std::unordered_map::const_iterator FineGrainedAccessPermissions::DeniesFind( + const std::string &key) const { + return Find(key, true); +} + bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { return first.grants() == second.grants() && first.denies() == second.denies(); } diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 678098001f..4787e00f44 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -13,6 +13,7 @@ #include #include +#include namespace memgraph::auth { // These permissions must have values that are applicable for usage in a @@ -63,7 +64,7 @@ inline bool operator&(uint64_t a, LabelPermission b) { return (a & static_cast grants_{}; std::unordered_map denies_{}; - void grant_(const std::string &permission, LabelPermission label_permission); - void deny_(const std::string &permission, LabelPermission label_permission); + void DoGrant(const std::string &permission, LabelPermission label_permission); + void DoDeny(const std::string &permission, LabelPermission label_permission); + + std::unordered_map::const_iterator Find(const std::string &key, bool in_denies) const; + std::unordered_map::const_iterator GrantsFind(const std::string &key) const; + std::unordered_map::const_iterator DeniesFind(const std::string &key) const; }; bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second); diff --git a/tests/unit/auth.cpp b/tests/unit/auth.cpp index 4c90fcbbaf..4c8ff7ac80 100644 --- a/tests/unit/auth.cpp +++ b/tests/unit/auth.cpp @@ -407,6 +407,286 @@ TEST(AuthWithoutStorage, PermissionsMaskTest) { ASSERT_EQ(p4.denies(), 2); } +TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + ASSERT_TRUE(fga_permissions1 == fga_permissions2); + } + + { + FineGrainedAccessPermissions fga_permissions; + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); + + ASSERT_FALSE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_FALSE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + + ASSERT_FALSE(fga_permissions.grants().empty()); + ASSERT_FALSE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("AnyString"); + + ASSERT_FALSE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("AnyString"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("*"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("*", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("AnyString"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_FALSE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("AnyString"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("*"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("Label", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("OtherLabel", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("*"); + + ASSERT_TRUE(fga_permissions.grants().empty()); + ASSERT_TRUE(fga_permissions.denies().empty()); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::EDIT); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::READ); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("*", LabelPermission::READ); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("*", LabelPermission::EDIT); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Deny("*", LabelPermission::READ); + + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::READ); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::EDIT); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::CREATE_DELETE); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("*", LabelPermission::CREATE_DELETE); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("*", LabelPermission::EDIT); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("*", LabelPermission::READ); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::READ); + fga_permissions.Revoke("*"); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::EDIT); + fga_permissions.Revoke("*"); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Deny("Label", LabelPermission::CREATE_DELETE); + fga_permissions.Revoke("*"); + + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + } +} + TEST(AuthWithoutStorage, UserSerializeDeserialize) { auto user = User("test"); user.permissions().Grant(Permission::MATCH); From 28554416a1fd184c9c971140c928af56966e91f2 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Sat, 13 Aug 2022 00:41:27 +0200 Subject: [PATCH 07/12] unnecessary includes removed; minor code improvements --- src/auth/models.cpp | 8 ++++---- src/auth/models.hpp | 1 - src/memgraph.cpp | 1 - 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index cc9573ca77..2bf097a7dc 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -297,8 +297,8 @@ const std::unordered_map &FineGrainedAccessPermissions::g const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } void FineGrainedAccessPermissions::DoGrant(const std::string &permission, LabelPermission label_permission) { - uint64_t shift = 1; - uint64_t perm = 0; + uint64_t shift{1}; + uint64_t perm{0}; auto u_label_perm = static_cast(label_permission); while (u_label_perm != 0) { @@ -310,8 +310,8 @@ void FineGrainedAccessPermissions::DoGrant(const std::string &permission, LabelP } void FineGrainedAccessPermissions::DoDeny(const std::string &permission, LabelPermission label_permission) { - uint64_t shift = 1; - uint64_t perm = 0; + uint64_t shift{1}; + uint64_t perm{0}; auto u_label_perm = static_cast(label_permission); while (u_label_perm <= LabelPermissionMax) { diff --git a/src/auth/models.hpp b/src/auth/models.hpp index 4787e00f44..b4ac9fc405 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -13,7 +13,6 @@ #include #include -#include namespace memgraph::auth { // These permissions must have values that are applicable for usage in a diff --git a/src/memgraph.cpp b/src/memgraph.cpp index 056a7521a1..0104e92871 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -32,7 +32,6 @@ #include #include -#include "auth/models.hpp" #include "communication/bolt/v1/constants.hpp" #include "communication/websocket/auth.hpp" #include "communication/websocket/server.hpp" From 8cf2715ae471f404e91a14701226f8955cd82485 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Tue, 16 Aug 2022 00:54:50 +0200 Subject: [PATCH 08/12] Access checker reworked; denies and grant merged into single permission object; Created global_permission that applies to all non-created permissions. Grant, Deny and Revoke reworked; Merge method reworked --- src/auth/models.cpp | 171 ++++++++------------- src/auth/models.hpp | 29 ++-- tests/unit/auth.cpp | 361 ++++++++++++++++++++++++++------------------ 3 files changed, 299 insertions(+), 262 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 2bf097a7dc..8da15a7d14 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -32,10 +32,6 @@ DEFINE_string(auth_password_strength_regex, default_password_regex.data(), namespace memgraph::auth { namespace { -const std::string ASTERISK = "*"; -const uint64_t LabelPermissionMax = static_cast(memgraph::auth::LabelPermission::CREATE_DELETE); -} // namespace -namespace { // Constant list of all available permissions. const std::vector kPermissionsAll = { Permission::MATCH, Permission::CREATE, Permission::MERGE, Permission::DELETE, @@ -104,17 +100,22 @@ std::string PermissionLevelToString(PermissionLevel level) { } } -std::unordered_map Merge(const std::unordered_map &first, - const std::unordered_map &second) { - std::unordered_map result{first}; +FineGrainedAccessPermissions Merge(const FineGrainedAccessPermissions &first, + const FineGrainedAccessPermissions &second) { + std::unordered_map permissions{first.permissions()}; + std::optional global_permission = std::nullopt; - for (const auto &it : second) { - if (result.find(it.first) != result.end()) { - result[it.first] |= it.second; - } + if (second.global_permission().has_value()) { + global_permission = second.global_permission().value(); + } else if (first.global_permission().has_value()) { + global_permission = first.global_permission().value(); } - return result; + for (const auto &it : second.permissions()) { + permissions[it.first] = it.second; + } + + return FineGrainedAccessPermissions(permissions, global_permission); } Permissions::Permissions(uint64_t grants, uint64_t denies) { @@ -202,86 +203,52 @@ bool operator==(const Permissions &first, const Permissions &second) { bool operator!=(const Permissions &first, const Permissions &second) { return !(first == second); } -FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_map &grants, - const std::unordered_map &denies) - : grants_(grants), denies_(denies) {} +FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_map &permissions, + const std::optional &global_permission) + : permissions_(permissions), global_permission_(global_permission) {} PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, const LabelPermission label_permission) { - auto grants_permission = PermissionLevel::DENY; - auto denies_permission = PermissionLevel::GRANT; + uint64_t concrete_permission = permissions_.contains(permission) + ? permissions_[permission] + : (global_permission_.has_value() ? global_permission_.value() : 0); - const auto &deny_it = DeniesFind(permission); - - if (deny_it != denies_.end() && (deny_it->second & label_permission)) { - denies_permission = PermissionLevel::DENY; - } - - const auto &grant_it = GrantsFind(permission); - - if (grant_it != grants_.end() && (grant_it->second & label_permission)) { - grants_permission = PermissionLevel::GRANT; - } + auto temp_permission = concrete_permission & label_permission; - return denies_permission >= grants_permission ? denies_permission : grants_permission; + return temp_permission > 0 ? PermissionLevel::GRANT : PermissionLevel::DENY; } void FineGrainedAccessPermissions::Grant(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { - grants_.clear(); - } else if (grants_.size() == 1 && grants_.contains(ASTERISK)) { - grants_.erase(ASTERISK); - - auto denied_permission_iter = denies_.find(permission); - - if (denied_permission_iter != denies_.end()) { - denies_.erase(denied_permission_iter); - } + global_permission_ = global_permission_.has_value() ? *global_permission_ |= CalculateGrant(label_permission) + : CalculateGrant(label_permission); + } else { + permissions_[permission] |= CalculateGrant(label_permission); } - - DoGrant(permission, label_permission); } void FineGrainedAccessPermissions::Revoke(const std::string &permission) { if (permission == ASTERISK) { - grants_.clear(); - denies_.clear(); - - return; - } - - auto denied_permission_iter = denies_.find(permission); - auto granted_permission_iter = grants_.find(permission); - - if (denied_permission_iter != denies_.end()) { - denies_.erase(denied_permission_iter); - } - - if (granted_permission_iter != grants_.end()) { - grants_.erase(granted_permission_iter); + permissions_.clear(); + global_permission_ = std::nullopt; + } else { + permissions_.erase(permission); } } void FineGrainedAccessPermissions::Deny(const std::string &permission, const LabelPermission label_permission) { if (permission == ASTERISK) { - denies_.clear(); - } else if (denies_.size() == 1 && denies_.find(ASTERISK) != denies_.end()) { - denies_.erase(ASTERISK); - - auto granted_permission_iter = grants_.find(permission); - - if (granted_permission_iter != grants_.end()) { - grants_.erase(granted_permission_iter); - } + global_permission_ = global_permission_.has_value() ? *global_permission_ &= CalculateDeny(label_permission) + : CalculateDeny(label_permission); + } else { + permissions_[permission] |= CalculateDeny(label_permission); } - - DoDeny(permission, label_permission); } nlohmann::json FineGrainedAccessPermissions::Serialize() const { nlohmann::json data = nlohmann::json::object(); - data["grants"] = grants_; - data["denies"] = denies_; + data["permissions"] = permissions_; + data["global_permission"] = global_permission_.has_value() ? std::to_string(global_permission_.value()) : "-1"; return data; } @@ -290,58 +257,50 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo throw AuthException("Couldn't load permissions data!"); } - return FineGrainedAccessPermissions(data["grants"], data["denies"]); -} - -const std::unordered_map &FineGrainedAccessPermissions::grants() const { return grants_; } -const std::unordered_map &FineGrainedAccessPermissions::denies() const { return denies_; } - -void FineGrainedAccessPermissions::DoGrant(const std::string &permission, LabelPermission label_permission) { - uint64_t shift{1}; - uint64_t perm{0}; - auto u_label_perm = static_cast(label_permission); + std::optional global_permission; - while (u_label_perm != 0) { - perm |= u_label_perm; - u_label_perm >>= shift; + if (data["global_permission"] == "-1") { + global_permission = std::nullopt; + } else { + global_permission = data["global_permission"]; } - grants_[permission] = perm; + return FineGrainedAccessPermissions(data["permissions"], global_permission); } -void FineGrainedAccessPermissions::DoDeny(const std::string &permission, LabelPermission label_permission) { +const std::unordered_map &FineGrainedAccessPermissions::permissions() const { + return permissions_; +} +const std::optional &FineGrainedAccessPermissions::global_permission() const { return global_permission_; }; + +uint64_t FineGrainedAccessPermissions::CalculateGrant(LabelPermission label_permission) { uint64_t shift{1}; - uint64_t perm{0}; - auto u_label_perm = static_cast(label_permission); + uint64_t result{0}; + auto uint_label_permission = static_cast(label_permission); - while (u_label_perm <= LabelPermissionMax) { - perm |= u_label_perm; - u_label_perm <<= shift; + while (uint_label_permission > 0) { + result |= uint_label_permission; + uint_label_permission >>= shift; } - denies_[permission] = perm; + return result; } -std::unordered_map::const_iterator FineGrainedAccessPermissions::Find(const std::string &key, - bool in_denies) const { - const auto &collection = in_denies ? denies_ : grants_; - const auto &it = collection.find("*"); - - return it != collection.end() ? it : collection.find(key); -} +uint64_t FineGrainedAccessPermissions::CalculateDeny(LabelPermission label_permission) { + uint64_t shift{1}; + uint64_t result{0}; + auto uint_label_permission = static_cast(label_permission); -std::unordered_map::const_iterator FineGrainedAccessPermissions::GrantsFind( - const std::string &key) const { - return Find(key, false); -} + while (uint_label_permission <= LabelPermissionMax) { + result |= uint_label_permission; + uint_label_permission <<= shift; + } -std::unordered_map::const_iterator FineGrainedAccessPermissions::DeniesFind( - const std::string &key) const { - return Find(key, true); + return LabelPermissionAll - result; } bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { - return first.grants() == second.grants() && first.denies() == second.denies(); + return first.permissions() == second.permissions() && first.global_permission() == second.global_permission(); } bool operator!=(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { @@ -448,9 +407,7 @@ Permissions User::GetPermissions() const { FineGrainedAccessPermissions User::GetFineGrainedAccessPermissions() const { if (role_) { - return FineGrainedAccessPermissions( - Merge(fine_grained_access_permissions_.grants(), role_->fine_grained_access_permissions().grants()), - Merge(fine_grained_access_permissions_.denies(), role_->fine_grained_access_permissions().denies())); + return Merge(role()->fine_grained_access_permissions(), fine_grained_access_permissions_); } return fine_grained_access_permissions_; } diff --git a/src/auth/models.hpp b/src/auth/models.hpp index b4ac9fc405..e05e91264f 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -15,6 +15,7 @@ #include namespace memgraph::auth { +const std::string ASTERISK = "*"; // These permissions must have values that are applicable for usage in a // bitmask. // clang-format off @@ -59,6 +60,11 @@ inline uint64_t operator|(uint64_t a, LabelPermission b) { return a | static_cas inline bool operator&(uint64_t a, LabelPermission b) { return (a & static_cast(b)) != 0; } +const uint64_t LabelPermissionAll = memgraph::auth::LabelPermission::CREATE_DELETE | + memgraph::auth::LabelPermission::EDIT | memgraph::auth::LabelPermission::READ; +const uint64_t LabelPermissionMax = static_cast(memgraph::auth::LabelPermission::CREATE_DELETE); +const uint64_t LabelPermissionMin = static_cast(memgraph::auth::LabelPermission::READ); + // Function that converts a permission to its string representation. std::string PermissionToString(Permission permission); @@ -103,8 +109,8 @@ bool operator!=(const Permissions &first, const Permissions &second); class FineGrainedAccessPermissions final { public: - explicit FineGrainedAccessPermissions(const std::unordered_map &grants = {}, - const std::unordered_map &denies = {}); + explicit FineGrainedAccessPermissions(const std::unordered_map &permissions = {}, + const std::optional &global_permission = std::nullopt); PermissionLevel Has(const std::string &permission, LabelPermission label_permission); @@ -119,19 +125,15 @@ class FineGrainedAccessPermissions final { /// @throw AuthException if unable to deserialize. static FineGrainedAccessPermissions Deserialize(const nlohmann::json &data); - const std::unordered_map &grants() const; - const std::unordered_map &denies() const; + const std::unordered_map &permissions() const; + const std::optional &global_permission() const; private: - std::unordered_map grants_{}; - std::unordered_map denies_{}; - - void DoGrant(const std::string &permission, LabelPermission label_permission); - void DoDeny(const std::string &permission, LabelPermission label_permission); + std::unordered_map permissions_{}; + std::optional global_permission_; - std::unordered_map::const_iterator Find(const std::string &key, bool in_denies) const; - std::unordered_map::const_iterator GrantsFind(const std::string &key) const; - std::unordered_map::const_iterator DeniesFind(const std::string &key) const; + static uint64_t CalculateGrant(LabelPermission label_permission); + static uint64_t CalculateDeny(LabelPermission label_permission); }; bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second); @@ -212,4 +214,7 @@ class User final { }; bool operator==(const User &first, const User &second); + +FineGrainedAccessPermissions Merge(const FineGrainedAccessPermissions &first, + const FineGrainedAccessPermissions &second); } // namespace memgraph::auth diff --git a/tests/unit/auth.cpp b/tests/unit/auth.cpp index 4c8ff7ac80..9cca882d85 100644 --- a/tests/unit/auth.cpp +++ b/tests/unit/auth.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -408,6 +409,11 @@ TEST(AuthWithoutStorage, PermissionsMaskTest) { } TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { + auto any_label = "AnyString"; + auto check_label = "Label"; + auto non_check_label = "OtherLabel"; + auto asterisk = "*"; + { FineGrainedAccessPermissions fga_permissions1, fga_permissions2; ASSERT_TRUE(fga_permissions1 == fga_permissions2); @@ -415,278 +421,347 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { { FineGrainedAccessPermissions fga_permissions; - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::DENY); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); - ASSERT_FALSE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_FALSE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_FALSE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_FALSE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); - ASSERT_FALSE(fga_permissions.grants().empty()); - ASSERT_FALSE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), LabelPermissionAll); + ASSERT_FALSE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("AnyString"); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(any_label); - ASSERT_FALSE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), LabelPermissionAll); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("AnyString"); + fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(any_label); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("AnyString", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("*"); + fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(asterisk); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("*", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("AnyString"); + fga_permissions.Deny(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(any_label); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_FALSE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), LabelPermission::EDIT | LabelPermission::READ); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("AnyString"); + fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(any_label); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("AnyString", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("*"); + fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(asterisk); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("Label", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("OtherLabel", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("*"); + fga_permissions.Grant(check_label, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(non_check_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(asterisk); - ASSERT_TRUE(fga_permissions.grants().empty()); - ASSERT_TRUE(fga_permissions.denies().empty()); + ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.permissions().empty()); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::EDIT); + fga_permissions.Grant(asterisk, LabelPermission::EDIT); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::READ); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("*", LabelPermission::READ); + fga_permissions.Deny(asterisk, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("*", LabelPermission::EDIT); + fga_permissions.Deny(asterisk, LabelPermission::EDIT); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Deny("*", LabelPermission::READ); + fga_permissions.Deny(asterisk, LabelPermission::READ); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("AnyString", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::DENY); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::READ); + fga_permissions.Grant(asterisk, LabelPermission::READ); + fga_permissions.Grant(check_label, LabelPermission::EDIT); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::EDIT); + fga_permissions.Grant(asterisk, LabelPermission::READ); + fga_permissions.Deny(check_label, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::CREATE_DELETE); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(check_label, LabelPermission::EDIT); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("*", LabelPermission::CREATE_DELETE); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(check_label, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("*", LabelPermission::EDIT); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(asterisk, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::GRANT); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("*", LabelPermission::READ); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(asterisk, LabelPermission::EDIT); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::READ); - fga_permissions.Revoke("*"); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(asterisk, LabelPermission::READ); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::READ), PermissionLevel::DENY); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::EDIT); - fga_permissions.Revoke("*"); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(check_label, LabelPermission::READ); + fga_permissions.Revoke(asterisk); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::DENY); } { FineGrainedAccessPermissions fga_permissions; - fga_permissions.Grant("*", LabelPermission::CREATE_DELETE); - fga_permissions.Deny("Label", LabelPermission::CREATE_DELETE); - fga_permissions.Revoke("*"); + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(check_label, LabelPermission::EDIT); + fga_permissions.Revoke(asterisk); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("Label", LabelPermission::READ), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::CREATE_DELETE), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::EDIT), PermissionLevel::DENY); - ASSERT_EQ(fga_permissions.Has("OtherLabel", LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::DENY); + } + + { + FineGrainedAccessPermissions fga_permissions; + fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); + fga_permissions.Deny(check_label, LabelPermission::CREATE_DELETE); + fga_permissions.Revoke(asterisk); + + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(check_label, LabelPermission::READ), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions.Has(non_check_label, LabelPermission::READ), PermissionLevel::DENY); } } +TEST_F(AuthWithStorage, FineGrainedAccessCheckerMerge) { + auto any_label = "AnyString"; + auto check_label = "Label"; + auto asterisk = "*"; + + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + fga_permissions1.Grant(asterisk, LabelPermission::READ); + + auto fga_permissions3 = memgraph::auth::Merge(fga_permissions1, fga_permissions2); + + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + fga_permissions2.Grant(asterisk, LabelPermission::READ); + + auto fga_permissions3 = memgraph::auth::Merge(fga_permissions1, fga_permissions2); + + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); + } + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + fga_permissions1.Grant(asterisk, LabelPermission::READ); + fga_permissions2.Grant(asterisk, LabelPermission::EDIT); + + auto fga_permissions3 = memgraph::auth::Merge(fga_permissions1, fga_permissions2); + + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions3.Has(any_label, LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + fga_permissions1.Grant(asterisk, LabelPermission::READ); + fga_permissions1.Grant(check_label, LabelPermission::EDIT); + fga_permissions2.Grant(asterisk, LabelPermission::EDIT); + + auto fga_permissions3 = memgraph::auth::Merge(fga_permissions1, fga_permissions2); + + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::EDIT), PermissionLevel::GRANT); + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + } + + { + FineGrainedAccessPermissions fga_permissions1, fga_permissions2; + fga_permissions1.Grant(asterisk, LabelPermission::READ); + fga_permissions1.Grant(check_label, LabelPermission::CREATE_DELETE); + fga_permissions2.Grant(asterisk, LabelPermission::EDIT); + fga_permissions2.Grant(check_label, LabelPermission::READ); + + auto fga_permissions3 = memgraph::auth::Merge(fga_permissions1, fga_permissions2); + + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::EDIT), PermissionLevel::DENY); + ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); + } +} TEST(AuthWithoutStorage, UserSerializeDeserialize) { auto user = User("test"); user.permissions().Grant(Permission::MATCH); From 22ae90b4083a238ffb3a6bb6ab5bdc41661b24d0 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Tue, 16 Aug 2022 08:38:09 +0200 Subject: [PATCH 09/12] Fixed wrong check; --- src/auth/models.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 8da15a7d14..e21fb4dc03 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -248,7 +248,7 @@ void FineGrainedAccessPermissions::Deny(const std::string &permission, const Lab nlohmann::json FineGrainedAccessPermissions::Serialize() const { nlohmann::json data = nlohmann::json::object(); data["permissions"] = permissions_; - data["global_permission"] = global_permission_.has_value() ? std::to_string(global_permission_.value()) : "-1"; + data["global_permission"] = global_permission_.has_value() ? global_permission_.value() : -1; return data; } @@ -259,7 +259,7 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo std::optional global_permission; - if (data["global_permission"] == "-1") { + if (data["global_permission"].empty() || data["global_permission"] == -1) { global_permission = std::nullopt; } else { global_permission = data["global_permission"]; From 88864c34ea0e0e8a02b14fd778f1c3f83f35994f Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Thu, 18 Aug 2022 13:54:49 +0200 Subject: [PATCH 10/12] PR review changes; Naming and const fixed, replaced double tertiary with lambda --- src/auth/models.cpp | 39 ++++++++++++++++++++------------- src/auth/models.hpp | 24 ++++++++++---------- src/memgraph.cpp | 34 ++++++++++++++--------------- tests/unit/auth.cpp | 53 +++++++++++++++++++++++---------------------- 4 files changed, 81 insertions(+), 69 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index cc16285f81..13f0d7371b 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -102,16 +102,16 @@ std::string PermissionLevelToString(PermissionLevel level) { FineGrainedAccessPermissions Merge(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { - std::unordered_map permissions{first.permissions()}; - std::optional global_permission = std::nullopt; + std::unordered_map permissions{first.GetPermissions()}; + std::optional global_permission; - if (second.global_permission().has_value()) { - global_permission = second.global_permission().value(); - } else if (first.global_permission().has_value()) { - global_permission = first.global_permission().value(); + if (second.GetGlobalPermission().has_value()) { + global_permission = second.GetGlobalPermission().value(); + } else if (first.GetGlobalPermission().has_value()) { + global_permission = first.GetGlobalPermission().value(); } - for (const auto &it : second.permissions()) { + for (const auto &it : second.GetPermissions()) { permissions[it.first] = it.second; } @@ -208,12 +208,20 @@ FineGrainedAccessPermissions::FineGrainedAccessPermissions(const std::unordered_ : permissions_(permissions), global_permission_(global_permission) {} PermissionLevel FineGrainedAccessPermissions::Has(const std::string &permission, - const LabelPermission label_permission) { - uint64_t concrete_permission = permissions_.contains(permission) - ? permissions_[permission] - : (global_permission_.has_value() ? global_permission_.value() : 0); + const LabelPermission label_permission) const { + const auto concrete_permission = std::invoke([&]() -> uint64_t { + if (permissions_.contains(permission)) { + return permissions_.at(permission); + } + + if (global_permission_.has_value()) { + return global_permission_.value(); + } + + return 0; + }); - auto temp_permission = concrete_permission & label_permission; + const auto temp_permission = concrete_permission & label_permission; return temp_permission > 0 ? PermissionLevel::GRANT : PermissionLevel::DENY; } @@ -266,10 +274,10 @@ FineGrainedAccessPermissions FineGrainedAccessPermissions::Deserialize(const nlo return FineGrainedAccessPermissions(data["permissions"], global_permission); } -const std::unordered_map &FineGrainedAccessPermissions::permissions() const { +const std::unordered_map &FineGrainedAccessPermissions::GetPermissions() const { return permissions_; } -const std::optional &FineGrainedAccessPermissions::global_permission() const { return global_permission_; }; +const std::optional &FineGrainedAccessPermissions::GetGlobalPermission() const { return global_permission_; }; uint64_t FineGrainedAccessPermissions::CalculateGrant(LabelPermission label_permission) { uint64_t shift{1}; @@ -298,7 +306,8 @@ uint64_t FineGrainedAccessPermissions::CalculateDeny(LabelPermission label_permi } bool operator==(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { - return first.permissions() == second.permissions() && first.global_permission() == second.global_permission(); + return first.GetPermissions() == second.GetPermissions() && + first.GetGlobalPermission() == second.GetGlobalPermission(); } bool operator!=(const FineGrainedAccessPermissions &first, const FineGrainedAccessPermissions &second) { diff --git a/src/auth/models.hpp b/src/auth/models.hpp index a06eb2bef7..f63345ee9d 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -52,18 +52,20 @@ enum class LabelPermission : uint64_t { }; // clang-format on -inline uint64_t operator|(LabelPermission a, LabelPermission b) { - return static_cast(a) | static_cast(b); +constexpr inline uint64_t operator|(LabelPermission lhs, LabelPermission rhs) { + return static_cast(lhs) | static_cast(rhs); } -inline uint64_t operator|(uint64_t a, LabelPermission b) { return a | static_cast(b); } +constexpr inline uint64_t operator|(uint64_t lhs, LabelPermission rhs) { return lhs | static_cast(rhs); } -inline uint64_t operator&(uint64_t a, LabelPermission b) { return (a & static_cast(b)) != 0; } +constexpr inline uint64_t operator&(uint64_t lhs, LabelPermission rhs) { + return (lhs & static_cast(rhs)) != 0; +} -const uint64_t LabelPermissionAll = memgraph::auth::LabelPermission::CREATE_DELETE | - memgraph::auth::LabelPermission::EDIT | memgraph::auth::LabelPermission::READ; -const uint64_t LabelPermissionMax = static_cast(memgraph::auth::LabelPermission::CREATE_DELETE); -const uint64_t LabelPermissionMin = static_cast(memgraph::auth::LabelPermission::READ); +constexpr uint64_t LabelPermissionAll = memgraph::auth::LabelPermission::CREATE_DELETE | + memgraph::auth::LabelPermission::EDIT | memgraph::auth::LabelPermission::READ; +constexpr uint64_t LabelPermissionMax = static_cast(memgraph::auth::LabelPermission::CREATE_DELETE); +constexpr uint64_t LabelPermissionMin = static_cast(memgraph::auth::LabelPermission::READ); // Function that converts a permission to its string representation. std::string PermissionToString(Permission permission); @@ -123,7 +125,7 @@ class FineGrainedAccessPermissions final { FineGrainedAccessPermissions &operator=(FineGrainedAccessPermissions &&) = default; ~FineGrainedAccessPermissions() = default; - PermissionLevel Has(const std::string &permission, LabelPermission label_permission); + PermissionLevel Has(const std::string &permission, LabelPermission label_permission) const; void Grant(const std::string &permission, LabelPermission label_permission); @@ -136,8 +138,8 @@ class FineGrainedAccessPermissions final { /// @throw AuthException if unable to deserialize. static FineGrainedAccessPermissions Deserialize(const nlohmann::json &data); - const std::unordered_map &permissions() const; - const std::optional &global_permission() const; + const std::unordered_map &GetPermissions() const; + const std::optional &GetGlobalPermission() const; private: std::unordered_map permissions_{}; diff --git a/src/memgraph.cpp b/src/memgraph.cpp index 2ea5091e95..6af5b3ebc0 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -759,14 +759,14 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { const std::vector &labels, const std::vector &edge_types) override { EditPermissions( user_or_role, privileges, labels, edge_types, - [](auto *permissions, const auto &permission) { + [](auto &permissions, const auto &permission) { // TODO (mferencevic): should we first check that the // privilege is granted/denied/revoked before // unconditionally granting/denying/revoking it? - permissions->Grant(permission); + permissions.Grant(permission); }, - [](auto *label_permissions, const auto &label) { - label_permissions->Grant(label, memgraph::auth::LabelPermission::CREATE_DELETE); + [](auto &label_permissions, const auto &label) { + label_permissions.Grant(label, memgraph::auth::LabelPermission::CREATE_DELETE); }); } @@ -775,14 +775,14 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { const std::vector &labels, const std::vector &edge_types) override { EditPermissions( user_or_role, privileges, labels, edge_types, - [](auto *permissions, const auto &permission) { + [](auto &permissions, const auto &permission) { // TODO (mferencevic): should we first check that the // privilege is granted/denied/revoked before // unconditionally granting/denying/revoking it? - permissions->Deny(permission); + permissions.Deny(permission); }, - [](auto *label_permissions, const auto &label) { - label_permissions->Deny(label, memgraph::auth::LabelPermission::READ); + [](auto &label_permissions, const auto &label) { + label_permissions.Deny(label, memgraph::auth::LabelPermission::READ); }); } @@ -791,13 +791,13 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { const std::vector &labels, const std::vector &edge_types) override { EditPermissions( user_or_role, privileges, labels, edge_types, - [](auto *permissions, const auto &permission) { + [](auto &permissions, const auto &permission) { // TODO (mferencevic): should we first check that the // privilege is granted/denied/revoked before // unconditionally granting/denying/revoking it? - permissions->Revoke(permission); + permissions.Revoke(permission); }, - [](auto *label_permissions, const auto &label) { label_permissions->Revoke(label); }); + [](auto &label_permissions, const auto &label) { label_permissions.Revoke(label); }); } private: @@ -823,25 +823,25 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { } if (user) { for (const auto &permission : permissions) { - edit_fun(&user->permissions(), permission); + edit_fun(user->permissions(), permission); } for (const auto &label : labels) { - edit_label_permisions_fun(&user->fine_grained_access_handler().label_permissions(), label); + edit_label_permisions_fun(user->fine_grained_access_handler().label_permissions(), label); } for (const auto &edgeType : edgeTypes) { - edit_label_permisions_fun(&user->fine_grained_access_handler().edge_type_permissions(), edgeType); + edit_label_permisions_fun(user->fine_grained_access_handler().edge_type_permissions(), edgeType); } locked_auth->SaveUser(*user); } else { for (const auto &permission : permissions) { - edit_fun(&role->permissions(), permission); + edit_fun(role->permissions(), permission); } for (const auto &label : labels) { - edit_label_permisions_fun(&user->fine_grained_access_handler().label_permissions(), label); + edit_label_permisions_fun(user->fine_grained_access_handler().label_permissions(), label); } for (const auto &edgeType : edgeTypes) { - edit_label_permisions_fun(&role->fine_grained_access_handler().edge_type_permissions(), edgeType); + edit_label_permisions_fun(role->fine_grained_access_handler().edge_type_permissions(), edgeType); } locked_auth->SaveRole(*role); diff --git a/tests/unit/auth.cpp b/tests/unit/auth.cpp index 88bd90047a..49cb72d1ae 100644 --- a/tests/unit/auth.cpp +++ b/tests/unit/auth.cpp @@ -485,10 +485,10 @@ TEST(AuthWithoutStorage, PermissionsMaskTest) { } TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { - auto any_label = "AnyString"; - auto check_label = "Label"; - auto non_check_label = "OtherLabel"; - auto asterisk = "*"; + const std::string any_label = "AnyString"; + const std::string check_label = "Label"; + const std::string non_check_label = "OtherLabel"; + const std::string asterisk = "*"; { FineGrainedAccessPermissions fga_permissions1, fga_permissions2; @@ -497,8 +497,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { { FineGrainedAccessPermissions fga_permissions; - ASSERT_TRUE(fga_permissions.permissions().empty()); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::CREATE_DELETE), PermissionLevel::DENY); ASSERT_EQ(fga_permissions.Has(any_label, LabelPermission::EDIT), PermissionLevel::DENY); @@ -509,16 +509,16 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { FineGrainedAccessPermissions fga_permissions; fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_FALSE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_FALSE(fga_permissions.GetPermissions().empty()); } { FineGrainedAccessPermissions fga_permissions; fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_FALSE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_FALSE(fga_permissions.GetPermissions().empty()); } { @@ -526,8 +526,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); - ASSERT_EQ(fga_permissions.global_permission(), LabelPermissionAll); - ASSERT_FALSE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), LabelPermissionAll); + ASSERT_FALSE(fga_permissions.GetPermissions().empty()); } { @@ -535,8 +535,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Grant(asterisk, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(any_label); - ASSERT_EQ(fga_permissions.global_permission(), LabelPermissionAll); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), LabelPermissionAll); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -544,8 +544,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(any_label); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -553,8 +553,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Grant(any_label, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(asterisk); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -562,8 +562,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Deny(asterisk, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(any_label); - ASSERT_EQ(fga_permissions.global_permission(), LabelPermission::EDIT | LabelPermission::READ); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), LabelPermission::EDIT | LabelPermission::READ); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -571,8 +571,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(any_label); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -580,8 +580,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Deny(any_label, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(asterisk); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -590,8 +590,8 @@ TEST(AuthWithoutStorage, FineGrainedAccessPermissions) { fga_permissions.Deny(non_check_label, LabelPermission::CREATE_DELETE); fga_permissions.Revoke(asterisk); - ASSERT_EQ(fga_permissions.global_permission(), std::nullopt); - ASSERT_TRUE(fga_permissions.permissions().empty()); + ASSERT_EQ(fga_permissions.GetGlobalPermission(), std::nullopt); + ASSERT_TRUE(fga_permissions.GetPermissions().empty()); } { @@ -838,6 +838,7 @@ TEST_F(AuthWithStorage, FineGrainedAccessCheckerMerge) { ASSERT_EQ(fga_permissions3.Has(check_label, LabelPermission::READ), PermissionLevel::GRANT); } } + TEST(AuthWithoutStorage, UserSerializeDeserialize) { auto user = User("test"); user.permissions().Grant(Permission::MATCH); From f8e43bb3763393bd2d6b1669b0260255db2a8beb Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Thu, 18 Aug 2022 14:02:04 +0200 Subject: [PATCH 11/12] unwrapping the iterator fix --- src/auth/models.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/auth/models.cpp b/src/auth/models.cpp index 13f0d7371b..2753b7df59 100644 --- a/src/auth/models.cpp +++ b/src/auth/models.cpp @@ -111,8 +111,8 @@ FineGrainedAccessPermissions Merge(const FineGrainedAccessPermissions &first, global_permission = first.GetGlobalPermission().value(); } - for (const auto &it : second.GetPermissions()) { - permissions[it.first] = it.second; + for (const auto &[label_name, permission] : second.GetPermissions()) { + permissions[label_name] = permission; } return FineGrainedAccessPermissions(permissions, global_permission); From dc0a768f9a11056f35783d09dd1c766a4a5c6387 Mon Sep 17 00:00:00 2001 From: Boris Tasevski Date: Thu, 18 Aug 2022 14:57:21 +0200 Subject: [PATCH 12/12] minor spelling fixes --- src/auth/models.hpp | 2 +- src/memgraph.cpp | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/auth/models.hpp b/src/auth/models.hpp index f63345ee9d..5556f43610 100644 --- a/src/auth/models.hpp +++ b/src/auth/models.hpp @@ -71,7 +71,7 @@ constexpr uint64_t LabelPermissionMin = static_cast(memgraph::auth::La std::string PermissionToString(Permission permission); // Class that indicates what permission level the user/role has. -enum class PermissionLevel : short { GRANT, NEUTRAL, DENY }; +enum class PermissionLevel : uint8_t { GRANT, NEUTRAL, DENY }; // Function that converts a permission level to its string representation. std::string PermissionLevelToString(PermissionLevel level); diff --git a/src/memgraph.cpp b/src/memgraph.cpp index 6af5b3ebc0..b611186d0c 100644 --- a/src/memgraph.cpp +++ b/src/memgraph.cpp @@ -801,11 +801,12 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { } private: - template + template void EditPermissions(const std::string &user_or_role, const std::vector &privileges, const std::vector &labels, const std::vector &edgeTypes, - const TEditFun &edit_fun, const TEditLabelPermisionsFun &edit_label_permisions_fun) { + const TEditPermissionsFun &edit_permissions_fun, + const TEditFineGrainedPermissionsFun &edit_fine_grained_permissions_fun) { if (!std::regex_match(user_or_role, name_regex_)) { throw memgraph::query::QueryRuntimeException("Invalid user or role name."); } @@ -823,25 +824,25 @@ class AuthQueryHandler final : public memgraph::query::AuthQueryHandler { } if (user) { for (const auto &permission : permissions) { - edit_fun(user->permissions(), permission); + edit_permissions_fun(user->permissions(), permission); } for (const auto &label : labels) { - edit_label_permisions_fun(user->fine_grained_access_handler().label_permissions(), label); + edit_fine_grained_permissions_fun(user->fine_grained_access_handler().label_permissions(), label); } for (const auto &edgeType : edgeTypes) { - edit_label_permisions_fun(user->fine_grained_access_handler().edge_type_permissions(), edgeType); + edit_fine_grained_permissions_fun(user->fine_grained_access_handler().edge_type_permissions(), edgeType); } locked_auth->SaveUser(*user); } else { for (const auto &permission : permissions) { - edit_fun(role->permissions(), permission); + edit_permissions_fun(role->permissions(), permission); } for (const auto &label : labels) { - edit_label_permisions_fun(user->fine_grained_access_handler().label_permissions(), label); + edit_fine_grained_permissions_fun(user->fine_grained_access_handler().label_permissions(), label); } for (const auto &edgeType : edgeTypes) { - edit_label_permisions_fun(role->fine_grained_access_handler().edge_type_permissions(), edgeType); + edit_fine_grained_permissions_fun(role->fine_grained_access_handler().edge_type_permissions(), edgeType); } locked_auth->SaveRole(*role);