Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] ConfidentialClientApplicationOptions Compile error with .NET8 Configuration Binder Source Generator #4453

Closed
pocki opened this issue Dec 5, 2023 · 3 comments · Fixed by #4460

Comments

@pocki
Copy link

pocki commented Dec 5, 2023

Library version used

4.58

.NET version

.NET 8.0

Scenario

ConfidentialClient - service to service (AcquireTokenForClient)

Is this a new or an existing app?

The app is in production, and I have upgraded to a new version of MSAL

Issue description and reproduction steps

.NET8 Web App with ConfidentialClientApp and config ConfidentialClientApplicationOptions in appsettings.json
Everything was working as expected.

I tried to enable the new Configuration binder Source generators coming with .NET8:
<EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator> in csproj

With source generator enabled, I get compile errors as there is an obsolete Property in the Options class:
Error CS0619 'ApplicationOptions.Component' is obsolete: 'Should use ClientName and ClientVersion properties instead of Component'

Relevant code snippets

//.csproj:
<EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>

//Startup.cs
services.Configure<ConfidentialClientApplicationOptions>(configuration.GetSection("AzureAD"));

//Generated Config code:

        public static void BindCore(IConfiguration configuration, ref ConfidentialClientApplicationOptions instance, bool defaultValueIfNotFound, BinderOptions? binderOptions)
        {
            ValidateConfigurationKeys(typeof(ConfidentialClientApplicationOptions), s_configKeys_ConfidentialClientApplicationOptions, configuration, binderOptions);

            if (configuration["ClientSecret"] is string value15)
            {
                instance.ClientSecret = value15;
            }

            if (configuration["AzureRegion"] is string value16)
            {
                instance.AzureRegion = value16;
            }

            if (configuration["EnableCacheSynchronization"] is string value17)
            {
                instance.EnableCacheSynchronization = ParseBool(value17, () => configuration.GetSection("EnableCacheSynchronization").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.EnableCacheSynchronization = default;
            }

            if (configuration["ClientId"] is string value18)
            {
                instance.ClientId = value18;
            }

            if (configuration["TenantId"] is string value19)
            {
                instance.TenantId = value19;
            }

            if (configuration["AadAuthorityAudience"] is string value20)
            {
                instance.AadAuthorityAudience = ParseEnum<AadAuthorityAudience>(value20, () => configuration.GetSection("AadAuthorityAudience").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.AadAuthorityAudience = default;
            }

            if (configuration["Instance"] is string value21)
            {
                instance.Instance = value21;
            }

            if (configuration["AzureCloudInstance"] is string value22)
            {
                instance.AzureCloudInstance = ParseEnum<AzureCloudInstance>(value22, () => configuration.GetSection("AzureCloudInstance").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.AzureCloudInstance = default;
            }

            if (configuration["RedirectUri"] is string value23)
            {
                instance.RedirectUri = value23;
            }

            if (configuration["Component"] is string value24)
            {
                instance.Component = value24;
            }

            if (configuration["ClientName"] is string value25)
            {
                instance.ClientName = value25;
            }

            if (configuration["ClientVersion"] is string value26)
            {
                instance.ClientVersion = value26;
            }

            if (AsConfigWithChildren(configuration.GetSection("ClientCapabilities")) is IConfigurationSection section27)
            {
                IEnumerable<string>? temp29 = instance.ClientCapabilities;
                temp29 = temp29 is null ? (IEnumerable<string>)new List<string>() : (IEnumerable<string>)new List<string>(temp29);
                BindCore(section27, ref temp29, defaultValueIfNotFound: false, binderOptions);
                instance.ClientCapabilities = temp29;
            }

            if (configuration["LegacyCacheCompatibilityEnabled"] is string value30)
            {
                instance.LegacyCacheCompatibilityEnabled = ParseBool(value30, () => configuration.GetSection("LegacyCacheCompatibilityEnabled").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.LegacyCacheCompatibilityEnabled = default;
            }

            if (configuration["KerberosServicePrincipalName"] is string value31)
            {
                instance.KerberosServicePrincipalName = value31;
            }

            if (configuration["TicketContainer"] is string value32)
            {
                instance.TicketContainer = ParseEnum<KerberosTicketContainer>(value32, () => configuration.GetSection("TicketContainer").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.TicketContainer = default;
            }

            if (configuration["LogLevel"] is string value33)
            {
                instance.LogLevel = ParseEnum<LogLevel>(value33, () => configuration.GetSection("LogLevel").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.LogLevel = default;
            }

            if (configuration["EnablePiiLogging"] is string value34)
            {
                instance.EnablePiiLogging = ParseBool(value34, () => configuration.GetSection("EnablePiiLogging").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.EnablePiiLogging = default;
            }

            if (configuration["IsDefaultPlatformLoggingEnabled"] is string value35)
            {
                instance.IsDefaultPlatformLoggingEnabled = ParseBool(value35, () => configuration.GetSection("IsDefaultPlatformLoggingEnabled").Path);
            }
            else if (defaultValueIfNotFound)
            {
                instance.IsDefaultPlatformLoggingEnabled = default;
            }
        }

Expected behavior

I can use the new Configuration Binder Source Generators of .NET8, no compile error.

Identity provider

Microsoft Entra ID (Work and School accounts and Personal Microsoft accounts)

Regression

have not tried with other/older versions

Solution and workarounds

Don't use Configuration Binder Source Generators of .NET8

@pocki pocki added needs attention Delete label after triage untriaged Do not delete. Needed for Automation labels Dec 5, 2023
@pmaytak pmaytak added this to the 4.59.0 milestone Dec 5, 2023
@pmaytak pmaytak added bug P2 confidential-client and removed untriaged Do not delete. Needed for Automation needs attention Delete label after triage labels Dec 5, 2023
@pmaytak
Copy link
Contributor

pmaytak commented Dec 5, 2023

Possible solutions:

  • Obsolete the Component property as a warning, not error.
  • Apply some "Ignore on source generation" attribute that may be available in .NET 6.

More info about the feature:
Configuration-binding source generator
Using the new configuration binder source generator

@pmaytak pmaytak modified the milestones: 4.59.0, 4.58.1 Dec 5, 2023
@bgavrilMS
Copy link
Member

How about we just delete the Component property @pmaytak ? It's been obsolete with error for a really long time and it never had any use.

@pmaytak
Copy link
Contributor

pmaytak commented Dec 6, 2023

Well, removing it is a binary breaking change at least. But even just obsoleting with a warning will not fully solve this for folks who have "treat warnings as errors" enabled. I think it's ok to remove it in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
3 participants