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

NSFS | set supplemental groups dynamically to users groups #8687

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

nadavMiz
Copy link
Contributor

@nadavMiz nadavMiz commented Jan 14, 2025

Explain the changes

  1. get user supplemental dynamically by calling getgrouplist. getgrouplist will scan the system for all groups that the user is a part of. that way will be able to set the users groups without specifying them in the account configuration. if specified, supplemental groups set by the account configuration will take precedence, and be set as the users groups. also in case no record of the user can be found in the os and no supplemental groups were defined in the account the supplemental groups list will be set as empty
  2. some tests were using the uid 5, which is a system reserved uid, causing the feature to give them root group permissions. changed the uid and gid to ones not reserved by the OS
  3. there was a suggestion to use getlogin_r to get the user name for getgrouplist. this function gets the name of the user logged to the terminal. this function uses the /var/run/utmp. in my implementation I used
    getpwuid instead which gets the user name by uid using the /etc/passwd file. I used it because its more flexible and according to documentation getlogin_r uses the /etc/passwd file anyways. also getlogin_r might not work because the uid was switched to be different then the one used to run the process(root)
  4. added implementation and tests for mac-os. run the tests same as on linux. only run on macos environment

Issues:

Fixed #8712

Gaps

#8710 - list objects fail when using supplemental groups

continue #8552

Testing Instructions:

before running all tests on the code changes please run:
npm run build:native
manual tests (examples for linux):

  1. create new group in the os:
    groupadd -g 1022 new_group
  2. create new user in the os:
    useradd -c test_user2 -m test_user2 -p -u 1033 -g 1034
  3. add user to the new group:
    usermod -a -G new_group test_user2
  4. create new noobaa account with the new groups gid:
    sudo node src/cmd/manage_nsfs.js account add --name test_user1 --uid 1022 --gid 1022
  5. create a second account with the new users gid and uid:
    sudo node src/cmd/manage_nsfs.js account add --name test_user1 --uid 1033 --gid 1034
  6. create a new bucket using the forst user
    test_user1_s3api create-bucket --bucket test_bcuket
  7. add bucket policy to allow putObject to test_user2
  8. put new object to the new bucket using the second user test_user2:
    test_user2_s3api put-object --bucket test_bcuket --key key1 --body <path_to_body>
  9. operation should succeed

automatic tests:
sudo node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_nsfs_access.js

Note: building and testing was done on Linux and Mac machines

  • Doc added/updated
  • Tests added

@nadavMiz nadavMiz force-pushed the dinamic_supplemental_groups branch 3 times, most recently from 9654fd5 to a36991b Compare January 14, 2025 16:43
@nadavMiz nadavMiz self-assigned this Jan 15, 2025
@nadavMiz nadavMiz force-pushed the dinamic_supplemental_groups branch from a36991b to b789a4e Compare January 15, 2025 10:48
src/native/util/os_linux.cpp Show resolved Hide resolved
src/native/util/os_linux.cpp Show resolved Hide resolved
src/native/util/os_linux.cpp Outdated Show resolved Hide resolved
src/test/system_tests/test_utils.js Outdated Show resolved Hide resolved
src/test/system_tests/test_utils.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Outdated Show resolved Hide resolved
src/test/system_tests/test_utils.js Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Outdated Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the dinamic_supplemental_groups branch 12 times, most recently from 7fdd5f1 to 77c2ee3 Compare January 22, 2025 09:47
@nadavMiz nadavMiz force-pushed the dinamic_supplemental_groups branch 6 times, most recently from 6b4a108 to 2b40e6e Compare January 26, 2025 10:57
Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

docs/NooBaaNonContainerized/AccountsAndBuckets.md Outdated Show resolved Hide resolved
src/native/util/os_darwin.cpp Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the dinamic_supplemental_groups branch from 2b40e6e to c639286 Compare January 26, 2025 14:23
@nadavMiz nadavMiz merged commit 630a2b9 into noobaa:master Jan 26, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NSFS | macos | add dynamic supplemental groups for macos
2 participants