Skip to content

Commit

Permalink
NAS-133768 / 25.04 / Validate groups on user create/update (#15479)
Browse files Browse the repository at this point in the history
* Remove inserted row if `_handle_relationships` fails

* Validate `groups` on user create/update

* Fix copypaste error
  • Loading branch information
themylogin authored Jan 24, 2025
1 parent 0552445 commit 0d4b584
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 1 deletion.
8 changes: 8 additions & 0 deletions src/middlewared/middlewared/plugins/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,15 @@ async def common_validation(self, verrors, data, schema, group_ids, old=None):
'A user cannot belong to more than 64 auxiliary groups.'
)

existing_groups = {g['id'] for g in await self.middleware.call('datastore.query', 'account_bsdgroups')}

for idx, dbid in enumerate(data.get('groups') or []):
if dbid not in existing_groups:
verrors.add(
f'{schema}.groups.{idx}',
'This group does not exist.'
)

if dbid >= BASE_SYNTHETIC_DATASTORE_ID:
verrors.add(
f'{schema}.groups.{idx}',
Expand Down
12 changes: 11 additions & 1 deletion src/middlewared/middlewared/plugins/datastore/write.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,17 @@ async def insert(self, name, data, options):
else:
pk = insert[pk_column.name]

await self._handle_relationships(pk, relationships)
try:
await self._handle_relationships(pk, relationships)
except Exception:
await self.middleware.call(
'datastore.execute_write',
table.delete().where(pk_column == pk),
{
'ha_sync': options['ha_sync'],
},
)
raise

if options['send_events']:
await self.middleware.call('datastore.send_insert_events', name, insert)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,15 @@ class SMARTTestDiskModel(Model):
disk_id = sa.Column(sa.Integer(), sa.ForeignKey('storage_disk.id'))


@pytest.mark.asyncio
async def test__bad_fk_insert_rollback():
async with datastore_test() as ds:
with pytest.raises(IntegrityError):
assert await ds.insert("tasks.smarttest", {"smarttest_disks": [1]})

assert await ds.query("tasks.smarttest") == []


@pytest.mark.asyncio
async def test__mtm_loader():
async with datastore_test() as ds:
Expand Down
14 changes: 14 additions & 0 deletions tests/api2/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,3 +166,17 @@ def test_create_local_user_ds_group():
pass

assert DS_GRP_VERR_STR in str(ve)


def test_create_account_invalid_gid():
with pytest.raises(ValidationErrors) as ve:
with user({
"username": "invalid_user",
"groups": [BASE_SYNTHETIC_DATASTORE_ID - 1],
"full_name": "invalid_user",
"group_create": True,
"password": "test1234",
}):
pass

assert "This group does not exist." in str(ve)

0 comments on commit 0d4b584

Please sign in to comment.