diff --git a/CHANGELOG.md b/CHANGELOG.md index ded816b3b90a9e3a4cde5f3bd1b9683a879ee977..61fb775373f331e6afeff20b8f9562eb6403351f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ to 2 and 3. ### Fixes - It's no longer possible to execute DML queries for tables that are not operable +- Fixed panic on user/role creation when max user number was exceeded ## [24.6.1] - 2024-10-28 diff --git a/src/cas.rs b/src/cas.rs index fdf2045fe2cc56819bd2547a87bd975c5c5070bb..4974275eb94110540581b12089fc2c535c95b225 100644 --- a/src/cas.rs +++ b/src/cas.rs @@ -8,7 +8,7 @@ use crate::tlog; use crate::traft; use crate::traft::error::Error as TraftError; use crate::traft::node; -use crate::traft::op::{Ddl, Dml, Op}; +use crate::traft::op::{Acl, Ddl, Dml, Op}; use crate::traft::EntryContext; use crate::traft::Result; use crate::traft::{RaftIndex, RaftTerm}; @@ -66,6 +66,14 @@ pub fn check_dml_prohibited(space_id: SpaceId) -> traft::Result<()> { Ok(()) } +pub fn check_acl_limits(storage: &Clusterwide, acl: &Acl) -> traft::Result<()> { + if matches!(acl, Acl::CreateUser { .. } | Acl::CreateRole { .. }) { + storage.users.check_user_limit() + } else { + Ok(()) + } +} + /// Performs a clusterwide compare and swap operation. Waits until the /// resulting entry is applied locally. /// @@ -292,6 +300,9 @@ fn proc_cas_local(req: &Request) -> Result<Response> { check_dml_prohibited(dml.space())?; } } + Op::Acl(acl) => { + check_acl_limits(storage, acl)?; + } _ => {} } diff --git a/src/sql.rs b/src/sql.rs index 7618f5e84dfc5b142336ddbd48e9c9eca7249ef3..e3a82f5b1c80a4e251b136cbd08e407ba3ba2204 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -972,6 +972,7 @@ fn acl_ir_node_to_op_or_result( .. }) => { check_name_emptyness(name)?; + storage.users.check_user_limit()?; let sys_user = Space::from(SystemSpace::User) .index("name") @@ -1007,6 +1008,8 @@ fn acl_ir_node_to_op_or_result( .. }) => { check_name_emptyness(name)?; + storage.users.check_user_limit()?; + let method = parse_auth_method(auth_method)?; validate_password(password, &method, storage)?; let data = AuthData::new(&method, name, password); diff --git a/src/storage.rs b/src/storage.rs index e5cbb2616a226783a6809e67c81e7e8fb8312937..a7abd153f2c0fa9c6dee5eee303346dc0b165f41 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -1846,6 +1846,9 @@ impl ToEntryIter<MP_SERDE> for Indexes { // Users //////////////////////////////////////////////////////////////////////////////// +/// The hard upper bound (32) for max users comes from tarantool BOX_USER_MAX +const MAX_USERS: usize = 32; + impl Users { pub fn new() -> tarantool::Result<Self> { let space = Space::builder(Self::TABLE_NAME) @@ -1988,6 +1991,17 @@ impl Users { self.space.update(&[user_id], ops)?; Ok(()) } + + #[inline] + pub fn check_user_limit(&self) -> traft::Result<()> { + if self.space.len()? >= MAX_USERS { + return Err(Error::Other( + format!("a limit on the total number of users has been reached: {MAX_USERS}") + .into(), + )); + } + Ok(()) + } } impl ToEntryIter<MP_SERDE> for Users { diff --git a/test/int/test_limits.py b/test/int/test_limits.py index e52f5f5f94f66e13aea81e7d3bb1a30e1f7f0bcf..0a71be8807a3da9bd5acbf8e47a6101f30f284aa 100644 --- a/test/int/test_limits.py +++ b/test/int/test_limits.py @@ -1,8 +1,12 @@ import pytest from conftest import Cluster, TarantoolError + +# The hard upper bound (32) for max users comes from tarantool BOX_USER_MAX +# 32 - sys users in picodata = 26 max_picodata_users = 26 + def test_user_limit(cluster: Cluster): cluster.deploy(instance_count=2) i1, i2 = cluster.instances @@ -20,7 +24,6 @@ def test_user_limit(cluster: Cluster): ) assert acl["row_count"] == 1 - # FIXME: should not panic, should be an error instead with pytest.raises( TarantoolError, match="a limit on the total number of users has been reached: 32", @@ -33,6 +36,7 @@ def test_user_limit(cluster: Cluster): """ ) + def test_role_limit(cluster: Cluster): cluster.deploy(instance_count=2) i1, i2 = cluster.instances @@ -43,7 +47,6 @@ def test_role_limit(cluster: Cluster): acl = i1.sql(f"create role {role}") assert acl["row_count"] == 1 - # FIXME: should not panic, should be an error instead with pytest.raises( TarantoolError, match="a limit on the total number of users has been reached: 32", diff --git a/test/pgproto/plugin_test.py b/test/pgproto/plugin_test.py index d365a8e1cc4136c0f55170b87380e113d2fbb7d3..015f909fa2d40cd04df4d417f7290622177d65c9 100644 --- a/test/pgproto/plugin_test.py +++ b/test/pgproto/plugin_test.py @@ -43,7 +43,7 @@ def test_create_plugin(postgres: Postgres): with pytest.raises(pg.DatabaseError, match="already exists"): cur.execute(f"CREATE PLUGIN {PLUGIN} {VERSION_1}") - cur.execute(f"DROP PLUGIN {PLUGIN} {VERSION_1 }") + cur.execute(f"DROP PLUGIN {PLUGIN} {VERSION_1}") cur.execute( f""" CREATE PLUGIN {PLUGIN} {VERSION_1} OPTION (TIMEOUT = 1.1)