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

Validate database connection before allowing a tenant to be added or setup #11822

Merged
merged 50 commits into from Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
998d170
Merge pull request #1 from OrchardCMS/main
MikeAlhayek Nov 13, 2021
70b66d3
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Dec 1, 2021
d1c31d2
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Dec 23, 2021
d277228
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Dec 28, 2021
718b31f
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Jan 4, 2022
40b4803
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Jan 5, 2022
25ea712
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Jan 16, 2022
bd905ca
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Jan 20, 2022
295b392
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Feb 23, 2022
7ac581c
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Apr 20, 2022
e4d5a2b
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Apr 22, 2022
0d02eb3
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Apr 27, 2022
4df6578
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 1, 2022
d46460c
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 10, 2022
606d821
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 10, 2022
028be3b
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 13, 2022
bcc0090
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 13, 2022
18a6f0b
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 16, 2022
d2b173a
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 18, 2022
18a4769
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 21, 2022
fac1a93
Merge branch 'OrchardCMS:main' into main
MikeAlhayek May 24, 2022
ab47e05
Merge branch 'OrchardCMS:main' into main
MikeAlhayek Jun 5, 2022
c726595
Add ConnectionFactoryProvider and validate tablePrefix before addnig …
malhayek2014 Jun 6, 2022
81bfeca
cleanup TenantValidator
malhayek2014 Jun 7, 2022
324bdc7
Update ConnectionFactoryProvider.cs
sebastienros Jun 9, 2022
9466019
Update IConnectionFactoryProvider.cs
sebastienros Jun 9, 2022
5457240
Update TenantValidatorTests.cs
sebastienros Jun 9, 2022
9d8e5f7
Instead of parsing out the connection string for all tenants, validat…
MikeAlhayek Jun 9, 2022
9d31031
Resolving conflict
MikeAlhayek Jun 9, 2022
61d0246
move DatabaseHelper to YesSql project and making it private
MikeAlhayek Jun 9, 2022
776385f
Clean up the code
MikeAlhayek Jun 10, 2022
02bcbdc
Check connection when editing a tenant during uninitilizing stage
MikeAlhayek Jun 11, 2022
d7c812d
update comment to restart build
MikeAlhayek Jun 12, 2022
4310203
Add a fix for #11876 and clean up the code
MikeAlhayek Jun 16, 2022
e05c321
Replace string base provider name to Enum and fix failing tests
MikeAlhayek Jun 17, 2022
c7df907
Clean up the SetupController to relay on the DbConnectionValidation t…
MikeAlhayek Jun 18, 2022
f046509
Remove unused services in the SetupController
MikeAlhayek Jun 18, 2022
f74d4a8
Merge branch 'main' into ValidateDatabasePrefix
MikeAlhayek Jul 9, 2022
4cda0bd
Fix labels
MikeAlhayek Jul 9, 2022
6dad07e
Merge branch 'ValidateDatabasePrefix' of https://github.com/CrestApps…
MikeAlhayek Jul 9, 2022
42953b7
Merge branch 'OrchardCMS:main' into ValidateDatabasePrefix
MikeAlhayek Jul 10, 2022
aa46120
Register ITableNameConvention and use it in the DbConnectionValidator
MikeAlhayek Jul 10, 2022
fb1b582
Move the TablePrefixSeparator into YesSqlOptions
MikeAlhayek Jul 10, 2022
7bc28e8
remove TablePrefixSeparator from the SetupConstants
MikeAlhayek Jul 11, 2022
7cc87e1
Merge branch 'OrchardCMS:main' into ValidateDatabasePrefix
MikeAlhayek Aug 1, 2022
213081d
update the TenantValidator to use shellHost.TryGetSettings() instead …
MikeAlhayek Aug 1, 2022
d078a1a
Update the tenant validator
MikeAlhayek Aug 2, 2022
1143c09
Update the TestValidator to check for exact Default tenant
MikeAlhayek Aug 4, 2022
c485d98
Merge branch 'main' into ValidateDatabasePrefix
MikeAlhayek Aug 17, 2022
ff89efe
Fix syntax, and format after conflict fix
MikeAlhayek Aug 17, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -214,7 +214,7 @@ public async Task<ShellSettings> CreateTenantSettingsAsync(TenantSetupOptions se

shellSettings["ConnectionString"] = setupOptions.DatabaseConnectionString;
shellSettings["TablePrefix"] = setupOptions.DatabaseTablePrefix;
shellSettings["DatabaseProvider"] = setupOptions.DatabaseProvider;
shellSettings["DatabaseProvider"] = setupOptions.DatabaseProvider.ToString();
shellSettings["Secret"] = Guid.NewGuid().ToString();
shellSettings["RecipeName"] = setupOptions.RecipeName;

Expand Down
Expand Up @@ -43,7 +43,7 @@ public class TenantSetupOptions
/// <summary>
/// Gets or sets the database provider.
/// </summary>
public string DatabaseProvider { get; set; }
public DatabaseProviderName DatabaseProvider { get; set; }

/// <summary>
/// Gets or sets the database connection string.
Expand Down
Expand Up @@ -26,7 +26,7 @@ public class SetupController : Controller
private readonly ISetupService _setupService;
private readonly ShellSettings _shellSettings;
private readonly IShellHost _shellHost;
private IdentityOptions _identityOptions;
private readonly IdentityOptions _identityOptions;
private readonly IEmailAddressValidator _emailAddressValidator;
private readonly IEnumerable<DatabaseProvider> _databaseProviders;
private readonly ILogger _logger;
Expand Down Expand Up @@ -59,13 +59,9 @@ public async Task<ActionResult> Index(string token)
var recipes = await _setupService.GetSetupRecipesAsync();
var defaultRecipe = recipes.FirstOrDefault(x => x.Tags.Contains("default")) ?? recipes.FirstOrDefault();

if (!string.IsNullOrWhiteSpace(_shellSettings["Secret"]))
if (!await ShouldProceedWithTokenAsync(token))
{
if (string.IsNullOrEmpty(token) || !await IsTokenValid(token))
{
_logger.LogWarning("An attempt to access '{TenantName}' without providing a secret was made", _shellSettings.Name);
return StatusCode(404);
}
return StatusCode(404);
}

var model = new SetupViewModel
Expand All @@ -90,28 +86,14 @@ public async Task<ActionResult> Index(string token)
[HttpPost, ActionName("Index")]
public async Task<ActionResult> IndexPOST(SetupViewModel model)
{
if (!string.IsNullOrWhiteSpace(_shellSettings["Secret"]))
if (!await ShouldProceedWithTokenAsync(model.Secret))
{
if (string.IsNullOrEmpty(model.Secret) || !await IsTokenValid(model.Secret))
{
_logger.LogWarning("An attempt to access '{TenantName}' without providing a valid secret was made", _shellSettings.Name);
return StatusCode(404);
}
return StatusCode(404);
}

model.DatabaseProviders = _databaseProviders;
model.Recipes = await _setupService.GetSetupRecipesAsync();

var selectedProvider = model.DatabaseProviders.FirstOrDefault(x => x.Value == model.DatabaseProvider);

if (!model.DatabaseConfigurationPreset)
{
if (selectedProvider != null && selectedProvider.HasConnectionString && String.IsNullOrWhiteSpace(model.ConnectionString))
{
ModelState.AddModelError(nameof(model.ConnectionString), S["The connection string is mandatory for this provider."]);
}
}

if (String.IsNullOrEmpty(model.Password))
{
ModelState.AddModelError(nameof(model.Password), S["The password is required."]);
Expand All @@ -123,7 +105,7 @@ public async Task<ActionResult> IndexPOST(SetupViewModel model)
}

RecipeDescriptor selectedRecipe = null;
if (!string.IsNullOrEmpty(_shellSettings["RecipeName"]))
if (!String.IsNullOrEmpty(_shellSettings["RecipeName"]))
{
selectedRecipe = model.Recipes.FirstOrDefault(x => x.Name == _shellSettings["RecipeName"]);
if (selectedRecipe == null)
Expand Down Expand Up @@ -169,8 +151,9 @@ public async Task<ActionResult> IndexPOST(SetupViewModel model)
}
};

if (!string.IsNullOrEmpty(_shellSettings["ConnectionString"]))
if (!String.IsNullOrEmpty(_shellSettings["ConnectionString"]))
{
model.DatabaseConfigurationPreset = true;
setupContext.Properties[SetupConstants.DatabaseProvider] = _shellSettings["DatabaseProvider"];
setupContext.Properties[SetupConstants.DatabaseConnectionString] = _shellSettings["ConnectionString"];
setupContext.Properties[SetupConstants.DatabaseTablePrefix] = _shellSettings["TablePrefix"];
Expand All @@ -184,7 +167,7 @@ public async Task<ActionResult> IndexPOST(SetupViewModel model)

var executionId = await _setupService.SetupAsync(setupContext);

// Check if a component in the Setup failed
// Check if any Setup component failed (e.g., database connection validation)
if (setupContext.Errors.Any())
{
foreach (var error in setupContext.Errors)
Expand Down Expand Up @@ -215,9 +198,13 @@ private void CopyShellSettingsValues(SetupViewModel model)
if (!String.IsNullOrEmpty(_shellSettings["DatabaseProvider"]))
{
model.DatabaseConfigurationPreset = true;
model.DatabaseProvider = _shellSettings["DatabaseProvider"];
if (Enum.TryParse(_shellSettings["DatabaseProvider"], out DatabaseProviderName providerName))
{
model.DatabaseProvider = providerName;
}
}
else

if (!model.DatabaseProvider.HasValue)
{
model.DatabaseProvider = model.DatabaseProviders.FirstOrDefault(p => p.IsDefault)?.Value;
}
Expand All @@ -228,6 +215,21 @@ private void CopyShellSettingsValues(SetupViewModel model)
}
}

private async Task<bool> ShouldProceedWithTokenAsync(string token)
{
if (!String.IsNullOrWhiteSpace(_shellSettings["Secret"]))
{
if (String.IsNullOrEmpty(token) || !await IsTokenValid(token))
{
_logger.LogWarning("An attempt to access '{TenantName}' without providing a secret was made", _shellSettings.Name);

return false;
}
}

return true;
}

private async Task<bool> IsTokenValid(string token)
{
try
Expand Down
@@ -1,6 +1,7 @@
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using OrchardCore.Data;
using OrchardCore.Recipes.Models;
using OrchardCore.Setup.Annotations;
Expand All @@ -15,7 +16,7 @@ public class SetupViewModel

public string Description { get; set; }

public string DatabaseProvider { get; set; }
public DatabaseProviderName? DatabaseProvider { get; set; }

public string ConnectionString { get; set; }

Expand All @@ -24,6 +25,7 @@ public class SetupViewModel
/// <summary>
/// True if the database configuration is preset and can't be changed or displayed on the Setup screen.
/// </summary>
[BindNever]
public bool DatabaseConfigurationPreset { get; set; }

[Required]
Expand All @@ -38,8 +40,10 @@ public class SetupViewModel
[DataType(DataType.Password)]
public string PasswordConfirmation { get; set; }

[BindNever]
public IEnumerable<DatabaseProvider> DatabaseProviders { get; set; } = Enumerable.Empty<DatabaseProvider>();

[BindNever]
public IEnumerable<RecipeDescriptor> Recipes { get; set; }

public bool RecipeNamePreset { get; set; }
Expand Down
Expand Up @@ -66,6 +66,7 @@
var passwordTooltip = T["Password must have at least {0}.", passwordOptions];
}
<form asp-action="Index">

<div class="bg-light p-2">
@if (LocOptions.Value.SupportedUICultures.Count() > 1)
{
Expand All @@ -92,6 +93,9 @@
<h1>@T["Setup"]</h1>
<p class="lead">@T["Please answer a few questions to configure your site."]</p>
</div>

<div asp-validation-summary="ModelOnly"></div>

@if (defaultRecipe == null)
{
<div class="alert alert-danger" role="alert">
Expand Down Expand Up @@ -173,7 +177,7 @@

<div class="mb-3 col-md-6 tablePrefix" asp-validation-class-for="TablePrefix">
<label asp-for="TablePrefix">@T["Table Prefix"]</label>
<input asp-for="TablePrefix" class="form-select" />
<input asp-for="TablePrefix" class="form-control" />
<span asp-validation-for="TablePrefix" class="text-danger"></span>
<span class="text-muted form-text small">@T["You can specify a table prefix if you intend to reuse the same database for multiple sites."]</span>
</div>
Expand Down Expand Up @@ -236,7 +240,7 @@
</form>
<script src="~/OrchardCore.Setup/Scripts/setup.min.js"></script>
<script>
$(function(){
$(function() {
$('#Password').strength({
minLength: @(options.Password.RequiredLength),
upperCase: @(options.Password.RequireUppercase ? "true" : "false"),
Expand All @@ -253,13 +257,13 @@

toggleConnectionString = document.querySelector('#toggleConnectionString');
if (toggleConnectionString) {
toggleConnectionString.addEventListener('click', function (e) {
toggleConnectionString.addEventListener('click', function(e) {
togglePasswordVisibility(document.querySelector('#ConnectionString'), document.querySelector('#toggleConnectionString'))
});
}

togglePassword = document.querySelector('#togglePassword');
togglePassword.addEventListener('click', function (e) {
togglePassword.addEventListener('click', function(e) {
togglePasswordVisibility(document.querySelector('#Password'), document.querySelector('#togglePassword'))
});

Expand Down
Expand Up @@ -180,7 +180,9 @@ public async Task<ActionResult> Setup(SetupApiViewModel model)
databaseProvider = model.DatabaseProvider;
}

var selectedProvider = _databaseProviders.FirstOrDefault(x => String.Equals(x.Value, databaseProvider, StringComparison.OrdinalIgnoreCase));
Enum.TryParse(databaseProvider, out DatabaseProviderName providerName);

var selectedProvider = _databaseProviders.FirstOrDefault(x => x.Value == providerName);

if (selectedProvider == null)
{
Expand Down
Expand Up @@ -6,6 +6,7 @@
using Microsoft.Extensions.Localization;
using OrchardCore.Data;
using OrchardCore.Environment.Shell;
using OrchardCore.Environment.Shell.Models;
using OrchardCore.Mvc.ModelBinding;
using OrchardCore.Tenants.ViewModels;

Expand All @@ -17,33 +18,24 @@ public class TenantValidator : ITenantValidator

private readonly IShellHost _shellHost;
private readonly IFeatureProfilesService _featureProfilesService;
private readonly IEnumerable<DatabaseProvider> _databaseProviders;
private readonly ShellSettings _shellSettings;
private readonly IStringLocalizer<TenantValidator> S;
private readonly IDbConnectionValidator _dbConnectionValidator;

public TenantValidator(
IShellHost shellHost,
IFeatureProfilesService featureProfilesService,
IEnumerable<DatabaseProvider> databaseProviders,
ShellSettings shellSettings,
IStringLocalizer<TenantValidator> stringLocalizer)
IStringLocalizer<TenantValidator> stringLocalizer,
IDbConnectionValidator dbConnectionValidator)
{
_shellHost = shellHost;
_featureProfilesService = featureProfilesService;
_databaseProviders = databaseProviders;
_shellSettings = shellSettings;
S = stringLocalizer;
_dbConnectionValidator = dbConnectionValidator;
}

public async Task<IEnumerable<ModelError>> ValidateAsync(TenantViewModel model)
{
var errors = new List<ModelError>();
var selectedProvider = _databaseProviders.FirstOrDefault(x => x.Value == model.DatabaseProvider);

if (selectedProvider != null && selectedProvider.HasConnectionString && String.IsNullOrWhiteSpace(model.ConnectionString))
{
errors.Add(new ModelError(nameof(model.ConnectionString), S["The connection string is mandatory for this provider."]));
}

if (String.IsNullOrWhiteSpace(model.Name))
{
Expand All @@ -64,36 +56,63 @@ public async Task<IEnumerable<ModelError>> ValidateAsync(TenantViewModel model)
errors.Add(new ModelError(nameof(model.Name), S["Invalid tenant name. Must contain characters only and no spaces."]));
}

if (!_shellSettings.IsDefaultShell() && String.IsNullOrWhiteSpace(model.RequestUrlHost) && String.IsNullOrWhiteSpace(model.RequestUrlPrefix))
if (!ShellHelper.DefaultShellName.Equals(model.Name, StringComparison.OrdinalIgnoreCase) && String.IsNullOrWhiteSpace(model.RequestUrlHost) && String.IsNullOrWhiteSpace(model.RequestUrlPrefix))
MikeAlhayek marked this conversation as resolved.
Show resolved Hide resolved
{
errors.Add(new ModelError(nameof(model.RequestUrlPrefix), S["Host and url prefix can not be empty at the same time."]));
}

if (!String.IsNullOrWhiteSpace(model.RequestUrlPrefix))
if (!String.IsNullOrWhiteSpace(model.RequestUrlPrefix) && model.RequestUrlPrefix.Contains('/'))
{
if (model.RequestUrlPrefix.Contains('/'))
{
errors.Add(new ModelError(nameof(model.RequestUrlPrefix), S["The url prefix can not contain more than one segment."]));
}
errors.Add(new ModelError(nameof(model.RequestUrlPrefix), S["The url prefix can not contain more than one segment."]));
}

var allSettings = _shellHost.GetAllSettings();
var allOtherSettings = _shellHost.GetAllSettings().Where(settings => !String.Equals(settings.Name, model.Name, StringComparison.OrdinalIgnoreCase));

if (model.IsNewTenant && allSettings.Any(tenant => String.Equals(tenant.Name, model.Name, StringComparison.OrdinalIgnoreCase)))
if (allOtherSettings.Any(settings => String.Equals(settings.RequestUrlPrefix, model.RequestUrlPrefix?.Trim(), StringComparison.OrdinalIgnoreCase) && DoesUrlHostExist(settings.RequestUrlHost, model.RequestUrlHost)))
{
errors.Add(new ModelError(nameof(model.Name), S["A tenant with the same name already exists."]));
errors.Add(new ModelError(nameof(model.RequestUrlPrefix), S["A tenant with the same host and prefix already exists."]));
}

var allOtherShells = allSettings.Where(t => !String.Equals(t.Name, model.Name, StringComparison.OrdinalIgnoreCase));
if (model.IsNewTenant)
{
if (_shellHost.TryGetSettings(model.Name, out _))
{
errors.Add(new ModelError(nameof(model.Name), S["A tenant with the same name already exists."]));
}

if (allOtherShells.Any(tenant => String.Equals(tenant.RequestUrlPrefix, model.RequestUrlPrefix?.Trim(), StringComparison.OrdinalIgnoreCase) && DoesUrlHostExist(tenant.RequestUrlHost, model.RequestUrlHost)))
await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors);
}
else
{
errors.Add(new ModelError(nameof(model.RequestUrlPrefix), S["A tenant with the same host and prefix already exists."]));
// At this point, we know we are validating existing tenant
if (!_shellHost.TryGetSettings(model.Name, out var existingSettings) || existingSettings.State == TenantState.Uninitialized)
{
// while the tenant is Uninitialized, we are still able to change the database settings
// let's validate the database for assurance

await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors);
}
}

return errors;
}

private async Task AssertConnectionValidityAndApplyErrorsAsync(string databaseProvider, string connectionString, string tablePrefix, List<ModelError> errors)
{
switch (await _dbConnectionValidator.ValidateAsync(databaseProvider, connectionString, tablePrefix))
{
case DbConnectionValidatorResult.UnsupportedProvider:
errors.Add(new ModelError(nameof(TenantViewModel.DatabaseProvider), S["The provided database provider is not supported."]));
break;
case DbConnectionValidatorResult.InvalidConnection:
errors.Add(new ModelError(nameof(TenantViewModel.ConnectionString), S["The provided connection string is invalid or server is unreachable."]));
break;
case DbConnectionValidatorResult.DocumentFound:
errors.Add(new ModelError(nameof(TenantViewModel.TablePrefix), S["The provided table prefix already exists."]));
break;
}
}

private static bool DoesUrlHostExist(string urlHost, string modelUrlHost)
{
if (String.IsNullOrEmpty(urlHost) && String.IsNullOrEmpty(modelUrlHost))
Expand Down
Expand Up @@ -3,7 +3,7 @@ namespace OrchardCore.Data
public class DatabaseProvider
{
public string Name { get; set; }
public string Value { get; set; }
public DatabaseProviderName Value { get; set; }
public bool HasConnectionString { get; set; }
public bool HasTablePrefix { get; set; }
public bool IsDefault { get; set; }
Expand Down
@@ -0,0 +1,10 @@
namespace OrchardCore.Data;

public enum DatabaseProviderName
{
None,
SqlConnection,
Sqlite,
MySql,
Postgres,
}
Expand Up @@ -19,7 +19,7 @@ public static class ServiceCollectionExtensions
/// <param name="isDefault">Whether the data provider is the default one.</param>
/// <param name="sampleConnectionString">A sample connection string, e.g. Server={Server Name};Database={Database Name};IntegratedSecurity=true</param>
/// <returns></returns>
public static IServiceCollection TryAddDataProvider(this IServiceCollection services, string name, string value, bool hasConnectionString, bool hasTablePrefix, bool isDefault, string sampleConnectionString = "")
public static IServiceCollection TryAddDataProvider(this IServiceCollection services, string name, DatabaseProviderName value, bool hasConnectionString, bool hasTablePrefix, bool isDefault, string sampleConnectionString = "")
{
for (var i = services.Count - 1; i >= 0; i--)
{
Expand Down