Fix NuGet/Home#1300: V3 install/restore only uses credentials for index.json. #126
Conversation
…ex.json. The problem is that in HttpHandlerResourceV3Provider, when there is a credential saved in nuget.config, the code createsCredentialCache, then adds NetworkCredential into the cache, with the uri of the source as key. However, forV3 source, other resources most likely have different URIs from index.json endpoint. Thus, this CredentialCache won't be able to provide credentials for other resources. It only works for index.json. The fix is to just create a normal NetworkCredential object.
| @@ -66,12 +64,7 @@ private HttpClientHandler TryGetCredentialAndProxy(PackageSource packageSource) | |||
| && !String.IsNullOrEmpty(packageSource.UserName) | |||
| && !String.IsNullOrEmpty(packageSource.Password)) | |||
| { | |||
| var cache = new CredentialCache(); | |||
There was a problem hiding this comment.
So with this we make host == account? Which means you can't host two private feeds on the same host? I think that's an ok assumption for now, we just have to document it (start a PR in the docs repo as well)
CC @csharpfritz
There was a problem hiding this comment.
Talking to the VSO folks, this will break some of their scenarios where the hosts are different between where index.json is hosted and where the registration blobs are at.
The suggestion is to make the code flow the credentials around. @zarenner is going to propose changes, and I'm going to ask @blowdart to take a look to see if we are worried about credential leak or not
There was a problem hiding this comment.
Looking at this again, David and I may have misunderstood something in our discussion with Yishai. My understanding now is that CredentialPromptWebRequestHandler gets non-domain-specific credentials, which it will happily use on all requests, regardless of the request hostname. That same CredentialPromptWebRequestHandler is re-used in multiple requests which may be for different hostnames.
If we're still misunderstanding something, please correct me, but otherwise this change looks fine to us. We're validating it now against our scenarios.
There was a problem hiding this comment.
@yishaigalatzer Right, with the current implementation, you can't host two different feeds that require different credentials on the same host. We need to redesign our authentication implementation to support this scenario.
|
|
when comments addressed and another signoff
|
Is the username updated on the package source after prompting? Is this relying on the call to index.json to handle all retries for getting the right credentials? |
|
What's the scenario where the password will differ? Sent from my Windows Phone From: Justin Emgartenmailto:[email protected] Is the username updated on the package source after prompting? Is this relying on the call to index.json to handle all retries for getting the right credentials? — |
|
@emgarten after prompting, the new username/password will get saved in CredentialStore.Instance. So the next time when this method is called, after this line
and the username/password saved in nuget.config will NOT get used. |
|
@yishaigalatzer when the nuget.config credentials are incorrect |
|
I get it. My question was when can they be incorrect for anything index.json represents but correct for index.json Sent from my Windows Phone From: Justin Emgartenmailto:[email protected] @yishaigalatzerhttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2fyishaigalatzer&data=01%7c01%7cyigalatz%40microsoft.com%7cfd0418e09e4b40d600cf08d2b273f89a%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=pIWc0oI%2b06%2bTiEki6V%2foAG9mmjU9DzMbgtgtMwzWEqQ%3d when the nuget.config credentials are incorrect @feilinghttps://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fgithub.com%2ffeiling&data=01%7c01%7cyigalatz%40microsoft.com%7cfd0418e09e4b40d600cf08d2b273f89a%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=epvyiONftIExdoVePWAmKF5ZMqYMhZEfeB4t4CTcMy8%3d [:shipit:] — |
The problem is that in HttpHandlerResourceV3Provider, when there is a credential saved in
nuget.config, the code creates CredentialCache, then adds NetworkCredential into the cache, with the
uri of the source as key. However, for V3 source, other resources most likely have different URIs
from index.json endpoint. Thus, this CredentialCache won't be able to provide credentials for other
resources. It only works for index.json.
The fix is to just create a normal NetworkCredential object.
@deepakaravindr @emgarten @yishaigalatzer @zhili1208