From afa1d81cbc7655284b05b188b29c25cff36f2902 Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Thu, 20 Aug 2026 14:15:59 -0700 Subject: [PATCH 1/9] AB#34137 Rework tenant creation: onboarding field mapping, New Tenant modal, Metabase post-creation step Onboarding "Create Tenant" modal: - Reorders field mapping so Name and the new Display Name field come first, then Ministry, Division, Branch, Program Area, Features, Program Managers. - Adds Display Name and Division as mappable fields (stored on the tenant's ExtraProperties / passed through to TenantCreateDto). - Fixes the Jaro-Winkler auto-detection matching a "ProgramManagerEmail"-style field to the Name canonical instead of a literal "Name" field, via an exact-match short-circuit before fuzzy scoring. New Tenant modal (Tenants/CreateModal): - Rebuilt to match the Edit Configuration modal's tabbed layout (Details, Program Managers, Features, Metabase) instead of a single flat form. - Details tab now actually renders Division/Branch/Description/CAS Client Code (previously present on the model but never shown) plus Display Name. - Program Managers search now uses the same field-selector UX as Edit. - Features tab lets you pick features to enable at creation time, reusing the existing FeatureKeys -> TenantCreatedEventHandler mechanism. - New Metabase tab manages the group's member emails, backed by ABP Settings (Global default list + per-tenant snapshot, with an "add for this tenant only" vs "save as default" option). Tenants list: adds a Display Name column. Post-tenant-creation pipeline (new): - IPostTenantCreationStep (Unity.SharedKernel) - pluggable, ordered, per-step ContinueOnError. - PostTenantCreationSequenceJob - a self-chaining ABP background job that runs steps in order, entirely via the job queue (durable/retryable per step), stopping on failure only where ContinueOnError is false. - MetabaseTenantRegistrationStep - first step. Registers the new tenant with Metabase (database connection over its readonly Postgres role, a permissions group with its configured members, and a collection), automating what manual_deploy_new_metabase_tenant.ps1 used to do by hand. Resolves the Metabase endpoint via the existing DynamicUrls mechanism (new METABASE_API_BASE key) and the API key via new TenantCreation: Steps:Metabase:ApiKey config. - IResilientHttpRequest gains an optional extraHeaders parameter (needed for Metabase's x-api-key auth), backward compatible with all existing call sites. Co-Authored-By: Claude Sonnet 5 --- .../Http/IResilientHttpRequest.cs | 4 +- .../Http/ResilientHttpRequest.cs | 22 +- .../IPostTenantCreationStep.cs | 27 +++ .../IOnboardingRequestAppService.cs | 2 +- .../Metabase/MetabaseSettings.cs | 11 + .../OnboardingColumnConfigDto.cs | 4 + .../OnboardingRequestDto.cs | 2 + .../TenantCreateDto.cs | 2 + .../TenantCreateOrUpdateDtoBase.cs | 1 + .../TenantDto.cs | 1 + .../MetabaseSettingDefinitionProvider.cs | 20 ++ ...ngColumnConfigSettingDefinitionProvider.cs | 2 + .../OnboardingColumnConfigSettings.cs | 2 + .../OnboardingRequestAppService.cs | 37 ++- .../TenantAppService.cs | 6 +- .../UnityTenantManagementMapperlyProfile.cs | 1 + .../OnboardingRequestController.cs | 6 +- .../Onboarding/CreateTenantModal.cshtml | 36 ++- .../TenantManagement/Onboarding/Index.js | 32 ++- .../Tenants/CreateModal.cshtml | 181 +++++++++++--- .../Tenants/CreateModal.cshtml.cs | 79 +++++-- .../Pages/TenantManagement/Tenants/Index.js | 221 ++++++++++++++---- ...UnityTenantManagementWebMapperlyProfile.cs | 5 +- .../GrantManagerApplicationModule.cs | 3 + .../Handlers/TenantCreatedEventHandler.cs | 34 ++- .../Metabase/IMetabaseApiClient.cs | 23 ++ .../Metabase/MetabaseApiClient.cs | 137 +++++++++++ .../Integrations/Metabase/MetabaseOptions.cs | 7 + .../PostTenantCreationSequenceJob.cs | 72 ++++++ .../PostTenantCreationStepArgs.cs | 12 + .../Steps/MetabaseTenantRegistrationStep.cs | 140 +++++++++++ .../Integrations/DynamicUrlKeyNames.cs | 1 + .../Localization/GrantManager/en.json | 7 +- .../Integrations/DynamicUrlDataSeeder.cs | 3 + .../Unity.GrantManager.Web/appsettings.json | 9 +- .../PostTenantCreationSequenceJobTests.cs | 123 ++++++++++ .../MetabaseTenantRegistrationStepTests.cs | 117 ++++++++++ 37 files changed, 1264 insertions(+), 128 deletions(-) create mode 100644 applications/Unity.GrantManager/modules/Unity.SharedKernel/PostTenantCreation/IPostTenantCreationStep.cs create mode 100644 applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/Metabase/MetabaseSettings.cs create mode 100644 applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Metabase/MetabaseSettingDefinitionProvider.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseOptions.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationStepArgs.cs create mode 100644 applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs create mode 100644 applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/PostTenantCreationSequenceJobTests.cs create mode 100644 applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs diff --git a/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/IResilientHttpRequest.cs b/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/IResilientHttpRequest.cs index ea461b2d2c..e61844102a 100644 --- a/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/IResilientHttpRequest.cs +++ b/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/IResilientHttpRequest.cs @@ -1,4 +1,5 @@ -using System.Net.Http; +using System.Collections.Generic; +using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Volo.Abp; @@ -18,6 +19,7 @@ Task HttpAsync( string? authToken = null, (string username, string password)? basicAuth = null, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, + IReadOnlyDictionary? extraHeaders = null, CancellationToken cancellationToken = default); /// diff --git a/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/ResilientHttpRequest.cs b/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/ResilientHttpRequest.cs index 109c0a816d..80814ccca6 100644 --- a/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/ResilientHttpRequest.cs +++ b/applications/Unity.GrantManager/modules/Unity.SharedKernel/Http/ResilientHttpRequest.cs @@ -1,6 +1,7 @@ using Polly; using Polly.Retry; using System; +using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; @@ -114,10 +115,11 @@ public async Task HttpAsync( string? authToken = null, (string username, string password)? basicAuth = null, HttpCompletionOption completionOption = HttpCompletionOption.ResponseContentRead, + IReadOnlyDictionary? extraHeaders = null, CancellationToken cancellationToken = default) { return await SendWithClientAsync( - _httpClient, httpVerb, resource, body, authToken, basicAuth, completionOption, cancellationToken); + _httpClient, httpVerb, resource, body, authToken, basicAuth, completionOption, extraHeaders, cancellationToken); } @@ -138,7 +140,7 @@ public Task HttpAsyncSecured( EnsureMutualTlsClient(certPath, certPassword); return SendWithClientAsync( - _mtlsClient!, httpVerb, resource, body, authToken, basicAuth, HttpCompletionOption.ResponseContentRead, cancellationToken); + _mtlsClient!, httpVerb, resource, body, authToken, basicAuth, HttpCompletionOption.ResponseContentRead, null, cancellationToken); } @@ -193,6 +195,7 @@ private async Task SendWithClientAsync( string? authToken, (string username, string password)? basicAuth, HttpCompletionOption completionOption, + IReadOnlyDictionary? extraHeaders, CancellationToken cancellationToken) { // Build final URL @@ -208,7 +211,7 @@ private async Task SendWithClientAsync( return await _pipeline.ExecuteAsync(async ct => { using var requestMessage = - BuildRequestMessage(httpVerb, fullUrl, body, authToken, basicAuth); + BuildRequestMessage(httpVerb, fullUrl, body, authToken, basicAuth, extraHeaders); return await client.SendAsync(requestMessage, completionOption, ct) .ConfigureAwait(false); @@ -226,7 +229,8 @@ private static HttpRequestMessage BuildRequestMessage( Uri fullUrl, object? body, string? authToken, - (string username, string password)? basicAuth) + (string username, string password)? basicAuth, + IReadOnlyDictionary? extraHeaders = null) { var requestMessage = new HttpRequestMessage(httpVerb, fullUrl); requestMessage.Headers.Accept.Clear(); @@ -248,6 +252,16 @@ private static HttpRequestMessage BuildRequestMessage( requestMessage.Headers.Add(AuthorizationHeader, $"Basic {encoded}"); } + // Additional headers (e.g. API keys) not covered by the auth-token/basic-auth cases above + if (extraHeaders != null) + { + foreach (var header in extraHeaders) + { + requestMessage.Headers.Remove(header.Key); + requestMessage.Headers.Add(header.Key, header.Value); + } + } + // Body if (body != null) { diff --git a/applications/Unity.GrantManager/modules/Unity.SharedKernel/PostTenantCreation/IPostTenantCreationStep.cs b/applications/Unity.GrantManager/modules/Unity.SharedKernel/PostTenantCreation/IPostTenantCreationStep.cs new file mode 100644 index 0000000000..28b328048f --- /dev/null +++ b/applications/Unity.GrantManager/modules/Unity.SharedKernel/PostTenantCreation/IPostTenantCreationStep.cs @@ -0,0 +1,27 @@ +using System; +using System.Threading.Tasks; + +namespace Unity.Modules.Shared.PostTenantCreation; + +/// +/// A single step in the post-tenant-creation sequence, run as part of +/// PostTenantCreationSequenceJob after a new tenant is created. Implement this in any +/// module and register it as an to +/// have it picked up automatically - no changes to the sequencing job are needed. +/// +public interface IPostTenantCreationStep +{ + /// Determines execution order relative to other steps (ascending). + int Order { get; } + + /// Short, human-readable name used in logging. + string StepName { get; } + + /// + /// When true, a failure in this step is logged and the sequence continues to the next step. + /// When false, a failure stops the sequence - later steps do not run. + /// + bool ContinueOnError { get; } + + Task ExecuteAsync(Guid tenantId); +} diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/IOnboardingRequestAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/IOnboardingRequestAppService.cs index 10cfd4acc0..24c0f6fcee 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/IOnboardingRequestAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/IOnboardingRequestAppService.cs @@ -11,7 +11,7 @@ public interface IOnboardingRequestAppService : IApplicationService { Task> GetListAsync(OnboardingListRequestDto input); Task GetAsync(Guid id); - Task ValidateAsync(Guid id, string? tenantNameFieldKey, string? superUsersFieldKey, string? branchFieldKey = null, string? featuresFieldKey = null, string? ministryFieldKey = null, string? programAreaFieldKey = null); + Task ValidateAsync(Guid id, string? tenantNameFieldKey, string? superUsersFieldKey, string? branchFieldKey = null, string? featuresFieldKey = null, string? ministryFieldKey = null, string? programAreaFieldKey = null, string? displayNameFieldKey = null, string? divisionFieldKey = null); Task CreateTenantAsync(Guid id, CreateTenantInputDto? input); Task GetColumnSchemaAsync(string? category = null); Task> GetAvailableCategoriesAsync(); diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/Metabase/MetabaseSettings.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/Metabase/MetabaseSettings.cs new file mode 100644 index 0000000000..3b59d51480 --- /dev/null +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/Metabase/MetabaseSettings.cs @@ -0,0 +1,11 @@ +namespace Unity.TenantManagement.Metabase; + +public static class MetabaseSettings +{ + /// + /// Comma-separated list of user emails to add to a tenant's Metabase group. + /// Stored Global (the running default applied to new tenants) and per-tenant, "T" provider + /// (the resolved snapshot captured when that tenant was created). + /// + public const string UserEmails = "Metabase.UserEmails"; +} diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs index 2a95c6a0af..86ba5c63e4 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs @@ -15,19 +15,23 @@ public class OnboardingColumnSchemaDto { public List Columns { get; set; } = []; public string? TenantNameFieldKey { get; set; } + public string? DisplayNameFieldKey { get; set; } public string? SuperUsersFieldKey { get; set; } public string? BranchFieldKey { get; set; } public string? FeaturesFieldKey { get; set; } public string? MinistryFieldKey { get; set; } + public string? DivisionFieldKey { get; set; } public string? ProgramAreaFieldKey { get; set; } } public class CreateTenantInputDto { public string? TenantNameFieldKey { get; set; } + public string? DisplayNameFieldKey { get; set; } public string? SuperUsersFieldKey { get; set; } public string? BranchFieldKey { get; set; } public string? FeaturesFieldKey { get; set; } public string? MinistryFieldKey { get; set; } + public string? DivisionFieldKey { get; set; } public string? ProgramAreaFieldKey { get; set; } } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingRequestDto.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingRequestDto.cs index c251999c11..4a773b44b2 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingRequestDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingRequestDto.cs @@ -9,6 +9,7 @@ public class OnboardingRequestDto public Guid Id { get; set; } public string SubmissionNumber { get; set; } = string.Empty; public string TenantName { get; set; } = string.Empty; + public string DisplayName { get; set; } = string.Empty; public string TenantDescription { get; set; } = string.Empty; public string ProgramAreaName { get; set; } = string.Empty; public string ProgramAreaDescription { get; set; } = string.Empty; @@ -18,6 +19,7 @@ public class OnboardingRequestDto public string ExecutiveDirector { get; set; } = string.Empty; public string Branch { get; set; } = string.Empty; public string Ministry { get; set; } = string.Empty; + public string Division { get; set; } = string.Empty; public string Status { get; set; } = string.Empty; public string Category { get; set; } = string.Empty; public DateTime? SubmissionDate { get; set; } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateDto.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateDto.cs index d9b7998e85..a751daa659 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateDto.cs @@ -6,4 +6,6 @@ public class TenantCreateDto : TenantCreateOrUpdateDtoBase public string UserIdentifier { get; set; } = string.Empty; /// Comma-separated ABP feature keys to enable on the new tenant (e.g. "Unity.Payments,Unity.Reporting"). public string? FeatureKeys { get; set; } + /// Comma-separated user emails to add to this tenant's Metabase group. + public string? MetabaseUserEmails { get; set; } } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateOrUpdateDtoBase.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateOrUpdateDtoBase.cs index 0b89d2ddf3..33495c44ab 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateOrUpdateDtoBase.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantCreateOrUpdateDtoBase.cs @@ -12,6 +12,7 @@ public abstract class TenantCreateOrUpdateDtoBase : ExtensibleObject [Display(Name = "TenantName")] public string Name { get; set; } + public string DisplayName { get; set; } = string.Empty; public string Division { get; set; } = string.Empty; public string Branch { get; set; } = string.Empty; public string Description { get; set; } = string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantDto.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantDto.cs index 5dc338b00c..ef8d496847 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/TenantDto.cs @@ -7,6 +7,7 @@ namespace Unity.TenantManagement; public class TenantDto : ExtensibleEntityDto, IHasConcurrencyStamp { public string Name { get; set; } + public string DisplayName { get; set; } = string.Empty; public string Division { get; set; } = string.Empty; public string Branch { get; set; } = string.Empty; public string Description { get; set; } = string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Metabase/MetabaseSettingDefinitionProvider.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Metabase/MetabaseSettingDefinitionProvider.cs new file mode 100644 index 0000000000..1f3874faed --- /dev/null +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Metabase/MetabaseSettingDefinitionProvider.cs @@ -0,0 +1,20 @@ +using Volo.Abp.Settings; +using Volo.Abp.SettingManagement; + +namespace Unity.TenantManagement.Metabase; + +public class MetabaseSettingDefinitionProvider : SettingDefinitionProvider +{ + public override void Define(ISettingDefinitionContext context) + { + context.Add( + new SettingDefinition( + MetabaseSettings.UserEmails, + defaultValue: null, + isVisibleToClients: false, + isInherited: false, + isEncrypted: false) + .WithProviders(GlobalSettingValueProvider.ProviderName, TenantSettingValueProvider.ProviderName) + ); + } +} diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettingDefinitionProvider.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettingDefinitionProvider.cs index c0000d45fc..d9affde02e 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettingDefinitionProvider.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettingDefinitionProvider.cs @@ -13,10 +13,12 @@ public override void Define(ISettingDefinitionContext context) { context.Add( OnboardingDef(OnboardingColumnConfigSettings.TenantNameFieldKey), + OnboardingDef(OnboardingColumnConfigSettings.DisplayNameFieldKey), OnboardingDef(OnboardingColumnConfigSettings.SuperUsersFieldKey), OnboardingDef(OnboardingColumnConfigSettings.BranchFieldKey), OnboardingDef(OnboardingColumnConfigSettings.FeaturesFieldKey), OnboardingDef(OnboardingColumnConfigSettings.MinistryFieldKey), + OnboardingDef(OnboardingColumnConfigSettings.DivisionFieldKey), OnboardingDef(OnboardingColumnConfigSettings.ProgramAreaFieldKey) ); } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettings.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettings.cs index a2041e1c22..49bc404a06 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettings.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/Onboarding/OnboardingColumnConfigSettings.cs @@ -3,9 +3,11 @@ namespace Unity.TenantManagement.Onboarding; public static class OnboardingColumnConfigSettings { public const string TenantNameFieldKey = "Onboarding.ColumnConfig.TenantNameFieldKey"; + public const string DisplayNameFieldKey = "Onboarding.ColumnConfig.DisplayNameFieldKey"; public const string SuperUsersFieldKey = "Onboarding.ColumnConfig.SuperUsersFieldKey"; public const string BranchFieldKey = "Onboarding.ColumnConfig.BranchFieldKey"; public const string FeaturesFieldKey = "Onboarding.ColumnConfig.FeaturesFieldKey"; public const string MinistryFieldKey = "Onboarding.ColumnConfig.MinistryFieldKey"; + public const string DivisionFieldKey = "Onboarding.ColumnConfig.DivisionFieldKey"; public const string ProgramAreaFieldKey = "Onboarding.ColumnConfig.ProgramAreaFieldKey"; } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs index 7b60631ad3..6e44749adb 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs @@ -256,13 +256,13 @@ public virtual async Task> GetAvailableCategoriesAsync() return categories; } - public virtual async Task ValidateAsync(Guid id, string? tenantNameFieldKey, string? superUsersFieldKey, string? branchFieldKey = null, string? featuresFieldKey = null, string? ministryFieldKey = null, string? programAreaFieldKey = null) + public virtual async Task ValidateAsync(Guid id, string? tenantNameFieldKey, string? superUsersFieldKey, string? branchFieldKey = null, string? featuresFieldKey = null, string? ministryFieldKey = null, string? programAreaFieldKey = null, string? displayNameFieldKey = null, string? divisionFieldKey = null) { var request = await GetAsync(id); if (request == null) return new OnboardingValidationResultDto { IsValid = false, Issues = ["Onboarding request not found."] }; - await ResolveFieldMappings(request, tenantNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, programAreaFieldKey); + await ResolveFieldMappings(request, tenantNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, programAreaFieldKey, displayNameFieldKey, divisionFieldKey); var issues = await RunValidationStepsAsync(request); @@ -274,10 +274,10 @@ public virtual async Task CreateTenantAsync(Guid id, CreateTenantInputDto? input var request = await GetAsync(id) ?? throw new UserFriendlyException("Onboarding request not found."); - await ResolveFieldMappings(request, input?.TenantNameFieldKey, input?.SuperUsersFieldKey, input?.BranchFieldKey, input?.FeaturesFieldKey, input?.MinistryFieldKey, input?.ProgramAreaFieldKey); + await ResolveFieldMappings(request, input?.TenantNameFieldKey, input?.SuperUsersFieldKey, input?.BranchFieldKey, input?.FeaturesFieldKey, input?.MinistryFieldKey, input?.ProgramAreaFieldKey, input?.DisplayNameFieldKey, input?.DivisionFieldKey); if (input != null) - await SaveFieldMappingAsync(input.TenantNameFieldKey, input.SuperUsersFieldKey, input.BranchFieldKey, input.FeaturesFieldKey, input.MinistryFieldKey, input.ProgramAreaFieldKey); + await SaveFieldMappingAsync(input.TenantNameFieldKey, input.SuperUsersFieldKey, input.BranchFieldKey, input.FeaturesFieldKey, input.MinistryFieldKey, input.ProgramAreaFieldKey, input.DisplayNameFieldKey, input.DivisionFieldKey); // Re-validate server-side even if the client already called ValidateAsync — the client // cannot be trusted to have done so, and skipping this would let a duplicate tenant name @@ -307,7 +307,9 @@ public virtual async Task CreateTenantAsync(Guid id, CreateTenantInputDto? input var tenantDto = await TenantAppService.CreateAsync(new TenantCreateDto { Name = request.TenantName, + DisplayName = request.DisplayName, Branch = request.Branch, + Division = request.Division, Description = request.TenantDescription, UserIdentifier = userGuids[0], FeatureKeys = featureKeys.Count > 0 ? string.Join(',', featureKeys) : null @@ -341,7 +343,8 @@ private async Task> RunValidationStepsAsync(OnboardingRequestDto re private async Task ResolveFieldMappings(OnboardingRequestDto request, string? tenantNameKey = null, string? superUsersKey = null, string? branchKey = null, string? featuresKey = null, - string? ministryKey = null, string? programAreaKey = null) + string? ministryKey = null, string? programAreaKey = null, + string? displayNameKey = null, string? divisionKey = null) { var saved = await ReadTenantMappingAsync(); tenantNameKey ??= saved.TenantNameFieldKey; @@ -350,9 +353,13 @@ private async Task ResolveFieldMappings(OnboardingRequestDto request, featuresKey ??= saved.FeaturesFieldKey; ministryKey ??= saved.MinistryFieldKey; programAreaKey ??= saved.ProgramAreaFieldKey; + displayNameKey ??= saved.DisplayNameFieldKey; + divisionKey ??= saved.DivisionFieldKey; if (!string.IsNullOrEmpty(tenantNameKey) && request.Fields.TryGetValue(tenantNameKey, out var tenantNameVal) && tenantNameVal is not null) request.TenantName = tenantNameVal.ToString()!; + if (!string.IsNullOrEmpty(displayNameKey) && request.Fields.TryGetValue(displayNameKey, out var displayNameVal) && displayNameVal is not null) + request.DisplayName = displayNameVal.ToString()!; if (!string.IsNullOrEmpty(superUsersKey) && request.Fields.TryGetValue(superUsersKey, out var superUsersVal) && superUsersVal is not null) request.SuperUsers = superUsersVal.ToString()!; if (!string.IsNullOrEmpty(branchKey) && request.Fields.TryGetValue(branchKey, out var branchVal) && branchVal is not null) @@ -361,51 +368,61 @@ private async Task ResolveFieldMappings(OnboardingRequestDto request, request.Features = featuresVal.ToString()!; if (!string.IsNullOrEmpty(ministryKey) && request.Fields.TryGetValue(ministryKey, out var ministryVal) && ministryVal is not null) request.Ministry = ministryVal.ToString()!; + if (!string.IsNullOrEmpty(divisionKey) && request.Fields.TryGetValue(divisionKey, out var divisionVal) && divisionVal is not null) + request.Division = divisionVal.ToString()!; if (!string.IsNullOrEmpty(programAreaKey) && request.Fields.TryGetValue(programAreaKey, out var programAreaVal) && programAreaVal is not null) request.ProgramAreaName = programAreaVal.ToString()!; } - private async Task SaveFieldMappingAsync(string? tenantNameKey, string? superUsersKey, string? branchKey, string? featuresKey, string? ministryKey, string? programAreaKey) + private async Task SaveFieldMappingAsync(string? tenantNameKey, string? superUsersKey, string? branchKey, string? featuresKey, string? ministryKey, string? programAreaKey, string? displayNameKey, string? divisionKey) { var userId = CurrentUser.Id?.ToString(); if (string.IsNullOrEmpty(userId)) return; await _settingManager.SetAsync(OnboardingColumnConfigSettings.TenantNameFieldKey, tenantNameKey, UserProvider, userId); + await _settingManager.SetAsync(OnboardingColumnConfigSettings.DisplayNameFieldKey, displayNameKey, UserProvider, userId); await _settingManager.SetAsync(OnboardingColumnConfigSettings.SuperUsersFieldKey, superUsersKey, UserProvider, userId); await _settingManager.SetAsync(OnboardingColumnConfigSettings.BranchFieldKey, branchKey, UserProvider, userId); await _settingManager.SetAsync(OnboardingColumnConfigSettings.FeaturesFieldKey, featuresKey, UserProvider, userId); await _settingManager.SetAsync(OnboardingColumnConfigSettings.MinistryFieldKey, ministryKey, UserProvider, userId); + await _settingManager.SetAsync(OnboardingColumnConfigSettings.DivisionFieldKey, divisionKey, UserProvider, userId); await _settingManager.SetAsync(OnboardingColumnConfigSettings.ProgramAreaFieldKey, programAreaKey, UserProvider, userId); } private async Task ReadTenantMappingAsync() { var userId = CurrentUser.Id?.ToString(); - string? tenantNameKey = null, superUsersKey = null, branchKey = null, featuresKey = null, ministryKey = null, programAreaKey = null; + string? tenantNameKey = null, displayNameKey = null, superUsersKey = null, branchKey = null, featuresKey = null, ministryKey = null, divisionKey = null, programAreaKey = null; if (!string.IsNullOrEmpty(userId)) { tenantNameKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.TenantNameFieldKey, UserProvider, userId); + displayNameKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.DisplayNameFieldKey, UserProvider, userId); superUsersKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.SuperUsersFieldKey, UserProvider, userId); branchKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.BranchFieldKey, UserProvider, userId); featuresKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.FeaturesFieldKey, UserProvider, userId); ministryKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.MinistryFieldKey, UserProvider, userId); + divisionKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.DivisionFieldKey, UserProvider, userId); programAreaKey = await _settingManager.GetOrNullAsync(OnboardingColumnConfigSettings.ProgramAreaFieldKey, UserProvider, userId); } tenantNameKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.TenantNameFieldKey); + displayNameKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.DisplayNameFieldKey); superUsersKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.SuperUsersFieldKey); branchKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.BranchFieldKey); featuresKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.FeaturesFieldKey); ministryKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.MinistryFieldKey); + divisionKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.DivisionFieldKey); programAreaKey ??= await _settingManager.GetOrNullGlobalAsync(OnboardingColumnConfigSettings.ProgramAreaFieldKey); return new OnboardingColumnSchemaDto { TenantNameFieldKey = tenantNameKey, + DisplayNameFieldKey = displayNameKey, SuperUsersFieldKey = superUsersKey, BranchFieldKey = branchKey, FeaturesFieldKey = featuresKey, MinistryFieldKey = ministryKey, + DivisionFieldKey = divisionKey, ProgramAreaFieldKey = programAreaKey }; } @@ -526,6 +543,7 @@ private static OnboardingRequestDto MapToDto( switch (fv.Key.ToLowerInvariant().Replace("-", "").Replace("_", "").Replace(" ", "")) { + case "displayname": dto.DisplayName = fv.Value; break; case "tenantdescription": case "description": dto.TenantDescription = fv.Value; break; case "programareaname": case "programarea": dto.ProgramAreaName = fv.Value; break; case "programareadescription": dto.ProgramAreaDescription = fv.Value; break; @@ -534,6 +552,7 @@ private static OnboardingRequestDto MapToDto( case "executivedirector": dto.ExecutiveDirector = fv.Value; break; case "branch": dto.Branch = fv.Value; break; case "ministry": dto.Ministry = fv.Value; break; + case "division": dto.Division = fv.Value; break; } } } @@ -545,6 +564,8 @@ private static OnboardingRequestDto MapToDto( if (!string.IsNullOrEmpty(mapping.TenantNameFieldKey) && dto.Fields.TryGetValue(mapping.TenantNameFieldKey, out var tn) && tn != null) dto.TenantName = tn.ToString()!; + if (!string.IsNullOrEmpty(mapping.DisplayNameFieldKey) && dto.Fields.TryGetValue(mapping.DisplayNameFieldKey, out var dn) && dn != null) + dto.DisplayName = dn.ToString()!; if (!string.IsNullOrEmpty(mapping.SuperUsersFieldKey) && dto.Fields.TryGetValue(mapping.SuperUsersFieldKey, out var su) && su != null) dto.SuperUsers = su.ToString()!; if (!string.IsNullOrEmpty(mapping.BranchFieldKey) && dto.Fields.TryGetValue(mapping.BranchFieldKey, out var br) && br != null) @@ -553,6 +574,8 @@ private static OnboardingRequestDto MapToDto( dto.Features = ft.ToString()!; if (!string.IsNullOrEmpty(mapping.MinistryFieldKey) && dto.Fields.TryGetValue(mapping.MinistryFieldKey, out var mn) && mn != null) dto.Ministry = mn.ToString()!; + if (!string.IsNullOrEmpty(mapping.DivisionFieldKey) && dto.Fields.TryGetValue(mapping.DivisionFieldKey, out var dv) && dv != null) + dto.Division = dv.ToString()!; if (!string.IsNullOrEmpty(mapping.ProgramAreaFieldKey) && dto.Fields.TryGetValue(mapping.ProgramAreaFieldKey, out var pa) && pa != null) dto.ProgramAreaName = pa.ToString()!; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs index 3e5bed3287..d2dc87ebc4 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs @@ -34,6 +34,7 @@ public class TenantAppService( { private IIdentityUserRepository IdentityUserRepository => LazyServiceProvider.LazyGetRequiredService(); + private const string ExtraPropDisplayName = "DisplayName"; private const string ExtraPropDivision = "Division"; private const string ExtraPropBranch = "Branch"; private const string ExtraPropDescription = "Description"; @@ -169,6 +170,7 @@ public virtual async Task CreateAsync(TenantCreateDto input) // Set ExtraProperties from input tenant.ExtraProperties[UnityTenantManagementConsts.TenantLicencePlateExtraPropertyKey] = credentials.DbName; + tenant.ExtraProperties[ExtraPropDisplayName] = input.DisplayName ?? string.Empty; tenant.ExtraProperties[ExtraPropDivision] = input.Division ?? string.Empty; tenant.ExtraProperties[ExtraPropBranch] = input.Branch ?? string.Empty; tenant.ExtraProperties[ExtraPropDescription] = input.Description ?? string.Empty; @@ -188,7 +190,8 @@ await localEventBus.PublishAsync( Properties = { { "UserIdentifier", input.UserIdentifier }, - { "FeatureKeys", input.FeatureKeys ?? string.Empty } + { "FeatureKeys", input.FeatureKeys ?? string.Empty }, + { "MetabaseUserEmails", input.MetabaseUserEmails ?? string.Empty } } } ); @@ -206,6 +209,7 @@ public virtual async Task UpdateAsync(Guid id, TenantUpdateDto input) tenant.SetConcurrencyStampIfNotNull(input.ConcurrencyStamp); // Update ExtraProperties from input + tenant.ExtraProperties[ExtraPropDisplayName] = input.DisplayName ?? string.Empty; tenant.ExtraProperties[ExtraPropDivision] = input.Division ?? string.Empty; tenant.ExtraProperties[ExtraPropBranch] = input.Branch ?? string.Empty; tenant.ExtraProperties[ExtraPropDescription] = input.Description ?? string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/UnityTenantManagementMapperlyProfile.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/UnityTenantManagementMapperlyProfile.cs index c0af541474..d42c060cd1 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/UnityTenantManagementMapperlyProfile.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/UnityTenantManagementMapperlyProfile.cs @@ -19,6 +19,7 @@ public override void Map(Tenant source, TenantDto destination) destination.Id = source.Id; destination.Name = source.Name; destination.ConcurrencyStamp = source.ConcurrencyStamp; + destination.DisplayName = GetExtraProperty(source, "DisplayName") ?? string.Empty; destination.CasClientCode = GetExtraProperty(source, "CasClientCode") ?? string.Empty; destination.LicencePlate = GetExtraProperty(source, "LicencePlate") ?? string.Empty; destination.Division = GetExtraProperty(source, "Division") ?? string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.HttpApi/OnboardingRequestController.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.HttpApi/OnboardingRequestController.cs index e00ada72a8..8443ff3990 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.HttpApi/OnboardingRequestController.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.HttpApi/OnboardingRequestController.cs @@ -40,10 +40,12 @@ public virtual Task ValidateAsync( [FromQuery] string? branchFieldKey = null, [FromQuery] string? featuresFieldKey = null, [FromQuery] string? ministryFieldKey = null, - [FromQuery] string? programAreaFieldKey = null) + [FromQuery] string? programAreaFieldKey = null, + [FromQuery] string? displayNameFieldKey = null, + [FromQuery] string? divisionFieldKey = null) { if (!ModelState.IsValid) throw new UserFriendlyException("OnboardingRequestController->ValidateAsync: ModelState Invalid"); - return OnboardingRequestAppService.ValidateAsync(id, tenantNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, programAreaFieldKey); + return OnboardingRequestAppService.ValidateAsync(id, tenantNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, programAreaFieldKey, displayNameFieldKey, divisionFieldKey); } [HttpPost("{id}/create-tenant")] diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml index f396127cdf..c58060cfc1 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml @@ -17,6 +17,24 @@ bool ContinueOnError { get; } + /// + /// Validated before runs. When false, the step is skipped (logged, + /// not treated as a failure) and the sequence moves on to the next step. Defaults to true - + /// override to check preconditions such as required configuration being present. + /// + Task CanExecuteAsync(Guid tenantId) => Task.FromResult(true); + Task ExecuteAsync(Guid tenantId); } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs index 86ba5c63e4..20338724e1 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application.Contracts/OnboardingColumnConfigDto.cs @@ -34,4 +34,13 @@ public class CreateTenantInputDto public string? MinistryFieldKey { get; set; } public string? DivisionFieldKey { get; set; } public string? ProgramAreaFieldKey { get; set; } + + /// Comma-separated emails checked in the Metabase tab - sent to TenantCreateDto as-is. + public string? MetabaseUserEmails { get; set; } + + /// Comma-separated subset of newly-added Metabase emails to persist as the new Global default. + public string? MetabaseNewDefaultUserEmails { get; set; } + + /// Comma-separated default Metabase emails explicitly removed - deleted from the Global default. + public string? MetabaseRemovedDefaultUserEmails { get; set; } } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs index 6e44749adb..6c440ef31a 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/OnboardingRequestAppService.cs @@ -10,6 +10,7 @@ using Unity.Flex.WorksheetInstances; using Unity.Modules.Shared.Correlation; using Unity.Modules.Shared.Permissions; +using Unity.TenantManagement.Metabase; using Unity.TenantManagement.Onboarding; using Unity.TenantManagement.Validation; using Volo.Abp; @@ -312,7 +313,8 @@ public virtual async Task CreateTenantAsync(Guid id, CreateTenantInputDto? input Division = request.Division, Description = request.TenantDescription, UserIdentifier = userGuids[0], - FeatureKeys = featureKeys.Count > 0 ? string.Join(',', featureKeys) : null + FeatureKeys = featureKeys.Count > 0 ? string.Join(',', featureKeys) : null, + MetabaseUserEmails = input?.MetabaseUserEmails }); foreach (var userGuid in userGuids.Skip(1)) @@ -324,10 +326,29 @@ await TenantAppService.AssignManagerAsync(new TenantAssignManagerDto }); } + if (!string.IsNullOrWhiteSpace(input?.MetabaseNewDefaultUserEmails) || !string.IsNullOrWhiteSpace(input?.MetabaseRemovedDefaultUserEmails)) + await UpdateMetabaseDefaultUserEmailsAsync(input.MetabaseNewDefaultUserEmails, input.MetabaseRemovedDefaultUserEmails); + if (ApplicationProvider != null) await ApplicationProvider.CloseApplicationAsync(id); } + private async Task UpdateMetabaseDefaultUserEmailsAsync(string? newEmailsCsv, string? removedEmailsCsv) + { + var removed = SplitEmails(removedEmailsCsv); + var updated = SplitEmails(await _settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails)) + .Concat(SplitEmails(newEmailsCsv)) + .Where(email => !removed.Contains(email, StringComparer.OrdinalIgnoreCase)) + .Distinct(StringComparer.OrdinalIgnoreCase); + + await _settingManager.SetGlobalAsync(MetabaseSettings.UserEmails, string.Join(",", updated)); + } + + private static List SplitEmails(string? emailsCsv) => + (emailsCsv ?? string.Empty) + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .ToList(); + private async Task> RunValidationStepsAsync(OnboardingRequestDto request) { var issues = new List(); diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml index c58060cfc1..c0f0c0d763 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml @@ -15,6 +15,26 @@ + + +
+
+ + +
+

@L["CreateTenantModal:MetabaseDescription"]

+

@L["CreateTenantModal:MetabaseAccountNote"]

+ +
+ @foreach (var email in Model.DefaultMetabaseUserEmails) + { +
+ + + +
+ } +
+ +
+ + +
+
+ + +
+ + + + +
+ +
+
@L["CreateTenantModal:Validating"] diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml.cs index bf3d068f36..5ed6f9b8a8 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/CreateTenantModal.cshtml.cs @@ -1,14 +1,18 @@ #nullable enable using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Unity.Modules.Shared.Permissions; +using Unity.TenantManagement.Metabase; +using Volo.Abp.SettingManagement; namespace Unity.TenantManagement.Web.Pages.TenantManagement.Onboarding; [Authorize(IdentityConsts.ITOperationsPolicyName)] -public class CreateTenantModalModel(IOnboardingRequestAppService onboardingRequestAppService) +public class CreateTenantModalModel(IOnboardingRequestAppService onboardingRequestAppService, ISettingManager settingManager) : OnboardingPageModel { [BindProperty(SupportsGet = true)] @@ -16,10 +20,18 @@ public class CreateTenantModalModel(IOnboardingRequestAppService onboardingReque public OnboardingRequestDto? OnboardingRequest { get; set; } + public List DefaultMetabaseUserEmails { get; set; } = []; + public virtual async Task OnGetAsync() { OnboardingRequest = await onboardingRequestAppService.GetAsync(Id); if (OnboardingRequest == null) return NotFound(); + + var defaultEmailsCsv = await settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails); + DefaultMetabaseUserEmails = (defaultEmailsCsv ?? string.Empty) + .Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .ToList(); + return Page(); } } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js index 42ac513786..613cb5b44b 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js @@ -400,11 +400,14 @@ const ministryFieldKey = $('#create-tenant-ministry-field').val() || null; const divisionFieldKey = $('#create-tenant-division-field').val() || null; const programAreaFieldKey = $('#create-tenant-program-area-field').val() || null; + const metabaseUserEmails = $('#create-tenant-metabase-user-emails').val() || null; + const metabaseNewDefaultUserEmails = $('#create-tenant-metabase-new-default-user-emails').val() || null; + const metabaseRemovedDefaultUserEmails = $('#create-tenant-metabase-removed-default-user-emails').val() || null; abp.ajax({ url: abp.appPath + 'api/onboarding-requests/' + applicationId + '/create-tenant', type: 'POST', - data: JSON.stringify({ tenantNameFieldKey, displayNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, divisionFieldKey, programAreaFieldKey }), + data: JSON.stringify({ tenantNameFieldKey, displayNameFieldKey, superUsersFieldKey, branchFieldKey, featuresFieldKey, ministryFieldKey, divisionFieldKey, programAreaFieldKey, metabaseUserEmails, metabaseNewDefaultUserEmails, metabaseRemovedDefaultUserEmails }), contentType: 'application/json' }).done(function () { abp.notify.success(l('OnboardingModal:CreateSuccess')); @@ -491,6 +494,76 @@ }); } + // ─── Metabase tab: user list ─────────────────────────────────────────────── + + let _metabaseNewlyAddedEmails = []; + let _metabaseRemovedDefaultEmails = []; + + function _captureMetabaseUsersToForm() { + let checked = []; + $('#create-tenant-metabase-user-list .create-tenant-metabase-user-checkbox:checked').each(function () { + checked.push($(this).val()); + }); + $('#create-tenant-metabase-user-emails').val(checked.join(',')); + $('#create-tenant-metabase-removed-default-user-emails').val(_metabaseRemovedDefaultEmails.join(',')); + + if ($('#create-tenant-metabase-save-as-default').prop('checked')) { + let newDefaults = _metabaseNewlyAddedEmails.filter(function (email) { + return checked.indexOf(email) !== -1; + }); + $('#create-tenant-metabase-new-default-user-emails').val(newDefaults.join(',')); + } else { + $('#create-tenant-metabase-new-default-user-emails').val(''); + } + } + + function _addMetabaseUser(email) { + email = (email || '').trim(); + if (!email) return; + + let exists = $('#create-tenant-metabase-user-list .create-tenant-metabase-user-checkbox').toArray().some(function (el) { + return $(el).val().toLowerCase() === email.toLowerCase(); + }); + if (exists) { + abp.notify.warn(l('CreateTenantModal:MetabaseAlreadyInList')); + return; + } + + let id = 'create-tenant-metabase-user-' + $('#create-tenant-metabase-user-list .create-tenant-metabase-user-checkbox').length + '-' + Date.now(); + let $checkbox = $('') + .attr('id', id).val(email); + let $label = $('').attr('for', id).text(email); + $('
').append($checkbox).append($label).appendTo('#create-tenant-metabase-user-list'); + + _metabaseNewlyAddedEmails.push(email); + _captureMetabaseUsersToForm(); + } + + function _wireMetabaseTabHandlers() { + _metabaseNewlyAddedEmails = []; + _metabaseRemovedDefaultEmails = []; + _captureMetabaseUsersToForm(); + $('#create-tenant-metabase-user-list').on('change', '.create-tenant-metabase-user-checkbox', _captureMetabaseUsersToForm); + $('#create-tenant-metabase-save-as-default').on('change', _captureMetabaseUsersToForm); + $('#create-tenant-metabase-add-user-btn').on('click', function (e) { + e.preventDefault(); + _addMetabaseUser($('#create-tenant-metabase-new-user-email').val()); + $('#create-tenant-metabase-new-user-email').val(''); + }); + $('#create-tenant-metabase-new-user-email').on('keypress', function (e) { + if (e.which === 13) { + e.preventDefault(); + $('#create-tenant-metabase-add-user-btn').click(); + } + }); + $('#create-tenant-metabase-user-list').on('click', '.create-tenant-metabase-remove-default-btn', function (e) { + e.preventDefault(); + _metabaseRemovedDefaultEmails.push($(this).data('email')); + $(this).closest('.form-check').remove(); + _captureMetabaseUsersToForm(); + }); + } + abp.modals.createTenantModal = function () { return { initModal: function (publicApi, args) { @@ -502,6 +575,7 @@ } catch { _fieldValues = {}; } _loadCreateTenantFields(applicationId); + _wireMetabaseTabHandlers(); $('#btn-confirm-create-tenant').on('click', _onCreateTenantConfirm(applicationId)); } }; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml index 19049e0b29..47f190aa55 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml @@ -142,13 +142,15 @@

Users checked here will be granted access to this tenant's data in Metabase.

+

Note: a user must already have a Metabase account (via SSO login, or created under Admin > People) before they can be added to a group. Emails without an existing account are skipped during registration.

@foreach (var email in Model.DefaultMetabaseUserEmails) { -
+
- + +
}
@@ -164,6 +166,7 @@ +
} diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs index 0fd4ac3ea0..aa59358719 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs @@ -57,22 +57,23 @@ public virtual async Task OnPostAsync() var input = ObjectMapper.Map(Tenant); await TenantAppService.CreateAsync(input); - if (!string.IsNullOrWhiteSpace(Tenant.MetabaseNewDefaultUserEmails)) + if (!string.IsNullOrWhiteSpace(Tenant.MetabaseNewDefaultUserEmails) || !string.IsNullOrWhiteSpace(Tenant.MetabaseRemovedDefaultUserEmails)) { - await SaveNewMetabaseDefaultUserEmailsAsync(Tenant.MetabaseNewDefaultUserEmails); + await UpdateMetabaseDefaultUserEmailsAsync(Tenant.MetabaseNewDefaultUserEmails, Tenant.MetabaseRemovedDefaultUserEmails); } return NoContent(); } - private async Task SaveNewMetabaseDefaultUserEmailsAsync(string newEmailsCsv) + private async Task UpdateMetabaseDefaultUserEmailsAsync(string? newEmailsCsv, string? removedEmailsCsv) { - var existing = SplitEmails(await SettingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails)); - var merged = existing + var removed = SplitEmails(removedEmailsCsv); + var updated = SplitEmails(await SettingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails)) .Concat(SplitEmails(newEmailsCsv)) + .Where(email => !removed.Contains(email, StringComparer.OrdinalIgnoreCase)) .Distinct(StringComparer.OrdinalIgnoreCase); - await SettingManager.SetGlobalAsync(MetabaseSettings.UserEmails, string.Join(",", merged)); + await SettingManager.SetGlobalAsync(MetabaseSettings.UserEmails, string.Join(",", updated)); } private static List SplitEmails(string? emailsCsv) => @@ -103,6 +104,9 @@ public class TenantInfoModel : ExtensibleObject /// Comma-separated subset of newly-added Metabase emails to persist as the new Global default. public string? MetabaseNewDefaultUserEmails { get; set; } + /// Comma-separated default Metabase emails explicitly removed - deleted from the Global default. + public string? MetabaseRemovedDefaultUserEmails { get; set; } + [Required] public string UserIdentifier { get; set; } = string.Empty; } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js index c3ac2bb895..4076dc7d7b 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js @@ -232,6 +232,7 @@ $('#create-features-content').on('change', 'input[type="checkbox"]', _captureCreateFeaturesToForm); _metabaseNewlyAddedEmails = []; + _metabaseRemovedDefaultEmails = []; _captureMetabaseUsersToForm(); $('#metabase-user-list').on('change', '.metabase-user-checkbox', _captureMetabaseUsersToForm); $('#metabase-save-as-default').on('change', _captureMetabaseUsersToForm); @@ -246,6 +247,12 @@ $('#metabase-add-user-btn').click(); } }); + $('#metabase-user-list').on('click', '.metabase-remove-default-btn', function (e) { + e.preventDefault(); + _metabaseRemovedDefaultEmails.push($(this).data('email')); + $(this).closest('.form-check').remove(); + _captureMetabaseUsersToForm(); + }); $('#create-pane-features').closest('form').on('invalid-form.validate', function (e, validator) { if (validator.errorList.length > 0) { @@ -260,6 +267,7 @@ // ─── Metabase tab: user list ─────────────────────────────────────────────── let _metabaseNewlyAddedEmails = []; + let _metabaseRemovedDefaultEmails = []; function _captureMetabaseUsersToForm() { let checked = []; @@ -267,6 +275,7 @@ checked.push($(this).val()); }); $('#metabase-user-emails').val(checked.join(',')); + $('#metabase-removed-default-user-emails').val(_metabaseRemovedDefaultEmails.join(',')); if ($('#metabase-save-as-default').prop('checked')) { let newDefaults = _metabaseNewlyAddedEmails.filter(function (email) { diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/OnboardingRequestAppServiceTests.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/OnboardingRequestAppServiceTests.cs index 7667f51dfa..cce0b7c74c 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/OnboardingRequestAppServiceTests.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/OnboardingRequestAppServiceTests.cs @@ -8,6 +8,7 @@ using Unity.Flex.Worksheets; using Unity.Flex.Worksheets.Values; using Unity.Flex.WorksheetInstances; +using Unity.TenantManagement.Metabase; using Unity.TenantManagement.Onboarding; using Volo.Abp; using Volo.Abp.Application.Dtos; @@ -469,4 +470,87 @@ await _tenantAppService.Received(1).AssignManagerAsync(Arg.Is>(), ApplicationCorrelationProvider).Returns(new List + { + WorksheetInstanceFor(id, ("tn", "Metabase Co"), ("su", "first@example.com")) + }); + _userLookup.FindUserGuidByEmailAsync("first@example.com").Returns("guid-1"); + _tenantAppService.CreateAsync(Arg.Any()) + .Returns(new TenantDto { Id = newTenantId, Name = "Metabase Co" }); + + await _appService.CreateTenantAsync(id, new CreateTenantInputDto + { + TenantNameFieldKey = "tn", + SuperUsersFieldKey = "su", + MetabaseUserEmails = "a@gov.bc.ca,b@gov.bc.ca" + }); + + await _tenantAppService.Received(1).CreateAsync(Arg.Is(d => + d.MetabaseUserEmails == "a@gov.bc.ca,b@gov.bc.ca")); + } + + [Fact] + public async Task CreateTenantAsync_MetabaseNewDefaultUserEmailsProvided_MergesIntoGlobalSetting() + { + var id = Guid.NewGuid(); + var newTenantId = Guid.NewGuid(); + + _applicationProvider.GetByIdAsync(id).Returns(new OnboardingApplicationRecord { Id = id, Category = "Onboarding" }); + _worksheetInstanceAppService.GetListByCorrelationIdsAsync(Arg.Any>(), ApplicationCorrelationProvider).Returns(new List + { + WorksheetInstanceFor(id, ("tn", "Metabase Defaults Co"), ("su", "first@example.com")) + }); + _userLookup.FindUserGuidByEmailAsync("first@example.com").Returns("guid-1"); + _tenantAppService.CreateAsync(Arg.Any()) + .Returns(new TenantDto { Id = newTenantId, Name = "Metabase Defaults Co" }); + _settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails) + .Returns("existing@gov.bc.ca"); + + await _appService.CreateTenantAsync(id, new CreateTenantInputDto + { + TenantNameFieldKey = "tn", + SuperUsersFieldKey = "su", + MetabaseUserEmails = "existing@gov.bc.ca,new@gov.bc.ca", + MetabaseNewDefaultUserEmails = "new@gov.bc.ca" + }); + + await _settingManager.Received(1).SetGlobalAsync( + MetabaseSettings.UserEmails, "existing@gov.bc.ca,new@gov.bc.ca"); + } + + [Fact] + public async Task CreateTenantAsync_MetabaseRemovedDefaultUserEmailsProvided_RemovesFromGlobalSetting() + { + var id = Guid.NewGuid(); + var newTenantId = Guid.NewGuid(); + + _applicationProvider.GetByIdAsync(id).Returns(new OnboardingApplicationRecord { Id = id, Category = "Onboarding" }); + _worksheetInstanceAppService.GetListByCorrelationIdsAsync(Arg.Any>(), ApplicationCorrelationProvider).Returns(new List + { + WorksheetInstanceFor(id, ("tn", "Metabase Removal Co"), ("su", "first@example.com")) + }); + _userLookup.FindUserGuidByEmailAsync("first@example.com").Returns("guid-1"); + _tenantAppService.CreateAsync(Arg.Any()) + .Returns(new TenantDto { Id = newTenantId, Name = "Metabase Removal Co" }); + _settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails) + .Returns("keep@gov.bc.ca,stale@gov.bc.ca"); + + await _appService.CreateTenantAsync(id, new CreateTenantInputDto + { + TenantNameFieldKey = "tn", + SuperUsersFieldKey = "su", + MetabaseUserEmails = "keep@gov.bc.ca", + MetabaseRemovedDefaultUserEmails = "stale@gov.bc.ca" + }); + + await _settingManager.Received(1).SetGlobalAsync(MetabaseSettings.UserEmails, "keep@gov.bc.ca"); + } } diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs index 89507ea589..aa1d760237 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs @@ -11,7 +11,7 @@ namespace Unity.GrantManager.Integrations.Metabase; /// public interface IMetabaseApiClient { - Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, CancellationToken cancellationToken = default); + Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default); Task SyncDatabaseSchemaAsync(int databaseId, CancellationToken cancellationToken = default); Task RescanDatabaseValuesAsync(int databaseId, CancellationToken cancellationToken = default); Task CreateGroupAsync(string name, CancellationToken cancellationToken = default); diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs index 27ed590c3a..4b8fae41ac 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs @@ -18,14 +18,14 @@ public class MetabaseApiClient( { private const string ApiKeyHeader = "x-api-key"; - public async Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, CancellationToken cancellationToken = default) + public async Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default) { var body = new { engine = "postgres", name, is_full_sync = true, - details = new { host, port, dbname = dbName, user = username, password, ssl = true } + details = new { host, port, dbname = dbName, user = username, password, ssl } }; var result = await PostAsync("/api/database", body, cancellationToken); return result.Value("id"); diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseOptions.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseOptions.cs index 0a3f62c106..487528d539 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseOptions.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseOptions.cs @@ -4,4 +4,28 @@ public class MetabaseOptions { /// Admin API key - same key the Metabase admin UI/PowerShell runbook uses (x-api-key header). public string ApiKey { get; set; } = string.Empty; + + /// + /// Local-dev-only override for the Postgres host passed to Metabase when registering a + /// tenant's database connection. Tenant readonly connection strings store "localhost" as the + /// host (correct for the .NET app, which runs on the host machine) - but a dockerized local + /// Metabase container can't reach "localhost" that way, since that resolves to the container + /// itself. Set this to whatever hostname your local Metabase container can actually reach + /// Postgres by - e.g. the Postgres container's name/service (like "unitydb") if both containers + /// share a Docker network, or "host.docker.internal" if Metabase needs to reach out to the host + /// machine instead (which may also require a Windows Firewall inbound allow rule for 5432, and + /// doesn't resolve for every local Docker setup - verify with a direct call to Metabase's + /// POST /api/database before assuming it's this setting). Leave unset in deployed environments - + /// there both the app and Metabase reach Postgres via its OpenShift service name, so the stored + /// host is already correct. + /// + public string DbHostOverride { get; set; } = string.Empty; + + /// + /// Local-dev-only override for whether Metabase connects to the tenant's Postgres database + /// over SSL. Deployed Postgres (Crunchy on OpenShift) requires SSL, so the default (null, no + /// override) sends ssl: true. A plain local `postgres` Docker image has SSL disabled + /// out of the box, so set this to false locally or Metabase's connection attempt fails. + /// + public bool? DbSslOverride { get; set; } } diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs index 1134170d01..d0e68fdcb6 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs @@ -41,11 +41,20 @@ public override async Task ExecuteAsync(PostTenantCreationStepArgs args) { using (currentTenant.Change(args.TenantId)) { - logger.LogInformation( - "{Prefix} Running step {StepIndex} '{StepName}' for tenant {TenantId}", - LogPrefix, args.StepIndex, step.StepName, args.TenantId); + if (!await step.CanExecuteAsync(args.TenantId)) + { + logger.LogInformation( + "{Prefix} Skipping step {StepIndex} '{StepName}' for tenant {TenantId} - CanExecuteAsync returned false", + LogPrefix, args.StepIndex, step.StepName, args.TenantId); + } + else + { + logger.LogInformation( + "{Prefix} Running step {StepIndex} '{StepName}' for tenant {TenantId}", + LogPrefix, args.StepIndex, step.StepName, args.TenantId); - await step.ExecuteAsync(args.TenantId); + await step.ExecuteAsync(args.TenantId); + } } } catch (Exception ex) diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs index b971624f64..bf45bb8d60 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs @@ -4,9 +4,11 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Unity.GrantManager.Integrations.Metabase; using Unity.Modules.Shared.PostTenantCreation; using Unity.TenantManagement.Metabase; +using Volo.Abp; using Volo.Abp.DependencyInjection; using Volo.Abp.Security.Encryption; using Volo.Abp.Settings; @@ -40,11 +42,14 @@ namespace Unity.GrantManager.Tenants.PostCreation.Steps; /// is true - a Metabase outage is logged but doesn't block tenant /// creation or later post-creation steps. /// +[RemoteService(false)] +[ExposeServices(typeof(IPostTenantCreationStep))] public class MetabaseTenantRegistrationStep( IMetabaseApiClient metabaseApiClient, ITenantRepository tenantRepository, IStringEncryptionService stringEncryptionService, ISettingManager settingManager, + IOptions metabaseOptions, ILogger logger) : IPostTenantCreationStep, ITransientDependency { @@ -58,6 +63,19 @@ public class MetabaseTenantRegistrationStep( // A Metabase outage shouldn't block other post-creation steps from running. public bool ContinueOnError => true; + public virtual Task CanExecuteAsync(Guid tenantId) + { + if (string.IsNullOrWhiteSpace(metabaseOptions.Value.ApiKey)) + { + logger.LogInformation( + "{Prefix} No Metabase API key configured - skipping registration for tenant {TenantId}.", + LogPrefix, tenantId); + return Task.FromResult(false); + } + + return Task.FromResult(true); + } + public virtual async Task ExecuteAsync(Guid tenantId) { var tenant = await tenantRepository.GetAsync(tenantId, includeDetails: true); @@ -74,7 +92,17 @@ public virtual async Task ExecuteAsync(Guid tenantId) var (host, port, dbName, username, password) = ParseConnectionString(stringEncryptionService.Decrypt(encryptedReadOnlyConnectionString)); - var databaseId = await metabaseApiClient.CreateDatabaseAsync(tenant.Name, host, port, dbName, username, password); + if (!string.IsNullOrWhiteSpace(metabaseOptions.Value.DbHostOverride)) + { + logger.LogInformation( + "{Prefix} Overriding Postgres host '{OriginalHost}' with '{OverrideHost}' for tenant {TenantId} (Metabase:DbHostOverride is set).", + LogPrefix, host, metabaseOptions.Value.DbHostOverride, tenantId); + host = metabaseOptions.Value.DbHostOverride; + } + + var ssl = metabaseOptions.Value.DbSslOverride ?? true; + + var databaseId = await metabaseApiClient.CreateDatabaseAsync(tenant.Name, host, port, dbName, username, password, ssl); await metabaseApiClient.SyncDatabaseSchemaAsync(databaseId); await metabaseApiClient.RescanDatabaseValuesAsync(databaseId); diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain.Shared/Localization/GrantManager/en.json b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain.Shared/Localization/GrantManager/en.json index 4b32807332..edd337fc6e 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain.Shared/Localization/GrantManager/en.json +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain.Shared/Localization/GrantManager/en.json @@ -631,6 +631,14 @@ "CreateTenantModal:CreatingWarningBody": "Tenant provisioning is in progress — please do not close this window or navigate away until the process completes.", "CreateTenantModal:ConfirmButton": "Create Tenant", "CreateTenantModal:NoFieldsWarning": "No worksheet data is available for this request. Tenant creation requires at least one submitted worksheet with field data.", + "CreateTenantModal:DetailsTab": "Details", + "CreateTenantModal:MetabaseTab": "Metabase", + "CreateTenantModal:MetabaseDescription": "Users checked here will be granted access to this tenant's data in Metabase.", + "CreateTenantModal:MetabaseAccountNote": "Note: a user must already have a Metabase account (via SSO login, or created under Admin > People) before they can be added to a group. Emails without an existing account are skipped during registration.", + "CreateTenantModal:MetabaseAddButton": "Add", + "CreateTenantModal:MetabaseSaveAsDefaultLabel": "Save newly added users as default for future tenants", + "CreateTenantModal:MetabaseRemoveDefaultTitle": "Remove from default list", + "CreateTenantModal:MetabaseAlreadyInList": "That user is already in the list.", "TenantList:ActionsButton": "Actions", "TenantList:ConfigurationAction": "Configuration", "TenantList:LicencePlate": "Licence Plate", diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.Development.json b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.Development.json index f317444a18..c80ec41d0d 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.Development.json +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.Development.json @@ -135,11 +135,19 @@ "ReportingAI": { "JWTSecret": "" }, - "Azure": { "OpenAI": { "ApiKey": "", "Endpoint": "" } + }, + "TenantCreation": { + "Steps": { + "Metabase": { + "ApiKey": "", + "DbHostOverride": "", + "DbSslOverride": false + } + } } } diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json index 8a7fba49f6..0919e5791c 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json @@ -39,13 +39,6 @@ "ChesClientId": "", "ChesClientSecret": "" }, - "TenantCreation": { - "Steps": { - "Metabase": { - "ApiKey": "" - } - } - }, "Intake": { "FormId": "", "ApiKey": "", @@ -169,5 +162,14 @@ "Endpoint": "" }, "UNITY_GITHUB_PAT": "" + }, + "TenantCreation": { + "Steps": { + "Metabase": { + "ApiKey": "", + "DbHostOverride": "", + "DbSslOverride": false + } + } } } diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/PostTenantCreationSequenceJobTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/PostTenantCreationSequenceJobTests.cs index f7025a34d4..a13e112e85 100644 --- a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/PostTenantCreationSequenceJobTests.cs +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/PostTenantCreationSequenceJobTests.cs @@ -13,7 +13,7 @@ namespace Unity.GrantManager.Tenants.PostCreation; public class PostTenantCreationSequenceJobTests { - private sealed class FakeStep(int order, string name, bool continueOnError, Func? onExecute = null) + private sealed class FakeStep(int order, string name, bool continueOnError, Func? onExecute = null, bool canExecute = true) : IPostTenantCreationStep { public int Order { get; } = order; @@ -21,6 +21,8 @@ private sealed class FakeStep(int order, string name, bool continueOnError, Func public bool ContinueOnError { get; } = continueOnError; public bool Executed { get; private set; } + public Task CanExecuteAsync(Guid tenantId) => Task.FromResult(canExecute); + public async Task ExecuteAsync(Guid tenantId) { Executed = true; @@ -106,6 +108,20 @@ public async Task ExecuteAsync_StepThrows_ContinueOnErrorFalse_StopsSequence() enqueued.ShouldBeEmpty(); } + [Fact] + public async Task ExecuteAsync_CanExecuteAsyncReturnsFalse_SkipsExecuteButStillEnqueuesNextStep() + { + var step0 = new FakeStep(0, "Step0", continueOnError: false, canExecute: false); + var step1 = new FakeStep(1, "Step1", continueOnError: false); + var (job, enqueued) = CreateJob([step0, step1]); + + await job.ExecuteAsync(new PostTenantCreationStepArgs { TenantId = Guid.NewGuid(), StepIndex = 0 }); + + step0.Executed.ShouldBeFalse(); + var next = enqueued.ShouldHaveSingleItem(); + next.StepIndex.ShouldBe(1); + } + [Fact] public async Task ExecuteAsync_RunsSteps_InAscendingOrder_RegardlessOfRegistrationOrder() { diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs index d57258797b..f5efbee7f2 100644 --- a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs @@ -2,6 +2,7 @@ using System.Reflection; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using NSubstitute; using Shouldly; using Unity.GrantManager.Integrations.Metabase; @@ -32,7 +33,7 @@ private static Tenant CreateTenant(string name) } private static (MetabaseTenantRegistrationStep Step, IMetabaseApiClient MetabaseApiClient, ISettingManager SettingManager, Tenant Tenant) - CreateStep(string? encryptedReadOnlyConnectionString = "encrypted-blob") + CreateStep(string? encryptedReadOnlyConnectionString = "encrypted-blob", string? apiKey = "test-api-key", string? dbHostOverride = null, bool? dbSslOverride = null) { var tenant = CreateTenant("AG-MARB"); if (encryptedReadOnlyConnectionString != null) @@ -48,9 +49,15 @@ private static (MetabaseTenantRegistrationStep Step, IMetabaseApiClient Metabase var settingManager = Substitute.For(); var metabaseApiClient = Substitute.For(); + var metabaseOptions = Options.Create(new MetabaseOptions + { + ApiKey = apiKey ?? string.Empty, + DbHostOverride = dbHostOverride ?? string.Empty, + DbSslOverride = dbSslOverride + }); var step = new MetabaseTenantRegistrationStep( - metabaseApiClient, tenantRepository, encryptionService, settingManager, + metabaseApiClient, tenantRepository, encryptionService, settingManager, metabaseOptions, Substitute.For>()); return (step, metabaseApiClient, settingManager, tenant); @@ -64,6 +71,25 @@ public void ContinueOnError_IsTrue_SoAMetabaseOutageDoesNotBlockLaterSteps() step.ContinueOnError.ShouldBeTrue(); } + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public async Task CanExecuteAsync_NoApiKeyConfigured_ReturnsFalse(string? apiKey) + { + var (step, _, _, tenant) = CreateStep(apiKey: apiKey); + + (await step.CanExecuteAsync(tenant.Id)).ShouldBeFalse(); + } + + [Fact] + public async Task CanExecuteAsync_ApiKeyConfigured_ReturnsTrue() + { + var (step, _, _, tenant) = CreateStep(); + + (await step.CanExecuteAsync(tenant.Id)).ShouldBeTrue(); + } + [Fact] public async Task ExecuteAsync_NoReadonlyConnectionString_SkipsWithoutCallingMetabase() { @@ -71,7 +97,7 @@ public async Task ExecuteAsync_NoReadonlyConnectionString_SkipsWithoutCallingMet await step.ExecuteAsync(tenant.Id); - await metabaseApiClient.DidNotReceiveWithAnyArgs().CreateDatabaseAsync(default!, default!, default, default!, default!, default!); + await metabaseApiClient.DidNotReceiveWithAnyArgs().CreateDatabaseAsync(default!, default!, default, default!, default!, default!, default); } [Fact] @@ -83,7 +109,7 @@ public async Task ExecuteAsync_CreatesDatabaseGroupAndCollection_UsingParsedConn settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns("user1@gov.bc.ca,user2@gov.bc.ca"); metabaseApiClient.CreateDatabaseAsync( - tenant.Name, "dev-crunchy-postgres-primary.ce395f-dev.svc", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t") + tenant.Name, "dev-crunchy-postgres-primary.ce395f-dev.svc", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", true) .Returns(11); metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); metabaseApiClient.FindUserIdByEmailAsync("user1@gov.bc.ca").Returns(101); @@ -100,6 +126,38 @@ public async Task ExecuteAsync_CreatesDatabaseGroupAndCollection_UsingParsedConn await metabaseApiClient.Received(1).GrantGroupCollectionAccessAsync(22, 33); } + [Fact] + public async Task ExecuteAsync_DbHostOverrideConfigured_UsesOverrideHostInsteadOfConnectionStringHost() + { + var (step, metabaseApiClient, settingManager, tenant) = CreateStep(dbHostOverride: "host.docker.internal"); + settingManager.GetOrNullAsync(MetabaseSettings.UserEmails, TenantSettingValueProvider.ProviderName, tenant.Id.ToString()) + .Returns((string?)null); + settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns((string?)null); + metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.CreateCollectionAsync(tenant.Name).Returns(33); + + await step.ExecuteAsync(tenant.Id); + + await metabaseApiClient.Received(1).CreateDatabaseAsync( + tenant.Name, "host.docker.internal", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", true); + } + + [Fact] + public async Task ExecuteAsync_DbSslOverrideFalse_DisablesSslForDatabaseConnection() + { + var (step, metabaseApiClient, settingManager, tenant) = CreateStep(dbSslOverride: false); + settingManager.GetOrNullAsync(MetabaseSettings.UserEmails, TenantSettingValueProvider.ProviderName, tenant.Id.ToString()) + .Returns((string?)null); + settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns((string?)null); + metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.CreateCollectionAsync(tenant.Name).Returns(33); + + await step.ExecuteAsync(tenant.Id); + + await metabaseApiClient.Received(1).CreateDatabaseAsync( + tenant.Name, "dev-crunchy-postgres-primary.ce395f-dev.svc", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", false); + } + [Fact] public async Task ExecuteAsync_TenantScopedUserEmailsSetting_TakesPrecedenceOverGlobalDefault() { From 0a80aec6cd2c0c7c9cd8a86452fcf9c409b3af1e Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Fri, 21 Aug 2026 14:04:57 -0700 Subject: [PATCH 3/9] AB#34137 PR updates --- .../Pages/TenantManagement/Onboarding/Index.js | 2 +- .../Pages/TenantManagement/Tenants/Index.js | 17 ++++++++++++----- .../Steps/MetabaseTenantRegistrationStep.cs | 13 +++++++++++-- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js index 613cb5b44b..3d97ca663c 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Onboarding/Index.js @@ -509,7 +509,7 @@ if ($('#create-tenant-metabase-save-as-default').prop('checked')) { let newDefaults = _metabaseNewlyAddedEmails.filter(function (email) { - return checked.indexOf(email) !== -1; + return checked.includes(email); }); $('#create-tenant-metabase-new-default-user-emails').val(newDefaults.join(',')); } else { diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js index 4076dc7d7b..1344561c3a 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js @@ -279,7 +279,7 @@ if ($('#metabase-save-as-default').prop('checked')) { let newDefaults = _metabaseNewlyAddedEmails.filter(function (email) { - return checked.indexOf(email) !== -1; + return checked.includes(email); }); $('#metabase-new-default-user-emails').val(newDefaults.join(',')); } else { @@ -320,10 +320,17 @@ function _generateGuid() { if (globalThis.crypto?.randomUUID) return globalThis.crypto.randomUUID(); - return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { - let r = Math.random() * 16 | 0, v = c === 'x' ? r : (r & 0x3 | 0x8); - return v.toString(16); - }); + + // Fallback for environments without crypto.randomUUID - still CSPRNG-backed via + // crypto.getRandomValues, not Math.random(), since this key is used as a cache-busting + // provider key sent to the server, not truly security-sensitive, but there's no reason + // to reach for a weaker PRNG when getRandomValues is universally available. + const bytes = new Uint8Array(16); + globalThis.crypto.getRandomValues(bytes); + bytes[6] = (bytes[6] & 0x0f) | 0x40; + bytes[8] = (bytes[8] & 0x3f) | 0x80; + const hex = Array.from(bytes, function (b) { return b.toString(16).padStart(2, '0'); }).join(''); + return hex.slice(0, 8) + '-' + hex.slice(8, 12) + '-' + hex.slice(12, 16) + '-' + hex.slice(16, 20) + '-' + hex.slice(20); } function _renderFeatureItem(feature) { diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs index bf45bb8d60..a3609f6c7c 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs @@ -114,8 +114,8 @@ public virtual async Task ExecuteAsync(Guid tenantId) if (userId == null) { logger.LogWarning( - "{Prefix} User '{Email}' not found in Metabase for tenant {TenantId} - they must log in via LDAP or be created under Admin > People before they can be added to a group.", - LogPrefix, email, tenantId); + "{Prefix} User '{MaskedEmail}' not found in Metabase for tenant {TenantId} - they must log in via LDAP or be created under Admin > People before they can be added to a group.", + LogPrefix, MaskEmail(email), tenantId); continue; } await metabaseApiClient.AddGroupMemberAsync(groupId, userId.Value); @@ -142,6 +142,15 @@ private async Task> GetUserEmailsAsync(Guid tenantId) .ToList(); } + // Avoids writing a user's full email address to logs (CodeQL: exposure of private + // information) while keeping enough of it for an admin to correlate a "not found" warning + // with a known user. + private static string MaskEmail(string email) + { + var atIndex = email.IndexOf('@', StringComparison.Ordinal); + return atIndex <= 1 ? "***" : string.Concat(email.AsSpan(0, 1), "***", email.AsSpan(atIndex)); + } + private static (string Host, int Port, string DbName, string Username, string Password) ParseConnectionString(string connectionString) { string? Get(string key) From 9c7078e2262aac7cfd5da14f81902c31333ff7ba Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Fri, 21 Aug 2026 14:52:04 -0700 Subject: [PATCH 4/9] AB#34137 more codeQL suggestions --- .../Handlers/TenantCreatedEventHandler.cs | 8 +- .../Metabase/MetabaseApiClient.cs | 81 ++++++++---- .../Unity.GrantManager.Web/appsettings.json | 3 +- .../Metabase/MetabaseApiClientTests.cs | 125 ++++++++++++++++++ 4 files changed, 185 insertions(+), 32 deletions(-) create mode 100644 applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs index 5b69356174..670bfe8302 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs @@ -76,11 +76,15 @@ await _backgroundJobManager.EnqueueAsync(new PostTenantCreationStepArgs // Global default changes in the meantime. private async Task SaveMetabaseUserEmailsAsync(TenantCreatedEto eto, Guid tenantId) { - if (!eto.Properties.TryGetValue("MetabaseUserEmails", out var emailsRaw) || string.IsNullOrWhiteSpace(emailsRaw)) + // An empty string is a deliberate "no Metabase users for this tenant" choice - it must + // still be persisted (not skipped), otherwise MetabaseTenantRegistrationStep's + // GetUserEmailsAsync falls back to the Global default list and grants users the + // tenant creator explicitly unchecked. + if (!eto.Properties.TryGetValue("MetabaseUserEmails", out var emailsRaw)) return; await _settingManager.SetAsync( - MetabaseSettings.UserEmails, emailsRaw, TenantSettingValueProvider.ProviderName, tenantId.ToString()); + MetabaseSettings.UserEmails, emailsRaw ?? string.Empty, TenantSettingValueProvider.ProviderName, tenantId.ToString()); } private async Task EnableRequestedFeaturesAsync(TenantCreatedEto eto, Guid tenantId) diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs index 4b8fae41ac..d315a138ab 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net; using System.Net.Http; using System.Threading; using System.Threading.Tasks; @@ -18,6 +19,12 @@ public class MetabaseApiClient( { private const string ApiKeyHeader = "x-api-key"; + // Metabase's permissions/collection graph endpoints use an optimistic-concurrency "revision" + // number - a PUT with a stale revision (because another tenant registration updated the graph + // first) is rejected. Retry the whole read-mutate-write cycle against a freshly-fetched graph + // rather than surfacing a transient conflict as a permanent failure. + private const int MaxGraphUpdateAttempts = 3; + public async Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default) { var body = new @@ -54,23 +61,19 @@ public async Task CreateGroupAsync(string name, CancellationToken cancellat public Task AddGroupMemberAsync(int groupId, int userId, CancellationToken cancellationToken = default) => PostAsync("/api/permissions/membership", new { group_id = groupId, user_id = userId }, cancellationToken); - public async Task GrantGroupDatabaseAccessAsync(int groupId, int databaseId, CancellationToken cancellationToken = default) - { - var graph = await GetAsync("/api/permissions/graph", cancellationToken); - var groups = (JObject?)graph["groups"] ?? new JObject(); - var groupKey = groupId.ToString(); - var groupNode = (JObject?)groups[groupKey] ?? new JObject(); - - groupNode[databaseId.ToString()] = new JObject + public Task GrantGroupDatabaseAccessAsync(int groupId, int databaseId, CancellationToken cancellationToken = default) => + UpdateGraphWithRetryAsync("/api/permissions/graph", groups => { - ["view-data"] = "unrestricted", - ["create-queries"] = "query-builder-and-native" - }; - groups[groupKey] = groupNode; + var groupKey = groupId.ToString(); + var groupNode = (JObject?)groups[groupKey] ?? new JObject(); - await PutAsync("/api/permissions/graph", - new { groups, revision = graph.Value("revision") }, cancellationToken); - } + groupNode[databaseId.ToString()] = new JObject + { + ["view-data"] = "unrestricted", + ["create-queries"] = "query-builder-and-native" + }; + groups[groupKey] = groupNode; + }, cancellationToken); public async Task CreateCollectionAsync(string name, CancellationToken cancellationToken = default) { @@ -78,18 +81,41 @@ public async Task CreateCollectionAsync(string name, CancellationToken canc return result.Value("id"); } - public async Task GrantGroupCollectionAccessAsync(int groupId, int collectionId, CancellationToken cancellationToken = default) - { - var graph = await GetAsync("/api/collection/graph", cancellationToken); - var groups = (JObject?)graph["groups"] ?? new JObject(); - var groupKey = groupId.ToString(); - var groupNode = (JObject?)groups[groupKey] ?? new JObject(); + public Task GrantGroupCollectionAccessAsync(int groupId, int collectionId, CancellationToken cancellationToken = default) => + UpdateGraphWithRetryAsync("/api/collection/graph", groups => + { + var groupKey = groupId.ToString(); + var groupNode = (JObject?)groups[groupKey] ?? new JObject(); - groupNode[collectionId.ToString()] = "write"; - groups[groupKey] = groupNode; + groupNode[collectionId.ToString()] = "write"; + groups[groupKey] = groupNode; + }, cancellationToken); - await PutAsync("/api/collection/graph", - new { groups, revision = graph.Value("revision") }, cancellationToken); + private async Task UpdateGraphWithRetryAsync(string graphPath, Action applyMutation, CancellationToken cancellationToken) + { + for (var attempt = 1; ; attempt++) + { + var graph = await GetAsync(graphPath, cancellationToken); + var groups = (JObject?)graph["groups"] ?? new JObject(); + applyMutation(groups); + + var response = await PutRawAsync(graphPath, + new { groups, revision = graph.Value("revision") }, cancellationToken); + + if (response.IsSuccessStatusCode) + return; + + var isRevisionConflict = response.StatusCode is HttpStatusCode.Conflict or HttpStatusCode.BadRequest; + if (!isRevisionConflict || attempt >= MaxGraphUpdateAttempts) + { + var content = response.Content == null ? string.Empty : await response.Content.ReadAsStringAsync(cancellationToken); + throw new IntegrationServiceException( + $"Metabase API call to '{graphPath}' failed with status {response.StatusCode}: {content}"); + } + + // Stale revision - another concurrent tenant registration updated the graph first. + // Loop around to re-fetch the latest graph and reapply this mutation on top of it. + } } private async Task GetBaseUrlAsync() => @@ -114,12 +140,11 @@ private async Task PostAsync(string path, object body, CancellationToke return await ReadJsonAsync(response, path); } - private async Task PutAsync(string path, object body, CancellationToken cancellationToken) + private async Task PutRawAsync(string path, object body, CancellationToken cancellationToken) { var baseUrl = await GetBaseUrlAsync(); - var response = await resilientHttpRequest.HttpAsync( + return await resilientHttpRequest.HttpAsync( HttpMethod.Put, $"{baseUrl}{path}", body, extraHeaders: BuildHeaders(), cancellationToken: cancellationToken); - return await ReadJsonAsync(response, path); } private static async Task ReadJsonAsync(HttpResponseMessage response, string path) diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json index 0919e5791c..91383e7d3e 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Web/appsettings.json @@ -167,8 +167,7 @@ "Steps": { "Metabase": { "ApiKey": "", - "DbHostOverride": "", - "DbSslOverride": false + "DbHostOverride": "" } } } diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs new file mode 100644 index 0000000000..ad2589ef05 --- /dev/null +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs @@ -0,0 +1,125 @@ +using System.Net; +using System.Net.Http; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Options; +using NSubstitute; +using Shouldly; +using Unity.GrantManager.Integrations.Exceptions; +using Unity.Modules.Shared.Http; +using Xunit; + +namespace Unity.GrantManager.Integrations.Metabase; + +public class MetabaseApiClientTests +{ + private const string BaseUrl = "https://metabase.example"; + + private static (MetabaseApiClient Client, IResilientHttpRequest Http) CreateClient() + { + var http = Substitute.For(); + var endpointService = Substitute.For(); + endpointService.GetUgmUrlByKeyNameAsync(DynamicUrlKeyNames.METABASE_API_BASE).Returns(BaseUrl); + var options = Options.Create(new MetabaseOptions { ApiKey = "test-api-key" }); + + return (new MetabaseApiClient(http, endpointService, options), http); + } + + private static HttpResponseMessage JsonResponse(HttpStatusCode status, string json) => + new(status) { Content = new StringContent(json, Encoding.UTF8, "application/json") }; + + private static void SetupHttpSequence(IResilientHttpRequest http, params HttpResponseMessage[] responses) => + http.HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()) + .Returns(responses[0], responses[1..]); + + [Fact] + public async Task GrantGroupDatabaseAccessAsync_NoConflict_SucceedsOnFirstAttempt() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":1}"), + JsonResponse(HttpStatusCode.OK, "{}")); + + await client.GrantGroupDatabaseAccessAsync(groupId: 5, databaseId: 11); + + await http.Received(2).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task GrantGroupDatabaseAccessAsync_StaleRevisionOnFirstPut_RefetchesGraphAndRetries() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":1}"), // GET #1 + JsonResponse(HttpStatusCode.Conflict, "{\"message\":\"stale\"}"), // PUT #1 - stale revision + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":2}"), // GET #2 (retry re-fetch) + JsonResponse(HttpStatusCode.OK, "{}")); // PUT #2 - succeeds + + await Should.NotThrowAsync(() => client.GrantGroupDatabaseAccessAsync(groupId: 5, databaseId: 11)); + + await http.Received(4).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task GrantGroupDatabaseAccessAsync_ConflictOnEveryAttempt_ThrowsAfterMaxAttempts() + { + var (client, http) = CreateClient(); + // 3 attempts allowed: GET/PUT-conflict x3 (6 calls total), all conflicting. + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":1}"), + JsonResponse(HttpStatusCode.Conflict, "{}"), + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":2}"), + JsonResponse(HttpStatusCode.Conflict, "{}"), + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":3}"), + JsonResponse(HttpStatusCode.Conflict, "{}")); + + await Should.ThrowAsync( + () => client.GrantGroupDatabaseAccessAsync(groupId: 5, databaseId: 11)); + + await http.Received(6).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task GrantGroupDatabaseAccessAsync_NonConflictFailure_ThrowsWithoutRetrying() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":1}"), + JsonResponse(HttpStatusCode.InternalServerError, "{}")); + + await Should.ThrowAsync( + () => client.GrantGroupDatabaseAccessAsync(groupId: 5, databaseId: 11)); + + // Only the initial GET + PUT - a non-conflict failure isn't retried at this layer. + await http.Received(2).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task GrantGroupCollectionAccessAsync_StaleRevisionOnFirstPut_RefetchesGraphAndRetries() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":1}"), + JsonResponse(HttpStatusCode.BadRequest, "{}"), + JsonResponse(HttpStatusCode.OK, "{\"groups\":{},\"revision\":2}"), + JsonResponse(HttpStatusCode.OK, "{}")); + + await Should.NotThrowAsync(() => client.GrantGroupCollectionAccessAsync(groupId: 5, collectionId: 22)); + } +} From c4b9ffdbb81c16385bbe7aa3296b714af3ff6648 Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Fri, 21 Aug 2026 15:32:40 -0700 Subject: [PATCH 5/9] AB#34137 more codeQL feedback --- .../TenantAppService.cs | 12 ++++++--- .../Tenants/ConfigurationModal.cshtml | 1 + .../Tenants/ConfigurationModal.cshtml.cs | 1 + .../Tenants/CreateModal.cshtml.cs | 16 +++++++++++ .../TenantManagement/Tenants/EditModal.cshtml | 1 + .../Tenants/EditModal.cshtml.cs | 1 + ...UnityTenantManagementWebMapperlyProfile.cs | 4 +++ .../TenantAppService_Tests.cs | 27 +++++++++++++++++++ .../PostTenantCreationSequenceJob.cs | 13 ++++++--- .../Steps/MetabaseTenantRegistrationStep.cs | 21 ++++++++------- .../Integrations/DynamicUrlDataSeeder.cs | 14 +++++++++- 11 files changed, 93 insertions(+), 18 deletions(-) diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs index d2dc87ebc4..bb645051b7 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs @@ -60,7 +60,7 @@ public virtual async Task> GetListAsync(GetTenantsInpu var extraPropertySortFields = new HashSet(StringComparer.OrdinalIgnoreCase) { - ExtraPropDivision, ExtraPropBranch, ExtraPropDescription, ExtraPropCasClientCode + ExtraPropDisplayName, ExtraPropDivision, ExtraPropBranch, ExtraPropDescription, ExtraPropCasClientCode }; var dbSortFields = new HashSet(StringComparer.OrdinalIgnoreCase) @@ -91,10 +91,13 @@ public virtual async Task> GetListAsync(GetTenantsInpu ); } - // In-memory path: needed when filtering on ExtraProperties or sorting on ExtraProperties - // Keep native name filtering in SQL and only layer ExtraProperties matching on top. + // In-memory path: needed when filtering on ExtraProperties or sorting on ExtraProperties. + // Fetch unfiltered here - the underlying repository's filter only matches against Name, so + // passing it through would exclude an ExtraProperty-only match (e.g. filtering by Division + // text that isn't also in the tenant's Name) before the OR-based filter below ever runs. + // Name + ExtraProperties matching is applied together, in memory, further down. var dbSorting = dbSortFields.Contains(sortField) ? input.Sorting : nameof(Tenant.Name); - var filteredTenants = await tenantRepository.GetListAsync(dbSorting, int.MaxValue, 0, input.Filter); + var filteredTenants = await tenantRepository.GetListAsync(dbSorting, int.MaxValue, 0, filter: null); IEnumerable result = filteredTenants; @@ -104,6 +107,7 @@ public virtual async Task> GetListAsync(GetTenantsInpu var filter = input.Filter.Trim(); result = result.Where(t => (t.Name != null && t.Name.Contains(filter, StringComparison.OrdinalIgnoreCase)) || + MatchesExtraProperty(t, ExtraPropDisplayName, filter) || MatchesExtraProperty(t, ExtraPropDivision, filter) || MatchesExtraProperty(t, ExtraPropBranch, filter) || MatchesExtraProperty(t, ExtraPropDescription, filter) || diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml index 8cea4764ca..11441b601f 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml @@ -65,6 +65,7 @@
+ diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs index 6966884120..1ce3203b66 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs @@ -121,6 +121,7 @@ public class TenantInfoModel : ExtensibleObject, IHasConcurrencyStamp [Display(Name = "DisplayName:TenantName")] public string Name { get; set; } = string.Empty; + public string DisplayName { get; set; } = string.Empty; public string Division { get; set; } = string.Empty; public string Branch { get; set; } = string.Empty; public string Description { get; set; } = string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs index aa59358719..85e09d9058 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs @@ -54,6 +54,22 @@ public virtual async Task OnPostAsync() { ValidateModel(); + // The Features/Metabase tabs are only hidden client-side for non-IT-Admin/Ops callers - + // TenantAppService.CreateAsync itself is reachable by anyone with plain Tenants.Create + // permission (TenantsCreateOrITOps), so a forged POST could otherwise set arbitrary + // FeatureKeys/MetabaseUserEmails. Re-check the same policy server-side and strip these + // privileged fields when it fails, mirroring ConfigurationModalModel's FeaturesJson guard. + var canManageFeatures = (await AuthorizationService + .AuthorizeAsync(User, IdentityConsts.ITAdminOrITOperationsPolicyName)).Succeeded; + + if (!canManageFeatures) + { + Tenant.FeatureKeys = null; + Tenant.MetabaseUserEmails = null; + Tenant.MetabaseNewDefaultUserEmails = null; + Tenant.MetabaseRemovedDefaultUserEmails = null; + } + var input = ObjectMapper.Map(Tenant); await TenantAppService.CreateAsync(input); diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml index 1421d24d89..22926d0357 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml @@ -20,6 +20,7 @@ + diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml.cs index 703aadd6fe..55773d9503 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/EditModal.cshtml.cs @@ -49,6 +49,7 @@ public class TenantInfoModel : ExtensibleObject, IHasConcurrencyStamp [DynamicStringLength(typeof(TenantConsts), nameof(TenantConsts.MaxNameLength))] [Display(Name = "DisplayName:TenantName")] public string Name { get; set; } + public string DisplayName { get; set; } = string.Empty; public string Division { get; set; } = string.Empty; public string Branch { get; set; } = string.Empty; public string Description { get; set; } = string.Empty; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebMapperlyProfile.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebMapperlyProfile.cs index 020c1f0bc0..c09d7676fa 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebMapperlyProfile.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebMapperlyProfile.cs @@ -29,6 +29,7 @@ public override void Map(TenantDto source, EditModalModel.TenantInfoModel destin { destination.Id = source.Id; destination.Name = source.Name; + destination.DisplayName = source.DisplayName; destination.Division = source.Division; destination.Branch = source.Branch; destination.Description = source.Description; @@ -74,6 +75,7 @@ public override TenantUpdateDto Map(EditModalModel.TenantInfoModel source) public override void Map(EditModalModel.TenantInfoModel source, TenantUpdateDto destination) { destination.Name = source.Name; + destination.DisplayName = source.DisplayName; destination.Division = source.Division; destination.Branch = source.Branch; destination.Description = source.Description; @@ -96,6 +98,7 @@ public override void Map(TenantDto source, TenantInfoModel destination) { destination.Id = source.Id; destination.Name = source.Name; + destination.DisplayName = source.DisplayName; destination.Division = source.Division; destination.Branch = source.Branch; destination.Description = source.Description; @@ -117,6 +120,7 @@ public override TenantUpdateDto Map(TenantInfoModel source) public override void Map(TenantInfoModel source, TenantUpdateDto destination) { destination.Name = source.Name; + destination.DisplayName = source.DisplayName; destination.Division = source.Division; destination.Branch = source.Branch; destination.Description = source.Description; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs index 96f8c73743..ebcd99601a 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs @@ -75,6 +75,33 @@ public async Task GetListAsync_Sorted_Descending_By_Name() tenants.FindIndex(t => t.Name == "acme").ShouldBeGreaterThan(tenants.FindIndex(t => t.Name == "volosoft")); } + [Fact] + public async Task GetListAsync_Sorted_By_DisplayName() + { + var acme = UsingDbContext(dbContext => dbContext.Tenants.Single(t => t.Name == "acme")); + var volo = UsingDbContext(dbContext => dbContext.Tenants.Single(t => t.Name == "volosoft")); + + await _tenantAppService.UpdateAsync(acme.Id, new TenantUpdateDto { Name = "acme", DisplayName = "Zeta Corp" }); + await _tenantAppService.UpdateAsync(volo.Id, new TenantUpdateDto { Name = "volosoft", DisplayName = "Alpha Corp" }); + + var result = await _tenantAppService.GetListAsync(new GetTenantsInput { Sorting = "DisplayName ASC" }); + var tenants = result.Items.ToList(); + + tenants.FindIndex(t => t.Name == "volosoft").ShouldBeLessThan(tenants.FindIndex(t => t.Name == "acme")); + } + + [Fact] + public async Task GetListAsync_Filtered_By_DisplayName() + { + var acme = UsingDbContext(dbContext => dbContext.Tenants.Single(t => t.Name == "acme")); + await _tenantAppService.UpdateAsync(acme.Id, new TenantUpdateDto { Name = "acme", DisplayName = "UniqueDisplayNameXyz" }); + + var result = await _tenantAppService.GetListAsync(new GetTenantsInput { Filter = "UniqueDisplayNameXyz" }); + + result.Items.ShouldContain(t => t.Name == "acme"); + result.Items.ShouldNotContain(t => t.Name == "volosoft"); + } + [Fact] public async Task CreateAsync() { diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs index d0e68fdcb6..11ef370f63 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/PostTenantCreationSequenceJob.cs @@ -13,9 +13,16 @@ namespace Unity.GrantManager.Tenants.PostCreation; /// /// Runs the registered steps, one per job execution, in /// ascending order. Each execution re-enqueues itself -/// for the next step, so the sequence is driven entirely by ABP's background job queue - every -/// step is individually durable and retryable. A step whose -/// is false stops the sequence on failure; later steps are not run. +/// for the next step, so the sequence is driven entirely by ABP's background job queue. +/// +/// A step's exception is always caught and logged here rather than rethrown, so ABP's own +/// background-job retry mechanism (which only engages when ExecuteAsync throws) never +/// applies to an individual step - failures are best-effort, not retried automatically. A step +/// whose is true is logged and the sequence +/// moves on to the next step regardless; one whose ContinueOnError is false stops the sequence +/// entirely on failure (later steps do not run). Either way, the failed step itself does not get +/// another automatic attempt - recovering it currently requires manually re-enqueuing a +/// for that step index. /// public class PostTenantCreationSequenceJob( IEnumerable steps, diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs index a3609f6c7c..418b20af9a 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Security.Cryptography; +using System.Text; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -114,8 +116,8 @@ public virtual async Task ExecuteAsync(Guid tenantId) if (userId == null) { logger.LogWarning( - "{Prefix} User '{MaskedEmail}' not found in Metabase for tenant {TenantId} - they must log in via LDAP or be created under Admin > People before they can be added to a group.", - LogPrefix, MaskEmail(email), tenantId); + "{Prefix} User (hash {EmailHash}) not found in Metabase for tenant {TenantId} - they must log in via LDAP or be created under Admin > People before they can be added to a group.", + LogPrefix, HashEmail(email), tenantId); continue; } await metabaseApiClient.AddGroupMemberAsync(groupId, userId.Value); @@ -142,14 +144,13 @@ private async Task> GetUserEmailsAsync(Guid tenantId) .ToList(); } - // Avoids writing a user's full email address to logs (CodeQL: exposure of private - // information) while keeping enough of it for an admin to correlate a "not found" warning - // with a known user. - private static string MaskEmail(string email) - { - var atIndex = email.IndexOf('@', StringComparison.Ordinal); - return atIndex <= 1 ? "***" : string.Concat(email.AsSpan(0, 1), "***", email.AsSpan(atIndex)); - } + // Avoids writing a user's email address to logs (CodeQL: exposure of private information). + // A substring/mask still contains real characters from the source string, so CodeQL's + // dataflow analysis still treats it as the same private data - a one-way hash is what's + // actually recognized as breaking that taint, while still letting an admin correlate repeated + // "not found" warnings against a known list of emails (by hashing candidates themselves). + private static string HashEmail(string email) => + Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(email.Trim().ToLowerInvariant())))[..8]; private static (string Host, int Port, string DbName, string Username, string Password) ParseConnectionString(string connectionString) { diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs index b34a84a2bb..5b68b508a4 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs @@ -52,6 +52,18 @@ private static string GetMatomoUrl() return DynamicUrls.MATOMO_PROD_URL; } + // Unlike Matomo, only dev has a known route baked in here. Test/UAT/prod are deliberately + // left blank - ops sets the real URL once via the Endpoint Management admin page, and + // (unlike Matomo) it's never overwritten afterward: this only ever inserts a row when one + // doesn't already exist, so whatever's in the database always takes precedence. + private static string GetMetabaseUrl() + { + var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? string.Empty; + return string.IsNullOrEmpty(env) || env.StartsWith("dev", StringComparison.OrdinalIgnoreCase) + ? DynamicUrls.METABASE_DEV_URL + : string.Empty; + } + private async Task SeedDynamicUrlAsync() { if (currentTenant == null || currentTenant.Id == null) @@ -73,7 +85,7 @@ private async Task SeedDynamicUrlAsync() new() { KeyName = DynamicUrlKeyNames.ANALYTICS_MATOMO_BASE, Url = GetMatomoUrl(), Description = "Matomo Analytics" }, new() { KeyName = DynamicUrlKeyNames.GITHUB_REPO, Url = DynamicUrls.GITHUB_REPO, Description = "GitHub Repository" }, new() { KeyName = DynamicUrlKeyNames.GITHUB_GRAPHQL, Url = DynamicUrls.GITHUB_GRAPHQL, Description = "GitHub GraphQL Endpoint" }, - new() { KeyName = DynamicUrlKeyNames.METABASE_API_BASE, Url = DynamicUrls.METABASE_DEV_URL, Description = "Metabase Reporting API" }, + new() { KeyName = DynamicUrlKeyNames.METABASE_API_BASE, Url = GetMetabaseUrl(), Description = "Metabase Reporting API" }, new() { KeyName = $"{DynamicUrlKeyNames.DIRECT_MESSAGE_KEY_PREFIX}{messageIndex++}", Url = "", Description = $"Direct message webhook {messageIndex}" }, new() { KeyName = $"{DynamicUrlKeyNames.DIRECT_MESSAGE_KEY_PREFIX}{messageIndex++}", Url = "", Description = $"Direct message webhook {messageIndex}" }, new() { KeyName = $"{DynamicUrlKeyNames.DIRECT_MESSAGE_KEY_PREFIX}{messageIndex++}", Url = "", Description = $"Direct message webhook {messageIndex}" }, From 6f57be61363a8860cba2f88fa44fb172ffc00d47 Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Tue, 25 Aug 2026 10:23:39 -0700 Subject: [PATCH 6/9] AB#34137 merge the reporting roles section into tenant --- .../ITenantViewRoleAppService.cs | 13 +- .../Configuration/TenantViewRoleDto.cs | 36 ++- .../Configuration/TenantViewRoleAppService.cs | 103 ++++++-- .../Menus/ReportingMenuContributor.cs | 46 ---- .../Menus/ReportingMenus.cs | 17 -- .../ReportingAdmin/Configuration/Index.cshtml | 97 -------- .../Configuration/Index.cshtml.cs | 67 ------ .../ReportingAdmin/Configuration/Index.css | 219 ------------------ .../ReportingAdmin/Configuration/Index.js | 180 -------------- .../Unity.Reporting.Web/ReportingWebModule.cs | 13 +- .../client-proxies/tenant-view-role-proxy.js | 38 --- .../TenantViewRoleAppServiceTests.cs | 212 +++++++++++++++++ .../Tenants/ConfigurationModal.cshtml | 93 ++++++++ .../Tenants/ConfigurationModal.cshtml.cs | 19 +- .../Pages/TenantManagement/Tenants/Index.js | 98 ++++++++ .../Unity.TenantManagement.Web.csproj | 1 + .../UnityTenantManagementWebModule.cs | 2 + 17 files changed, 549 insertions(+), 705 deletions(-) delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenuContributor.cs delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenus.cs delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml.cs delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.css delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.js delete mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/wwwroot/client-proxies/tenant-view-role-proxy.js create mode 100644 applications/Unity.GrantManager/modules/Unity.Reporting/test/Unity.Reporting.Application.Tests/Configuration/TenantViewRoleAppServiceTests.cs diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs index 443b6731cd..43f11ef2b9 100644 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Threading.Tasks; namespace Unity.Reporting.Configuration @@ -12,14 +11,16 @@ namespace Unity.Reporting.Configuration public interface ITenantViewRoleAppService { /// - /// Retrieves the view role configuration for all tenants in the system. - /// Returns both explicitly configured roles and default inferred roles based on tenant naming patterns. + /// Retrieves the view role configuration for a specific tenant. + /// Returns either the explicitly configured role or a default inferred role based on the tenant's name. /// + /// The unique identifier of the tenant to retrieve the view role configuration for. /// - /// A list of objects containing the tenant information and their associated view roles. - /// Default roles follow the pattern {tenantname}_readonly when not explicitly configured. + /// A containing the tenant information and its associated view role. + /// Defaults to the {tenantname}_readonly pattern when not explicitly configured. /// - Task> GetAllAsync(); + /// Thrown when the specified tenant is not found. + Task GetAsync(Guid tenantId); /// /// Updates the view role configuration for a specific tenant. diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/TenantViewRoleDto.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/TenantViewRoleDto.cs index 2012ef9dc6..02272962c9 100644 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/TenantViewRoleDto.cs +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/TenantViewRoleDto.cs @@ -20,15 +20,45 @@ public class TenantViewRoleDto /// /// Gets or sets the database role name that will be granted SELECT permissions on reporting views for this tenant. - /// Defaults to {tenantname}_readonly if not explicitly configured. + /// Defaults to the tenant's {LicencePlate}_readonly role when a license plate is on record + /// (the role automatically provisioned for new tenants), falling back to the legacy + /// {tenantname}_readonly pattern for older tenants with no license plate, unless a role has + /// been explicitly saved for this tenant. /// public string ViewRole { get; set; } = string.Empty; /// /// Gets or sets a value indicating whether the current ViewRole value is an inferred default /// that has not been explicitly saved to the database. When true, indicates the role name - /// follows the default pattern (e.g., {tenantname}_readonly) and requires explicit saving - /// to persist as a tenant-specific setting. + /// follows a default pattern (see ) and requires explicit saving to + /// persist as a tenant-specific setting. /// public bool IsDefaultInferred { get; set; } + + /// + /// Gets or sets the tenant's license plate (its database name, e.g. "T_ABC123"), used since + /// tenant provisioning to name the tenant's two automatically-created database roles: the + /// license plate itself (read-write) and {LicencePlate}_readonly. Null for tenants that + /// predate this convention. + /// + public string? LicencePlate { get; set; } + + /// + /// Gets or sets the {LicencePlate}_readonly role name expected to exist for this tenant. Null + /// when is null. + /// + public string? ExpectedReadOnlyRole { get; set; } + + /// + /// Gets or sets whether actually exists as a role in the + /// tenant's database, checked live. Always false when is null. + /// + public bool ReadOnlyRoleExists { get; set; } + + /// + /// Gets or sets whether the tenant's main (read-write) role - named after + /// - actually exists in the tenant's database, checked live. + /// Always false when is null. + /// + public bool MainRoleExists { get; set; } } \ No newline at end of file diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application/Configuration/TenantViewRoleAppService.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application/Configuration/TenantViewRoleAppService.cs index e434779455..dc1a5a3ef8 100644 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application/Configuration/TenantViewRoleAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application/Configuration/TenantViewRoleAppService.cs @@ -7,6 +7,7 @@ using Unity.Reporting.BackgroundJobs; using Unity.Reporting.Domain.Configuration; using Unity.Reporting.Settings; +using Volo.Abp; using Volo.Abp.Application.Services; using Volo.Abp.BackgroundJobs; using Volo.Abp.MultiTenancy; @@ -31,37 +32,68 @@ public class TenantViewRoleAppService( IReportColumnsMapRepository reportColumnsMapRepository, ICurrentTenant currentTenant) : ApplicationService, ITenantViewRoleAppService { + private const string LicencePlateExtraPropertyKey = "LicencePlate"; + /// - /// Retrieves all tenant view role configurations. - /// Returns a list of all tenants with their current view role assignments, defaulting to {tenantname}_readonly if not configured. + /// Retrieves the view role configuration for a specific tenant, along with the live existence + /// state of the two roles automatically provisioned for the tenant's license plate (its + /// read-write role and {LicencePlate}_readonly). /// - public async Task> GetAllAsync() + public async Task GetAsync(Guid tenantId) { - var tenants = await tenantRepository.GetListAsync(); - var tenantViewRoles = new List(); + var tenant = await tenantRepository.GetAsync(tenantId); + + var licencePlate = tenant.ExtraProperties.TryGetValue(LicencePlateExtraPropertyKey, out var lp) + ? lp?.ToString() + : null; + var expectedReadOnlyRole = string.IsNullOrWhiteSpace(licencePlate) ? null : $"{licencePlate}_readonly"; + + // An explicitly saved role always wins (covers legacy tenants whose role doesn't follow + // either naming convention). Otherwise prefer the license-plate readonly role - the one + // actually provisioned for new tenants - falling back to the legacy {tenantname}_readonly + // pattern only when there's no license plate on record at all. + var savedViewRole = await settingManager.GetOrNullAsync(ReportingSettings.TenantViewRole, "T", tenant.Id.ToString()); - foreach (var tenant in tenants) + string viewRole; + bool isDefaultInferred; + if (!string.IsNullOrEmpty(savedViewRole)) + { + viewRole = savedViewRole; + isDefaultInferred = false; + } + else if (expectedReadOnlyRole != null) + { + viewRole = expectedReadOnlyRole; + isDefaultInferred = true; + } + else { - // Get tenant-specific setting first, fallback to default pattern - var viewRole = await settingManager.GetOrNullAsync(ReportingSettings.TenantViewRole, "T", tenant.Id.ToString()); + viewRole = $"{tenant.Name.ToLowerInvariant()}_readonly"; + isDefaultInferred = true; + } - bool isDefaultInferred = false; - if (string.IsNullOrEmpty(viewRole)) + bool readOnlyRoleExists = false; + bool mainRoleExists = false; + if (!string.IsNullOrWhiteSpace(licencePlate)) + { + using (currentTenant.Change(tenantId)) { - viewRole = $"{tenant.Name.ToLowerInvariant()}_readonly"; - isDefaultInferred = true; + readOnlyRoleExists = await reportColumnsMapRepository.RoleExistsAsync(expectedReadOnlyRole!); + mainRoleExists = await reportColumnsMapRepository.RoleExistsAsync(licencePlate); } - - tenantViewRoles.Add(new TenantViewRoleDto - { - TenantId = tenant.Id, - TenantName = tenant.Name, - ViewRole = viewRole, - IsDefaultInferred = isDefaultInferred - }); } - return tenantViewRoles; + return new TenantViewRoleDto + { + TenantId = tenant.Id, + TenantName = tenant.Name, + ViewRole = viewRole, + IsDefaultInferred = isDefaultInferred, + LicencePlate = licencePlate, + ExpectedReadOnlyRole = expectedReadOnlyRole, + ReadOnlyRoleExists = readOnlyRoleExists, + MainRoleExists = mainRoleExists + }; } /// @@ -72,6 +104,8 @@ public async Task UpdateAsync(Guid tenantId, UpdateTenantView { var tenant = await tenantRepository.GetAsync(tenantId); + await EnsureRoleExistsAsync(tenantId, input.ViewRole); + await settingManager.SetAsync(ReportingSettings.TenantViewRole, input.ViewRole, "T", tenantId.ToString()); return new TenantViewRoleDto @@ -91,6 +125,17 @@ public async Task AssignRoleToViewsAsync(Guid tenantId) { Logger.LogInformation("Starting role assignment for tenant: {TenantId}", tenantId); + var role = await settingManager.GetOrNullAsync(ReportingSettings.TenantViewRole, "T", tenantId.ToString()); + if (string.IsNullOrWhiteSpace(role)) + { + throw new UserFriendlyException("No view role is configured for this tenant yet. Save a role first."); + } + + // Fail fast here rather than relying solely on AssignViewRoleBackgroundJob's own check - + // that check only logs a warning and silently no-ops, so without this the queue call + // always reports success even for a role that doesn't exist in the tenant's database. + await EnsureRoleExistsAsync(tenantId, role); + var jobArgs = new AssignViewRoleBackgroundJobArgs { TenantId = tenantId @@ -100,6 +145,22 @@ public async Task AssignRoleToViewsAsync(Guid tenantId) Logger.LogInformation("Queued role assignment job for tenant: {TenantId}", tenantId); } + /// + /// Throws a if the given role does not exist as a real + /// PostgreSQL role in the tenant's own database. + /// + private async Task EnsureRoleExistsAsync(Guid tenantId, string role) + { + using (currentTenant.Change(tenantId)) + { + if (!await reportColumnsMapRepository.RoleExistsAsync(role)) + { + throw new UserFriendlyException( + $"Role '{role}' does not exist in this tenant's database. Create the role first, or choose an existing one from View DB Info."); + } + } + } + /// /// Retrieves database information for a specific tenant, including available roles and reporting views. /// This method queries the tenant's database to return a comprehensive list of database roles diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenuContributor.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenuContributor.cs deleted file mode 100644 index 0819c74972..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenuContributor.cs +++ /dev/null @@ -1,46 +0,0 @@ -using System.Threading.Tasks; -using Unity.Modules.Shared.Navigation; -using Unity.Modules.Shared.Permissions; -using Volo.Abp.UI.Navigation; - -namespace Unity.Reporting.Web.Menus; - -/// -/// ABP Framework menu contributor for the Unity.Reporting module navigation system. -/// Responsible for adding reporting-related menu items to the application's main navigation, -/// including administrative pages for reconciliation and reporting configuration management. -/// All menu items require IT Admin permissions for security and proper access control. -/// -public class ReportingMenuContributor : IMenuContributor -{ - /// - /// Configures the application menu by adding Unity.Reporting module menu items. - /// Delegates to the private method to set up reporting-specific navigation items - /// with appropriate permissions and routing configuration. - /// - /// The menu configuration context containing the menu to be configured. - /// A task representing the asynchronous menu configuration operation. - public async Task ConfigureMenuAsync(MenuConfigurationContext context) - { - await ConfigureReportingMenuAsync(context); - } - - /// - /// Configures reporting-specific menu items including reconciliation and reporting configuration pages. - /// Adds navigation items with IT Admin permission requirements and proper routing to ensure - /// administrative functionality is accessible only to authorized users with appropriate permissions. - /// - /// The menu configuration context for adding reporting menu items. - /// A completed task representing the synchronous menu item addition operations. - private static async Task ConfigureReportingMenuAsync(MenuConfigurationContext context) - { - // Add Reporting Configuration menu item for IT Admin users - await context.AddItemAsync( - new ApplicationMenuItem( - ReportingMenus.Prefix, - displayName: "Reporting", - "~/ReportingAdmin/Configuration") - .OnlyWhenInRole(IdentityConsts.ITAdminRoleName) - ); - } -} diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenus.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenus.cs deleted file mode 100644 index 5e55024442..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Menus/ReportingMenus.cs +++ /dev/null @@ -1,17 +0,0 @@ -namespace Unity.Reporting.Web.Menus; - -/// -/// Static class containing menu identifier constants for the Unity.Reporting web navigation system. -/// Defines standardized menu prefixes and identifiers used throughout the reporting module's -/// navigation infrastructure to ensure consistent menu organization and avoid naming conflicts -/// with other modules in the Unity application ecosystem. -/// -public static class ReportingMenus -{ - /// - /// The base prefix for all Unity.Reporting menu items and navigation elements. - /// Used to namespace reporting-related menu items and ensure they don't conflict - /// with menu items from other modules in the application. - /// - public const string Prefix = "Reporting"; -} diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml deleted file mode 100644 index dd0b637f99..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml +++ /dev/null @@ -1,97 +0,0 @@ -@page -@model Unity.Reporting.Web.Pages.ReportingAdmin.IndexModel -@{ - ViewData["Title"] = "Reporting Configuration"; -} - -@section styles -{ - -} -@section scripts -{ - -} - -
-
-

Reporting Configuration

-
-
- -
-
Tenant View Role Management
-

Configure tenant-specific database roles for reporting views. Each tenant can have a custom role, or will default to {tenantname}_readonly if not specified.

- -
- - - - - - - - - - @foreach (var tenantRole in Model.TenantViewRoles) - { - - - - - - } - -
Tenant NameView RoleActions
@tenantRole.TenantName -
- - @if (tenantRole.IsDefaultInferred) - { - - } -
- 		-
- - - -
-
-
- - @if (!Model.TenantViewRoles.Any()) - { -
-

No tenants found.

-
- } -
-
-
- diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml.cs deleted file mode 100644 index f8fcf1ee3e..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.cshtml.cs +++ /dev/null @@ -1,67 +0,0 @@ -using Microsoft.AspNetCore.Authorization; -using Microsoft.AspNetCore.Mvc; -using System; -using System.Collections.Generic; -using System.Threading.Tasks; -using Unity.Modules.Shared.Permissions; -using Unity.Reporting.Configuration; - -namespace Unity.Reporting.Web.Pages.ReportingAdmin -{ - /// - /// Razor Page model for the Reporting Administration configuration interface. - /// Provides functionality for IT administrators to manage tenant-specific reporting settings including - /// database role configuration for view access control per tenant. Displays tenant view role configurations - /// in a management table interface for easy administration. - /// Requires IT Admin permissions for all operations to ensure secure configuration management. - /// - [Authorize(IdentityConsts.ITAdminPermissionName)] - public class IndexModel : ReportingPageModel - { - private readonly ITenantViewRoleAppService _tenantViewRoleAppService; - - /// - /// Gets or sets the list of tenant view role configurations for display in the DataTable. - /// Contains all tenants with their current view role assignments for the management interface. - /// - public List TenantViewRoles { get; set; } = new(); - - /// - /// Initializes a new instance of the IndexModel with required dependency injection services. - /// Sets up the tenant view role service for managing per-tenant configurations. - /// - /// The application service for tenant-specific view role management. - public IndexModel(ITenantViewRoleAppService tenantViewRoleAppService) - { - _tenantViewRoleAppService = tenantViewRoleAppService; - } - - /// - /// Handles GET requests to display the reporting configuration page with current settings. - /// Loads all tenant view role configurations for display in the management interface. - /// - /// A task representing the asynchronous page loading operation. - public async Task OnGetAsync() - { - TenantViewRoles = await _tenantViewRoleAppService.GetAllAsync(); - } - - /// - /// AJAX handler to get tenant database information including roles and views. - /// - /// The tenant ID to get database information for. - /// JSON result containing tenant database information. - public async Task OnGetTenantDatabaseInfoAsync(Guid tenantId) - { - try - { - var databaseInfo = await _tenantViewRoleAppService.GetTenantDatabaseInfoAsync(tenantId); - return new JsonResult(new { success = true, data = databaseInfo }); - } - catch (Exception ex) - { - return new JsonResult(new { success = false, error = ex.Message }); - } - } - } -} diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.css b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.css deleted file mode 100644 index df3a7d6839..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.css +++ /dev/null @@ -1,219 +0,0 @@ -.run-sync-jobs { - padding: 1rem; -} - -.reporting-configuration-section { - background-color: #f8f9fa; - border: 1px solid #dee2e6; - border-radius: 0.375rem; - padding: 1.5rem; - margin-bottom: 1.5rem; -} - -.reporting-configuration-section h5 { - color: #495057; - border-bottom: 1px solid #dee2e6; - padding-bottom: 0.5rem; - margin-bottom: 1rem; -} - -.unity-page-titlebar { - border-bottom: 1px solid #dee2e6; - margin-bottom: 1rem; - padding-bottom: 1rem; -} - -.d-flex { - display: flex; -} - -.gap-2 { - gap: 0.5rem; -} - -/* Tenant View Role Management Styles */ -.view-role-input { - width: 100%; - padding: 0.375rem 0.75rem; - font-size: 1rem; - line-height: 1.5; - border: 1px solid #ced4da; - border-radius: 0.25rem; - min-width: 200px; - max-width: 300px; - transition: border-color 0.15s ease-in-out, box-shadow 0.15s ease-in-out; -} - -.view-role-input:focus { - border-color: #80bdff; - outline: 0; - box-shadow: 0 0 0 0.2rem rgba(0, 123, 255, 0.25); -} - -.action-buttons-cell .btn { - min-width: 80px; - padding: 0.5rem 1rem; - font-weight: 500; - transition: all 0.15s ease-in-out; -} - -.default-role-indicator { - flex-shrink: 0; - cursor: help; - font-size: 1rem; -} - -.default-role-indicator:hover { - color: #ff6b35 !important; -} - -.view-role-cell .d-flex { - align-items: center; -} - -/* Table header styling - match the user config pattern */ -#TenantViewRoleTable thead th { - background-color: #55698A !important; - color: white !important; - font-weight: 500 !important; - font-size: 18px !important; - text-align: left !important; - border: none; -} - -#TenantViewRoleTable thead th:hover { - background-color: #3E4C63 !important; - color: white !important; -} - -/* Sort arrow styling for headers */ -#TenantViewRoleTable table.dataTable thead th.sorting:before, -#TenantViewRoleTable table.dataTable thead th.sorting:after, -#TenantViewRoleTable table.dataTable thead th.sorting_asc:before, -#TenantViewRoleTable table.dataTable thead th.sorting_asc:after, -#TenantViewRoleTable table.dataTable thead th.sorting_desc:before, -#TenantViewRoleTable table.dataTable thead th.sorting_desc:after { - color: white !important; -} - -#TenantViewRoleTable { - margin-top: 1rem; -} - -#TenantViewRoleTable tbody tr:hover { - background-color: #f1f3f5; -} - -/* DataTables wrapper styling - match general pattern */ -.dataTables_wrapper .dataTables_filter { - margin-bottom: 1rem; -} - -.dataTables_wrapper .dataTables_filter input { - width: 100%; - margin-left: 0.5rem; - border-radius: 0.25rem; - border: 1px solid #ced4da; - padding: 0.375rem 0.75rem; - font-size: 1rem; - line-height: 1.5; - transition: border-color 0.15s ease-in-out, box-shadow 0.15s ease-in-out; -} - -.dataTables_wrapper .dataTables_filter input:focus { - border-color: #80bdff; - outline: 0; - box-shadow: 0 0 0 0.2rem rgba(0, 123, 255, 0.25); -} - -.table-responsive { - border-radius: 0.375rem; - border: 1px solid #dee2e6; -} - -/* Tooltip styling */ -.tooltip { - font-size: 0.875rem; -} - -.tooltip-inner { - max-width: 300px; - padding: 0.5rem 0.75rem; - background-color: #343a40; - border-radius: 0.375rem; -} - -/* Action bar styling - add consistency */ -.action-bar { - display: flex; - justify-content: space-between; - align-items: center; - gap: 1rem; - margin-bottom: 1rem; -} - -.filter-search-action-bar_search-wrapper { - flex: 1; - max-width: 400px; -} - -.tbl-search { - width: 100%; -} - -/* Responsive Design */ -@media (max-width: 768px) { - .d-flex.gap-2 { - flex-direction: column; - gap: 0.5rem !important; - } - - .action-buttons-cell .btn { - min-width: auto; - width: 100%; - padding: 0.375rem 0.75rem; - font-size: 0.875rem; - } - - .view-role-input { - min-width: auto; - max-width: none; - width: 100%; - } - - .view-role-cell .d-flex { - flex-direction: column; - align-items: flex-start; - gap: 0.5rem; - } - - .default-role-indicator { - align-self: flex-end; - } - - .action-bar { - flex-direction: column; - gap: 0.5rem; - } - - .action-bar .btn { - width: 100%; - } - - /* Table header responsive adjustments */ - #TenantViewRoleTable thead th { - font-size: 16px !important; - } -} - -@media (max-width: 576px) { - .view-role-input { - font-size: 0.875rem; - padding: 0.25rem 0.5rem; - } - - #TenantViewRoleTable thead th { - font-size: 14px !important; - padding: 0.5rem 0.25rem; - } -} \ No newline at end of file diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.js b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.js deleted file mode 100644 index 7abdccfd0c..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/Pages/ReportingAdmin/Configuration/Index.js +++ /dev/null @@ -1,180 +0,0 @@ -$(function () { - let _tenantViewRoleAppService = unity.reporting.configuration.tenantViewRole; - let _databaseInfoModal = new abp.ModalManager({ - viewUrl: abp.appPath + 'ReportingAdmin/Configuration/DatabaseInfoModal' - }); - - // Initialize tooltips for default role indicators - function initializeTooltips() { - $('[data-bs-toggle="tooltip"]').tooltip(); - } - - // Initialize DataTable for tenant view role management - let tenantViewRoleTable = $('#TenantViewRoleTable').DataTable({ - order: [[0, 'asc']], // Sort by tenant name - processing: false, - serverSide: false, - paging: true, - searching: true, - pageLength: 25, - autoWidth: false, - scrollY: 'calc(100vh - 325px)', - scrollCollapse: true, - columnDefs: [ - { - targets: [2], // Actions column - orderable: false - } - ], - dom: 'frtip', // Show filter, table, info, pagination - language: { - emptyTable: "No tenants found", - search: "Search tenants:", - lengthMenu: "Show _MENU_ tenants per page", - info: "Showing _START_ to _END_ of _TOTAL_ tenants" - }, - drawCallback: function() { - // Reinitialize tooltips after table redraws - initializeTooltips(); - } - }); - - // Keep the scroll body sized so the header/pagination stay within the viewport - // instead of the table overflowing past the bottom of the screen (same plugin - // used by initializeDataTable's fixedHeaders option elsewhere in the app). - // Stashed on the settings object, matching table-utils.js's own usage of the plugin. - if ($.fn.dataTable.ScrollResize) { - tenantViewRoleTable.settings()[0]._scrollResize = new $.fn.dataTable.ScrollResize(tenantViewRoleTable); - } - - // Initialize tooltips on page load - initializeTooltips(); - - // Handle save role button click - $(document).on('click', '.save-role-btn', function () { - const button = $(this); - const tenantId = button.data('tenant-id'); - const row = button.closest('tr'); - const viewRoleInput = row.find('.view-role-input'); - const viewRole = viewRoleInput.val().trim(); - - if (!viewRole) { - abp.notify.warn('Please enter a view role name.'); - return; - } - - button.prop('disabled', true).html(' Saving...'); - - _tenantViewRoleAppService.update(tenantId, { viewRole: viewRole }) - .done(function (_) { - // Remove the default indicator since it's now saved - const indicator = row.find('.default-role-indicator'); - if (indicator.length) { - indicator.tooltip('dispose'); - indicator.remove(); - } - - // Update the data attribute - viewRoleInput.attr('data-is-default', 'false'); - - abp.notify.success('View role saved successfully.'); - }) - .fail(function () { - abp.notify.error('Failed to save view role.'); - }) - .always(function () { - button.prop('disabled', false).html(' Save'); - }); - }); - - // Handle assign role to views button click - $(document).on('click', '.assign-role-btn', function () { - const button = $(this); - const tenantId = button.data('tenant-id'); - const tenantName = button.data('tenant-name'); - const row = button.closest('tr'); - const viewRoleInput = row.find('.view-role-input'); - const viewRole = viewRoleInput.val().trim(); - const isDefault = viewRoleInput.attr('data-is-default') === 'true'; - - if (!viewRole) { - abp.notify.warn('Please enter a view role name before assigning it to views.'); - return; - } - - // Check if role needs to be saved first - if (isDefault) { - abp.message.confirm( - `The role "${viewRole}" is using the default pattern and hasn't been saved yet. Would you like to save it first and then assign it to views?`, - 'Save and Assign Role', - function (isConfirmed) { - if (isConfirmed) { - // Save first, then assign - saveAndAssignRole(tenantId, tenantName, viewRole, button, row); - } - } - ); - } else { - // Role is already saved, proceed with assignment - assignRoleToViews(tenantId, tenantName, viewRole, button); - } - }); - - // Handle view database info button click - $(document).on('click', '.view-database-info-btn', function () { - const button = $(this); - const tenantId = button.data('tenant-id'); - const tenantName = button.data('tenant-name'); - - _databaseInfoModal.open({ - tenantId: tenantId, - tenantName: tenantName - }); - }); - - // Function to save role and then assign to views - function saveAndAssignRole(tenantId, tenantName, viewRole, button, row) { - button.prop('disabled', true).html(' Saving & Assigning...'); - - _tenantViewRoleAppService.update(tenantId, { viewRole: viewRole }) - .done(function (result) { - // Remove the default indicator - const indicator = row.find('.default-role-indicator'); - if (indicator.length) { - indicator.tooltip('dispose'); - indicator.remove(); - } - - // Update the data attribute - row.find('.view-role-input').attr('data-is-default', 'false'); - - // Now assign to views - assignRoleToViews(tenantId, tenantName, viewRole, button); - }) - .fail(function () { - abp.notify.error('Failed to save view role.'); - button.prop('disabled', false).html(' Assign to Views'); - }); - } - - // Function to assign role to views - function assignRoleToViews(tenantId, tenantName, viewRole, button) { - button.prop('disabled', true).html(' Assigning...'); - - _tenantViewRoleAppService.assignRoleToViews(tenantId) - .done(function () { - abp.notify.success(`Role assignment jobs have been queued for tenant "${tenantName}". The process will complete in the background.`); - }) - .fail(function () { - abp.notify.error('Failed to queue role assignment jobs.'); - }) - .always(function () { - button.prop('disabled', false).html(' Assign to Views'); - }); - } - - // Add refresh functionality - window.refreshTenantTable = function() { - window.location.reload(); - }; -}); diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/ReportingWebModule.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/ReportingWebModule.cs index 34b15c6436..3d8bcbe338 100644 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/ReportingWebModule.cs +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/ReportingWebModule.cs @@ -1,12 +1,10 @@ using Microsoft.Extensions.DependencyInjection; using Unity.Reporting.Localization; -using Unity.Reporting.Web.Menus; using Volo.Abp.AspNetCore.Mvc.Localization; using Volo.Abp.AspNetCore.Mvc.UI.Theme.Shared; using Volo.Abp.Mapperly; using Volo.Abp.Modularity; using Volo.Abp.SettingManagement.Web; -using Volo.Abp.UI.Navigation; using Volo.Abp.VirtualFileSystem; namespace Unity.Reporting.Web; @@ -45,18 +43,13 @@ public override void PreConfigureServices(ServiceConfigurationContext context) } /// - /// Configures main services for the Unity.Reporting Web module including navigation, virtual file system, and AutoMapper. - /// Registers the ReportingMenuContributor for navigation menu setup, configures embedded virtual file system resources - /// for CSS/JS assets and views, and sets up AutoMapper object mapping with validation for web-layer data transformations. + /// Configures main services for the Unity.Reporting Web module including virtual file system and AutoMapper. + /// Configures embedded virtual file system resources for CSS/JS assets and views, and sets up AutoMapper + /// object mapping with validation for web-layer data transformations. /// /// The service configuration context for dependency injection and module configuration. public override void ConfigureServices(ServiceConfigurationContext context) { - Configure(options => - { - options.MenuContributors.Add(new ReportingMenuContributor()); - }); - Configure(options => { options.FileSets.AddEmbedded(); diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/wwwroot/client-proxies/tenant-view-role-proxy.js b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/wwwroot/client-proxies/tenant-view-role-proxy.js deleted file mode 100644 index 844a584771..0000000000 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Web/wwwroot/client-proxies/tenant-view-role-proxy.js +++ /dev/null @@ -1,38 +0,0 @@ -/* This file is automatically generated by ABP framework to use MVC Controllers from javascript. */ - -// module reporting - -(function(){ - - // controller unity.reporting.tenantViewRole - - (function(){ - - abp.utils.createNamespace(window, 'unity.reporting.tenantViewRole'); - - unity.reporting.tenantViewRole.getAll = function(ajaxParams) { - return abp.ajax($.extend(true, { - url: abp.appPath + 'api/reporting/tenant-view-roles', - type: 'GET' - }, ajaxParams)); - }; - - unity.reporting.tenantViewRole.update = function(tenantId, input, ajaxParams) { - return abp.ajax($.extend(true, { - url: abp.appPath + 'api/reporting/tenant-view-roles/' + tenantId + '', - type: 'PUT', - data: JSON.stringify(input) - }, ajaxParams)); - }; - - unity.reporting.tenantViewRole.assignRoleToViews = function(tenantId, ajaxParams) { - return abp.ajax($.extend(true, { - url: abp.appPath + 'api/reporting/tenant-view-roles/' + tenantId + '/assign-role-to-views', - type: 'POST', - dataType: null - }, ajaxParams)); - }; - - })(); - -})(); \ No newline at end of file diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/test/Unity.Reporting.Application.Tests/Configuration/TenantViewRoleAppServiceTests.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/test/Unity.Reporting.Application.Tests/Configuration/TenantViewRoleAppServiceTests.cs new file mode 100644 index 0000000000..00a0e984de --- /dev/null +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/test/Unity.Reporting.Application.Tests/Configuration/TenantViewRoleAppServiceTests.cs @@ -0,0 +1,212 @@ +using Microsoft.Extensions.Logging; +using NSubstitute; +using Shouldly; +using System; +using System.Reflection; +using System.Threading.Tasks; +using Unity.Reporting.Configuration; +using Unity.Reporting.Domain.Configuration; +using Volo.Abp; +using Volo.Abp.BackgroundJobs; +using Volo.Abp.DependencyInjection; +using Volo.Abp.MultiTenancy; +using Volo.Abp.SettingManagement; +using Volo.Abp.TenantManagement; +using Xunit; +using Xunit.Abstractions; + +namespace Unity.Reporting.Application.Tests.Configuration; + +public class TenantViewRoleAppServiceTests : ReportingApplicationTestBase +{ + private const string SettingName = "GrantManager.Reporting.TenantViewRole"; + + private readonly ITenantRepository _tenantRepository; + private readonly ISettingManager _settingManager; + private readonly IBackgroundJobManager _backgroundJobManager; + private readonly IReportColumnsMapRepository _reportColumnsMapRepository; + private readonly ICurrentTenant _currentTenant; + private readonly TenantViewRoleAppService _service; + + public TenantViewRoleAppServiceTests(ITestOutputHelper outputHelper) : base(outputHelper) + { + _tenantRepository = Substitute.For(); + _settingManager = Substitute.For(); + _backgroundJobManager = Substitute.For(); + _reportColumnsMapRepository = Substitute.For(); + _currentTenant = Substitute.For(); + _currentTenant.Change(Arg.Any()).Returns(Substitute.For()); + + _service = new TenantViewRoleAppService( + _tenantRepository, _settingManager, _backgroundJobManager, _reportColumnsMapRepository, _currentTenant); + + SetupServicePropertiesForTesting(_service, Substitute.For>()); + } + + // Tenant's constructors are all non-public (ABP requires going through ITenantManager to + // create one) - reflection is the standard workaround for exercising it in a plain unit test. + private static Tenant CreateTenant(string name) + { + var ctor = typeof(Tenant).GetConstructor( + BindingFlags.NonPublic | BindingFlags.Instance, + null, [typeof(Guid), typeof(string), typeof(string)], null)!; + return (Tenant)ctor.Invoke([Guid.NewGuid(), name, name.ToUpperInvariant()]); + } + + // Mirrors ReportMappingServiceTests' approach: TenantViewRoleAppService is constructed + // directly (not resolved via DI) so tests don't need a real Postgres-backed EF Core context - + // its Logger property (from ApplicationService's LazyServiceProvider) is wired up manually. + private static void SetupServicePropertiesForTesting(object service, ILogger logger) + { + var mockLazyServiceProvider = Substitute.For(); + mockLazyServiceProvider.LazyGetService(Arg.Any>()) + .Returns(logger); + + var lazyServiceProviderProperty = typeof(TenantViewRoleAppService) + .GetProperty("LazyServiceProvider", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance); + + if (lazyServiceProviderProperty == null) + { + var currentType = typeof(TenantViewRoleAppService).BaseType; + while (currentType != null && lazyServiceProviderProperty == null) + { + lazyServiceProviderProperty = currentType.GetProperty("LazyServiceProvider", + BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly); + currentType = currentType.BaseType; + } + } + + if (lazyServiceProviderProperty != null && lazyServiceProviderProperty.CanWrite) + { + lazyServiceProviderProperty.SetValue(service, mockLazyServiceProvider); + } + else + { + throw new InvalidOperationException("Could not find or set LazyServiceProvider property"); + } + } + + [Fact] + public async Task GetAsync_TenantHasLicencePlate_NoSavedRole_DefaultsToLicencePlateReadOnlyRole() + { + var tenant = CreateTenant("acme"); + tenant.ExtraProperties["LicencePlate"] = "T_ABC123"; + _tenantRepository.GetAsync(tenant.Id).Returns(tenant); + _settingManager.GetOrNullAsync(SettingName, "T", tenant.Id.ToString()).Returns((string?)null); + _reportColumnsMapRepository.RoleExistsAsync("T_ABC123_readonly").Returns(true); + _reportColumnsMapRepository.RoleExistsAsync("T_ABC123").Returns(false); + + var result = await _service.GetAsync(tenant.Id); + + result.LicencePlate.ShouldBe("T_ABC123"); + result.ExpectedReadOnlyRole.ShouldBe("T_ABC123_readonly"); + result.ViewRole.ShouldBe("T_ABC123_readonly"); + result.IsDefaultInferred.ShouldBeTrue(); + result.ReadOnlyRoleExists.ShouldBeTrue(); + result.MainRoleExists.ShouldBeFalse(); + } + + [Fact] + public async Task GetAsync_TenantHasNoLicencePlate_FallsBackToLegacyTenantNamePattern() + { + var tenant = CreateTenant("acme"); + _tenantRepository.GetAsync(tenant.Id).Returns(tenant); + _settingManager.GetOrNullAsync(SettingName, "T", tenant.Id.ToString()).Returns((string?)null); + + var result = await _service.GetAsync(tenant.Id); + + result.LicencePlate.ShouldBeNull(); + result.ExpectedReadOnlyRole.ShouldBeNull(); + result.ViewRole.ShouldBe("acme_readonly"); + result.IsDefaultInferred.ShouldBeTrue(); + result.ReadOnlyRoleExists.ShouldBeFalse(); + result.MainRoleExists.ShouldBeFalse(); + // No license plate - existence isn't checked against anything, so the repository should + // never be queried for this tenant. + await _reportColumnsMapRepository.DidNotReceiveWithAnyArgs().RoleExistsAsync(default!); + } + + [Fact] + public async Task GetAsync_SavedRoleExists_TakesPrecedenceOverLicencePlateDefault() + { + var tenant = CreateTenant("acme"); + tenant.ExtraProperties["LicencePlate"] = "T_ABC123"; + _tenantRepository.GetAsync(tenant.Id).Returns(tenant); + _settingManager.GetOrNullAsync(SettingName, "T", tenant.Id.ToString()).Returns("legacy_custom_role"); + _reportColumnsMapRepository.RoleExistsAsync(Arg.Any()).Returns(true); + + var result = await _service.GetAsync(tenant.Id); + + result.ViewRole.ShouldBe("legacy_custom_role"); + result.IsDefaultInferred.ShouldBeFalse(); + // The license-plate role info is still surfaced alongside the saved (legacy) role. + result.LicencePlate.ShouldBe("T_ABC123"); + result.ExpectedReadOnlyRole.ShouldBe("T_ABC123_readonly"); + } + + [Fact] + public async Task UpdateAsync_RoleDoesNotExist_ThrowsUserFriendlyException() + { + var tenant = CreateTenant("acme"); + _tenantRepository.GetAsync(tenant.Id).Returns(tenant); + _reportColumnsMapRepository.RoleExistsAsync("nonexistent_role").Returns(false); + + await Should.ThrowAsync( + () => _service.UpdateAsync(tenant.Id, new UpdateTenantViewRoleDto { ViewRole = "nonexistent_role" })); + + await _settingManager.DidNotReceive().SetAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task UpdateAsync_RoleExists_PersistsSetting() + { + var tenant = CreateTenant("acme"); + _tenantRepository.GetAsync(tenant.Id).Returns(tenant); + _reportColumnsMapRepository.RoleExistsAsync("acme_readonly").Returns(true); + + var result = await _service.UpdateAsync(tenant.Id, new UpdateTenantViewRoleDto { ViewRole = "acme_readonly" }); + + result.ViewRole.ShouldBe("acme_readonly"); + result.IsDefaultInferred.ShouldBeFalse(); + await _settingManager.Received(1).SetAsync( + SettingName, "acme_readonly", "T", tenant.Id.ToString(), Arg.Any()); + } + + [Fact] + public async Task AssignRoleToViewsAsync_NoRoleConfigured_ThrowsUserFriendlyException() + { + var tenantId = Guid.NewGuid(); + _settingManager.GetOrNullAsync(SettingName, "T", tenantId.ToString()).Returns((string?)null); + + await Should.ThrowAsync(() => _service.AssignRoleToViewsAsync(tenantId)); + + await _backgroundJobManager.DidNotReceiveWithAnyArgs().EnqueueAsync(default!); + } + + [Fact] + public async Task AssignRoleToViewsAsync_RoleDoesNotExist_ThrowsUserFriendlyException() + { + var tenantId = Guid.NewGuid(); + _settingManager.GetOrNullAsync(SettingName, "T", tenantId.ToString()).Returns("ghost_role"); + _reportColumnsMapRepository.RoleExistsAsync("ghost_role").Returns(false); + + await Should.ThrowAsync(() => _service.AssignRoleToViewsAsync(tenantId)); + + await _backgroundJobManager.DidNotReceiveWithAnyArgs().EnqueueAsync(default!); + } + + [Fact] + public async Task AssignRoleToViewsAsync_RoleExists_EnqueuesJob() + { + var tenantId = Guid.NewGuid(); + _settingManager.GetOrNullAsync(SettingName, "T", tenantId.ToString()).Returns("acme_readonly"); + _reportColumnsMapRepository.RoleExistsAsync("acme_readonly").Returns(true); + + await _service.AssignRoleToViewsAsync(tenantId); + + await _backgroundJobManager.Received(1).EnqueueAsync( + Arg.Is(a => a.TenantId == tenantId), + Arg.Any(), Arg.Any()); + } +} diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml index 11441b601f..e5f8a3d03c 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml @@ -59,6 +59,16 @@ } + @if (Model.CanManageReporting) + { +
  • + +
    • + }
    @@ -185,6 +195,89 @@
    } + @if (Model.CanManageReporting) + { +
    +

    + Configure the database role granted SELECT access to this tenant's reporting views. +

    + + @if (!string.IsNullOrEmpty(Model.ViewRole?.LicencePlate)) + { +
    +
    License Plate: @Model.ViewRole.LicencePlate
    +
    + Main role @Model.ViewRole.LicencePlate: + @if (Model.ViewRole.MainRoleExists) + { + Found + } + else + { + Not found + } +
    +
    + Readonly role @Model.ViewRole.ExpectedReadOnlyRole: + @if (Model.ViewRole.ReadOnlyRoleExists) + { + Found + } + else + { + Not found + } +
    +
    + } + else + { +

    + No license plate on record for this tenant (a legacy tenant, created before + automatic role provisioning) - defaulting to + @($"{Model.Tenant.Name.ToLowerInvariant()}_readonly") unless a role + has been explicitly saved below. +

    + } + +
    + +
    + + @if (Model.ViewRole?.IsDefaultInferred == true) + { + + } +
    +
    + +
    + + + +
    +
    + } + diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs index 1ce3203b66..8ab08b458d 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/ConfigurationModal.cshtml.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Mvc; using Unity.GrantManager.Integrations; using Unity.Modules.Shared.Permissions; +using Unity.Reporting.Configuration; using Volo.Abp.Domain.Entities; using Volo.Abp.FeatureManagement; using Volo.Abp.Features; @@ -21,7 +22,8 @@ namespace Unity.TenantManagement.Web.Pages.TenantManagement.Tenants; public class ConfigurationModalModel( ITenantAppService tenantAppService, ICasClientCodeLookupService lookupService, - IFeatureAppService featureAppService) : TenantManagementPageModel + IFeatureAppService featureAppService, + ITenantViewRoleAppService tenantViewRoleAppService) : TenantManagementPageModel { [BindProperty] public TenantInfoModel Tenant { get; set; } = null!; @@ -45,6 +47,10 @@ public class ConfigurationModalModel( public bool CanManageManagers { get; set; } + public bool CanManageReporting { get; set; } + + public TenantViewRoleDto? ViewRole { get; set; } + public virtual async Task OnGetAsync(Guid id) { var tenantDto = await tenantAppService.GetAsync(id); @@ -61,11 +67,22 @@ public virtual async Task OnGetAsync(Guid id) CanManageManagers = CanManageFeatures; + // Stricter than CanManageFeatures - this mirrors the reporting database-role admin page's + // original ITAdministrator-only gating (IdentityConsts.ITAdminPermissionName on + // TenantViewRoleAppService itself), not the broader ITAdminOrITOperations used above. + CanManageReporting = (await AuthorizationService + .AuthorizeAsync(User, IdentityConsts.ITAdminPolicyName)).Succeeded; + if (CanManageConnectionStrings) { ConnectionStrings = await tenantAppService.GetConnectionStringsAsync(id); } + if (CanManageReporting) + { + ViewRole = await tenantViewRoleAppService.GetAsync(id); + } + return Page(); } diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js index 1344561c3a..c35e7a4c14 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/Index.js @@ -17,6 +17,10 @@ } ); + let _reportingDatabaseInfoModal = new abp.ModalManager({ + viewUrl: abp.appPath + 'ReportingAdmin/Configuration/DatabaseInfoModal' + }); + let _dataTable = null; // ─── Actions column renderer ────────────────────────────────────────────── @@ -461,10 +465,104 @@ $('#create-features-json').val(featureKeys.join(',')); } + // ─── Configuration modal: Reporting tab (view role) ─────────────────────── + + let _tenantViewRoleAppService = unity.reporting.configuration.tenantViewRole; + + function _saveReportingViewRole(tenantId, onSaved) { + let $btn = $('#config-save-role-btn'); + let viewRole = $('#config-view-role-input').val().trim(); + + if (!viewRole) { + abp.notify.warn('Please enter a view role name.'); + return; + } + + $btn.prop('disabled', true).html(' Saving...'); + + _tenantViewRoleAppService.update(tenantId, { viewRole: viewRole }) + .done(function () { + let $indicator = $('#pane-reporting .default-role-indicator'); + if ($indicator.length) { + $indicator.tooltip('dispose'); + $indicator.remove(); + } + $('#config-view-role-input').attr('data-is-default', 'false'); + + abp.notify.success('View role saved successfully.'); + if (onSaved) onSaved(viewRole); + }) + .fail(function () { + abp.notify.error('Failed to save view role.'); + }) + .always(function () { + $btn.prop('disabled', false).html(' Save'); + }); + } + + function _assignReportingRoleToViews(tenantId, tenantName, viewRole) { + let $btn = $('#config-assign-role-btn'); + $btn.prop('disabled', true).html(' Assigning...'); + + _tenantViewRoleAppService.assignRoleToViews(tenantId) + .done(function () { + abp.notify.success('Role assignment jobs have been queued for tenant "' + tenantName + '". The process will complete in the background.'); + }) + .fail(function () { + abp.notify.error('Failed to queue role assignment jobs.'); + }) + .always(function () { + $btn.prop('disabled', false).html(' Assign to Views'); + }); + } + + function _wireReportingTabHandlers(tenantId) { + $('#config-save-role-btn').off('click').on('click', function () { + _saveReportingViewRole(tenantId); + }); + + $('#config-assign-role-btn').off('click').on('click', function () { + let $btn = $(this); + let tenantName = $btn.data('tenant-name'); + let viewRole = $('#config-view-role-input').val().trim(); + let isDefault = $('#config-view-role-input').attr('data-is-default') === 'true'; + + if (!viewRole) { + abp.notify.warn('Please enter a view role name before assigning it to views.'); + return; + } + + if (isDefault) { + abp.message.confirm( + 'The role "' + viewRole + '" is using the default pattern and hasn\'t been saved yet. Would you like to save it first and then assign it to views?', + 'Save and Assign Role', + function (isConfirmed) { + if (isConfirmed) { + _saveReportingViewRole(tenantId, function (savedViewRole) { + _assignReportingRoleToViews(tenantId, tenantName, savedViewRole); + }); + } + } + ); + } else { + _assignReportingRoleToViews(tenantId, tenantName, viewRole); + } + }); + + $('#config-view-database-info-btn').off('click').on('click', function () { + let $btn = $(this); + _reportingDatabaseInfoModal.open({ + tenantId: tenantId, + tenantName: $btn.data('tenant-name') + }); + }); + } + function _configurationModalInitModal(publicApi, args) { _configTenantId = args.id; _loadManagersTab(_configTenantId); + _wireReportingTabHandlers(_configTenantId); _configFilterDataTable = $('#ConfigUserSearchTable').DataTable( abp.libs.datatables.normalizeConfiguration({ diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Unity.TenantManagement.Web.csproj b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Unity.TenantManagement.Web.csproj index 8b033bd1ce..f10327ca73 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Unity.TenantManagement.Web.csproj +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Unity.TenantManagement.Web.csproj @@ -30,6 +30,7 @@ + diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebModule.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebModule.cs index f7bffd53c2..72bc7e970f 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebModule.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/UnityTenantManagementWebModule.cs @@ -15,6 +15,7 @@ using Volo.Abp.VirtualFileSystem; using Volo.Abp.Threading; using Unity.Modules.Shared.Permissions; +using Unity.Reporting; using Unity.TenantManagement.Web.Navigation; namespace Unity.TenantManagement.Web; @@ -23,6 +24,7 @@ namespace Unity.TenantManagement.Web; [DependsOn(typeof(AbpAspNetCoreMvcUiBootstrapModule))] [DependsOn(typeof(AbpFeatureManagementWebModule))] [DependsOn(typeof(AbpMapperlyModule))] +[DependsOn(typeof(ReportingApplicationContractsModule))] public class UnityTenantManagementWebModule : AbpModule { private static readonly OneTimeRunner OneTimeRunner = new(); From 8c5dc94d40b83a390f32df621864d0768f270e56 Mon Sep 17 00:00:00 2001 From: Andre Goncalves <98196495+AndreGAot@users.noreply.github.com> Date: Tue, 25 Aug 2026 13:03:42 -0700 Subject: [PATCH 7/9] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../Configuration/ITenantViewRoleAppService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs index 43f11ef2b9..a0ac9914e4 100644 --- a/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.Reporting/src/Unity.Reporting.Application.Contracts/Configuration/ITenantViewRoleAppService.cs @@ -17,7 +17,7 @@ public interface ITenantViewRoleAppService /// The unique identifier of the tenant to retrieve the view role configuration for. /// /// A containing the tenant information and its associated view role. - /// Defaults to the {tenantname}_readonly pattern when not explicitly configured. + /// Defaults to {LicencePlate}_readonly when a licence plate exists, falling back to {tenantname}_readonly for legacy tenants. /// /// Thrown when the specified tenant is not found. Task GetAsync(Guid tenantId); From 1e2a35e75391ac21d52cd3eff326ba6e3017b271 Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Tue, 25 Aug 2026 13:09:41 -0700 Subject: [PATCH 8/9] AB#34137 address copilot comments --- .../TenantAppService.cs | 56 +++++++--- .../Tenants/CreateModal.cshtml.cs | 8 +- .../TenantAppService_Tests.cs | 35 ++++++ .../Handlers/TenantCreatedEventHandler.cs | 22 ++-- .../Metabase/IMetabaseApiClient.cs | 11 +- .../Metabase/MetabaseApiClient.cs | 71 ++++++++++-- .../Steps/MetabaseTenantRegistrationStep.cs | 22 ++-- .../TenantCreatedEventHandlerTests.cs | 56 ++++++++++ .../Metabase/MetabaseApiClientTests.cs | 104 ++++++++++++++++++ .../MetabaseTenantRegistrationStepTests.cs | 22 ++-- 10 files changed, 350 insertions(+), 57 deletions(-) diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs index bb645051b7..9f2163c3c4 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Application/TenantAppService.cs @@ -5,6 +5,7 @@ using System.Security.Cryptography; using System.Threading.Tasks; using Microsoft.AspNetCore.Authorization; +using Unity.Modules.Shared.Permissions; using Unity.TenantManagement.Abstractions; using Unity.TenantManagement.Application; using Unity.TenantManagement.Application.Contracts; @@ -147,6 +148,16 @@ private static string GetExtraPropertyValue(Tenant tenant, string key) [Authorize(TenantManagementPermissions.Policies.TenantsCreateOrITOps)] public virtual async Task CreateAsync(TenantCreateDto input) { + // TenantsCreateOrITOps also grants access to any caller holding the plain Tenants.Create + // permission, not just IT Admin/Operations. CreateModalModel.OnPostAsync already strips + // FeatureKeys/MetabaseUserEmails for non-admin callers, but that only guards the OOTB + // Razor Page - a caller can still invoke this application service directly (its own + // dynamic API controller) and set them. Re-check the stricter policy here so a forged call + // can't enable arbitrary features or - more importantly - grant arbitrary email addresses + // Metabase access to the new tenant's database via the post-creation registration step. + StripPrivilegedFieldsUnlessAuthorized( + input, await AuthorizationService.IsGrantedAsync(IdentityConsts.ITAdminOrITOperationsPolicyName)); + Tenant? tenant = null; using (var uow = unitOfWorkManager.Begin(true, false)) @@ -186,23 +197,42 @@ public virtual async Task CreateAsync(TenantCreateDto input) await uow.CompleteAsync(); } - await localEventBus.PublishAsync( - new TenantCreatedEto - { - Id = tenant.Id, - Name = tenant.Name, - Properties = - { - { "UserIdentifier", input.UserIdentifier }, - { "FeatureKeys", input.FeatureKeys ?? string.Empty }, - { "MetabaseUserEmails", input.MetabaseUserEmails ?? string.Empty } - } - } - ); + var tenantCreatedEto = new TenantCreatedEto + { + Id = tenant.Id, + Name = tenant.Name, + Properties = + { + { "UserIdentifier", input.UserIdentifier }, + { "FeatureKeys", input.FeatureKeys ?? string.Empty } + } + }; + + // Distinguish "field omitted" (input.MetabaseUserEmails is null - an older/API caller + // that never set it) from "explicitly cleared" (empty string - a deliberate "no Metabase + // users for this tenant" choice) - only the latter should be persisted as an override. + // TenantCreatedEventHandler falls back to the Global default when the property is absent. + if (input.MetabaseUserEmails != null) + { + tenantCreatedEto.Properties["MetabaseUserEmails"] = input.MetabaseUserEmails; + } + + await localEventBus.PublishAsync(tenantCreatedEto); return ObjectMapper.Map(tenant); } + // Extracted so the stripping decision itself is unit-testable without driving ABP's + // authorization pipeline through an integration test host. + internal static void StripPrivilegedFieldsUnlessAuthorized(TenantCreateDto input, bool callerIsAuthorized) + { + if (!callerIsAuthorized) + { + input.FeatureKeys = null; + input.MetabaseUserEmails = null; + } + } + [Authorize(TenantManagementPermissions.Policies.TenantsUpdateOrITOps)] public virtual async Task UpdateAsync(Guid id, TenantUpdateDto input) { diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs index 85e09d9058..55f8851a38 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/src/Unity.TenantManagement.Web/Pages/TenantManagement/Tenants/CreateModal.cshtml.cs @@ -54,11 +54,11 @@ public virtual async Task OnPostAsync() { ValidateModel(); - // The Features/Metabase tabs are only hidden client-side for non-IT-Admin/Ops callers - + // The Features/Metabase tabs are only hidden client-side for non-IT-Admin/Ops callers, and // TenantAppService.CreateAsync itself is reachable by anyone with plain Tenants.Create - // permission (TenantsCreateOrITOps), so a forged POST could otherwise set arbitrary - // FeatureKeys/MetabaseUserEmails. Re-check the same policy server-side and strip these - // privileged fields when it fails, mirroring ConfigurationModalModel's FeaturesJson guard. + // permission (TenantsCreateOrITOps) - it now re-checks this same policy and strips these + // privileged fields itself, so this is defense in depth (a clean 4xx-free UX for this + // page), not the only guard. Mirrors ConfigurationModalModel's FeaturesJson guard. var canManageFeatures = (await AuthorizationService .AuthorizeAsync(User, IdentityConsts.ITAdminOrITOperationsPolicyName)).Succeeded; diff --git a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs index ebcd99601a..cfe7bd2210 100644 --- a/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs +++ b/applications/Unity.GrantManager/modules/Unity.TenantManagement/test/Unity.TenantManagement.Application.Tests/TenantAppService_Tests.cs @@ -125,6 +125,41 @@ await Assert.ThrowsAsync(async () => }); } + [Fact] + public void StripPrivilegedFieldsUnlessAuthorized_CallerNotAuthorized_ClearsFeatureKeysAndMetabaseUserEmails() + { + // A caller with only the plain Tenants.Create permission (not IT Admin/Operations) must + // not be able to enable arbitrary features or grant arbitrary email addresses Metabase + // access to a tenant's database via the post-creation registration step. + var input = new TenantCreateDto + { + Name = "acme2", + FeatureKeys = "Unity.Payments", + MetabaseUserEmails = "someone@gov.bc.ca" + }; + + TenantAppService.StripPrivilegedFieldsUnlessAuthorized(input, callerIsAuthorized: false); + + input.FeatureKeys.ShouldBeNull(); + input.MetabaseUserEmails.ShouldBeNull(); + } + + [Fact] + public void StripPrivilegedFieldsUnlessAuthorized_CallerAuthorized_PreservesFeatureKeysAndMetabaseUserEmails() + { + var input = new TenantCreateDto + { + Name = "acme2", + FeatureKeys = "Unity.Payments", + MetabaseUserEmails = "someone@gov.bc.ca" + }; + + TenantAppService.StripPrivilegedFieldsUnlessAuthorized(input, callerIsAuthorized: true); + + input.FeatureKeys.ShouldBe("Unity.Payments"); + input.MetabaseUserEmails.ShouldBe("someone@gov.bc.ca"); + } + [Fact] public async Task UpdateAsync() { diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs index 670bfe8302..a8bf788eae 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Handlers/TenantCreatedEventHandler.cs @@ -76,17 +76,25 @@ await _backgroundJobManager.EnqueueAsync(new PostTenantCreationStepArgs // Global default changes in the meantime. private async Task SaveMetabaseUserEmailsAsync(TenantCreatedEto eto, Guid tenantId) { - // An empty string is a deliberate "no Metabase users for this tenant" choice - it must - // still be persisted (not skipped), otherwise MetabaseTenantRegistrationStep's - // GetUserEmailsAsync falls back to the Global default list and grants users the - // tenant creator explicitly unchecked. - if (!eto.Properties.TryGetValue("MetabaseUserEmails", out var emailsRaw)) - return; + var emails = await ResolveMetabaseUserEmailsAsync( + eto.Properties, () => _settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails)); await _settingManager.SetAsync( - MetabaseSettings.UserEmails, emailsRaw ?? string.Empty, TenantSettingValueProvider.ProviderName, tenantId.ToString()); + MetabaseSettings.UserEmails, emails, TenantSettingValueProvider.ProviderName, tenantId.ToString()); } + // TenantAppService.CreateAsync only adds "MetabaseUserEmails" to eto.Properties when the + // caller explicitly set it (even to an empty string - a deliberate "no Metabase users for + // this tenant" choice, which must still be persisted as-is). When the property is absent - + // an older/API caller that never set it - snapshot the *current* Global default here + // rather than leaving the tenant setting unset, so the step reads a stable list even if the + // Global default changes before it (async, queued) actually runs. + internal static async Task ResolveMetabaseUserEmailsAsync( + IReadOnlyDictionary etoProperties, Func> getGlobalDefaultAsync) => + etoProperties.TryGetValue("MetabaseUserEmails", out var emailsRaw) + ? emailsRaw + : await getGlobalDefaultAsync() ?? string.Empty; + private async Task EnableRequestedFeaturesAsync(TenantCreatedEto eto, Guid tenantId) { if (!eto.Properties.TryGetValue("FeatureKeys", out var featureKeysRaw)) diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs index aa1d760237..7886949ebc 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/IMetabaseApiClient.cs @@ -8,16 +8,21 @@ namespace Unity.GrantManager.Integrations.Metabase; /// manual_deploy_new_metabase_tenant.ps1 runbook: create a database connection for the tenant's /// readonly Postgres role, create a permissions group and add members to it, grant the group /// access to the database, and create/grant a collection. +/// +/// The FindOrCreate* methods and are idempotent - they +/// look for an existing database/group/collection/membership by its natural key (name, or +/// group+user) before creating one, so the caller can safely retry after a partial failure +/// (e.g. re-enqueuing MetabaseTenantRegistrationStep) without duplicating resources. /// public interface IMetabaseApiClient { - Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default); + Task FindOrCreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default); Task SyncDatabaseSchemaAsync(int databaseId, CancellationToken cancellationToken = default); Task RescanDatabaseValuesAsync(int databaseId, CancellationToken cancellationToken = default); - Task CreateGroupAsync(string name, CancellationToken cancellationToken = default); + Task FindOrCreateGroupAsync(string name, CancellationToken cancellationToken = default); Task FindUserIdByEmailAsync(string email, CancellationToken cancellationToken = default); Task AddGroupMemberAsync(int groupId, int userId, CancellationToken cancellationToken = default); Task GrantGroupDatabaseAccessAsync(int groupId, int databaseId, CancellationToken cancellationToken = default); - Task CreateCollectionAsync(string name, CancellationToken cancellationToken = default); + Task FindOrCreateCollectionAsync(string name, CancellationToken cancellationToken = default); Task GrantGroupCollectionAccessAsync(int groupId, int collectionId, CancellationToken cancellationToken = default); } diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs index d315a138ab..f4ceaa8e19 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Integrations/Metabase/MetabaseApiClient.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Linq; using System.Net; using System.Net.Http; @@ -25,8 +26,14 @@ public class MetabaseApiClient( // rather than surfacing a transient conflict as a permanent failure. private const int MaxGraphUpdateAttempts = 3; - public async Task CreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default) + public async Task FindOrCreateDatabaseAsync(string name, string host, int port, string dbName, string username, string password, bool ssl, CancellationToken cancellationToken = default) { + var existingId = await FindIdByNameAsync("/api/database", name, cancellationToken); + if (existingId != null) + { + return existingId.Value; + } + var body = new { engine = "postgres", @@ -44,8 +51,14 @@ public Task SyncDatabaseSchemaAsync(int databaseId, CancellationToken cancellati public Task RescanDatabaseValuesAsync(int databaseId, CancellationToken cancellationToken = default) => PostAsync($"/api/database/{databaseId}/rescan_values", new { }, cancellationToken); - public async Task CreateGroupAsync(string name, CancellationToken cancellationToken = default) + public async Task FindOrCreateGroupAsync(string name, CancellationToken cancellationToken = default) { + var existingId = await FindIdByNameAsync("/api/permissions/group", name, cancellationToken); + if (existingId != null) + { + return existingId.Value; + } + var result = await PostAsync("/api/permissions/group", new { name }, cancellationToken); return result.Value("id"); } @@ -58,8 +71,24 @@ public async Task CreateGroupAsync(string name, CancellationToken cancellat return match?.Value("id"); } - public Task AddGroupMemberAsync(int groupId, int userId, CancellationToken cancellationToken = default) => - PostAsync("/api/permissions/membership", new { group_id = groupId, user_id = userId }, cancellationToken); + public async Task AddGroupMemberAsync(int groupId, int userId, CancellationToken cancellationToken = default) + { + // A rerun after a partial failure must not re-POST an existing membership - Metabase + // treats that as a conflict rather than a no-op. + if (await IsGroupMemberAsync(groupId, userId, cancellationToken)) + { + return; + } + + await PostAsync("/api/permissions/membership", new { group_id = groupId, user_id = userId }, cancellationToken); + } + + private async Task IsGroupMemberAsync(int groupId, int userId, CancellationToken cancellationToken) + { + var memberships = await GetAsync("/api/permissions/membership", cancellationToken); + var groupMembers = memberships[groupId.ToString(CultureInfo.InvariantCulture)] as JArray; + return groupMembers?.Any(m => m.Value("user_id") == userId) ?? false; + } public Task GrantGroupDatabaseAccessAsync(int groupId, int databaseId, CancellationToken cancellationToken = default) => UpdateGraphWithRetryAsync("/api/permissions/graph", groups => @@ -75,8 +104,14 @@ public Task GrantGroupDatabaseAccessAsync(int groupId, int databaseId, Cancellat groups[groupKey] = groupNode; }, cancellationToken); - public async Task CreateCollectionAsync(string name, CancellationToken cancellationToken = default) + public async Task FindOrCreateCollectionAsync(string name, CancellationToken cancellationToken = default) { + var existingId = await FindIdByNameAsync("/api/collection", name, cancellationToken); + if (existingId != null) + { + return existingId.Value; + } + var result = await PostAsync("/api/collection", new { name, color = "#509EE3" }, cancellationToken); return result.Value("id"); } @@ -124,12 +159,25 @@ private async Task GetBaseUrlAsync() => private IReadOnlyDictionary BuildHeaders() => new Dictionary { [ApiKeyHeader] = options.Value.ApiKey }; - private async Task GetAsync(string path, CancellationToken cancellationToken) + private async Task GetRawAsync(string path, CancellationToken cancellationToken) { var baseUrl = await GetBaseUrlAsync(); - var response = await resilientHttpRequest.HttpAsync( + return await resilientHttpRequest.HttpAsync( HttpMethod.Get, $"{baseUrl}{path}", extraHeaders: BuildHeaders(), cancellationToken: cancellationToken); - return await ReadJsonAsync(response, path); + } + + private async Task GetAsync(string path, CancellationToken cancellationToken) => + await ReadJsonAsync(await GetRawAsync(path, cancellationToken), path); + + // Metabase's list endpoints are inconsistent about pagination - /api/database wraps results in + // {"data": [...]}, while /api/permissions/group and /api/collection return a raw JSON array. + // Handling both shapes here keeps the find-or-create callers simple. + private async Task FindIdByNameAsync(string listPath, string name, CancellationToken cancellationToken) + { + var root = await ReadJsonTokenAsync(await GetRawAsync(listPath, cancellationToken), listPath); + var items = root as JArray ?? root["data"] as JArray ?? new JArray(); + var match = items.FirstOrDefault(item => string.Equals(item.Value("name"), name, StringComparison.Ordinal)); + return match?.Value("id"); } private async Task PostAsync(string path, object body, CancellationToken cancellationToken) @@ -147,7 +195,10 @@ private async Task PutRawAsync(string path, object body, Ca HttpMethod.Put, $"{baseUrl}{path}", body, extraHeaders: BuildHeaders(), cancellationToken: cancellationToken); } - private static async Task ReadJsonAsync(HttpResponseMessage response, string path) + private static async Task ReadJsonAsync(HttpResponseMessage response, string path) => + (JObject)await ReadJsonTokenAsync(response, path); + + private static async Task ReadJsonTokenAsync(HttpResponseMessage response, string path) { var content = response.Content == null ? string.Empty : await response.Content.ReadAsStringAsync(); @@ -157,6 +208,6 @@ private static async Task ReadJsonAsync(HttpResponseMessage response, s $"Metabase API call to '{path}' failed with status {response.StatusCode}: {content}"); } - return string.IsNullOrWhiteSpace(content) ? new JObject() : JObject.Parse(content); + return string.IsNullOrWhiteSpace(content) ? new JObject() : JToken.Parse(content); } } diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs index 418b20af9a..eb3ab6839c 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Application/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStep.cs @@ -26,14 +26,15 @@ namespace Unity.GrantManager.Tenants.PostCreation.Steps; /// EntityFrameworkCoreGrantManagerDbSchemaMigrator provisions them automatically at tenant creation): /// /// 1. Connects the tenant's data as a read-only source - decrypts the tenant's stored -/// Tenant_Readonly connection string and calls the Metabase API to create a database +/// Tenant_Readonly connection string and calls the Metabase API to find or create a database /// connection named after the tenant, then triggers a schema sync + value rescan. -/// 2. Creates a Metabase permissions group named after the tenant and adds the configured member -/// emails to it (see ). A user who isn't already a Metabase -/// user (via LDAP login or Admin > People) is skipped with a warning, not a hard failure. +/// 2. Finds or creates a Metabase permissions group named after the tenant and adds the +/// configured member emails to it (see ). A user who isn't +/// already a Metabase user (via LDAP login or Admin > People) is skipped with a warning, not +/// a hard failure. /// 3. Grants that group unrestricted view/query access to the new database connection, scoped to /// just this tenant's data. -/// 4. Creates a Metabase collection for the tenant and grants the group write access to it. +/// 4. Finds or creates a Metabase collection for the tenant and grants the group write access to it. /// /// The member email list comes from ABP Settings: a Global "default" list (editable via the New /// Tenant modal's Metabase tab) plus any ad-hoc emails added just for this tenant. The resolved @@ -42,7 +43,10 @@ namespace Unity.GrantManager.Tenants.PostCreation.Steps; /// changes before this (async, queued) step actually runs. /// /// is true - a Metabase outage is logged but doesn't block tenant -/// creation or later post-creation steps. +/// creation or later post-creation steps. Recovery from a partial failure is a manual re-enqueue +/// of this step (see PostTenantCreationSequenceJob) - every Metabase call this step makes +/// (via ) is idempotent by design (find-or-create by tenant name, +/// membership/permission checks before writing) so a rerun is always safe. /// [RemoteService(false)] [ExposeServices(typeof(IPostTenantCreationStep))] @@ -104,11 +108,11 @@ public virtual async Task ExecuteAsync(Guid tenantId) var ssl = metabaseOptions.Value.DbSslOverride ?? true; - var databaseId = await metabaseApiClient.CreateDatabaseAsync(tenant.Name, host, port, dbName, username, password, ssl); + var databaseId = await metabaseApiClient.FindOrCreateDatabaseAsync(tenant.Name, host, port, dbName, username, password, ssl); await metabaseApiClient.SyncDatabaseSchemaAsync(databaseId); await metabaseApiClient.RescanDatabaseValuesAsync(databaseId); - var groupId = await metabaseApiClient.CreateGroupAsync(tenant.Name); + var groupId = await metabaseApiClient.FindOrCreateGroupAsync(tenant.Name); foreach (var email in await GetUserEmailsAsync(tenantId)) { @@ -125,7 +129,7 @@ public virtual async Task ExecuteAsync(Guid tenantId) await metabaseApiClient.GrantGroupDatabaseAccessAsync(groupId, databaseId); - var collectionId = await metabaseApiClient.CreateCollectionAsync(tenant.Name); + var collectionId = await metabaseApiClient.FindOrCreateCollectionAsync(tenant.Name); await metabaseApiClient.GrantGroupCollectionAccessAsync(groupId, collectionId); logger.LogInformation( diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Handlers/TenantCreatedEventHandlerTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Handlers/TenantCreatedEventHandlerTests.cs index c47f6fe57c..4ed5b04830 100644 --- a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Handlers/TenantCreatedEventHandlerTests.cs +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Handlers/TenantCreatedEventHandlerTests.cs @@ -1,3 +1,5 @@ +using System.Collections.Generic; +using System.Threading.Tasks; using Shouldly; using Xunit; @@ -5,6 +7,60 @@ namespace Unity.GrantManager.Handlers; public class TenantCreatedEventHandlerTests { + [Fact] + public async Task ResolveMetabaseUserEmailsAsync_PropertyOmitted_SnapshotsCurrentGlobalDefault() + { + var etoProperties = new Dictionary(); + + var result = await TenantCreatedEventHandler.ResolveMetabaseUserEmailsAsync( + etoProperties, () => Task.FromResult("global1@gov.bc.ca,global2@gov.bc.ca")); + + result.ShouldBe("global1@gov.bc.ca,global2@gov.bc.ca"); + } + + [Fact] + public async Task ResolveMetabaseUserEmailsAsync_PropertyOmittedAndNoGlobalDefaultSet_ReturnsEmpty() + { + var etoProperties = new Dictionary(); + + var result = await TenantCreatedEventHandler.ResolveMetabaseUserEmailsAsync( + etoProperties, () => Task.FromResult(null)); + + result.ShouldBe(string.Empty); + } + + [Fact] + public async Task ResolveMetabaseUserEmailsAsync_PropertyExplicitlyEmpty_ReturnsEmptyWithoutFallingBackToGlobal() + { + var etoProperties = new Dictionary { ["MetabaseUserEmails"] = string.Empty }; + var globalDefaultLookedUp = false; + + var result = await TenantCreatedEventHandler.ResolveMetabaseUserEmailsAsync(etoProperties, () => + { + globalDefaultLookedUp = true; + return Task.FromResult("should-not-be-used@gov.bc.ca"); + }); + + result.ShouldBe(string.Empty); + globalDefaultLookedUp.ShouldBeFalse(); + } + + [Fact] + public async Task ResolveMetabaseUserEmailsAsync_PropertyExplicitlySet_ReturnsThatValueWithoutFallingBackToGlobal() + { + var etoProperties = new Dictionary { ["MetabaseUserEmails"] = "tenant-specific@gov.bc.ca" }; + var globalDefaultLookedUp = false; + + var result = await TenantCreatedEventHandler.ResolveMetabaseUserEmailsAsync(etoProperties, () => + { + globalDefaultLookedUp = true; + return Task.FromResult("should-not-be-used@gov.bc.ca"); + }); + + result.ShouldBe("tenant-specific@gov.bc.ca"); + globalDefaultLookedUp.ShouldBeFalse(); + } + [Fact] public void BuildFeatureUpdates_NullInput_ReturnsEmpty() { diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs index ad2589ef05..27b8cba6b6 100644 --- a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Integrations/Metabase/MetabaseApiClientTests.cs @@ -1,3 +1,4 @@ +using System; using System.Net; using System.Net.Http; using System.Text; @@ -122,4 +123,107 @@ public async Task GrantGroupCollectionAccessAsync_StaleRevisionOnFirstPut_Refetc await Should.NotThrowAsync(() => client.GrantGroupCollectionAccessAsync(groupId: 5, collectionId: 22)); } + + // /api/database wraps its list in {"data": [...]}. + [Fact] + public async Task FindOrCreateDatabaseAsync_DatabaseWithSameNameAlreadyExists_ReturnsExistingIdWithoutCreating() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"data\":[{\"id\":11,\"name\":\"AG-MARB\"}]}")); + + var databaseId = await client.FindOrCreateDatabaseAsync( + "AG-MARB", "host", 5432, "db", "user", "pass", ssl: true); + + databaseId.ShouldBe(11); + await http.Received(1).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task FindOrCreateDatabaseAsync_NoDatabaseWithThatName_CreatesNewDatabase() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"data\":[]}"), + JsonResponse(HttpStatusCode.OK, "{\"id\":12}")); + + var databaseId = await client.FindOrCreateDatabaseAsync( + "AG-MARB", "host", 5432, "db", "user", "pass", ssl: true); + + databaseId.ShouldBe(12); + await http.Received(1).HttpAsync( + HttpMethod.Post, Arg.Is(url => url != null && url.EndsWith("/api/database", StringComparison.Ordinal)), + Arg.Any(), Arg.Any(), Arg.Any<(string username, string password)?>(), + Arg.Any(), Arg.Any?>(), + Arg.Any()); + } + + // /api/permissions/group returns a raw JSON array, unlike /api/database. + [Fact] + public async Task FindOrCreateGroupAsync_GroupWithSameNameAlreadyExists_ReturnsExistingIdWithoutCreating() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "[{\"id\":7,\"name\":\"AG-MARB\"}]")); + + var groupId = await client.FindOrCreateGroupAsync("AG-MARB"); + + groupId.ShouldBe(7); + await http.Received(1).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + // /api/collection also returns a raw JSON array. + [Fact] + public async Task FindOrCreateCollectionAsync_CollectionWithSameNameAlreadyExists_ReturnsExistingIdWithoutCreating() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "[{\"id\":33,\"name\":\"AG-MARB\"}]")); + + var collectionId = await client.FindOrCreateCollectionAsync("AG-MARB"); + + collectionId.ShouldBe(33); + await http.Received(1).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task AddGroupMemberAsync_UserAlreadyAMember_DoesNotPostMembershipAgain() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"5\":[{\"user_id\":101,\"membership_id\":1}]}")); + + await client.AddGroupMemberAsync(groupId: 5, userId: 101); + + await http.Received(1).HttpAsync( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), + Arg.Any<(string username, string password)?>(), Arg.Any(), + Arg.Any?>(), Arg.Any()); + } + + [Fact] + public async Task AddGroupMemberAsync_UserNotYetAMember_PostsMembership() + { + var (client, http) = CreateClient(); + SetupHttpSequence(http, + JsonResponse(HttpStatusCode.OK, "{\"5\":[{\"user_id\":101,\"membership_id\":1}]}"), + JsonResponse(HttpStatusCode.OK, "{}")); + + await client.AddGroupMemberAsync(groupId: 5, userId: 202); + + await http.Received(1).HttpAsync( + HttpMethod.Post, Arg.Is(url => url != null && url.EndsWith("/api/permissions/membership", StringComparison.Ordinal)), + Arg.Any(), Arg.Any(), Arg.Any<(string username, string password)?>(), + Arg.Any(), Arg.Any?>(), + Arg.Any()); + } } diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs index f5efbee7f2..1799aeaa20 100644 --- a/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Application.Tests/Tenants/PostCreation/Steps/MetabaseTenantRegistrationStepTests.cs @@ -97,7 +97,7 @@ public async Task ExecuteAsync_NoReadonlyConnectionString_SkipsWithoutCallingMet await step.ExecuteAsync(tenant.Id); - await metabaseApiClient.DidNotReceiveWithAnyArgs().CreateDatabaseAsync(default!, default!, default, default!, default!, default!, default); + await metabaseApiClient.DidNotReceiveWithAnyArgs().FindOrCreateDatabaseAsync(default!, default!, default, default!, default!, default!, default); } [Fact] @@ -108,13 +108,13 @@ public async Task ExecuteAsync_CreatesDatabaseGroupAndCollection_UsingParsedConn .Returns((string?)null); settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns("user1@gov.bc.ca,user2@gov.bc.ca"); - metabaseApiClient.CreateDatabaseAsync( + metabaseApiClient.FindOrCreateDatabaseAsync( tenant.Name, "dev-crunchy-postgres-primary.ce395f-dev.svc", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", true) .Returns(11); - metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.FindOrCreateGroupAsync(tenant.Name).Returns(22); metabaseApiClient.FindUserIdByEmailAsync("user1@gov.bc.ca").Returns(101); metabaseApiClient.FindUserIdByEmailAsync("user2@gov.bc.ca").Returns((int?)null); - metabaseApiClient.CreateCollectionAsync(tenant.Name).Returns(33); + metabaseApiClient.FindOrCreateCollectionAsync(tenant.Name).Returns(33); await step.ExecuteAsync(tenant.Id); @@ -133,12 +133,12 @@ public async Task ExecuteAsync_DbHostOverrideConfigured_UsesOverrideHostInsteadO settingManager.GetOrNullAsync(MetabaseSettings.UserEmails, TenantSettingValueProvider.ProviderName, tenant.Id.ToString()) .Returns((string?)null); settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns((string?)null); - metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); - metabaseApiClient.CreateCollectionAsync(tenant.Name).Returns(33); + metabaseApiClient.FindOrCreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.FindOrCreateCollectionAsync(tenant.Name).Returns(33); await step.ExecuteAsync(tenant.Id); - await metabaseApiClient.Received(1).CreateDatabaseAsync( + await metabaseApiClient.Received(1).FindOrCreateDatabaseAsync( tenant.Name, "host.docker.internal", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", true); } @@ -149,12 +149,12 @@ public async Task ExecuteAsync_DbSslOverrideFalse_DisablesSslForDatabaseConnecti settingManager.GetOrNullAsync(MetabaseSettings.UserEmails, TenantSettingValueProvider.ProviderName, tenant.Id.ToString()) .Returns((string?)null); settingManager.GetOrNullGlobalAsync(MetabaseSettings.UserEmails).Returns((string?)null); - metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); - metabaseApiClient.CreateCollectionAsync(tenant.Name).Returns(33); + metabaseApiClient.FindOrCreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.FindOrCreateCollectionAsync(tenant.Name).Returns(33); await step.ExecuteAsync(tenant.Id); - await metabaseApiClient.Received(1).CreateDatabaseAsync( + await metabaseApiClient.Received(1).FindOrCreateDatabaseAsync( tenant.Name, "dev-crunchy-postgres-primary.ce395f-dev.svc", 5432, "T_ABC123", "t_abc123_readonly", "s3cr3t", false); } @@ -164,7 +164,7 @@ public async Task ExecuteAsync_TenantScopedUserEmailsSetting_TakesPrecedenceOver var (step, metabaseApiClient, settingManager, tenant) = CreateStep(); settingManager.GetOrNullAsync(MetabaseSettings.UserEmails, TenantSettingValueProvider.ProviderName, tenant.Id.ToString()) .Returns("tenant-scoped@gov.bc.ca"); - metabaseApiClient.CreateGroupAsync(tenant.Name).Returns(22); + metabaseApiClient.FindOrCreateGroupAsync(tenant.Name).Returns(22); metabaseApiClient.FindUserIdByEmailAsync(Arg.Any()).Returns((int?)null); await step.ExecuteAsync(tenant.Id); From 7e4f595393facadd8c898e8ba8434646d43963b9 Mon Sep 17 00:00:00 2001 From: Andre Goncalves Date: Tue, 25 Aug 2026 14:04:11 -0700 Subject: [PATCH 9/9] AB#34137 align metabase url seed pattern --- .../Integrations/DynamicUrlDataSeeder.cs | 43 ++++++++++------- .../Integrations/DynamicUrlDataSeederTests.cs | 48 +++++++++++++++++++ 2 files changed, 75 insertions(+), 16 deletions(-) create mode 100644 applications/Unity.GrantManager/test/Unity.GrantManager.Domain.Tests/Integrations/DynamicUrlDataSeederTests.cs diff --git a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs index 5b68b508a4..39c8371e14 100644 --- a/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs +++ b/applications/Unity.GrantManager/src/Unity.GrantManager.Domain/Integrations/DynamicUrlDataSeeder.cs @@ -39,30 +39,28 @@ public static class DynamicUrls public const string GITHUB_GRAPHQL = $"{PROTOCOL}//api.github.com/graphql"; // No separate dev2 hostname - dev2 shares the dev Metabase route. public const string METABASE_DEV_URL = $"{PROTOCOL}//dev-unity-reporting.apps.gold.devops.gov.bc.ca"; + public const string METABASE_TEST_URL = $"{PROTOCOL}//test-unity-reporting.apps.gold.devops.gov.bc.ca"; + public const string METABASE_PROD_URL = $"{PROTOCOL}//prod-unity-reporting.apps.gold.devops.gov.bc.ca"; } - private static string GetMatomoUrl() + internal static string GetEnvironmentUrl(string? aspNetCoreEnvironment, string devUrl, string testUrl, string prodUrl) { - var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? string.Empty; + var env = aspNetCoreEnvironment ?? string.Empty; if (string.IsNullOrEmpty(env) || env.StartsWith("dev", StringComparison.OrdinalIgnoreCase)) - return DynamicUrls.MATOMO_DEV_URL; + return devUrl; if (env.StartsWith("test", StringComparison.OrdinalIgnoreCase) || env.Equals("uat", StringComparison.OrdinalIgnoreCase)) - return DynamicUrls.MATOMO_TEST_URL; - return DynamicUrls.MATOMO_PROD_URL; + return testUrl; + return prodUrl; } - // Unlike Matomo, only dev has a known route baked in here. Test/UAT/prod are deliberately - // left blank - ops sets the real URL once via the Endpoint Management admin page, and - // (unlike Matomo) it's never overwritten afterward: this only ever inserts a row when one - // doesn't already exist, so whatever's in the database always takes precedence. - private static string GetMetabaseUrl() - { - var env = Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT") ?? string.Empty; - return string.IsNullOrEmpty(env) || env.StartsWith("dev", StringComparison.OrdinalIgnoreCase) - ? DynamicUrls.METABASE_DEV_URL - : string.Empty; - } + private static string GetMatomoUrl() => + GetEnvironmentUrl(Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"), + DynamicUrls.MATOMO_DEV_URL, DynamicUrls.MATOMO_TEST_URL, DynamicUrls.MATOMO_PROD_URL); + + private static string GetMetabaseUrl() => + GetEnvironmentUrl(Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT"), + DynamicUrls.METABASE_DEV_URL, DynamicUrls.METABASE_TEST_URL, DynamicUrls.METABASE_PROD_URL); private async Task SeedDynamicUrlAsync() { @@ -108,6 +106,19 @@ private async Task SeedDynamicUrlAsync() existing.Url = dynamicUrl.Url; await DynamicUrlRepository.UpdateAsync(existing); } + // Unlike Matomo, Metabase is never kept in sync with the environment default + // once a row exists - ops may point it at a different route via the Endpoint + // Management admin page, and that choice must stick. Only fill in the + // env-default value when the existing (host-level) row is still blank, so a + // pre-existing row from before test/prod URLs were known here gets backfilled + // exactly once, and a deliberately-set value is never overwritten. + else if (existing.KeyName == DynamicUrlKeyNames.METABASE_API_BASE && + string.IsNullOrWhiteSpace(existing.Url) && + !string.IsNullOrWhiteSpace(dynamicUrl.Url)) + { + existing.Url = dynamicUrl.Url; + await DynamicUrlRepository.UpdateAsync(existing); + } } } } diff --git a/applications/Unity.GrantManager/test/Unity.GrantManager.Domain.Tests/Integrations/DynamicUrlDataSeederTests.cs b/applications/Unity.GrantManager/test/Unity.GrantManager.Domain.Tests/Integrations/DynamicUrlDataSeederTests.cs new file mode 100644 index 0000000000..272b46040f --- /dev/null +++ b/applications/Unity.GrantManager/test/Unity.GrantManager.Domain.Tests/Integrations/DynamicUrlDataSeederTests.cs @@ -0,0 +1,48 @@ +using Shouldly; +using Unity.GrantManager.Integrations; +using Xunit; + +namespace Unity.GrantManager.Domain.Tests.Integrations; + +public class DynamicUrlDataSeederTests +{ + private const string DevUrl = "https://dev-example.gov.bc.ca"; + private const string TestUrl = "https://test-example.gov.bc.ca"; + private const string ProdUrl = "https://prod-example.gov.bc.ca"; + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("Development")] + [InlineData("dev2")] + public void GetEnvironmentUrl_DevOrUnset_ReturnsDevUrl(string? aspNetCoreEnvironment) + { + var result = DynamicUrlDataSeeder.GetEnvironmentUrl( + aspNetCoreEnvironment, DevUrl, TestUrl, ProdUrl); + + result.ShouldBe(DevUrl); + } + + [Theory] + [InlineData("Test")] + [InlineData("test2")] + [InlineData("UAT")] + public void GetEnvironmentUrl_TestOrUat_ReturnsTestUrl(string aspNetCoreEnvironment) + { + var result = DynamicUrlDataSeeder.GetEnvironmentUrl( + aspNetCoreEnvironment, DevUrl, TestUrl, ProdUrl); + + result.ShouldBe(TestUrl); + } + + [Theory] + [InlineData("Production")] + [InlineData("Staging")] + public void GetEnvironmentUrl_AnythingElse_ReturnsProdUrl(string aspNetCoreEnvironment) + { + var result = DynamicUrlDataSeeder.GetEnvironmentUrl( + aspNetCoreEnvironment, DevUrl, TestUrl, ProdUrl); + + result.ShouldBe(ProdUrl); + } +}