Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NAS-131456 / 25.04 / Update user-group memberships using datastore #14601

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

themylogin
Copy link
Contributor

@themylogin themylogin commented Oct 1, 2024

It seems that the original code for updating user-group relationships (that was never touched since it was written in 2017) contains an error:

https://github.com/truenas/middleware/blob/master/src/middlewared/middlewared/plugins/account.py#L1367

gms is an array of account.bsdgroupmembership objects, so gm["id"] has a Row ID type for account.bsdgroupmembership table. groups is an array of Row IDs for account.bsdgroup table. gm['id'] not in groups check has no sense, it compares Row IDs of different tables.

This is probably the root cause of a rare bug where the following code

from truenas_api_client import Client


c = Client()

while True:
    user = c.call('user.query', [['username', '=', 'smbuser']], {'get': True})

    pk = c.call('user.update', user['id'], {'ssh_password_enabled': True})
    pk = c.call('user.update', user['id'], {'groups': [43, 40, 90]})
    user = c.call('user.get_instance', pk)
    assert set(user['groups']) == set([43, 40, 90])

very rarely fails with assertion failure, user["groups"] being

    "groups": [
      90,
      43,
      90
    ],

Our datastore plugin has the ability to manage many-to-many relationships table, let's use it.

@themylogin
Copy link
Contributor Author

themylogin commented Oct 1, 2024

@themylogin
Copy link
Contributor Author

time 1:00

@bugclerk bugclerk changed the title Update user-group memberships using datastore NAS-131456 / 25.04 / Update user-group memberships using datastore Oct 1, 2024
@bugclerk
Copy link
Contributor

bugclerk commented Oct 1, 2024

@truenas truenas deleted a comment from bugclerk Oct 1, 2024
@themylogin themylogin force-pushed the fix-user-groups-update branch from b552e17 to 002f5fa Compare October 3, 2024 17:20
@anodos325 anodos325 force-pushed the fix-user-groups-update branch from 002f5fa to c039e32 Compare October 11, 2024 19:34
@bugclerk
Copy link
Contributor

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Oct 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants