search

OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.23k stars 2.34k forks source link

Add configuration overrides for the OrchardCore.Microsoft.Authentication module. #8437

Open willnationsdev opened 3 years ago

willnationsdev commented 3 years ago

Describe the bug

Problems with configuring AzureADSettings in answer to #8432.

To Reproduce

Steps to reproduce the behavior:

  1. Turn on the Microsoft Azure Active Directory Authentication feature.
  2. Update the root appsettings.json to look like this:
{
  "OrchardCore": {
    "OrchardCore_Microsoft_Authentication": {
      "AzureADSettings": {
        "DisplayName": "test",
        "AppId": "test",
        "TenantId": "test",
        "CallbackPath": ""
      }
    }
  }
}

Expected behavior

  1. The data in the config should be respected. As it stands, even though it is an ISiteSettings value stored in the database, it is not registered as an Options (should it be?) and the IShellConfiguration is never consulted, so users cannot control these values from the config at all. Merely assigning values to the config values does NOT, e.g. override the display name of the existing AzureADSettings you've assigned via the admin GUI.

  2. Because the values are forcibly being set via config, the UI should preset the values in the DisplayDriver and modify the View to disable editing of the fields, providing a hint that the values are being overridden by a config value (that way, people understand the reason it is blocking interaction).

Same is likely true for Microsoft Authentication since I haven't seen any mention of config in the module's source code at all. Possibly other External login providers, but I haven't checked those.

Screenshots

--

deanmarcussen commented 3 years ago

Two issues

  1. Configure the AzureADSettings with IConfigureOptions
  2. How to present all the ISiteSettings when they are also configured via IShellConfiguration - bearing in mind only some of the values may be set this way, some may still be set by the site settings.

For 1, I think it would be fine too refactor that to use IConfigureOptions as is done for many, but not all (there are many that it hasn't been done for) settings. Plenty of examples in the source code for this.

If you wanted to do a pr for this that would be great.

For 2 see https://github.com/OrchardCMS/OrchardCore/issues/6036 for some discussion about it, again if someone wanted to work towards a solution, that would be great.