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 | NC | add option to set account supplemental groups #8552

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

nadavMiz
Copy link
Contributor

@nadavMiz nadavMiz commented Nov 21, 2024

Explain the changes

supplemental groups are additional groups an account can be part of besides his main group (gid). add option to set a users supplemental groups through the nsfs cli. see Supplementary group IDs in https://man7.org/linux/man-pages/man7/credentials.7.html. having addition groups allows account to access files and directory that allow access to one of the accounts supplemental groups (similar to main group access ). supplementary group IDs are used mainly for adding permissions. for other purposes the accounts main GID will be used (for example determining the group of a file created by the account). note that this enables access only on file system level, s3 commands will still require bucket policy permissions. in the same manner account will still be block from accessing the file system even if it has bucket policy permissions

  1. add option to set account supplemental groups:
  2. supplemental groups can be added by nsfs cli either by passing a single positive number (202) or string of comma separated positive number ("200,101")
  3. supplemental groups can be added using from_file as wither one of the previously mentioned types or as an array of positive numbers ([212,313])
  4. supplemental groups will be saved as part of the account configuration in the section nsfs_account_config
  5. set supplemental groups as part of setting the user id for both linux and darwin (see os_linux.cpp and os_darwin.cpp)
  6. add tests for both adding and updating account configuration (including from_file, and anonymous access)
  7. change documentation to include data about supplemental_groups option
  8. since mac-os creates new directories with parent directory gid and not the process gid. change new directories owner in test_nsfs_access.js. opened a new issue for this problem NSFS | macos | buckets created with parent directory GID instead of thread (user) GID #8665

Issues: Fixed #7274

Testing Instructions:

  1. sudo npx jest test_nc_nsfs_account_cli.test.js
  2. sudo npx jest test_nc_nsfs_anonymous_cli.test.js
  3. sudo node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_nsfs_access.js
  4. sudo NC_CORETEST=true node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_nsfs_integration.js

Gaps

#8665
noobaa/noobaa-operator#1505

  • Doc added/updated
  • Tests added

@nadavMiz nadavMiz requested a review from romayalon November 21, 2024 16:21
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 4 times, most recently from 702fd42 to 9abd913 Compare November 24, 2024 09:34
src/sdk/nb.d.ts Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_help_utils.js Outdated Show resolved Hide resolved
src/native/fs/fs_napi.cpp Outdated Show resolved Hide resolved
src/native/util/os_linux.cpp Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_help_utils.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_constants.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_validations.js Show resolved Hide resolved
src/server/system_services/schemas/nsfs_account_schema.js Outdated Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_validations.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_validations.js Outdated Show resolved Hide resolved
src/native/fs/fs_napi.cpp Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 11 times, most recently from f2ee5e0 to 6f7239b Compare December 11, 2024 16:53
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.

I added my comments (without the native files).

I would list things that you can add in this PR:

  • Tests for the noobaa-cli that combines the supplemental groups with the --from_file flag.
  • Tests for the noobaa-cli that combines the supplemental groups with the --anonymous flag
  • Update the S3Ops.md about permission - currently only UID, GID is mentioned.

src/api/common_api.js Outdated Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_cli_utils.js Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_cli_utils.js Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_constants.js Show resolved Hide resolved
src/native/fs/fs_napi.cpp Outdated Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Show resolved Hide resolved
src/util/native_fs_utils.js Outdated Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 5 times, most recently from 08a66b9 to 406c2ed Compare December 15, 2024 17:51
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 5 times, most recently from b948756 to cbb9118 Compare December 23, 2024 19:36
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 3 times, most recently from 3ec6ddd to 1ef9a41 Compare January 8, 2025 10:41
@nadavMiz nadavMiz requested a review from shirady January 8, 2025 10:49
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 3 times, most recently from 0e324bf to 87ed095 Compare January 8, 2025 12:11
@nadavMiz nadavMiz self-assigned this Jan 8, 2025
docs/NooBaaNonContainerized/AccountsAndBuckets.md Outdated Show resolved Hide resolved
src/manage_nsfs/manage_nsfs_validations.js Outdated Show resolved Hide resolved
src/native/util/os_darwin.cpp Outdated Show resolved Hide resolved
src/test/unit_tests/test_nsfs_access.js Outdated Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 4 times, most recently from 500d0e5 to be3a2b0 Compare January 9, 2025 10:20
@nadavMiz nadavMiz requested a review from shirady January 9, 2025 10:21
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

src/native/util/os_darwin.cpp Outdated Show resolved Hide resolved
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.

Supporting supplemental groups for accounts
4 participants