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-129686 / 24.04.2 / Fix SMB ACL form (by bvasilenko) #10290

Merged
merged 4 commits into from
Jul 5, 2024

Conversation

bugclerk
Copy link
Contributor

@bugclerk bugclerk commented Jul 5, 2024

Automatic cherry-pick failed. Please resolve conflicts by running:

git cherry-pick -x 594ce987a9f58c7fd0927bbe79a47c39048961fd
git cherry-pick -x df35b0190c725640c742c5fd4bbd6f3029e4cdce
git cherry-pick -x b8609ae68bc07c83acd1468f3b80c8e7d8d644f9

If the original PR was merged via a squash, you can just cherry-pick the squashed commit:

git reset --hard HEAD~3
git cherry-pick -x 324fa0f9a287f5d9d742e132d275ec5061a58177

Summary

  1. when AD cache is empty (see ticket), when Group combobox does not contain any suitable values to choose from,
    I've changed to code to add the ability to type in a custom value from keyboard . When user presses Save, every value is validated for being a valid existing entry in the Active Directory

  2. Middleware returns special type of AD entry BOTH , and when AD cache is empty (see ticket) it's not possible to distinguish a user from a group.
    It was agreed that, it would be unreasonable to try to squeeze in last-second middleware update for 24.04.2.
    So, in such cases, the WebUI code was updated to make an attempt of detecting the type, otherwise it treats an AD entry as GROUP.

Testing

  1. Log into WebUI
  2. Setup the Active directory
  3. Disable AD User / Group Cache and rebuild cache
  4. Save the configuration
  5. Create an SMB share
  6. Edit the SMB ACL
  7. Add a known to exist AD Group using keyboard entry, not picking any of options of the dropdown menu (for example AD02\domain users)
    and/or add a User entry by typing on the keyboard (for example type in AD02\administrator)
  8. Save the ACL
  9. Edit the SMB ACL
  10. Observe the AD Group and/or User entry

Original PR: #10271
Jira URL: https://ixsystems.atlassian.net/browse/NAS-129686

Copy link

codecov bot commented Jul 5, 2024

Codecov Report

Attention: Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.

Please upload report for BASE (release/24.04.2@9a9bc85). Learn more about missing BASE report.

Files Patch % Lines
...app/pages/sharing/smb/smb-acl/smb-acl.component.ts 80.00% 5 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             release/24.04.2   #10290   +/-   ##
==================================================
  Coverage                   ?   66.05%           
==================================================
  Files                      ?     1426           
  Lines                      ?    54657           
  Branches                   ?     6579           
==================================================
  Hits                       ?    36103           
  Misses                     ?    18554           
  Partials                   ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@undsoft undsoft marked this pull request as ready for review July 5, 2024 10:48
@undsoft undsoft requested a review from a team as a code owner July 5, 2024 10:48
@undsoft undsoft requested review from AlexKarpov98 and removed request for a team July 5, 2024 10:48
Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

backported.

@undsoft undsoft enabled auto-merge (squash) July 5, 2024 11:19
@undsoft undsoft merged commit a9f463d into release/24.04.2 Jul 5, 2024
8 checks passed
@undsoft undsoft deleted the NAS-129686-24.04.2 branch July 5, 2024 11:19
@bugclerk
Copy link
Contributor Author

bugclerk commented Jul 5, 2024

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 Jul 5, 2024
@undsoft
Copy link
Collaborator

undsoft commented Jul 7, 2024

backport

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