Feature to edit shell settings from the admin (OSOE-638)

[Piedone]

Since shell settings can serve as a source for configuration (see the discussion under #65) then it'll be really useful to be able to edit those shell settings from the admin area. So, add a textbox-based editor (perhaps Monaco, so whitespace can be easily managed) to the tenant editor that lets you change all shell settings (i.e. the whole ShellSettings dictionary).

• See the comments under Configuration provider that loads configs from shell settings (OSOE-637) #65 for implementation suggestions.
• To demonstrate this working, configure Media storage quota for a tenant.

Jira issue

[wAsnk]

Should this be a new project or is it okay to implement it in Lombiq.Hosting.Tenants.Management?

[Piedone]

Lombiq.Hosting.Tenants.Management is OK for this.

[wAsnk]

Current state:
All tenant affecting settings are listed in formatted JSON:

Save settings button calls the following controller:

[HttpPost]
    [ValidateAntiForgeryToken]
    public async Task<IActionResult> Edit(ShellSettingsEditorViewModel model)
    {
        if (!await _authorizationService.AuthorizeAsync(User, ShellSettingsEditPermissions.ShellSettingsEditPermission))
        {
            return Forbid();
        }

        if (!_shellHost.TryGetSettings(model.TenantId, out var shellSettings))
        {
            return NotFound();
        }

        var jsonConfigurationParser = new JsonConfigurationParser();
        var shellSettingsDictionary = jsonConfigurationParser.ParseConfiguration(model.Json);
        foreach (var key in shellSettingsDictionary.Keys)
        {
            shellSettings[key] = shellSettingsDictionary[key];
        }

        await _shellHost.UpdateShellSettingsAsync(shellSettings);

        return RedirectToAction(
            nameof(AdminController.Edit),
            typeof(AdminController).ControllerName(),
            new
            {
                area = "OrchardCore.Tenants",
                id = model.TenantId,
            });
    }

The shellSettings which I get from _shellHost contains the changes when I'm calling UpdateShellSettingsAsync but still the changes are not saved.

What I found is that I can only modify tenant configs such as:

It looks like using UpdateShellSettingsAsync won't update other keys, just those:
https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore/Shell/ShellSettingsManager.cs#L144

[Piedone]

Displaying all currently editable settings by default is a good idea, but make sure that it's apparent that it's for scaffolding only. We don't want people to downright save what's displayed (since that would copy all existing config).

Also, does this display settings coming from appsettings or everywhere? If the latter then rather don't display them because secret values can come from the environment.

[wAsnk]

Yeah you are right, we shouldn't display everything, only the tenant level settings.

There is a problem with saving shellSettings, it is not possible to save settings with more than one children because of this in shellHost.UpdateShellSettingsAsync:

var sections = settings.ShellConfiguration.GetChildren()
  .Where(s => !s.GetChildren().Any())
  .ToArray();

https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore/OrchardCore/Shell/ShellSettingsManager.cs#L192-L194

Meaning we can't save e.g. Lombiq_Hosting_Tenants_MediaStorageManagement:MaximumStorageQuotaBytes: 2147483648 currently through the IShellHost api.

[Piedone]

Please also reply to this:

Also, does this display settings coming from appsettings or everywhere? If the latter then rather don't display them because secret values can come from the environment.

I can't comment on the rest.

[wAsnk]

Yes, from everywhere.

[Piedone]

I see, then that's not something that should happen due to it potentially exposing private data.

[jtkech]

@Piedone @wAsnk Just for infos and only from memory

ShellSettings has 2 ShellConfiguration, _settings with the minimal data to manage tenants states, coming by default for all tenants from the same tenants.json, this is this one that is flat (only one level of childrens). Then _configuration, the ShellConfiguration property, the IShellConfiguration that we can resolve from the DI.

Both take into account all config sources but we only allow a set of fields to be edited, on saving the final values are saved (if different), by default for _settings in tenants.json, for _configuration in the tenant specific appsettings.json, both being config sources having higher precedences.

Edited: I would say that you may need to override IShellConfigurationSources, see our current implementations for file, database or blob, but SaveAsync() only has a IDictionary<string, string> data parameter, so here also the config data to save are flat.