Get the URL authority when retrieving it from the HttpClient BaseAddress

[0xced]

When no explicit discovery address is specified, the authority is computed from the underlying HttpClient BaseAddress property.

Before this commit: the computed authority is the full URL.

After this commit: the computed authority is the authority part of the URL.

A HttpClient BaseAddress would typically include some base path to access an API. This base path is not part of the authority.

[damianh]

Merged. The test is 🎯 . Thanks for the contribution!

[josephdecock]

I think we actually may want to back this out, because we sometimes do need the path portion of the url to get to the correct discovery document. It is allowed by the spec and used by some cloud providers (e.g., Entra).

In the OIDC Discovery spec, paths are allowed in the url to the discovery document. That url is just the issuer with "/.well-known/openid-configuration" appended. But the issuer is "a URI with a scheme component that MUST be https, a host component, and optionally, port and path components". So just following the spec, I think we have to not use the Authority part of the url.

In practice, Entra puts the tenant id into the discovery url: https://login.microsoftonline.com/{tenant}/v2.0/.well-known/openid-configuration. So it seems likely that there would be bugs for at least Entra users and possibly more.

[0xced]

Getting the authority from the HttpClient only happens when no address is explicitly specified.

It's not obvious when looking at the diff from this pull request but the GetDiscoveryDocumentAsync extension method comes in two flavours where the address can be explicitly specified, either through the address parameter or the Address property of the DiscoveryDocumentRequest parameter.

public static async Task<DiscoveryDocumentResponse> GetDiscoveryDocumentAsync(this HttpClient client, string? address = null, CancellationToken cancellationToken = default);

public static async Task<DiscoveryDocumentResponse> GetDiscoveryDocumentAsync(this HttpMessageInvoker client, DiscoveryDocumentRequest request, CancellationToken cancellationToken = default);

And there are many tests in DiscoveryPolicyTests_AuthorityStringComparison and DiscoveryPolicyTests_AuthorityUriComparison that cover those cases.

[josephdecock]

What I'm thinking about is that there are a lot of different ways to organize the URLs of APIs and identity providers. In the specific case of the demo server, this PR's change is more convenient, but in other cases it would be less convenient. Even though the other way of setting the address exists, this is still a breaking change in this api.

[damianh]

@josephdecock @0xced it seems there is a mis-alignment on the validity of change that requires further discussion. Therefore we'll revert the change and take the topic to an issue for the details to be trashed out first.

@josephdecock thanks for validating