Fixes for a couple @azure/core-http issues found during arm-* migration

[daviwil]

This change fixes two issues that were identified when trying to use ms-rest-nodeauth with @azure/arm-resources when testing PR #4315:

1. When a credential type from ms-rest-nodeauth which implements TokenClientCredentials was passed to the ResourceManagementClient, isTokenCredential was mistaking it for a TokenCredential (they both have a getToken method) and then used the wrong authentication policy. This change checks whether the credential's getToken function accepts parameters before assuming it's a TokenCredential since the TokenClientCredentials.getToken method takes no parameters.

2. In Preview 1, @azure/core-http's ServiceClient implementation was incorrectly hardcoding the scope of all authentication requests to /.default instead of using the service-qualified scope name (i.e. https://management.azure.com/.default). This worked OK for data plane services but failed immediately for management plane services that do not accept the backcompat /.default scope.

The solution for #2 is unfortunately a bit of a necessary hack. ServiceClient implementations that are generated for ARM libraries must set the baseUri of their subclass instance to the service URI after the base ServiceClient constructor completes. Because baseUri is set late, the bearerTokenAuthenticationPolicy factory cannot be given a scope which includes the baseUri.

The fix here is to use a factory wrapper which accesses the ServiceClient.baseUri property right at the time the bearerTokenAuthenticationPolicy factory is first invoked so that the populated value gets used to create the scope string.

The alternative would be to change the constructor (or options type) of ServiceClient to accept the baseUri so that it can be used earlier in the initialization process, but that would then require additional changes to autorest.typescript to wire up the baseUri assignment correctly. I thought that this solution would be less trouble, but I'm happy to go for the full solution if folks think it's more appropriate.

[bterlson]

I can't say that this is bad per se, but in my experience function length checks come back to haunt. Preserving function length during transpilation is... hit or miss. Parameters with defaults don't contribute to length either, so making getToken's parameter optional would make it pass this check. If there's no other way this seems fine but let's be careful!

[ramya-rao-a]

Where is it exactly that we want to differentiate between the ms-rest-nodeauth credential and the TokenCredential?
I wonder, instead of checking for isTokenCredential(), if we should check for the other type

The TokenClientCredentials has 2 functions signRequest and getToken where as TokenCredential has only one of these. So there is our differentiator...

[daviwil]

Yep, that's what I was planning to try after @bterlson's comment. Just updated the PR with that fix and a test to verify it.

[ramya-rao-a]

Perhaps add a comment here as to why the explicit check on signRequest? Will help others who have no idea of ms-rest-nodeauth

[daviwil]

Good point, thanks!

[daviwil]

Comment added:

  // Check for an object with a 'getToken' function but without
  // a 'signRequest' function.  We do this check to make sure that
  // a ServiceClientCredentials implementor (like TokenClientCredentials
  // in ms-rest-nodeauth) doesn't get mistaken for a TokenCredential.