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

No way to delete a tenant site if tenant has only one site #2250

Closed
dkoeder opened this issue Jun 22, 2022 · 4 comments
Closed

No way to delete a tenant site if tenant has only one site #2250

dkoeder opened this issue Jun 22, 2022 · 4 comments

Comments

@dkoeder
Copy link

dkoeder commented Jun 22, 2022

Oqtane: current dev branch
Database: LocalDB
Hosting Model - Runtime: Blazor WebAssembly
Hosting Model – Prerender?: Yes

  1. Installed oqtane
  2. Added site, “TestA”, with new tenant, “TestTenantA”
  3. Restarted oqtane
  4. Went to Site Mangement
  5. Went to Edit on “TestA”
  6. Clicked on “Delete Site”, confirmed.
  7. Always returns “You Are Not Authorized To Delete The Site” due to the (sites.Count > 1) in Index.razor

Oqtane.Client\Modules\Admin\Site\Index.razor

private async Task DeleteSite()
{
	try
	{
		var sites = await SiteService.GetSitesAsync();
		if (sites.Count > 1)
		{
                     .
                     .
		}
		else
		{
			AddModuleMessage(Localizer["Message.FailAuth.DeleteSite"], MessageType.Warning);
		}
@dkoeder
Copy link
Author

dkoeder commented Jun 25, 2022

Here is my work around -
Original code -

private async Task DeleteSite()
{
	try
	{
		var sites = await SiteService.GetSitesAsync();
		if (sites.Count > 1)
		{
			await SiteService.DeleteSiteAsync(PageState.Site.SiteId);
			await logger.LogInformation("Site Deleted {SiteId}", PageState.Site.SiteId);

			var aliases = await AliasService.GetAliasesAsync();
			foreach (Alias a in aliases.Where(item => item.SiteId == PageState.Site.SiteId && item.TenantId == PageState.Site.TenantId))
			{
				await AliasService.DeleteAliasAsync(a.AliasId);
			}

			NavigationManager.NavigateTo(NavigateUrl("admin/sites"));
		}
		else
		{
			AddModuleMessage(Localizer["Message.FailAuth.DeleteSite"], MessageType.Warning);
		}
	}

Modified code -

private async Task DeleteSite()
{
	try
	{
		var tenant = await TenantService.GetTenantAsync(PageState.Site.TenantId);
		if (!tenant.Name.Equals(TenantNames.Master, StringComparison.OrdinalIgnoreCase))
		{
			await SiteService.DeleteSiteAsync(PageState.Site.SiteId);
			await logger.LogInformation("Site Deleted {SiteId}", PageState.Site.SiteId);

			var aliases = await AliasService.GetAliasesAsync();
			foreach (Alias a in aliases.Where(item => item.SiteId == PageState.Site.SiteId && item.TenantId == PageState.Site.TenantId))
			{
				await AliasService.DeleteAliasAsync(a.AliasId);
			}

			NavigationManager.NavigateTo(NavigationManager.BaseUri, true);
		}
		else
		{
			AddModuleMessage(Localizer["Message.FailAuth.DeleteSite"], MessageType.Warning);
		}

The "NavigationManager.NavigateTo(NavigationManager.BaseUri, true);" is just my guess at a navigation that works. If this was left at "NavigationManager.NavigateTo(NavigateUrl("admin/sites"));" it would cause an error in SiteRouter. Could this be due to navigating to a site that was just deleted? With these changes the issue #2249 seems not to be a problem.

@sbwalker
Copy link
Member

sbwalker commented Jun 27, 2022

The logic for validating if there is more than 1 site is intended to prevent a host user from removing all sites from an installation - as there needs to be at least 1 site in the installation to enable the framework to function properly (ie. this validation is to prevent a host user from accidentally "shooting themself in the foot").

However, as you identified above, the logic is not correct as instead of checking if there is more than 1 site in the entire installation, it is checking if there is more than 1 site in the current tenant (an installation can have many tenants). The proper fix is actually a bit different than what you suggested... it should utilize the Alias service as an Alias is the cross reference between tenants and sites.

sbwalker added a commit that referenced this issue Jun 27, 2022
Fix #2249 Fix #2250 - issues with site deletion
@dkoeder
Copy link
Author

dkoeder commented Jun 28, 2022

@sbwalker I tried it with the latest changes and now, when repeating the steps I define above, it says "Error Deleting Site".

Looks like the GetAliasesAsync in AliasService.cs need to be changed to something like this

    public async Task<List<Alias>> GetAliasesAsync()
    {
        List<Alias> aliases = await GetJsonAsync<List<Alias>>(ApiUrl);
        if (aliases == null)
        {
            var masterUrl = CreateApiUrl("Alias", null, ControllerRoutes.ApiRoute);
            aliases = await GetJsonAsync<List<Alias>>(masterUrl);
        }
        return aliases.OrderBy(item => item.Name).ToList();
    }

Not sure if that is the right way to get the master tenant url. Should the Alias service always use the master tenant url since it looks like Alias table is only in the host DB?

@sbwalker
Copy link
Member

@dkoeder the problem was with the redirect logic - it has been resolved in #2260. Oqtane's multi-tenant capability relies on Aliases to determine the appropriate mapping for the Urls in API/UI requests... so this means that when an Alias is deleted, all requests that are still utilizing that Alias will fail. As a result it is necessary to redirect the client to another valid Alias as soon as the current alias is deleted. One other concept which is not entirely obvious is that the "Master Tenant" has nothing to do with Sites or Aliases... a "tenant" in Oqtane simply refers to a database. The "Master Tenant" is the database where the global information related to Tenants, Aliases, Jobs, etc... is stored. The "Master Tenant" may contain Sites as well - however it is possible to remove all Sites from the Master Tenant and still have a functional installation - in this scenario all of your Sites would be in other Tenants (databases).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants