search

pnp / PnP-PowerShell

SharePoint PnP PowerShell CmdLets
https://pnp.github.io/powershell
Other
987 stars 665 forks source link

Connect-PnPOnline throws "Exception has been thrown by the target of an invocation" #2752

Closed jagsridharan closed 4 years ago

jagsridharan commented 4 years ago

Reporting an Issue or Missing Feature

Connect-PnPOnline with Thumbprint throws error "Exception has been thrown by the target of an invocation"

Expected behavior

Successfully connect to the PnP site

Actual behavior

Connect-PnPOnline : Exception has been thrown by the target of an invocation.

image

Steps to reproduce behavior

Which version of the PnP-PowerShell Cmdlets are you using?

What is the version of the Cmdlet module you are running?

3.22.2006.2

How did you install the PnP-PowerShell Cmdlets?

ghost commented 4 years ago

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

fastlaneb commented 4 years ago

Hi @jagsridharan I'm seeing this much more consistently than you - basically every single time. It appears we are now routing the Connect-PnPOnline thumbprint auth method to an AuthenticationMangager(PnP-Sites-Core) function that wants the private key of the cert. Continued research pending.

fastlaneb commented 4 years ago

Hi again @jagsridharan and @KoenZomers With the help of my colleagues we isolated this issue to Disconnect-PnPOnline. After we call Disconnect-PnPOnline it removes the certificate private key file that is placed on disk here: C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys. Without this private key we will not be able to connect referencing the certs thumbprint. @jagsridharan Were you seeing the failure after calling Disconnect-PnPOnline? If you recreate a new certificate and don't call Disconnect-PnPOnline I would expect you to continue being able to connect.

fastlaneb commented 4 years ago

Oh, also, this looks like a resurface of this: https://github.com/pnp/PnP-PowerShell/issues/2101 It's just now the flow of control is going through Disconnect-PnPOnline. @wobba

jagsridharan commented 4 years ago

@fastlaneb You are absolutely right. Now it makes sense. The reason that it stops working the second time is to the fact that i have a Finally block that Disconnects the connection. Let me try and confirm if this is the behaviour.

KoenZomers commented 4 years ago

Thanks for reporting this guys. I however don't seem to be able to reproduce this on my end. I use a connect like this all the time without any issues. It's the most popular way of connecting, so we should have seen more reports if it really was broken. What also stumbles me a bit is the finding that it would copy the certificate to the temporary location. Didn't double check this in code, but to what I remember, this only happens if you point to a certificate file but not if you use the thumbnail to reference it from the Windows Certificate store, like you are doing.

So something must be different in what you guys are doing vs what I am doing and we need to figure out what that is. One thing which is different, but validated "your way" of doing it and it still doesn't give me the same result ifI do the same, is that you are using the -ReturnConnection. I see that happen with many people. Not sure why because it's totally not necessary in 99% of the scenarios and clutters your code.

Can you share more details on what you're doing?

fastlaneb commented 4 years ago

Thanks for the prompt attention @KoenZomers Did you call Disconnect-PnPOnline and then try?

KoenZomers commented 4 years ago

@fastlaneb I did. Here's the output I get: image

fastlaneb commented 4 years ago

Thank you @KoenZomers I just ran the exact same code as yours and it deleted my private key from C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys. And it then fails. That being the case I'm wondering if this is somehow operating system related. I'm on Windows 10, build 1809. What OS are you on? And @jagsridharan what OS are you on?

yumoraby commented 4 years ago

Is there a difference between the 2 commands?

Disconnect-PnpOnline -Connection $c
Disconnect-PnPOnline

Could the null Connection flag, trigger a blanket purge as opposed to just closing the session?

KoenZomers commented 4 years ago

Is there a difference between the 2 commands?

Disconnect-PnpOnline -Connection $c
Disconnect-PnPOnline

Could the null Connection flag, trigger a blanket purge as opposed to just closing the session?

There actually is. Did some further debugging with this.

I was right on what I seemed to remember above: only when you use -CertificatePath with Connect-PnPOnline and really only then and in no other scenario, it would copy the certificate from that path you provide to the C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys folder. If instead you use a certificate from the Windows Certificate store by using Connect-PnPOnline with -Thumbprint, it will not, in no way, copy the certificate to that folder. Instead it will use it directly from the Windows Certificate Store.

With regards to Disconnect-PnPOnline, I see it will always try to delete a certificate from the C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys folder, even when having connected through -Thumbprint. It would normally just ignore it as it would not be able to find the file there. Now I do see in the code that if you are working with the somewhat funky -Connection parameter on the Disconnect-PnPOnline cmdlet, it would NOT reach the code where it tries to delete the certificate file. Instead it will try to do so on the current context. This "bug" has however been introduced by a commit by Erwin on February 15, 2018 already, so by no means could play a role here if you say it worked before April 2020. The May 2020 release did not contain pretty much any code changes, too, so if you really state it works with April 2020, but not with May 2020, we would have to look inside PnP Sites Core what has changed there. Only in June 2020, a lot of changes have been merged into PnP PowerShell.

The only scenario I can think of which could be causing what you are describing is that you are connecting in the current context somewhere using Connect-PnPOnline -CertificatePath and then in that same context, maybe further in the script, you do another $variable = Connect-PnPOnline -Thumbnail -ReturnConnection. If you then do a Disconnect-PnPOnline -Connection $variable, it would delete the certificate of the initial connect and cause issues on that connection, so whenever you would run a PnP PowerShell command, omitting -Connection with it.

Can you check if that could be the case? I'll submit a PR to get that bug from 2018 fixed in the next release, but don't put your hopes on that that will fix your issue, as it's very likely unrelated.

And please also clarify what your reason is to make use of the -ReturnConnection and -Connection parameters as these should only be used in 1% of the scenarios and are therefore way less used and tested than using PnP PowerShell without it.

wobba commented 4 years ago

Very good debugging and I'm sure not all connections scenarios have been worked thru. The cleanup of certs on disconnect was added due to disks being filled up with repeated connection's, to ensure people have a way to remove the temp files generated.

wobba commented 4 years ago

If people on the thread are able to build and test a version with @KoenZomers fix which that would be appreciated. It's merged to the dev branch.

jagsridharan commented 4 years ago

Thanks @KoenZomers and @wobba for the update and a quick response. I checked my code now and I can confirm that there is only one Connect-PnPOnline at the start and in the Finally block, I have Disconnect-PnPOnline. Only thing I am not doing is passing the returned connection object to the Disconnect-PnPOnline command, instead I am simply calling Disconnect-PnPOnline. When I was passing the Connection object, it was throwing error that no connection was open. As soon as I remove the Connection parameter, it doesn't throw the error. I will try to pass the Connection object and confirm if it resolves the issue. After confirming the same, I will build and test Koen's fix.

fastlaneb commented 4 years ago

Hi Team @KoenZomers @wobba @yumoraby @jagsridharan , I'm confused here. It is my understanding that if we delete these hashed private keys from disk we will never be able to connect with that particular certificate until returning the hashed private key back to disk. Koen, are you suggesting that connecting with the thumbprint doesn't need this key? Where is my logic flawed?

wobba commented 4 years ago

@fastlaneb When connecting it should create a new version of the temp file I believe - for that scenario.

KoenZomers commented 4 years ago

It's like a temporary cache folder which is only used if you reference a certificate by its path on a file system. If its in the Windows Certificate Store, it doesn't need to go to or be in this local cache folder as it can read from the store directly. Believe to have read somewhere that the copy is done to ensure some Windows process dealing with the certificates will be able to access the location as it may not be able to access the original location.

jagsridharan commented 4 years ago

hmm interesting, i have never used the authentication specifying the certificationpath (always used the certificate store).

fastlaneb commented 4 years ago

@wobba We are not seeing the creation of the new temp file when using Connect-PnPOnline with the thumbprint method. If this did happen everything would work. @KoenZomers I am not seeing that behavior. We(@yumoraby and I) only see the temp file created on disk when we import the file into the certificate store. If the temp file is gone we are not able to use Connect-PnPOnline with the thumbprint method because there is no longer a private key associated with that cert in the store. This can be verified by interrogating the certificate's "PrivateKey" property with PowerShell. I believe this is the same behavior that @jagsridharan is seeing.

yumoraby commented 4 years ago

@wobba We are not seeing the creation of the new temp file when using Connect-PnPOnline with the thumbprint method. If this did happen everything would work. @KoenZomers I am not seeing that behavior. We(@yumoraby and I) only see the temp file created on disk when we import the file into the certificate store. If the temp file is gone we are not able to use Connect-PnPOnline with the thumbprint method because there is no longer a private key associated with that cert in the store. This can be verified by interrogating the certificate's "PrivateKey" property with PowerShell. I believe this is the same behavior that @jagsridharan is seeing.

This is the test we ran yesterday:

  1. Create a Cert
  2. C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys is empty
  3. Add cert to Personal store on Cert Manager
  4. New item created in folder path C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys
  5. Check private key exists:
$cert = Get-ChildItem "cert:\LocalMachine\My" | Where-Object {$_.Thumbprint -eq $id.Thumbprint}
$cert.PrivateKey
  1. Key returns data
  2. Connect to cloud using either: Connect-AzureAD or Connect-PnPOnline SUCCESS
  3. Connect-PnPOnline
  4. Disconnect-PnPOnline
  5. Item created in folder path C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys no longer present
  6. Connect to cloud using either: Connect-AzureAD or Connect-PnPOnline FAIL
  7. Check private key exists: 
    $cert = Get-ChildItem "cert:\LocalMachine\My" | Where-Object {$_.Thumbprint -eq $id.Thumbprint}
$cert.PrivateKey
  8. No key returned

As @KoenZomers something was reintroduced into the code that triggered this issue, that @wobba addressed for issue #2101 is there anything i can do on my end to help test, debug let me know.

KoenZomers commented 4 years ago

Thanks for sharing these detailed steps @yumoraby. Still can't explain what technically could be causing this. Must almost be a difference in Windows 10. I just tried your steps and for me it does not add anything to C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys when I import my PFX into the Windows Certificate store.

What I will do to close this issue is the following. Since the file deletion from C:\ProgramData\Microsoft\Crypto\RSA\MachineKeys should only occur anyway when -CertificatePath is being used as that's the only method where it will copy something into the MachineKeys folder potentially filling it up, I will rewrite the code so it will only do the delete when that method has been used to connect as well. Based on what I'm reading here how it behaves for you, that should fix it.

I'll compile a pre-release version once I'm done with the change so you can test it out to see if it really works as I can't test it myself due to not being able to reproduce the issue.

KoenZomers commented 4 years ago

Here's a version in which what I wrote in my previous comment has been applied: SharePointPnPPowerShellOnline.zip

  1. Extract the contents of this ZIP to any location on your machine
  2. Ensure you have a new clean PowerShell window and run: Import-Module <path to where you extracted the files>\SharePointPnP.PowerShell.Online.Commands.dll
  3. Perform your test

Let me know if it fixes the issue for you.

yumoraby commented 4 years ago

Here's a version in which what I wrote in my previous comment has been applied: SharePointPnPPowerShellOnline.zip

  1. Extract the contents of this ZIP to any location on your machine
  2. Ensure you have a new clean PowerShell window and run: Import-Module <path to where you extracted the files>\SharePointPnP.PowerShell.Online.Commands.dll
  3. Perform your test

Let me know if it fixes the issue for you.

Will look to get this done later today. Should have a response on my findings in approx 5 hours.

fastlaneb commented 4 years ago

Hi Team, I tested and it no longer deletes file from the MachineKeys directory and I can connect and disconnect without error. Thanks @KoenZomers

Another tidbit, I couldn't use the zip provided(didn't want to go in and unblock all the files) so I just grabbed the code changes from #2759 and rebuilt.

KoenZomers commented 4 years ago

Awesome, thanks for confirming @fastlaneb. I'll activate the PR. It will be merged and released in the July 2020 release around July 7th, 2020.

yumoraby commented 4 years ago

Downloaded @KoenZomers module, closed and reopened PowerShell ISE:

Import-Module MSOnline -ErrorAction SilentlyContinue
Import-Module C:\Users\yumoraby\Downloads\SharePointPnPPowerShellOnline\3.22.2006.3\SharePointPnP.PowerShell.Online.Commands.dll 
Get-Module | Select-Object -Property Name, Version

Name                                     Version    
----                                     -------    
CredentialManager                        2.0        
ISE                                      1.0.0.0    
Microsoft.PowerShell.Management          3.1.0.0    
Microsoft.PowerShell.Security            3.0.0.0    
Microsoft.PowerShell.Utility             3.1.0.0    
pki                                      1.0.0.0    
SharePointPnP.PowerShell.Online.Commands 3.22.2006.2

My findings running the following:

$id = Get-ItemProperty 'HKLM:\SOFTWARE\Microsoft\Microsoft Assessments\'
$cert = Get-ChildItem "cert:\LocalMachine\My" | Where-Object {$_.Thumbprint -eq $id.Thumbprint}
$cert.PrivateKey
$domainName = ($id.TenantDomain.split('.')[0])

Connect-PnPOnline -Tenant $id.TenantDomain -ClientId $id.ApplicationId -Thumbprint $id.Thumbprint -Url https://$domainName-admin.sharepoint.com 

$masterList = Get-PnPList -Identity "DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS"
$query = "<View Scope='RecursiveAll'><RowLimit>4000</RowLimit></View>"
$sites = Get-PnPListItem $masterList -Query $query
$counter = 0

Disconnect-PnPOnline

Private Key was deleted, and am not able to reconnect.

Did the new package actually get imported, as the module version number is the same.

My Windows Version is: Version: 10.0.18363 aka Windows 10 1909

I have an issue where it does not upgrade to Windows 10 2004

KoenZomers commented 4 years ago

@yumoraby It's correct that you still see 3.22.2006.2, I didn't update the version number in this build. I'm testing this from Windows 10 build 2004. Just double checked the IL of the version I shared and it's truly the updated version in which the certificate will not be deleted anymore. Not sure why you're still experiencing this, as the code line that deleted the certificate file will not be hit anymore unless you specify the -CertificatePath on Connect-PnPOnline. I'm pretty confident that with @fastlaneb his test we can conclude that it should work.

yumoraby commented 4 years ago

thanks @KoenZomers will look to see what @fastlaneb did in his tests, and see if we can share this with the team to test. My machine maybe due a rebuild

fastlaneb commented 4 years ago

Yes, trying to get isolated modules working can certainly be a challenge. I'll sync with you tomorrow @yumoraby and we'll get you a good build to test.

wobba commented 4 years ago

2759 is now merged so I believe this issue can be closed?

KoenZomers commented 4 years ago

Let's do that for now indeed. @fastlaneb @yumoraby @jagsridharan feel free to reopen if your tests still show issues with the updated code. @fastlaneb you can compile an internal test version from the dev branch now if you wish.