From 7ebd80741c30ac98114cad3c7d64a822509d8495 Mon Sep 17 00:00:00 2001 From: Kory Draughn Date: Thu, 25 Feb 2021 17:35:29 -0500 Subject: [PATCH] [#16][#17] Plugin does not throw exception on missing JSON config. Only users meeting the requirements of the JSON configuration are allowed to manipulate protected metadata. This now includes adding metadata. --- .../test_rule_engine_plugin_metadata_guard.py | 38 +++++++- src/main.cpp | 92 +++++++------------ 2 files changed, 67 insertions(+), 63 deletions(-) diff --git a/packaging/test_rule_engine_plugin_metadata_guard.py b/packaging/test_rule_engine_plugin_metadata_guard.py index 6ecff9e..d5d5169 100644 --- a/packaging/test_rule_engine_plugin_metadata_guard.py +++ b/packaging/test_rule_engine_plugin_metadata_guard.py @@ -58,7 +58,7 @@ def test_incorrect_configuration_does_not_block_usage(self): self.assertTrue(count > 0) @unittest.skipIf(test.settings.RUN_IN_TOPOLOGY, "Skip for Topology Testing") - def test_users_can_modify_guarded_metadata(self): + def test_authorized_users_can_modify_guarded_metadata(self): config = IrodsConfig() # Set JSON configuration for the root collection. @@ -91,7 +91,7 @@ def test_users_can_modify_guarded_metadata(self): self.rods.assert_icommand(['imeta', 'rm', '-C', root_coll, self.metadata_guard_attribute_name(), json_config]) @unittest.skipIf(test.settings.RUN_IN_TOPOLOGY, "Skip for Topology Testing") - def test_users_cannot_modify_guarded_metadata(self): + def test_unauthorized_users_cannot_modify_guarded_metadata(self): config = IrodsConfig() root_coll = os.path.join('/', self.admin.zone_name) @@ -128,15 +128,49 @@ def test_users_cannot_modify_guarded_metadata(self): coll = self.admin.session_collection attribute_name = 'irods::guarded_attribute' self.admin.assert_icommand(['imeta', 'set', '-C', coll, attribute_name, 'abc']) + self.admin.assert_icommand(['imeta', 'add', '-C', coll, attribute_name, 'def']) + + def check_metadata(): + out, err, ec = self.admin.run_icommand(['imeta', 'ls', '-C', coll]) + self.assertEquals(ec, 0) + self.assertEquals(len(err), 0) + self.assertTrue('attribute: {0}\nvalue: {1}'.format(attribute_name, 'abc') in out) + self.assertTrue('attribute: {0}\nvalue: {1}'.format(attribute_name, 'def') in out) + self.assertTrue('attribute: {0}\nvalue: {1}'.format(attribute_name, 'DEF') not in out) + self.assertTrue('attribute: {0}\nvalue: {1}'.format(attribute_name, 'GHI') not in out) + + # Verify that the metadata is what we expect. + check_metadata() + + # Give an unauthorized user write access to the collection with protected metadata. self.admin.assert_icommand(['ichmod', 'write', self.user.username, coll]) self.user.assert_icommand(['imeta', 'set', '-C', coll, attribute_name, 'DEF'], 'STDERR', ['CAT_INSUFFICIENT_PRIVILEGE_LEVEL']) + self.user.assert_icommand(['imeta', 'add', '-C', coll, attribute_name, 'GHI'], 'STDERR', ['CAT_INSUFFICIENT_PRIVILEGE_LEVEL']) + + # Show that the plugin protected the metadata. + check_metadata() # Clean up. self.rods.assert_icommand(['imeta', 'rm', '-C', root_coll, self.metadata_guard_attribute_name(), json_config]) self.rods.assert_icommand(['iadmin', 'rfg', 'rodsadmin', self.admin.username]) + @unittest.skipIf(test.settings.RUN_IN_TOPOLOGY, "Skip for Topology Testing") + def test_plugin_does_not_throw_exception_when_json_config_has_not_been_set_as_metadata(self): + config = IrodsConfig() + + with lib.file_backed_up(config.server_config_path): + self.enable_rule_engine_plugin(config) + + log_offset = lib.get_file_size_by_path(paths.server_log_path()) + self.admin.assert_icommand(['imeta', 'set', '-C', self.admin.session_collection, 'a', 'v']) + self.admin.assert_icommand(['imeta', 'ls', '-C', self.admin.session_collection], 'STDOUT', ['attribute: a', 'value: v']) + + self.assertEquals(lib.count_occurrences_of_string_in_log(paths.server_log_path(), 'error', log_offset), 0) + self.assertEquals(lib.count_occurrences_of_string_in_log(paths.server_log_path(), 'exception', log_offset), 0) + self.assertEquals(lib.count_occurrences_of_string_in_log(paths.server_log_path(), 'SYS_CONFIG_FILE_ERR', log_offset), 0) + # # Utility Functions # diff --git a/src/main.cpp b/src/main.cpp index cf754cf..a005c14 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,5 +1,3 @@ -#include "user_administration.hpp" - #include #include #include @@ -12,7 +10,10 @@ #include #include #include +#include +#include +#include #include #include @@ -21,6 +22,7 @@ #include #include #include +#include namespace { @@ -38,50 +40,31 @@ namespace return *rei; } - template - auto sudo(ruleExecInfo_t& _rei, Function _func) -> decltype(_func()) - { - auto& auth_flag = _rei.rsComm->clientUser.authInfo.authFlag; - const auto old_auth_flag = auth_flag; - - // Elevate privileges. - auth_flag = LOCAL_PRIV_USER_AUTH; - - // Restore authorization flags on exit. - irods::at_scope_exit at_scope_exit{[&auth_flag, old_auth_flag] { - auth_flag = old_auth_flag; - }}; - - return _func(); - } - - auto load_plugin_config(ruleExecInfo_t& _rei) -> json + auto load_plugin_config(ruleExecInfo_t& _rei) -> std::optional { // Must elevate privileges so that the configuration can be retrieved. // Users who aren't administrators cannot retrieve metadata they don't own. - return sudo(_rei, [&_rei] { - std::string json_string; + irods::experimental::scoped_privileged_client spc{*_rei.rsComm}; - std::string gql = "select META_COLL_ATTR_VALUE " - "where META_COLL_ATTR_NAME = 'irods::metadata_guard' and COLL_NAME = '/"; - gql += _rei.rsComm->myEnv.rodsZone; - gql += "'"; + const auto gql = fmt::format("select META_COLL_ATTR_VALUE " + "where META_COLL_ATTR_NAME = 'irods::metadata_guard' and COLL_NAME = '/{}'", + _rei.rsComm->myEnv.rodsZone); - for (auto&& row : irods::query{_rei.rsComm, gql}) { - json_string = row[0]; + if (irods::query q{_rei.rsComm, gql}; q.size() > 0) { + try { + return json::parse(q.front()[0]); } - - if (json_string.empty()) { - const char* msg = "Rule Engine Plugin Configuration not set as metadata"; - rodsLog(LOG_ERROR, "[metadata_guard] %s", msg); + catch (const json::exception&) { + const char* msg = "Cannot parse Rule Engine Plugin configuration"; + rodsLog(LOG_ERROR, "[metadata_guard] %s.", msg); THROW(SYS_CONFIG_FILE_ERR, msg); } + } - return json::parse(json_string); - }); + return std::nullopt; } - auto user_is_administrator(const rsComm_t& conn) -> irods::error + auto user_is_administrator(rsComm_t& conn) -> irods::error { if (irods::is_privileged_client(conn)) { return CODE(RULE_ENGINE_CONTINUE); @@ -119,21 +102,12 @@ namespace try { auto* input = boost::any_cast(*std::next(std::begin(_rule_arguments), 2)); auto& rei = get_rei(_effect_handler); + const auto config = load_plugin_config(rei); - const auto is_modification = [op = std::string_view{input->arg0}]() noexcept - { - static const auto ops = {"set", "mod", "rm", "rmw", "rmi"}; - return std::any_of(std::begin(ops), std::end(ops), [&op](auto&& mod_op) { - return op == mod_op; - }); - }; - - if (!is_modification()) { + if (!config) { return CODE(RULE_ENGINE_CONTINUE); } - const auto config = load_plugin_config(rei); - // JSON Configuration structure: // { // "prefixes": ["irods::"], @@ -144,30 +118,29 @@ namespace // {"type": "user", "name": "jane#otherZone"} // ] // } - - for (auto&& prefix : config.at("prefixes")) { + for (auto&& prefix : config->at("prefixes")) { // If the metadata attribute starts with the prefix, then verify that the user // can modify the metadata attribute. if (boost::starts_with(input->arg3, prefix.get())) { // The "admin_only" flag supersedes the "editors" configuration option. - if (config.count("admin_only") && config.at("admin_only").get()) { + if (config->count("admin_only") && config->at("admin_only").get()) { return user_is_administrator(*rei.rsComm); } - namespace ua = irods::experimental::administration; + namespace adm = irods::experimental::administration; - const ua::user user{rei.uoic->userName, rei.uoic->rodsZone}; + const adm::user user{rei.uoic->userName, rei.uoic->rodsZone}; - for (auto&& editor : config.at("editors")) { + for (auto&& editor : config->at("editors")) { if (const auto type = editor.at("type").get(); type == "group") { - const ua::group group{editor.at("name").get()}; + const adm::group group{editor.at("name").get()}; - if (ua::server::user_is_member_of_group(*rei.rsComm, group, user)) { + if (adm::server::user_is_member_of_group(*rei.rsComm, group, user)) { return CODE(RULE_ENGINE_CONTINUE); } } else if (type == "user") { - if (editor.at("name").get() == ua::server::local_unique_name(*rei.rsComm, user)) { + if (editor.at("name").get() == adm::server::local_unique_name(*rei.rsComm, user)) { return CODE(RULE_ENGINE_CONTINUE); } } @@ -177,15 +150,12 @@ namespace } } - rodsLog(LOG_ERROR, "[metadata_guard] User is not allowed to modify metadata [attribute => %s].", input->arg3); + rodsLog(LOG_ERROR, "[metadata_guard] User is not allowed to modify metadata [attribute=%s].", input->arg3); return ERROR(CAT_INSUFFICIENT_PRIVILEGE_LEVEL, "User is not allowed to modify metadata"); } - catch (const json::parse_error& e) { - rodsLog(LOG_ERROR, "[metadata_guard] Cannot parse Rule Engine Plugin configuration."); - } - catch (const json::type_error& e) { - rodsLog(LOG_ERROR, "[metadata_guard] Missing or incorrect configuration properties."); + catch (const json::exception&) { + rodsLog(LOG_ERROR, "[metadata_guard] Unexpected JSON access or type error."); } catch (const std::exception& e) { rodsLog(LOG_ERROR, "[metadata_guard] %s", e.what());