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.

Sign up for GitHub

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

Jump to bottom

Gracefully handle smb acls for users without a domain #28062

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Gracefully handle smb acls for users without a domain #28062

merged 1 commit into from
Aug 13, 2021

Conversation

st3iny
Copy link
Member

@st3iny st3iny commented Jul 20, 2021 •
edited
Loading

Fixes #27809

Currently, checking ACLs of SMB storages fails if the user in the ACL has no domain.

@st3iny st3iny self-assigned this Jul 20, 2021
@st3iny
Copy link
Member Author

st3iny commented Jul 20, 2021 •
edited
Loading

@icewind1991 Or should we ignore all "users" with no domain in ACLs? I'm not sure what is the right call here.

Excerpt from the data leading to the error:

{
  "acls": {
    "DOMAIN\\user1": {
      "__class__": "Icewind\\SMB\\ACL"
    },
    "DOMAIN\\user2": {
      "__class__": "Icewind\\SMB\\ACL"
    },
    "Everyone": {
      "__class__": "Icewind\\SMB\\ACL"
    },
    "Other User": {
      "__class__": "Icewind\\SMB\\ACL"
    }
  }
}

@szaimen
Copy link
Contributor

szaimen commented Jul 20, 2021

@icewind1991

He is on vacations currently...

@st3iny st3iny force-pushed the fix/27809/smb-acl-users-without-domain branch from ba7a9c8 to d1faffa Compare July 28, 2021 06:58
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jul 28, 2021
@st3iny st3iny requested a review from icewind1991 July 28, 2021 06:59
@st3iny st3iny marked this pull request as ready for review July 28, 2021 06:59
juliusknorr
juliusknorr approved these changes Aug 11, 2021
View reviewed changes
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Code makes sense 👍

@juliusknorr
Copy link
Member

🏓 @icewind1991

@st3iny st3iny force-pushed the fix/27809/smb-acl-users-without-domain branch from d1faffa to 2e53cf4 Compare August 13, 2021 07:44
@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 13, 2021
@st3iny
Copy link
Member Author

st3iny commented Aug 13, 2021

/backport to stable20

@st3iny
Copy link
Member Author

st3iny commented Aug 13, 2021

/backport to stable21

@st3iny
Copy link
Member Author

st3iny commented Aug 13, 2021

/backport to stable22

@st3iny st3iny merged commit fa56c13 into master Aug 13, 2021
@st3iny st3iny deleted the fix/27809/smb-acl-users-without-domain branch August 13, 2021 08:38
@st3iny st3iny added this to the Nextcloud 23 milestone Aug 13, 2021
This was referenced Aug 13, 2021
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@juliusknorr juliusknorr juliusknorr approved these changes

@icewind1991 icewind1991 icewind1991 approved these changes

Assignees

@st3iny st3iny

Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: external storage
Projects
None yet
Milestone
Nextcloud 23
Development

Successfully merging this pull request may close these issues.

Undefined offset: 1 at /apps/files_external/lib/Lib/Storage/SMB.php#226
4 participants
@st3iny @szaimen @juliusknorr @icewind1991