From 484a32eddfc518e46e31f431cb4a7acd240428f6 Mon Sep 17 00:00:00 2001
From: Egor Ivkov <e.o.ivkov@gmail.com>
Date: Thu, 12 Dec 2024 23:10:36 +0300
Subject: [PATCH] fix: check max users on user creation
---
CHANGELOG.md | 1 +
src/cas.rs | 13 ++++++++++++-
src/sql.rs | 3 +++
src/storage.rs | 14 ++++++++++++++
test/int/test_limits.py | 7 +++++--
test/pgproto/plugin_test.py | 2 +-
6 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ded816b3b9..61fb775373 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 fdf2045fe2..4974275eb9 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 7618f5e84d..e3a82f5b1c 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 e5cbb2616a..a7abd153f2 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 e52f5f5f94..0a71be8807 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 d365a8e1cc..015f909fa2 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)
--
GitLab