Why was OpenIdConnectClientConfiguration.Authority removed from RC to final?

[natelaff]

Why from AccessTokenManagement was OpenIdConnectClientConfiguration.Authority commented out? This was in place in 4.0.0-RC1.5 then commented out.

This is literally the one thing I use this library for to switch the authority for a multi-tenant app to include the tenant name in the path to identity server. I can't find any comments about its removal in the release notes, and the commit has no information.

There was obvious intent here but I'm not versed enough in OIDC plumbing i suppose to understand the reasoning or the alternative method.

[Erwinvandervalk]

Hi @natelaff,

Unfortunately during the development of AccessTokenManagement V4, we had a pivot moment. We were about to release ATM V4 (hence the RC you're referring to), but then due to customer feedback and internal design reviews, we decided to go back to the drawing board and perform some extensive internal and external refactoring. So we withdrew the RC1.5 and started to work on a newer version.

As part of these extensive changes, we also looked closely at the usage of the Authority property. This property wasn't actually used anywhere in the library, because what's actually used is the TokenEndpoint url. Originally, the intention was to extend ATM to allow you to only set the authority, which would then be used to fetch the discovery document and set the tokenendpoint url automatically. We did some spikes around this, but decided had unintended side effects and wasn't worth the effort.

At that point, we decided to drop the Authority property, as it wasn't used in the library. This should have been added to the release notes, but was forgotten amongst the massive other changes that have happened. I'll add this to the release notes.

Now that obviously doesn't help you here, so I'm curious to learn more about your scenario. Can you tell me a bit more about how you were using this property?

[natelaff]

Thanks for the response @Erwinvandervalk!

I use https://github.com/Finbuckle/Finbuckle.MultiTenant for multi-tenancy. Here you can do very simple things in your pipeline for overriding options per tenant. So very easy to just do like options.Authority = "https://auth.mydomain.com/" + tenantName and the MVC pipeline will do all the stuff.

So naturally I would set this in my IOpenIdConnectConfigurationService implementation, which looks just like the default with the exception of specifically using that IOptionsMonitor version of the authority

       var config = new OpenIdConnectClientConfiguration
       {
           Scheme = configScheme,

           Authority = options.Authority,
           TokenEndpoint = configuration.TokenEndpoint,
           RevocationEndpoint = configuration.RevocationEndpoint,

           ClientId = options.ClientId,
           ClientSecret = options.ClientSecret,
           HttpClient = options.Backchannel,
       };
       return config;

I suppose I understand why you aren't using it if you're relying only on the TokenEndpoint, but that Authority also controls what is considered a valid token, yeah? So if that is not going to match I'm going to have an issue.

Hope that illuminates! I didn't test this after I noticed the break, so i started to look into the why first (breaking change! research instead of just code correct!) so I guess it's possible things still work ok, but something told me this wasn't going to work (otherwise I never would have implemented a custom IOpenIdConnectConfigurationService in the first place ;))

I can try to return to the 4.0 release and remove that and check, but I just wanted to see what the thought process was in removing it.

[natelaff]

In (limited) testing all indications seem to be that I actually do not need Authority (as you stated), which would actually mean I no longer even need a custom implementation and can rely on the default one 🤷‍♂️. I'm supposing at the time you added TokenEndpoint and RevocationEndpoint I probably could have dropped my implementation as it would have been using the tenant name in the path as required.

[Erwinvandervalk]

Thank you for reporting the issue because it triggered us to improve the documentation!
Also thanks for confirming that it still works for you!