(#1021) Authcred #1031
(#1021) Authcred #1031
Conversation
| .PARAMETER Credential | ||
| OPTIONAL - A System.Net.ICredentials object that contains credentials to | ||
| use to authenticate to the URL server. This is just ultimately passed | ||
| onto System.Net.HttpWebRequest Crentials property. Available in 0.9.11+ |
There was a problem hiding this comment.
Will need to update version on all of these.
| @@ -261,7 +267,7 @@ param( | |||
| if ($url.StartsWith('http:')) { | |||
| try { | |||
| $httpsUrl = $url.Replace("http://", "https://") | |||
| Get-WebHeaders -Url $httpsUrl -ErrorAction "Stop" | Out-Null | |||
| Get-WebHeaders -Url $httpsUrl -ErrorAction "Stop" -Credential $credential | Out-Null | |||
There was a problem hiding this comment.
Is it okay to pass a $null credential or will that error?
There was a problem hiding this comment.
I think so (though I'm no powershell expert). Get-WebHeaders has a check for a $null cred (around line 71). Is that what you mean?
There was a problem hiding this comment.
This is still showing up in github as a requested change....
| @@ -62,7 +68,10 @@ param( | |||
|
|
|||
| $request = [System.Net.HttpWebRequest]::Create($url); | |||
| $defaultCreds = [System.Net.CredentialCache]::DefaultCredentials | |||
| if ($defaultCreds -ne $null) { | |||
| if ($credential -ne $null) { | |||
| Write-Host "Using credential." | |||
There was a problem hiding this comment.
This should be either Write-Verbose or Write-Debug. And it should provide more information. What credential? User passed or something default?
|
What happened here? |
Apteryx0
force-pushed
the
authcred
branch
3 times, most recently
from
03bd227 to
1f89144
Compare
|
Sorry, I made a real mess of that. Hopefully I have reset everything back to what it should have been and fixed what you asked for. |
| @@ -53,6 +58,7 @@ Get-WebFile | |||
| param( | |||
| [parameter(Mandatory=$false, Position=0)][string] $url = '', | |||
| [parameter(Mandatory=$false, Position=1)][string] $userAgent = 'chocolatey command line', | |||
| [parameter(Mandatory=$false, Position=2)][Object] $credential = $null, | |||
There was a problem hiding this comment.
Remove position. Go for name only.
|
This will probably be added to Feel free to squash these commits into one single commit, then make that commit summary and body pretty good. 👍 |
|
As far as I am aware all requested changes have been made (unless I'm missing something....?) - not sure why this is still showing as "Changes requested" |
|
@spitzbub only because I haven't been back here to review again. Notice the date of my request for changes. Don't worry, will be reviewing these over the next week. |
|
This was reviewed for 0.10.4 but it didn't make it in. It was due to line endings changing on one file making it harder to review - the line endings change was necessary to ensure authenticode signing the scripts does not fail due to issues in PowerShell. |
|
Also the don't merge master comment applies here. |
|
You may also wish to fix the git username in your commits since you changed your username - otherwise it doesn't know how to apply contributor credit for your work. I'm not sure on Authcred here - there is probably a better name that summarizes this feature. |
Extends the variaboue helper APIs that call Get-ChocolateyWebFile to take a authentication credential and then pass that credential to the various web calls. This allows us to download resources from webservers that require authentication. The credential eventually gets used in a System.Net.HttpWebRequest object and thus can be either a NetworkCredential or a CredentialCache object. The previous behaviour was just to use the default credentials which is generally the windows user credential, which is often unsuitable.
|
Russell Mora seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Is there any update on this? |
|
As the CLA hasn't been signed we can't pull this change in. |
Sadly, this hasn't been done for a few years now :/ Is there anything I can do to help with this topic? Would it be helpful if I create a new PR? I'm tagging the original author of this PR in this comment. Maybe he can do something about this |
We also need the ability to pass credentials and it seems like this will stuck forever if no one takes action. Kind regards |
|
@norbertstoll @we-mi A new PR would be best at this stage to move this along. To remove any misunderstanding about this specific PR I'm going to go and ahead and close it. |
Hi @we-mi, are you still interested in getting this fixed? It's obviously up to us, since @Apteryx0 didn't respond the your tagging. Do you have any other suggestions on this? Regards |
|
Hi @norbertstoll, thanks for bringing this up again 👍 I'm totally interested in getting this fixed. You can expect a new PR in the next couple of days. I just need to read a bit more about how to create correct PRs in this repo :D |
|
Hi @we-mi, you're welcome ;) Let me know if I can assist at any point. I'm really looking forward to get this fixed/implemented. |
This is a pretty simple change - the MS web APIs take a credential object. This change adds that cred object to the appropriate chocolatey helper functions and then passes it down to the MS web APIs.
Closes #1021