Fix Google auth interop test

[JamesNK]

No description provided.

[JamesNK]

@apolcyn @jskeet Interop tests pass after downgrading Google.Apis.Auth.

Does that mean the change in Google.Apis.Auth that causes the interop error is intentional or a regression?

Problematic line:

grpc-dotnet/testassets/Shared/InteropClient.cs

Line 248 in c397bc7

Assert.IsFalse(googleCredential.IsCreateScopedRequired);

[jskeet]

cc @amanda-tarafa who is on holiday at the moment, but will want to look at this later.

I strongly suspect this is basically just a test issue, due to the change in googleapis/google-api-dotnet-client@bd80f0a which introduced the feature of Compute Credentials accepting scopes. (The name of IsCreateScopedRequired is unfortunate - it's more "permitted" than "required" IMO, but Amanda can give more details.)

If I'm right, then we should be able to narrow down the version range: I'd expect the current test to pass with 1.57.0 and fail with 1.58.0.

I suspect you could invert the assertion, or just remove it.

[apolcyn]

I think I see what happened here.

The "compute_engine_creds" test creates credentials via GoogleCredential.GetApplicationDefaultAsync();. This walks through an ordered list of credential mechanisms and picks the first one that works (depends on what's available). The way the "compute_engine_creds" test is run, we expect it to create a ComputeCredential (fetches a token from the GCE metadata server). I'm guessing the now-failing check was trying to do a sanity check that we did in fact resolve a ComputeCredential - because prior to https://github.com/googleapis/google-api-dotnet-client/pull/2103/files#diff-4ac910358ab8f456aef208d4ce8d2dc532d341bdc145c238683b3cb1bdee1af8, that type of credential did not support explicit scoping and so would have always been false.

googleapis/google-api-dotnet-client#2103 added support for explicit scoping (in some environments). That type of credential now always has SupportsExplicitScopes == true, so the check will of course fail.

Assuming this is all intentional, perhaps all we need for this test is a different, safer way to sanity check that we did resolve a ComputeCredential (if possible). We could perhaps remove it, the only downside is the test would be slightly fragile since it might not be using the credential we think it's using.

cc @jskeet

[jskeet]

I'm guessing the now-failing check was trying to do a sanity check that we did in fact resolve a ComputeCredential

If that's the case, there's a much simpler alternative, at least now - fetch googleCredential.UnderlyingCredential and check that it's a ComputeCredential.

[JamesNK]

Test is fixed. @apolcyn feel free to approve and merge.

[amanda-tarafa]

I'm back now, just catching up.

The name IsCreateScopeRequired is indeed very unfortunate but legacy and we cannot remove it without a significant breaking change. For reference, we internally call this SupportsExplicitScopes which is a way better name.

Although I cannot comment on the original intention of testing for "IsCreateScopeRequired" to be false, I can confirm the rest of @apolcyn's comment. ComputeCredential.IsCreateScopeRequired will always be true now because some (but we cannot tell which) ComputeCredentials support explicit scoping. The best way to check the credential to be a ComputeCredential is Jon's suggestion that's been implemented, so all good here.