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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nadavMiz
Copy link
Contributor

@nadavMiz nadavMiz commented Nov 21, 2024

Explain the changes

  1. add option to set account supplemental groups

Issues: Fixed #7274

Testing Instructions:

  1. sudo npx jest test_nc_nsfs_account_cli.test.js
  2. sudo node ./node_modules/mocha/bin/mocha src/test/unit_tests/test_nsfs_access.js
  • 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 Show resolved Hide resolved
src/cmd/manage_nsfs.js Outdated Show resolved Hide resolved
@@ -338,6 +338,7 @@ async function fetch_account_data(action, user_input) {
distinguished_name: user_input.user,
uid: user_input.user ? undefined : user_input.uid,
gid: user_input.user ? undefined : user_input.gid,
supplemental_groups: _.isUndefined(user_input.supplemental_groups) ? undefined : String(user_input.supplemental_groups),
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like supplemental_groups loading is missing in bucketspace_fs.read_account_by_access_key()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems like read_account_by_access_key gets the account from name, and just modify strings to be sensitiveStrings. do we need to do any modifications to supplemental groups? adding log there to print supplemental groups show the correct value

@@ -197,6 +201,19 @@ function validate_boolean_string_value(value) {
return false;
}

function validate_supplemental_groups(value) {
if (value && typeof value === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

need to add the same validation on account_server/accountspace_FS
CC: @shirady

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand - in which context do you want to add those validations?

src/native/fs/fs_napi.cpp Show resolved Hide resolved
@nadavMiz nadavMiz force-pushed the suplemental-groups branch 10 times, most recently from 50fd414 to f2ee5e0 Compare December 11, 2024 15:36
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