diff --git a/include/caosdb/acm/permission_rule.h b/include/caosdb/acm/permission_rule.h index 0ea11218f40a0ae4e7fa167d866b0e4fbebd1f2f..a801af460a275be140b9e4507a76aea6b25dc72c 100644 --- a/include/caosdb/acm/permission_rule.h +++ b/include/caosdb/acm/permission_rule.h @@ -29,6 +29,7 @@ #ifndef CAOSDB_ACM_PERMISSION_RULE_H #define CAOSDB_ACM_PERMISSION_RULE_H +#include <stddef.h> // for size_t #include <unordered_set> // for unordered_set #include <string> // for string @@ -44,9 +45,9 @@ public: auto operator=(const PermissionRule &rule) -> PermissionRule &; ~PermissionRule(); auto ToString() const -> std::string; - [[nodiscard]] auto IsGrant() const -> const bool; + [[nodiscard]] auto IsGrant() const -> bool; auto SetGrant(bool isGrant) -> void; - [[nodiscard]] auto IsPriority() const -> const bool; + [[nodiscard]] auto IsPriority() const -> bool; auto SetPriority(bool isPriority) -> void; [[nodiscard]] auto GetPermission() const -> const std::string &; @@ -56,9 +57,7 @@ private: }; struct HashPermissionRule { - size_t operator()(const PermissionRule& rule) const { - return std::hash<std::string>()(rule.GetPermission()); - } + auto operator()(const PermissionRule& rule) const -> size_t; }; using PermissionRules = std::unordered_set<PermissionRule, HashPermissionRule>; diff --git a/include/caosdb/acm/role.h b/include/caosdb/acm/role.h index feb2138d0a1d671f9026d4e3df82d0807902884b..35b2e2f4ba6ffe577763c7a2c269414fc443f654 100644 --- a/include/caosdb/acm/role.h +++ b/include/caosdb/acm/role.h @@ -31,8 +31,8 @@ #define CAOSDB_ACM_ROLES_H #include "caosdb/acm/permission_rule.h" // for PermissionRule +#include <memory> // for unique_ptr #include <string> // for string -#include <vector> // for vector namespace caosdb::connection { class Connection; @@ -45,7 +45,7 @@ class RoleImpl; class Role { public: explicit Role(std::string name); - explicit Role(RoleImpl *wrapped); + explicit Role(std::unique_ptr<RoleImpl> wrapped); Role(std::string name, std::string description); Role(const Role &role); auto operator=(const Role &role) -> Role &; @@ -55,19 +55,20 @@ public: auto SetName(std::string name) -> void; [[nodiscard]] auto GetDescription() const -> const std::string &; auto SetDescription(std::string description) -> void; - // TODO(tf) declare and implement: [[nodiscard]] auto GetPermissionRules() const -> const PermissionRules &; // auto SetPermissionRules(PermissionRules rules) -> void; - // auto ClearPermissionRules() -> void; - auto Grant(std::string permission, bool priority) -> void; - // auto Deny(std::string permission, bool priority) -> void; - auto RevokeGrant(std::string permission, bool priority) -> void; - // auto RevokeDenial(std::string permission, bool priority) -> void; + auto ClearPermissionRules() -> void; + auto Grant(std::string permission, bool isPriority) -> void; + auto Deny(std::string permission, bool isPriority) -> void; + auto RevokeGrant(const std::string &permission, bool isPriority) -> void; + auto RevokeDenial(const std::string &permission, bool isPriority) -> void; + auto AddPermissionRule(std::string permission, bool isGrant, bool isPriority) -> void; + auto RemovePermissionRule(const std::string &permission, bool isGrant, bool isPriority) -> void; friend class caosdb::connection::Connection; private: - RoleImpl *wrapped; + std::unique_ptr<RoleImpl> wrapped; }; } // namespace caosdb::acm diff --git a/src/caosdb/acm/permission_rule.cpp b/src/caosdb/acm/permission_rule.cpp index 8ce21977d45b7e0ce59c7261787d87eeb44de3c4..66f962fa8542212dbb198450fb2a16d5b0bc24e0 100644 --- a/src/caosdb/acm/permission_rule.cpp +++ b/src/caosdb/acm/permission_rule.cpp @@ -22,19 +22,19 @@ #include "caosdb/acm/permission_rule_impl.h" // for PermissionRuleImpl #include "caosdb/acm/v1alpha1/main.pb.h" // for ListPermissionRulesRequest #include "caosdb/protobuf_helper.h" // for ProtoMessageWrapper +#include <type_traits> // for remove_reference<>::type #include <utility> // for move namespace caosdb::acm { using caosdb::utility::ScalarProtoMessageWrapper; using ProtoPermissionRule = caosdb::acm::v1alpha1::PermissionRule; -PermissionRuleImpl::PermissionRuleImpl() : ScalarProtoMessageWrapper<ProtoPermissionRule>() {} +PermissionRuleImpl::PermissionRuleImpl() = default; PermissionRuleImpl::PermissionRuleImpl(ProtoPermissionRule *rule) : ScalarProtoMessageWrapper<ProtoPermissionRule>(rule) {} -PermissionRuleImpl::PermissionRuleImpl(std::string permission, bool isGrant, bool isPriority) - : PermissionRuleImpl() { +PermissionRuleImpl::PermissionRuleImpl(std::string permission, bool isGrant, bool isPriority) { this->wrapped->set_permission(std::move(permission)); this->wrapped->set_grant(isGrant); this->wrapped->set_priority(isPriority); @@ -48,6 +48,9 @@ PermissionRule::PermissionRule(const PermissionRule &rule) : impl(new Permission } auto PermissionRule::operator=(const PermissionRule &rule) -> PermissionRule & { + if(this == &rule) { + return *this; + } this->impl->wrapped->CopyFrom(*(rule.impl->wrapped)); return *this; @@ -57,13 +60,13 @@ PermissionRule::~PermissionRule() { delete this->impl; } auto PermissionRule::ToString() const -> std::string { return this->impl->ToString(); } -[[nodiscard]] auto PermissionRule::IsGrant() const -> const bool { +[[nodiscard]] auto PermissionRule::IsGrant() const -> bool { return this->impl->wrapped->grant(); } auto PermissionRule::SetGrant(bool isGrant) -> void { this->impl->wrapped->set_grant(isGrant); } -[[nodiscard]] auto PermissionRule::IsPriority() const -> const bool { +[[nodiscard]] auto PermissionRule::IsPriority() const -> bool { return this->impl->wrapped->priority(); } @@ -72,7 +75,15 @@ auto PermissionRule::SetPriority(bool isPriority) -> void { } auto PermissionRule::operator==(const PermissionRule &other) const -> bool { - return this->GetPermission() == other.GetPermission(); + if(this == &other) { + return true; + } + return this->GetPermission() == other.GetPermission() && + this->IsPriority() == other.IsPriority() && this->IsGrant() == other.IsGrant(); +} + +auto HashPermissionRule::operator()(const PermissionRule &rule) const -> size_t { + return std::hash<std::string>()(rule.GetPermission()); } auto PermissionRule::GetPermission() const -> const std::string & { diff --git a/src/caosdb/acm/role.cpp b/src/caosdb/acm/role.cpp index 9294de142e8989078d82c334d0c021b07572e96c..c7d07357978a1a4cc0a81a9671153423c7a3b2c8 100644 --- a/src/caosdb/acm/role.cpp +++ b/src/caosdb/acm/role.cpp @@ -23,8 +23,9 @@ #include "caosdb/acm/role_impl.h" // for RoleImpl #include "caosdb/acm/v1alpha1/main.pb.h" // for ListRolesRequest #include "caosdb/protobuf_helper.h" // for ProtoMessageWrapper +#include <type_traits> // for remove_reference<>::type +#include <unordered_set> // for _Node_iterator, operator!= #include <utility> // for move -#include <vector> // for vector namespace caosdb::acm { using caosdb::utility::ScalarProtoMessageWrapper; @@ -33,8 +34,7 @@ using ProtoRolePermissions = caosdb::acm::v1alpha1::RolePermissions; using ProtoRoleCapabilities = caosdb::acm::v1alpha1::RoleCapabilities; using ProtoListRoleItem = caosdb::acm::v1alpha1::ListRoleItem; -RoleImpl::RoleImpl(std::string name, std::string description) - : ScalarProtoMessageWrapper<ProtoRole>() { +RoleImpl::RoleImpl(std::string name, std::string description) { if (!name.empty()) { this->wrapped->set_name(name); } @@ -50,22 +50,33 @@ RoleImpl::RoleImpl(ProtoListRoleItem &role_item) RoleImpl::RoleImpl(std::string name) : RoleImpl(std::move(name), "") {} -Role::Role(RoleImpl *wrapped) : wrapped(std::move(wrapped)) {} +[[ nodiscard ]] auto RoleImpl::GeneratePermissionRulesSet() -> std::unique_ptr<PermissionRules> { + auto result = std::make_unique<PermissionRules>(); + for(auto rule : this->wrapped->permission_rules()) { + result->emplace(rule.permission(), rule.grant(), rule.priority()); + } + return std::move(result); +} + +Role::Role(std::unique_ptr<RoleImpl> wrapped) : wrapped(std::move(wrapped)) {} Role::Role(std::string name, std::string description) - : wrapped(new RoleImpl(std::move(name), std::move(description))) {} + : wrapped(std::make_unique<RoleImpl>(std::move(name), std::move(description))) {} Role::Role(std::string name) : Role(std::move(name), {""}) {} Role::Role(const Role &role) : Role(role.GetName(), role.GetDescription()) {} auto Role::operator=(const Role &role) -> Role & { + if(this == &role) { + return *this; + } this->wrapped->wrapped->CopyFrom(*(role.wrapped->wrapped)); return *this; } -Role::~Role() { delete this->wrapped; } +Role::~Role() = default; auto Role::GetName() const -> const std::string & { return this->wrapped->wrapped->name(); } auto Role::SetName(std::string name) -> void { @@ -91,23 +102,53 @@ auto Role::SetDescription(std::string description) -> void { auto Role::ToString() const -> std::string { return this->wrapped->ToString(); } [[nodiscard]] auto Role::GetPermissionRules() const -> const PermissionRules & { - return this->wrapped->rules; + if(this->wrapped->rules == nullptr) { + this->wrapped->rules = std::move(this->wrapped->GeneratePermissionRulesSet()); + } + return *(this->wrapped->rules); } auto Role::Grant(std::string permission, bool isPriority) -> void { - this->wrapped->rules.emplace(std::move(permission), true, isPriority); + this->AddPermissionRule(permission, true, isPriority); +} + +auto Role::RevokeGrant(const std::string &permission, bool isPriority) -> void { + this->RemovePermissionRule(permission, true, isPriority); +} + +auto Role::Deny(std::string permission, bool isPriority) -> void { + this->AddPermissionRule(permission, false, isPriority); +} + +auto Role::RevokeDenial(const std::string &permission, bool isPriority) -> void { + this->RemovePermissionRule(permission, false, isPriority); +} + +auto Role::AddPermissionRule(std::string permission, bool isGrant, bool isPriority) -> void { + this->RemovePermissionRule(permission, isGrant, isPriority); + auto *new_rule = this->wrapped->wrapped->add_permission_rules(); + new_rule->set_permission(std::move(permission)); + new_rule->set_priority(isPriority); + new_rule->set_grant(isGrant); } -auto Role::RevokeGrant(std::string permission, bool isPriority) -> void { - auto rule = this->wrapped->rules.begin(); - while (rule != this->wrapped->rules.end()) { - if (rule->IsGrant() && rule->IsPriority() == isPriority && - rule->GetPermission() == permission) { - this->wrapped->rules.erase(rule); - return; +auto Role::RemovePermissionRule(const std::string &permission, bool isGrant, bool isPriority) -> void { + this->wrapped->rules.reset(); + auto rule = this->wrapped->wrapped->mutable_permission_rules()->begin(); + while (rule != this->wrapped->wrapped->mutable_permission_rules()->end()) { + if (rule->grant() == isGrant && rule->priority() == isPriority && + rule->permission() == permission) { + this->wrapped->wrapped->mutable_permission_rules()->erase(rule); + rule = this->wrapped->wrapped->mutable_permission_rules()->begin(); } else { rule++; } } } + +auto Role::ClearPermissionRules() -> void { + this->wrapped->rules.reset(); + this->wrapped->wrapped->clear_permission_rules(); +} + } // namespace caosdb::acm diff --git a/src/caosdb/acm/role_impl.h b/src/caosdb/acm/role_impl.h index b30e6e11faa248f5babfeb9234dc276aaccad9b4..5a419796e23536ece7e9d06da794acc8d1fb006e 100644 --- a/src/caosdb/acm/role_impl.h +++ b/src/caosdb/acm/role_impl.h @@ -21,6 +21,7 @@ #include "caosdb/acm/role.h" #include "caosdb/acm/v1alpha1/main.pb.h" // for ListRolesRequest #include "caosdb/protobuf_helper.h" // for ProtoMessageWrapper +#include <memory> // for unique_ptr #include <utility> // for move namespace caosdb::acm { @@ -58,7 +59,8 @@ public: friend class caosdb::connection::Connection; private: - PermissionRules rules; + std::unique_ptr<PermissionRules> rules; + [[ nodiscard ]] auto GeneratePermissionRulesSet() -> std::unique_ptr<PermissionRules>; }; } // namespace caosdb::acm diff --git a/src/caosdb/connection.cpp b/src/caosdb/connection.cpp index 6632933a7787a2ba1284f0cdb00d2c8a478bcdba..f13100c4dd8cf5dfe44f6c511e6cf1e0cda3c688 100644 --- a/src/caosdb/connection.cpp +++ b/src/caosdb/connection.cpp @@ -137,7 +137,7 @@ auto Connection::RetrieveVersionInfo() const -> const VersionInfo & { } status.ThrowExceptionIfError(); auto *role = response.release_role(); - return Role(new RoleImpl(role)); + return Role(std::make_unique<RoleImpl>(role)); } auto Connection::DeleteSingleRole(std::string name) const -> void { @@ -221,8 +221,7 @@ auto Connection::CreateSingleRole(const Role &role) const -> void { auto *roles = response.mutable_roles(); for (auto &role_item : *roles) { - auto *implementation = new RoleImpl(role_item); - result.emplace_back(implementation); + result.emplace_back(std::make_unique<RoleImpl>(role_item)); } return result; } diff --git a/test/test_role.cpp b/test/test_role.cpp index 8b3e68451dfa2bddecc9a88ab174866d2644a0e1..53a569fbe1d18767790da3f8ca47ebf6682e1c7d 100644 --- a/test/test_role.cpp +++ b/test/test_role.cpp @@ -21,10 +21,13 @@ */ #include "caosdb/acm/role.h" // for Role +#include "caosdb/acm/permission_rule.h" // for PermissionRules, PermissionRule #include <gtest/gtest-message.h> // for Message #include <gtest/gtest-test-part.h> // for TestPartResult, SuiteApiRe... #include <gtest/gtest_pred_impl.h> // for Test, EXPECT_EQ, TEST -#include <memory> // for allocator +#include <iostream> // for cout +#include <string> // for allocator, string +#include <unordered_set> // for _Node_const_iterator, operat... namespace caosdb::acm { @@ -50,21 +53,76 @@ TEST(test_role, role_description) { EXPECT_EQ(role2.GetName(), "role2"); } -TEST(test_role, get_permission_rules) { +TEST(test_role, grant_revoke) { auto role1 = Role("role1", "description1"); EXPECT_TRUE(role1.GetPermissionRules().empty()); + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\"\n}\n"); role1.Grant("SOME_PERMISSION", false); EXPECT_FALSE(role1.GetPermissionRules().empty()); + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\",\n \"permissionRules\": [\n {\n \"permission\": \"SOME_PERMISSION\",\n \"grant\": true\n }\n ]\n}\n"); + role1.Grant("SOME_OTHER_PERMISSION", true); - EXPECT_FALSE(role1.GetPermissionRules().empty()); + EXPECT_EQ(role1.GetPermissionRules().size(), 2); + + role1.Grant("SOME_OTHER_PERMISSION", true); + // is a set + EXPECT_EQ(role1.GetPermissionRules().size(), 2); role1.RevokeGrant("SOME_PERMISSION", false); EXPECT_FALSE(role1.GetPermissionRules().empty()); + role1.RevokeGrant("NOT_EXISTING_PERMISSION", true); + + // is immutable + for(const auto &rule : role1.GetPermissionRules()) { + EXPECT_EQ(rule.GetPermission(), "SOME_OTHER_PERMISSION"); + } role1.RevokeGrant("SOME_OTHER_PERMISSION", true); EXPECT_TRUE(role1.GetPermissionRules().empty()); + + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\"\n}\n"); +} + +TEST(test_role, deny_revoke) { + auto role1 = Role("role1", "description1"); + EXPECT_TRUE(role1.GetPermissionRules().empty()); + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\"\n}\n"); + + role1.Deny("SOME_PERMISSION", false); + EXPECT_FALSE(role1.GetPermissionRules().empty()); + + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\",\n \"permissionRules\": [\n {\n \"permission\": \"SOME_PERMISSION\"\n }\n ]\n}\n"); + + role1.Deny("SOME_OTHER_PERMISSION", true); + EXPECT_EQ(role1.GetPermissionRules().size(), 2); + + role1.Deny("SOME_OTHER_PERMISSION", true); + // is a set + EXPECT_EQ(role1.GetPermissionRules().size(), 2); + + role1.RevokeDenial("SOME_PERMISSION", false); + EXPECT_FALSE(role1.GetPermissionRules().empty()); + role1.RevokeDenial("NOT_EXISTING_PERMISSION", true); + + // is immutable + for(const auto &rule : role1.GetPermissionRules()) { + EXPECT_EQ(rule.GetPermission(), "SOME_OTHER_PERMISSION"); + } + + role1.ClearPermissionRules(); + EXPECT_TRUE(role1.GetPermissionRules().empty()); + + EXPECT_EQ(role1.ToString(), "{\n \"name\": \"role1\",\n \"description\": \"description1\"\n}\n"); +} + +TEST(test_role, permission_rule) { + auto rule = PermissionRule("SOME_PERMISSION", true, false); + EXPECT_EQ(rule.GetPermission(), "SOME_PERMISSION"); + EXPECT_TRUE(rule.IsGrant()); + EXPECT_FALSE(rule.IsPriority()); + EXPECT_EQ(rule.ToString(), "{\n \"permission\": \"SOME_PERMISSION\",\n \"grant\": true\n}\n"); } } // namespace caosdb::acm