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

Resource Manager permissions bug/issue in DNN 9.8.1 and above (HTML Editor Browse Server showing all folders) #4573

Closed
cgentryls opened this issue Mar 29, 2021 · 25 comments · Fixed by #4897

Comments

@cgentryls
Copy link

cgentryls commented Mar 29, 2021

I believe the Resource Manager has a folder permissions bug/issue. The HTML Editor "Browse Server" is showing ALL folders to Roles after upgrading from DNN 9.7.2 to 9.8.1 no matter how the permissions are set in the Resource Manager.

When using the HTML Editor, selecting the image or file icon, selecting the "Browse Server" button. The Folder Tree shows all folders no matter what the folder permissions are set to for Roles in the Resource Manager. In DNN 9.7.2 I had folder permissions set so that I could hide some of the folders from certain Roles, but after upgrading to 9.8.1 it shows all the folders.

I have tried different folder permission settings in the PB > Site Assets, but nothing works. I have even tried replacing with the new Resource Manger but that did not work either.

I have tested this on a test production site and a test dnndev.me site and both have the same issue.

To replicate the issue. Go to the Site Assets and assign folder permissions on a folder to hide it from a "Role". Then login as a user assigned to that Role and use the HTML module "Browse Server" and you will see the folder you are trying to hide.

I have attached a couple of screen shots.

DNN 9 8 1 Site Assets Folder permissions

DNN 9 8 1 HTML Editor Browse Server showing all folders

Thanks,
Craig

@cgentryls
Copy link
Author

Also, if you have even one folder, with the "Open File in Folders" and the "Write to Folder" set to deny, then the Folder Tree shows NO folders at all.
DNN 9 8 1 Site Assets Folder permissions-both-set

DNN 9 8 1 HTML Editor Browse Server showing no folders

@sleupold
Copy link
Contributor

Using deny permission should only be used, if necessary and if global roles "all Users" are granted access.
Deny for "all users" role should be disabled, because it prevents anyone from accessing files or subfolders.

@cgentryls
Copy link
Author

cgentryls commented Mar 29, 2021

Even if I only have the "Browse" checked and the "Open Files in Folder" and "Write to Folder" not set. I can still see the Test folder when logged in as the Role. Previously when our site was on DNN 9.3.2 this is how I had it set. Then after upgrading to DNN 9.7.2 I had to set the folders I did not want the Role to see to deny "Open Files in Folder". Now after upgrading to 9.8.1 not even the deny works when trying to hide a folder.

How would you set the folder permissions to restrict a Role from viewing?
Can you test your DNN site and see if you can restrict view to a folder to a role?

Thanks.

DNN 9 8 1 Site Assets Folder permissions-only-one
DNN 9 8 1 HTML Editor Browse Server showing all folders-2

@cgentryls
Copy link
Author

cgentryls commented Mar 29, 2021

We are a local city government and I have around 20 Roles setup to edit content. I need to have the Site Assets folder permissions set so All Users cannot see all the folders when using the HTML Editor to "Browse Server". Then I assign permissions in the Site Assets for only certain Roles to see their folders. This used to work prior to DNN 9.8.1

Note: the folder permissions should also allow anyone, when viewing our website, to see an image or file in the folder.

Can you test your DNN site and see if you can restrict viewing a folder to a Role when using the HTML Editor "Browse Server"?

@sleupold
Copy link
Contributor

you don't need to deny permission for "all users" - this is default.
Access will only be possible for users in roles with dedicated grants. "Deny" option is meant to be used for situations, where you grant (explicitely) for a large role like "registered users" and want to exclude just members of a smaller role (like "editors"),
I will test folder permissions later

@cgentryls
Copy link
Author

I understand. I only used deny after I upgraded to DNN 9.7.2 because the permissions were not working properly. Now in DNN 9.8.1 no matter how I have the permissions set in Asset Manager you cannot hide folders per Roles. Yes, please let me know how your tests go. Thanks

@cgentryls
Copy link
Author

Has anyone tested to verify?

@cgentryls
Copy link
Author

Can anyone test their DNN site 9.8.1 and above, and see if you can restrict viewing a folder to a "Role" when using the HTML Editor "Browse Server"?

To replicate the issue I am describing above, go to the Site Assets and assign folder permissions on a folder to hide it from a "Role". Then login as a user assigned to that Role and use the HTML module "Browse Server" and you will see the folder you are trying to hide.

@cgentryls cgentryls changed the title DNN 9.8.1 HTML Editor Browse Server showing all folders Resource Manager permissions bug/issue in DNN 9.8.1 and above (HTML Editor Browse Server showing all folders) Apr 26, 2021
@yog-it
Copy link
Contributor

yog-it commented Apr 29, 2021

I tested this out, first with a fresh install of DNN 9.9.1 before and after removing the Digital Asset Manager and the permissions do not work correctly as described. I then installed a fresh copy of DNN 9.3.2 and confirm they do work accordingly. I then upgraded to the DNN 9.7.2 and the users that should not be able to see a specific folder now can when browsing for files in the CKE file browser. Using the "deny" does stop users that should not see it from seeing it, but as @sleupold said, by default the users should not be able to see the folder or its contents without being given permissions to do so.

I don't think the problem is in the Resource Manager or the Digital Asset Manager, but rather with the CKE editor's file browser.

@cgentryls
Copy link
Author

Thank you for testing and verifying! Everything you said about 9.3.2, then upgrading to 9.7.2, and then to 9.8.1 or above is true and things I noticed when I upgrading. The only thing I would ask is, did anything change with the DNN CKE editor from versions 9.3.2 to 9.7.2 and then 9.8.1. If not, then you would think it would have to be something with the Core DNN software and maybe the Resource Manager. My thoughts were that it was with the Resource Manager because that is where the most changes have been taking place lately. At this point I CANNOT upgrade to anything above 9.7.2 because of the permissions issues described in this issue. I hope someone looks into this issue soon. Thanks, Craig

@yog-it
Copy link
Contributor

yog-it commented Apr 29, 2021

Actually I'm not sure 9.7.2 is the culprit. I tested upgrading the working copy 9.3.2 and the issue is partially introduced in version 9.4.0. I found that once I move to 9.4.0 the test user that was given rights to browse folder and files can see the test directory they should but can't see the files, and they can also see a second test folder and files in it that no users but the admin was supposed to see. I'm going to keep testing and checking the changes from version to version in the 9.4.x path.

@cgentryls
Copy link
Author

Thanks for looking into this further, I really appreciate it. I was looking back through the version notes of DNN above 9.3.2 and below is what I found. Could it be related to any of these updates/fixes?

CKE editor related fixes:
Noteworthy Changes in v9.8.1
Fixed an issue where Denying the ADD permission for a role on an asset would make it invisible to users on CKE Editor. #4364 Thanks @mikebigun
Noteworthy Changes in v9.8.0
Merged CKEditor Provider in our main repository #4096 Thanks @bdukes
Release Notes for v9.4.1
Fixed an issue where the CkEditor provider would have the dll twice in the install package. Thanks @valadas #126

Permissions, Resource Manager or the Digital Asset Manager related fixes:
Noteworthy Changes in v9.8.1
Fixed an issue where Denying the ADD permission for a role on an asset would make it invisible to users on CKE Editor. #4364 Thanks @mikebigun
Noteworthy Changes in v9.8.0
(there are a number of changes to the Asset Manager)
Noteworthy Changes in v9.4.3
Fixed an issue where Select All was not working in site assets. Thanks @mikebigun #3251
Noteworthy Changes in v9.4.2
Updated default portal template so it provides default permissions on portal folders. Thanks @SCullman #3024
Noteworthy Changes in v9.4.1
Fixed an issue where the assets manager activity wheel would constantly spin. Thanks @berkarslan-xo #2970

@yog-it
Copy link
Contributor

yog-it commented May 4, 2021

You directed me right to the issue. #4364 alters the GetFoldersByPermissions stored procedure. That change correctly fixes the check for deny permission per the UserID or User's Roles. It also needed the same check but for when AllowAccess is true in the second part of the query. The change can be found in my fork (yog-it). If you want to give it a test run in your development/test environment, feel free. Just copy and paste the code from the 09.09.02.SqlDataProvider file into your SQL Console in DNN and run.

@cgentryls
Copy link
Author

Will this issue be resolved when issue #4649 is resolved?
I cannot upgrade our website from DNN 9.7.2 because of this issue.

@valadas
Copy link
Contributor

valadas commented Jul 21, 2021

@sleupold you have any input on the correct fix for this issue ?

@cgentryls
Copy link
Author

@yog-it says that his #4649 fixes this issue. I was wanting to know if it would be resolved in DNN 9.10.0?

@valadas
Copy link
Contributor

valadas commented Jul 21, 2021

Well #4649 was refused due to the solution not being correct following up a discussion with @sleupold so that's why I pinged him up as I don't exactly remember what was wrong...

@yog-it
Copy link
Contributor

yog-it commented Jul 21, 2021

@cgentryls the code I originally wrote was not correct and @sleupold had already been working on a solution that he provided the code for. We altered the PR with his code, but he may still be working on it, so the PR was closed. I think he'll post a full solution with his code in a PR to solve this issue.

@sleupold
Copy link
Contributor

I am still looking for a proper solution - my stored procedure does not handle all permission correctly.
besides, I noticed another issue with API code using a different way of checking permission, which is not efficient.

@cgentryls
Copy link
Author

Do any of you know when this issue will be resolved? I cannot upgrade our website from DNN 9.7.2 because of this issue.

@valadas
Copy link
Contributor

valadas commented Oct 5, 2021

This being an open source project, we never know when any specific issue will get fixed until someone submits a fix. @sleupold any progress on this?

@yog-it
Copy link
Contributor

yog-it commented Oct 11, 2021

I propose I re-submit my code again from the original PR to fix the issue addressed. I understand that the current SP is in need of more work. I can see that by the wonderful code that was provided. I can also help out @sleupold with the efficiency and use of permissions issues you mentioned if you would need a hand. Version 10 could have a better working version of this SP.

@valadas
Copy link
Contributor

valadas commented Oct 12, 2021

Awesome, thanks for all the hard work, I'll assign you the issue.

@mitchelsellers
Copy link
Contributor

@yog-it That is awesome, and just to be 100% clear on this, we CANNOT accept just a copy of the code that was there from Sebastian as we must have a unique contribution (Can be enhancement from his) to be clear on ownership claims etc. (For you to be compliant with the CLA agreement etc)

@yog-it
Copy link
Contributor

yog-it commented Oct 12, 2021

@mitchelsellers crystal clear 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment