-
Notifications
You must be signed in to change notification settings - Fork 435
Description
Is your feature request related to a problem? Please describe.
Client certificates can be obtained from an mTLS session for authentication purposes. If the server is configured with any option other than ClientCertificateMethod.AllowRenegotiate
then obtaining this certificate not problematic. However, if it is set to AllowRenegotiation, then obtaining the certificate can cause issues on the client side if the certificate was not initially provided (exactly what occurs is client implementation specific, but it can be very disruptive). AllowRenegotiation is the default behavior for NetFramework and .net core versions prior to .net6.
As a result, when we handle web requests we do not initially populate the ClientCertificate property in HttpRequestData. Instead, we add an action to the PropertyBag which our authorization logic invokes to populate the ClientCertificates if, and only if, we need the certificate for authorization reasons. Whether we need the certificate is highly variable as our endpoints can authenticate via tokens or certificates.
However, when developers attempt to extend our logic, it causes problems as they expect ClientCertificate to be populated if a certificate is present. It being unpopulated until a "magic" method is invoked is confusing and requires additional documentation.
Describe the solution you'd like
A clean mechanism to lazily populate the ClientCertificate property in HttpRequestData.
Describe alternatives you've considered
Forcing all of our consumers to set the flag correctly, however that is a large breaking change that we do not want to handle. In addition, there are valid reasons to use AllowRenegotiation (although it is not recommended), so it is not a unilateral change.
We can not always detect the status of the AllowRenegiation flag at runtime, so we can not vary our behavior based on this setting. We need behavior which works as intended regardless of the setting.
Loading a certificate may also have some cost based on the web architecture. By moving it to lazy loading in all scenarios it can be a performance improvement.
Additional context
I have created a draft PR here https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/3155/files, however I believe that the API surface is not ideal and is deserving of discussion.