Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

Fix Add-PnPFile throwing "Access Denied" error with existing folders with Read permission at Web level #2478

Closed
wants to merge 1 commit into from

Conversation

jackpoz
Copy link
Contributor

@jackpoz jackpoz commented Jan 29, 2020

Type

  • Bug Fix
  • New Feature
  • Sample

Related Issues?

Partially fixes #2476

What is in this Pull Request ?

This PR fixes an issue where users with Read access at Web level but Full Control at Library level receive an "Access Denied" error when calling Add-PnPFile.

The current Add-PnPFile implementation relies on Web.EnsureFolder() to retrieve the folder where to add the file. Web.EnsureFolder() first tries to get the Web.RootWeb.ServerRelativeUrl property (which throws an "Access Denied" server-side error when the user has only Read access on the Web) and then tries to get the List.RootFolder folder of every list in the Web (which throws yet another "Access Denied" server-side erro).
This PR changes the PnP-PowerShell code so that the folder existance is checked with CSOM first and EnsureFolder() is called only if the folder doesn't exist.

This allows to upload files to existing folders with an account with Read permission at Web level and Write/Full Control at Library level. If the folder doesn't exist, an "Access Denied" exception is still thrown (with this permission configuration) as EnsureFolder() is still used in this case. A possible solution could be to use CreateFolder() recursiverly, or change EnsureFolder() implementation in PnP-Site-Core to avoid any "Access Denied" error.

Workaround a permission issue in Web.EnsureFolder() that throws an exception if the user doesn't have Full Control permissions on Web level.
@yzdoe
Copy link

yzdoe commented Feb 5, 2020

Actually, even with Edit permission at site level, Add-PnPFile throws "Access Denied" error. #2476. The same user can actually run Add-PnpFolder and upload files to the same document library through web browser.

@yzdoe
Copy link

yzdoe commented Feb 11, 2020

I merged recent dev branch to this pull request, compiled the code and it works fine.

@KoenZomers KoenZomers self-assigned this Mar 17, 2020
@KoenZomers KoenZomers added the status:tracked Triaged and are being investigated further label Mar 17, 2020
@KoenZomers
Copy link
Collaborator

KoenZomers commented Mar 17, 2020

@jackpoz I'm trying to test this PR. To set up the test scenario I have:

  • Created a new site collection
  • Added Test01 to the Site visitors
  • Broken rights inheritance on "Shared Documents" in the site and given Test01 Full Control to the library
  • Taken your PR code, connected to the site using user Test01
  • Used Add-PnPFile -Path c:\somefile.txt -Folder "Shared Documents"

For me this still results in a 401 Unauthorized at:
SelectedWeb.EnsureProperty(w => w.ServerRelativeUrl);

The same line of code you are using in your PR inside GetFolder(), so it still throws the exception for me.

Please elaborate on if my test case is not representative for what you're trying to fix.

@yzdoe
Copy link

yzdoe commented Mar 17, 2020

@KoenZomers From my experience, if you grant Test01 'Edit' site permission, with this PR, the user can use add-pnpfile. Without this PR, Test01 will get permission denied (even if they have site 'Edit' permission and full control of the library).

@KoenZomers
Copy link
Collaborator

Thanks for elaborating @yzdoe. Let me try to raise the permissions for Test01 on a site level from visitor to member and see what happens.

@KoenZomers
Copy link
Collaborator

Found out the error I was receiving was caused by a bug in PnP Sites Core which caused passwords with special characters not to work. PR-ed a fix for that. Will continue testing this PR with the fixed Sites Core build.

@KoenZomers KoenZomers mentioned this pull request Mar 17, 2020
@KoenZomers
Copy link
Collaborator

Works perfect now. Trying to change this in PnP Sites Core will be too tricky as we can't estimate what it will do to existing code using the EnsureFolder in it, so I deem fixing it in PnP PowerShell the better option.

Thanks again for your contribution @jackpoz and @yzdoe for testing this as well! Will be merged through PR #2583

@KoenZomers KoenZomers closed this Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:tracked Triaged and are being investigated further
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants