diff --git a/programs/server/config.xml b/programs/server/config.xml index 2ce3fe7754f..00a0608f607 100644 --- a/programs/server/config.xml +++ b/programs/server/config.xml @@ -640,6 +640,12 @@ executed by any user. You can change this behaviour by setting this to true. If it's set to true then this query requires "GRANT SELECT ON information_schema." just like as for ordinary tables. --> false + + + false diff --git a/src/Access/AccessControl.cpp b/src/Access/AccessControl.cpp index c6729459988..81a887dbaa4 100644 --- a/src/Access/AccessControl.cpp +++ b/src/Access/AccessControl.cpp @@ -171,6 +171,7 @@ void AccessControl::setUpFromMainConfig(const Poco::Util::AbstractConfiguration setOnClusterQueriesRequireClusterGrant(config_.getBool("access_control_improvements.on_cluster_queries_require_cluster_grant", false)); setSelectFromSystemDatabaseRequiresGrant(config_.getBool("access_control_improvements.select_from_system_db_requires_grant", false)); setSelectFromInformationSchemaRequiresGrant(config_.getBool("access_control_improvements.select_from_information_schema_requires_grant", false)); + setSettingsConstraintsReplacePrevious(config_.getBool("access_control_improvements.settings_constraints_replace_previous", false)); addStoragesFromMainConfig(config_, config_path_, get_zookeeper_function_); } diff --git a/src/Access/AccessControl.h b/src/Access/AccessControl.h index ab9cdba9ad1..e8a787ada0b 100644 --- a/src/Access/AccessControl.h +++ b/src/Access/AccessControl.h @@ -158,6 +158,9 @@ public: void setSelectFromInformationSchemaRequiresGrant(bool enable) { select_from_information_schema_requires_grant = enable; } bool doesSelectFromInformationSchemaRequireGrant() const { return select_from_information_schema_requires_grant; } + void setSettingsConstraintsReplacePrevious(bool enable) { settings_constraints_replace_previous = enable; } + bool doesSettingsConstraintsReplacePrevious() const { return settings_constraints_replace_previous; } + std::shared_ptr getContextAccess( const UUID & user_id, const std::vector & current_roles, @@ -223,6 +226,7 @@ private: std::atomic_bool on_cluster_queries_require_cluster_grant = false; std::atomic_bool select_from_system_db_requires_grant = false; std::atomic_bool select_from_information_schema_requires_grant = false; + std::atomic_bool settings_constraints_replace_previous = false; }; } diff --git a/src/Access/SettingsConstraints.cpp b/src/Access/SettingsConstraints.cpp index a9264cb4ccd..f49847ad701 100644 --- a/src/Access/SettingsConstraints.cpp +++ b/src/Access/SettingsConstraints.cpp @@ -36,65 +36,53 @@ void SettingsConstraints::clear() } -void SettingsConstraints::constrainMinValue(const String & setting_name, const Field & min_value) +void SettingsConstraints::setMinValue(const String & setting_name, const Field & min_value) { constraints[setting_name].min_value = Settings::castValueUtil(setting_name, min_value); } -void SettingsConstraints::constrainMaxValue(const String & setting_name, const Field & max_value) +void SettingsConstraints::setMaxValue(const String & setting_name, const Field & max_value) { constraints[setting_name].max_value = Settings::castValueUtil(setting_name, max_value); } -void SettingsConstraints::constrainReadOnly(const String & setting_name, bool read_only) +void SettingsConstraints::setIsConst(const String & setting_name, bool is_const) { - constraints[setting_name].read_only = read_only; + constraints[setting_name].is_const = is_const; } -void SettingsConstraints::allowMinValue(const String & setting_name, const Field & min_value) +void SettingsConstraints::setChangableInReadonly(const String & setting_name, bool changeable_in_readonly) { - allowances[setting_name].min_value = Settings::castValueUtil(setting_name, min_value); + constraints[setting_name].changeable_in_readonly = changeable_in_readonly; } -void SettingsConstraints::allowMaxValue(const String & setting_name, const Field & max_value) -{ - allowances[setting_name].max_value = Settings::castValueUtil(setting_name, max_value); -} - -void SettingsConstraints::allowReadOnly(const String & setting_name, bool read_only) -{ - allowances[setting_name].read_only = read_only; -} - -void SettingsConstraints::get(const Settings & current_settings, std::string_view setting_name, Field & min_value, Field & max_value, bool & read_only) const +void SettingsConstraints::get(const Settings & current_settings, std::string_view setting_name, Field & min_value, Field & max_value, bool & is_const) const { auto range = getRange(current_settings, setting_name); min_value = range.min_value; max_value = range.max_value; - read_only = range.read_only; + is_const = range.is_const; } void SettingsConstraints::merge(const SettingsConstraints & other) { - for (const auto & [other_name, other_constraint] : other.constraints) + if (access_control.doesSettingsConstraintsReplacePrevious()) { - auto & constraint = constraints[other_name]; - if (!other_constraint.min_value.isNull()) - constraint.min_value = other_constraint.min_value; - if (!other_constraint.max_value.isNull()) - constraint.max_value = other_constraint.max_value; - if (other_constraint.read_only) - constraint.read_only = true; + for (const auto & [other_name, other_constraint] : other.constraints) + constraints[other_name] = other_constraint; } - for (const auto & [other_name, other_allowance] : other.allowances) + else { - auto & allowance = allowances[other_name]; - if (!other_allowance.min_value.isNull()) - allowance.min_value = other_allowance.min_value; - if (!other_allowance.max_value.isNull()) - allowance.max_value = other_allowance.max_value; - if (other_allowance.read_only) - allowance.read_only = true; + for (const auto & [other_name, other_constraint] : other.constraints) + { + auto & constraint = constraints[other_name]; + if (!other_constraint.min_value.isNull()) + constraint.min_value = other_constraint.min_value; + if (!other_constraint.max_value.isNull()) + constraint.max_value = other_constraint.max_value; + if (other_constraint.is_const) + constraint.is_const = true; // NOTE: In this mode flag cannot be overriden to be false + } } } @@ -229,7 +217,7 @@ bool SettingsConstraints::Range::check(SettingChange & change, const Field & new return false; } - if (read_only) + if (is_const) { if (reaction == THROW_ON_VIOLATION) throw Exception("Setting " + setting_name + " should not be changed", ErrorCodes::SETTING_CONSTRAINT_VIOLATION); @@ -279,36 +267,34 @@ SettingsConstraints::Range SettingsConstraints::getRange(const Settings & curren /** The `readonly` value is understood as follows: * 0 - no read-only restrictions. - * 1 - only read requests, as well as changing explicitly allowed settings. + * 1 - only read requests, as well as changing settings with `changable_in_readonly` flag. * 2 - only read requests, as well as changing settings, except for the `readonly` setting. */ if (current_settings.readonly > 1 && setting_name == "readonly") return Range::forbidden("Cannot modify 'readonly' setting in readonly mode", ErrorCodes::READONLY); + auto it = constraints.find(setting_name); if (current_settings.readonly == 1) { - auto it = allowances.find(setting_name); - if (it == allowances.end()) + if (it == constraints.end() || !it->second.changeable_in_readonly) return Range::forbidden("Cannot modify '" + String(setting_name) + "' setting in readonly mode", ErrorCodes::READONLY); - return it->second; } else // For both readonly=0 and readonly=2 { - auto it = constraints.find(setting_name); if (it == constraints.end()) return Range::allowed(); - return it->second; } + return it->second; } bool SettingsConstraints::Range::operator==(const Range & other) const { - return read_only == other.read_only && min_value == other.min_value && max_value == other.max_value; + return is_const == other.is_const && changeable_in_readonly == other.changeable_in_readonly && min_value == other.min_value && max_value == other.max_value; } bool operator ==(const SettingsConstraints & left, const SettingsConstraints & right) { - return left.constraints == right.constraints && left.allowances == right.allowances; + return left.constraints == right.constraints; } } diff --git a/src/Access/SettingsConstraints.h b/src/Access/SettingsConstraints.h index c6cd54a9e68..f9dedb49b68 100644 --- a/src/Access/SettingsConstraints.h +++ b/src/Access/SettingsConstraints.h @@ -35,15 +35,12 @@ class AccessControl; * 20000000000 * * - * + * * + * + * + * * - * - * - * 200000 - * 10000000000 - * - * * * * @@ -51,7 +48,7 @@ class AccessControl; * If a setting cannot be change due to the read-only mode this class throws an exception. * The value of `readonly` is understood as follows: * 0 - not read-only mode, no additional checks. - * 1 - only read queries, as well as changing explicitly allowed settings. + * 1 - only read queries, as well as changing settings with flag. * 2 - only read queries and you can change the settings, except for the `readonly` setting. * */ @@ -68,14 +65,12 @@ public: void clear(); bool empty() const { return constraints.empty(); } - void constrainMinValue(const String & setting_name, const Field & min_value); - void constrainMaxValue(const String & setting_name, const Field & max_value); - void constrainReadOnly(const String & setting_name, bool read_only); - void allowMinValue(const String & setting_name, const Field & min_value); - void allowMaxValue(const String & setting_name, const Field & max_value); - void allowReadOnly(const String & setting_name, bool read_only); + void setMinValue(const String & setting_name, const Field & min_value); + void setMaxValue(const String & setting_name, const Field & max_value); + void setIsConst(const String & setting_name, bool is_const); + void setChangableInReadonly(const String & setting_name, bool is_const); - void get(const Settings & current_settings, std::string_view setting_name, Field & min_value, Field & max_value, bool & read_only) const; + void get(const Settings & current_settings, std::string_view setting_name, Field & min_value, Field & max_value, bool & is_const) const; void merge(const SettingsConstraints & other); @@ -99,7 +94,8 @@ private: struct Range { - bool read_only = false; + bool is_const = false; + bool changeable_in_readonly = false; Field min_value; Field max_value; @@ -119,7 +115,7 @@ private: static Range forbidden(const String & explain, int code) { - return Range{.read_only = true, .explain = explain, .code = code}; + return Range{.is_const = true, .explain = explain, .code = code}; } }; @@ -143,7 +139,6 @@ private: // Special container for heterogeneous lookups: to avoid `String` construction during `find(std::string_view)` using RangeMap = std::unordered_map>; RangeMap constraints; - RangeMap allowances; const AccessControl * access_control = nullptr; }; diff --git a/src/Access/SettingsProfileElement.cpp b/src/Access/SettingsProfileElement.cpp index 7ff182fd139..7cd5d127815 100644 --- a/src/Access/SettingsProfileElement.cpp +++ b/src/Access/SettingsProfileElement.cpp @@ -56,7 +56,23 @@ void SettingsProfileElement::init(const ASTSettingsProfileElement & ast, const A value = ast.value; min_value = ast.min_value; max_value = ast.max_value; - readonly = ast.readonly; + changeable_in_readonly = false; + switch (ast.type) + { + case ASTSettingsProfileElement::ConstraintType::NONE: + is_const.reset(); + break; + case ASTSettingsProfileElement::ConstraintType::WRITABLE: + is_const.emplace(false); + break; + case ASTSettingsProfileElement::ConstraintType::CONST: + is_const.emplace(true); + break; + case ASTSettingsProfileElement::ConstraintType::CHANGEABLE_IN_READONLY: + is_const.reset(); + changeable_in_readonly = true; + break; + } if (!value.isNull()) value = Settings::castValueUtil(setting_name, value); @@ -208,25 +224,14 @@ SettingsConstraints SettingsProfileElements::toSettingsConstraints(const AccessC { if (!elem.setting_name.empty() && (elem.setting_name != ALLOW_BACKUP_SETTING_NAME)) { - switch (elem.kind) - { - case SettingsProfileElement::RangeKind::Constrain: - if (!elem.min_value.isNull()) - res.constrainMinValue(elem.setting_name, elem.min_value); - if (!elem.max_value.isNull()) - res.constrainMaxValue(elem.setting_name, elem.max_value); - if (elem.readonly) - res.constrainReadOnly(elem.setting_name, *elem.readonly); - break; - case SettingsProfileElement::RangeKind::Allow: - if (!elem.min_value.isNull()) - res.allowMinValue(elem.setting_name, elem.min_value); - if (!elem.max_value.isNull()) - res.allowMaxValue(elem.setting_name, elem.max_value); - if (elem.readonly) - res.allowReadOnly(elem.setting_name, *elem.readonly); - break; - } + if (!elem.min_value.isNull()) + res.setMinValue(elem.setting_name, elem.min_value); + if (!elem.max_value.isNull()) + res.setMaxValue(elem.setting_name, elem.max_value); + if (elem.is_const) + res.setIsConst(elem.setting_name, *elem.is_const); + if (elem.changeable_in_readonly) + res.setChangableInReadonly(elem.setting_name, true); } } return res; diff --git a/src/Access/SettingsProfileElement.h b/src/Access/SettingsProfileElement.h index d8cd6a3ca81..2d6e5cf7b2f 100644 --- a/src/Access/SettingsProfileElement.h +++ b/src/Access/SettingsProfileElement.h @@ -21,20 +21,14 @@ struct SettingsProfileElement { std::optional parent_profile; - enum class RangeKind - { - Constrain = 0, - Allow - }; - String setting_name; Field value; - RangeKind kind = RangeKind::Constrain; Field min_value; Field max_value; - std::optional readonly; + std::optional is_const; + bool changeable_in_readonly; - auto toTuple() const { return std::tie(parent_profile, setting_name, value, kind, min_value, max_value, readonly); } + auto toTuple() const { return std::tie(parent_profile, setting_name, value, min_value, max_value, is_const, changeable_in_readonly); } friend bool operator==(const SettingsProfileElement & lhs, const SettingsProfileElement & rhs) { return lhs.toTuple() == rhs.toTuple(); } friend bool operator!=(const SettingsProfileElement & lhs, const SettingsProfileElement & rhs) { return !(lhs == rhs); } friend bool operator <(const SettingsProfileElement & lhs, const SettingsProfileElement & rhs) { return lhs.toTuple() < rhs.toTuple(); } diff --git a/src/Parsers/Access/ASTSettingsProfileElement.cpp b/src/Parsers/Access/ASTSettingsProfileElement.cpp index 23dba8a926f..12de74ab417 100644 --- a/src/Parsers/Access/ASTSettingsProfileElement.cpp +++ b/src/Parsers/Access/ASTSettingsProfileElement.cpp @@ -52,10 +52,22 @@ void ASTSettingsProfileElement::formatImpl(const FormatSettings & settings, Form << applyVisitor(FieldVisitorToString{}, max_value); } - if (readonly) + switch (type) { - settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << (*readonly ? " READONLY" : " WRITABLE") - << (settings.hilite ? IAST::hilite_none : ""); + case ConstraintType::NONE: + break; + case ConstraintType::WRITABLE: + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " WRITABLE" + << (settings.hilite ? IAST::hilite_none : ""); + break; + case ConstraintType::CONST: + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " READONLY" + << (settings.hilite ? IAST::hilite_none : ""); + break; + case ConstraintType::CHANGEABLE_IN_READONLY: + settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " CHANGEABLE_IN_READONLY" + << (settings.hilite ? IAST::hilite_none : ""); + break; } } diff --git a/src/Parsers/Access/ASTSettingsProfileElement.h b/src/Parsers/Access/ASTSettingsProfileElement.h index 6a54bca3213..d31f409cbd8 100644 --- a/src/Parsers/Access/ASTSettingsProfileElement.h +++ b/src/Parsers/Access/ASTSettingsProfileElement.h @@ -7,7 +7,7 @@ namespace DB { /** Represents a settings profile's element like the following - * {variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE]} | PROFILE 'profile_name' + * {variable [= value] [MIN [=] min_value] [MAX [=] max_value] [CONST|READONLY|WRITABLE|CHANGEABLE_IN_READONLY]} | PROFILE 'profile_name' */ class ASTSettingsProfileElement : public IAST { @@ -17,7 +17,13 @@ public: Field value; Field min_value; Field max_value; - std::optional readonly; + enum class ConstraintType { + NONE, // default + WRITABLE, + CONST, // equals READONLY + CHANGEABLE_IN_READONLY, + }; + ConstraintType type; bool id_mode = false; /// If true then `parent_profile` keeps UUID, not a name. bool use_inherit_keyword = false; /// If true then this element is a part of ASTCreateSettingsProfileQuery. @@ -30,7 +36,7 @@ public: /** Represents settings profile's elements like the following - * {{variable [= value] [MIN [=] min_value] [MAX [=] max_value] [READONLY|WRITABLE]} | PROFILE 'profile_name'} [,...] + * {{variable [= value] [MIN [=] min_value] [MAX [=] max_value] [CONST|READONLY|WRITABLE|CHANGEABLE_IN_READONLY]} | PROFILE 'profile_name'} [,...] */ class ASTSettingsProfileElements : public IAST { diff --git a/src/Storages/System/StorageSystemSettings.cpp b/src/Storages/System/StorageSystemSettings.cpp index a0a8803169b..35c9ea442d4 100644 --- a/src/Storages/System/StorageSystemSettings.cpp +++ b/src/Storages/System/StorageSystemSettings.cpp @@ -40,8 +40,9 @@ void StorageSystemSettings::fillData(MutableColumns & res_columns, ContextPtr co res_columns[3]->insert(setting.getDescription()); Field min, max; - bool read_only = false; - constraints.get(settings, setting_name, min, max, read_only); + bool is_const = false; + bool changeable_in_readonly = false; + constraints.get(settings, setting_name, min, max, is_const, changeable_in_readonly); /// These two columns can accept strings only. if (!min.isNull()) diff --git a/src/Storages/System/StorageSystemSettingsProfileElements.cpp b/src/Storages/System/StorageSystemSettingsProfileElements.cpp index b1f2ad36999..ea23b052764 100644 --- a/src/Storages/System/StorageSystemSettingsProfileElements.cpp +++ b/src/Storages/System/StorageSystemSettingsProfileElements.cpp @@ -29,7 +29,7 @@ NamesAndTypesList StorageSystemSettingsProfileElements::getNamesAndTypes() {"min", std::make_shared(std::make_shared())}, {"max", std::make_shared(std::make_shared())}, {"readonly", std::make_shared(std::make_shared())}, - {"allowance", std::make_shared(std::make_shared())}, + {"changeable_in_readonly", std::make_shared(std::make_shared())}, {"inherit_profile", std::make_shared(std::make_shared())}, }; return names_and_types; @@ -65,8 +65,8 @@ void StorageSystemSettingsProfileElements::fillData(MutableColumns & res_columns auto & column_max_null_map = assert_cast(*res_columns[i++]).getNullMapData(); auto & column_readonly = assert_cast(assert_cast(*res_columns[i]).getNestedColumn()).getData(); auto & column_readonly_null_map = assert_cast(*res_columns[i++]).getNullMapData(); - auto & column_allowance = assert_cast(assert_cast(*res_columns[i]).getNestedColumn()).getData(); - auto & column_allowance_null_map = assert_cast(*res_columns[i++]).getNullMapData(); + auto & column_changeable_in_readonly = assert_cast(assert_cast(*res_columns[i]).getNestedColumn()).getData(); + auto & column_changeable_in_readonly_null_map = assert_cast(*res_columns[i++]).getNullMapData(); auto & column_inherit_profile = assert_cast(assert_cast(*res_columns[i]).getNestedColumn()); auto & column_inherit_profile_null_map = assert_cast(*res_columns[i++]).getNullMapData(); @@ -104,23 +104,23 @@ void StorageSystemSettingsProfileElements::fillData(MutableColumns & res_columns } bool inserted_readonly = false; - if (element.readonly && !element.setting_name.empty()) + if (element.is_const && !element.setting_name.empty()) { column_readonly.push_back(*element.readonly); column_readonly_null_map.push_back(false); inserted_readonly = true; } - bool inserted_allowance = false; - if (element.kind == SettingsProfileElement::RangeKind::Allow && !element.setting_name.empty()) + bool inserted_changeable_in_readonly = false; + if (element.changeable_in_readonly && !element.setting_name.empty()) { - column_allowance.push_back(true); - column_allowance_null_map.push_back(false); - inserted_allowance = true; + column_changeable_in_readonly.push_back(true); + column_changeable_in_readonly_null_map.push_back(false); + inserted_changeable_in_readonly = true; } bool inserted_setting_name = false; - if (inserted_value || inserted_min || inserted_max || inserted_readonly || inserted_allowance) + if (inserted_value || inserted_min || inserted_max || inserted_readonly || inserted_changeable_in_readonly) { const auto & setting_name = element.setting_name; column_setting_name.insertData(setting_name.data(), setting_name.size()); diff --git a/tests/config/config.d/enable_access_control_improvements.xml b/tests/config/config.d/enable_access_control_improvements.xml index 5a186548098..564b656a0ad 100644 --- a/tests/config/config.d/enable_access_control_improvements.xml +++ b/tests/config/config.d/enable_access_control_improvements.xml @@ -4,5 +4,6 @@ true true true + true diff --git a/tests/integration/helpers/0_common_instance_config.xml b/tests/integration/helpers/0_common_instance_config.xml index 64f0ce9e361..27563e47c35 100644 --- a/tests/integration/helpers/0_common_instance_config.xml +++ b/tests/integration/helpers/0_common_instance_config.xml @@ -24,5 +24,6 @@ true true true + true diff --git a/tests/integration/test_disabled_access_control_improvements/configs/config.d/disable_access_control_improvements.xml b/tests/integration/test_disabled_access_control_improvements/configs/config.d/disable_access_control_improvements.xml index 7969c638fd7..a335c7f8a1f 100644 --- a/tests/integration/test_disabled_access_control_improvements/configs/config.d/disable_access_control_improvements.xml +++ b/tests/integration/test_disabled_access_control_improvements/configs/config.d/disable_access_control_improvements.xml @@ -3,5 +3,6 @@ +