Skip to content

Commit

Permalink
[irods#7778] User removal now also removes ACLs
Browse files Browse the repository at this point in the history
  • Loading branch information
@FifthPotato @alanking
FifthPotato authored and alanking committed Jan 9, 2025
1 parent 89d8b33 commit 05403d3
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 1 deletion.
14 changes: 13 additions & 1 deletion plugins/database/src/db_plugin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4505,13 +4505,25 @@ irods::error db_del_user_re_op(
&icss );
if ( status != 0 && status != CAT_SUCCESS_BUT_WITH_NO_INFO ) {
char errMsg[MAX_NAME_LEN + 40];
log_db::info("chlDelUserRE delete password failure {}", status);
log_db::error("chlDelUserRE delete password failure {}", status);
snprintf( errMsg, sizeof errMsg, "Error removing password entry" );
addRErrorMsg( &_ctx.comm()->rError, 0, errMsg );
_rollback( "chlDelUserRE" );
return ERROR( status, "Error removing password entry" );
}

// Remove any ACLs associated with the user
cllBindVars[cllBindVarCount++] = iValStr;

status = cmlExecuteNoAnswerSql("delete from R_OBJT_ACCESS where user_id=?", &icss);

if (status != 0 && status != CAT_SUCCESS_BUT_WITH_NO_INFO) {
log_db::error("chlDelUserRE delete ACL failure {}", status);
addRErrorMsg(&_ctx.comm()->rError, 0, "Error removing ACLs");
_rollback("chlDelUserRE");
return ERROR(status, "Error removing ACLs");
}

/* Remove both the special user_id = group_user_id entry and any
other access entries for this user (or group) */
cllBindVars[cllBindVarCount++] = iValStr;
Expand Down
132 changes: 132 additions & 0 deletions scripts/irods/test/test_iadmin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1708,6 +1708,138 @@ def test_iadmin_exits_nonzero_on_nonexistent_command__issue_7734(self):
ec, _, _ = self.admin.assert_icommand(['iadmin', 'nonexistentcommand'], 'STDERR');
self.assertNotEqual(ec, 0)

def test_iadmin_rmuser_removes_associated_acls__issue_7778(self):
test_user_name = "test_user_issue_7778"
test_group_name = "test_group_issue_7778"
test_collection_name = "test_collection_issue_7778"
test_dataobj_name = "test_data_object_issue_7778"
try:
self.admin.assert_icommand(["imkdir", test_collection_name])
self.admin.assert_icommand(["itouch", test_dataobj_name])
self.admin.assert_icommand(["iadmin", "mkgroup", test_group_name])
self.admin.assert_icommand(["iadmin", "mkuser", test_user_name, "rodsuser"])
self.admin.assert_icommand(
["iadmin", "atg", test_group_name, test_user_name]
)
self.admin.assert_icommand(
["ichmod", "read_object", test_group_name, test_collection_name]
)
self.admin.assert_icommand(
["ichmod", "modify_object", test_user_name, test_collection_name]
)
self.admin.assert_icommand(
["ichmod", "read_object", test_group_name, test_dataobj_name]
)
self.admin.assert_icommand(
["ichmod", "modify_object", test_user_name, test_dataobj_name]
)

# Collect IDs of user and group
_, out, _ = self.admin.assert_icommand(
[
"iquest",
"%s",
"SELECT USER_ID WHERE USER_NAME = '%s' || = '%s'"
% (test_user_name, test_group_name),
],
"STDOUT_MULTILINE",
r"^[0-9]+$",
use_regex=True,
)

# Create iquest query string for the IDs-- we don't care about order, we just want both
query_string = "= '%s' || = '%s'" % tuple(out.strip().split('\n'))

# Should see the permissions as expected
self.admin.assert_icommand(
[
"iquest",
"SELECT COLL_ACCESS_USER_ID, COLL_ACCESS_NAME WHERE COLL_NAME = '%s' AND COLL_ACCESS_USER_ID %s"
% (
os.path.join(
self.admin.session_collection, test_collection_name
),
query_string,
),
],
"STDOUT",
["COLL_ACCESS_NAME = read_object", "COLL_ACCESS_NAME = modify_object"],
)
# As above, but for data objects
self.admin.assert_icommand(
[
"iquest",
"SELECT DATA_ACCESS_USER_ID, DATA_ACCESS_NAME WHERE DATA_NAME = '%s' AND COLL_NAME = '%s' AND DATA_ACCESS_USER_ID %s"
% (test_dataobj_name, self.admin.session_collection, query_string),
],
"STDOUT",
["DATA_ACCESS_NAME = read_object", "DATA_ACCESS_NAME = modify_object"],
)

self.admin.assert_icommand(["iadmin", "rmuser", test_user_name])

# Should only see read_object, since that was the perm given to the group
_, out, _ = self.admin.assert_icommand(
[
"iquest",
"SELECT COLL_ACCESS_USER_ID, COLL_ACCESS_NAME WHERE COLL_NAME = '%s' AND COLL_ACCESS_USER_ID %s"
% (
os.path.join(
self.admin.session_collection, test_collection_name
),
query_string,
),
],
"STDOUT",
["COLL_ACCESS_NAME = read_object"],
)
self.assertNotIn("COLL_ACCESS_NAME = modify_object", out)

# As above, but for data objects
_, out, _ = self.admin.assert_icommand(
[
"iquest",
"SELECT DATA_ACCESS_USER_ID, DATA_ACCESS_NAME WHERE DATA_NAME = '%s' AND COLL_NAME = '%s' AND DATA_ACCESS_USER_ID %s"
% (test_dataobj_name, self.admin.session_collection, query_string),
],
"STDOUT",
["DATA_ACCESS_NAME = read_object"],
)
self.assertNotIn("DATA_ACCESS_NAME = modify_object", out)

self.admin.assert_icommand(["iadmin", "rmgroup", test_group_name])

# Perms should vanish after deletion of associated group
self.admin.assert_icommand(
[
"iquest",
"SELECT COLL_ACCESS_USER_ID, COLL_ACCESS_NAME WHERE COLL_NAME = '%s' AND COLL_ACCESS_USER_ID %s"
% (
os.path.join(
self.admin.session_collection, test_collection_name
),
query_string,
),
],
"STDOUT_SINGLELINE",
"CAT_NO_ROWS_FOUND: Nothing was found matching your query",
)

# As above, but for data objects
self.admin.assert_icommand(
[
"iquest",
"SELECT DATA_ACCESS_USER_ID, DATA_ACCESS_NAME WHERE DATA_NAME = '%s' AND COLL_NAME = '%s' AND DATA_ACCESS_USER_ID %s"
% (test_dataobj_name, self.admin.session_collection, query_string),
],
"STDOUT_SINGLELINE",
"CAT_NO_ROWS_FOUND: Nothing was found matching your query",
)
finally:
self.admin.run_icommand(["iadmin", "rmuser", test_user_name])
self.admin.run_icommand(["iadmin", "rmgroup", test_group_name])
self.admin.run_icommand(["irmdir", test_collection_name])
self.admin.run_icommand(["irm", "-f", test_dataobj_name])

class Test_Iadmin_Resources(resource_suite.ResourceBase, unittest.TestCase):

Expand Down

0 comments on commit 05403d3

Please sign in to comment.