From b5ead2e20ccac4947ea9d59e7f436368c336d978 Mon Sep 17 00:00:00 2001
From: Kaitmazian Maksim <m.kaitmazian@picodata.io>
Date: Mon, 16 Sep 2024 15:03:49 +0300
Subject: [PATCH] fix(test): granting privileges to nonexistent user
This commit restores the user name in the ALTER USER command to "Dave" after
it was mistakenly changed to "DAVE", a nonexistent user. This change
broke the test logic, causing the test to erroneously verify granting
privileges to a nonexistent user, rather than confirming that we can
grant default privileges. The previous implementation resulted in noop,
so this went unnoticed until we started validating user existence.
Here is the commit changing Dave to DAVE:
https://git.picodata.io/picodata/picodata/picodata/-/commit/ecbb520b559ceb7f2dbe7ae0462c8d062faf1589
---
src/sql.rs | 3 +++
test/int/test_acl.py | 18 +++++++++---------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/sql.rs b/src/sql.rs
index 9bd681da38..e2bf49ff74 100644
--- a/src/sql.rs
+++ b/src/sql.rs
@@ -725,6 +725,9 @@ fn alter_user_ir_node_to_op_or_result(
})))
}
AlterOption::Login => {
+ // Note: We do not check if the privilege has already been granted, since a user may
+ // have it on one node but not on another. This grant must be applied globally to
+ // ensure the user has login access across all nodes.
let priv_def = PrivilegeDef::login(
get_grantee_id(storage, name.as_str())?,
current_user,
diff --git a/test/int/test_acl.py b/test/int/test_acl.py
index 7996d1b696..c71ed51890 100644
--- a/test/int/test_acl.py
+++ b/test/int/test_acl.py
@@ -551,9 +551,9 @@ def test_builtin_users_and_roles(cluster: Cluster):
# granting already granted privilege does not raise an error
index = i1.call(".proc_get_index")
- i1.sql('ALTER USER "DAVE" WITH LOGIN')
+ i1.sql('ALTER USER "Dave" WITH LOGIN')
new_index = i1.call(".proc_get_index")
- assert index == new_index
+ assert index != new_index
i1.grant_privilege("Dave", "execute", "role", "super")
new_index = i1.call(".proc_get_index")
@@ -612,25 +612,25 @@ def test_grant_and_revoke_default_users_privileges(cluster: Cluster):
assert index == new_index
# granting default privilege does not raise an error
- i1.sql('ALTER USER "DAVE" WITH LOGIN')
+ i1.sql('ALTER USER "Dave" WITH LOGIN')
new_index = i1.call(".proc_get_index")
- assert index == new_index
+ assert index != new_index # AlterUser is always executed trough raft
# granting default privilege does not raise an error
i1.grant_privilege("Dave", "alter", "user", "Dave")
new_index = i1.call(".proc_get_index")
- assert index == new_index
+ assert index != new_index # AlterUser is always executed trough raft
# revoke default privilege does not raise an error
- i1.sql('ALTER USER "DAVE" WITH NOLOGIN')
+ i1.sql('ALTER USER "Dave" WITH NOLOGIN')
new_index = i1.call(".proc_get_index")
- assert index == new_index
+ assert index != new_index # AlterUser is always executed trough raft
index = new_index
# already revoked, so it should be idempotent
- i1.sql('ALTER USER "DAVE" WITH NOLOGIN')
+ i1.sql('ALTER USER "Dave" WITH NOLOGIN')
new_index = i1.call(".proc_get_index")
- assert new_index == index
+ assert index != new_index # AlterUser is always executed trough raft
# it's part of https://git.picodata.io/picodata/picodata/picodata/-/issues/421
--
GitLab