diff --git a/Core/Resgrid.Services/LimitsService.cs b/Core/Resgrid.Services/LimitsService.cs index 20926086a..ea6175e55 100644 --- a/Core/Resgrid.Services/LimitsService.cs +++ b/Core/Resgrid.Services/LimitsService.cs @@ -32,6 +32,12 @@ public async Task ValidateDepartmentIsWithinLimitsAsync(int departmentId) var departmentCount = await _subscriptionsService.GetPlanCountsForDepartmentAsync(departmentId); var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // GetPlanCountsForDepartmentAsync can return null (e.g. Billing API unavailable). Without counts + // we cannot confirm the department is within limits; treat as NOT within limits rather than + // failing open, so missing billing data can't be used to bypass plan caps. + if (departmentCount == null) + return false; + if (departmentCount.UsersCount > await GetPersonnelLimitForDepartmentAsync(departmentId, plan)) return false; @@ -78,6 +84,11 @@ public async Task CanDepartmentAddNewUnit(int departmentId) { var departmentCount = await _subscriptionsService.GetPlanCountsForDepartmentAsync(departmentId); + // GetPlanCountsForDepartmentAsync can return null (e.g. Billing API unavailable). Deny the add + // rather than failing open, so missing billing data can't be used to exceed plan caps. + if (departmentCount == null) + return false; + if (departmentCount.UnitsCount >= await GetUnitsLimitForDepartmentAsync(departmentId)) return false; @@ -100,6 +111,12 @@ public async Task GetEntityLimitForDepartmentAsync(int departmentId, Plan p if (plan == null) plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // The fetched plan can still be null (e.g. Billing API unavailable). Default to the free-plan + // limit (10, matching the plan==null fallback in GetLimitsForEntityPlanWithFallbackAsync) rather + // than granting an unlimited cap — never fail open on missing billing data. + if (plan == null) + return 10; + return plan.GetLimitForTypeAsInt(PlanLimitTypes.Entities); } @@ -128,11 +145,22 @@ public async Task CanDepartmentProvisionNumberAsync(int departmentId) { var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // GetCurrentPlanForDepartmentAsync can return null (e.g. Billing API unavailable). Guard before + // accessing plan.PlanId to avoid a NullReferenceException; treat an unknown plan as not allowed. + if (plan == null) + { + Resgrid.Framework.Logging.LogInfo($"[Twilio SMS] CanDepartmentProvisionNumber=false for DepartmentId={departmentId} — plan is null (billing unavailable / no current plan), so no SMS reply will be generated."); + return false; + } + if (plan.PlanId == 4 || plan.PlanId == 5 || plan.PlanId == 10 || plan.PlanId == 14 || plan.PlanId == 15 || plan.PlanId == 16 || plan.PlanId == 17 || plan.PlanId == 18 || plan.PlanId == 19 || plan.PlanId == 20 || plan.PlanId == 21 || plan.PlanId == 26 || plan.PlanId == 27 || plan.PlanId == 28 || plan.PlanId == 29 || plan.PlanId == 30 || plan.PlanId == 31 || plan.PlanId == 32 || plan.PlanId == 33 || plan.PlanId == 36 || plan.PlanId == 37) return true; + // Diagnostic: this gate silently blocks the inbound-SMS text-command reply path. Log the + // resolved plan so it is clear *why* a department gets no reply (plan not in the allow-list). + Resgrid.Framework.Logging.LogInfo($"[Twilio SMS] CanDepartmentProvisionNumber=false for DepartmentId={departmentId} PlanId={plan.PlanId} — plan is not in the number/text-feature allow-list, so no SMS reply will be generated."); return false; } @@ -140,6 +168,11 @@ public async Task CanDepartmentUseVoiceAsync(int departmentId) { var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // GetCurrentPlanForDepartmentAsync can return null (e.g. Billing API unavailable); guard before + // accessing plan.PlanId and treat an unknown plan as not allowed. + if (plan == null) + return false; + if (plan.PlanId == 5 || plan.PlanId == 10 || plan.PlanId == 15 || plan.PlanId == 16 || plan.PlanId == 17 || plan.PlanId == 18 || plan.PlanId == 19 || plan.PlanId == 20 || plan.PlanId == 21 || plan.PlanId == 28 || plan.PlanId == 29 || plan.PlanId == 30 || plan.PlanId == 31 || plan.PlanId == 32 || plan.PlanId == 33 || plan.PlanId == 36 || plan.PlanId == 37) @@ -152,6 +185,11 @@ public async Task CanDepartmentUseLinksAsync(int departmentId) { var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // GetCurrentPlanForDepartmentAsync can return null (e.g. Billing API unavailable); guard before + // accessing plan.PlanId and treat an unknown plan as not allowed. + if (plan == null) + return false; + if (plan.PlanId > 1) return true; @@ -162,6 +200,11 @@ public async Task CanDepartmentCreateOrdersAsync(int departmentId) { var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); + // GetCurrentPlanForDepartmentAsync can return null (e.g. Billing API unavailable); guard before + // accessing plan.PlanId and treat an unknown plan as not allowed. + if (plan == null) + return false; + if (plan.PlanId > 1) return true; @@ -175,6 +218,19 @@ async Task getCurrentPlanForDepartmentAsync() var limits = new DepartmentLimits(); var plan = await _subscriptionsService.GetCurrentPlanForDepartmentAsync(departmentId); var departmentCount = await _subscriptionsService.GetPlanCountsForDepartmentAsync(departmentId); + + // No usage data (e.g. Billing API unavailable): default to free-plan limits with usage assumed + // at the cap, so we never hand out capacity we can't verify (protect from abuse) — and so we + // don't throw a NullReferenceException dereferencing the counts below. + if (departmentCount == null) + { + limits.PersonnelLimit = 10; + limits.UnitsLimit = 10; + limits.PersonnelCount = 10; + limits.UnitsCount = 10; + return limits; + } + limits.PersonnelCount = departmentCount.UsersCount; limits.UnitsCount = departmentCount.UnitsCount; diff --git a/Core/Resgrid.Services/ServicesModule.cs b/Core/Resgrid.Services/ServicesModule.cs index 5bdf32ea9..95aa5fc44 100644 --- a/Core/Resgrid.Services/ServicesModule.cs +++ b/Core/Resgrid.Services/ServicesModule.cs @@ -106,8 +106,18 @@ protected override void Load(ContainerBuilder builder) builder.RegisterType() .As() .WithParameter( - (parameter, _) => parameter.ParameterType == typeof(RestClient), - (_, context) => context.ResolveNamed(TtsRestClientRegistrationName)) + (parameter, _) => parameter.ParameterType == typeof(Func), + (_, context) => + { + // Hand TtsAudioService a factory rather than an eagerly-resolved RestClient. This + // keeps activation of the service (and any controller that depends on it, e.g. the + // Twilio controller that also handles incoming SMS) from forcing the named TTS + // RestClient to be built. The ServiceBaseUrl check only runs when the factory is + // invoked, i.e. when TTS audio is actually generated. Capture the lifetime scope + // instead of the transient IComponentContext so the factory is safe to call later. + var scope = context.Resolve(); + return (Func)(() => scope.ResolveNamed(TtsRestClientRegistrationName)); + }) .InstancePerLifetimeScope(); // SSO / Security Policy diff --git a/Core/Resgrid.Services/TtsAudioService.cs b/Core/Resgrid.Services/TtsAudioService.cs index a5ab4c8d8..62f6c6192 100644 --- a/Core/Resgrid.Services/TtsAudioService.cs +++ b/Core/Resgrid.Services/TtsAudioService.cs @@ -13,11 +13,15 @@ namespace Resgrid.Services public class TtsAudioService : ITtsAudioService { private const string AdminKeyHeaderName = "X-Resgrid-Admin-Key"; - private readonly RestClient _restClient; + private readonly Func _restClientFactory; - public TtsAudioService(RestClient restClient) + public TtsAudioService(Func restClientFactory) { - _restClient = restClient ?? throw new ArgumentNullException(nameof(restClient)); + // The RestClient is resolved lazily (only when TTS audio is actually generated) so that + // merely constructing this service — and anything that transitively depends on it, such as + // the Twilio controller used for incoming SMS — never forces the TTS RestClient to be built. + // The TtsConfig.ServiceBaseUrl validation now fires on first use instead of at activation. + _restClientFactory = restClientFactory ?? throw new ArgumentNullException(nameof(restClientFactory)); } public async Task GenerateSpeechUrlAsync(string text, string voice = null, int? speed = null, CancellationToken cancellationToken = default) @@ -25,6 +29,8 @@ public async Task GenerateSpeechUrlAsync(string text, string voice = null, if (string.IsNullOrWhiteSpace(text)) throw new ArgumentException("Text is required.", nameof(text)); + var restClient = _restClientFactory(); + var request = new RestRequest("tts", Method.Post); request.AddJsonBody(new GenerateSpeechRequest { @@ -33,7 +39,7 @@ public async Task GenerateSpeechUrlAsync(string text, string voice = null, Speed = speed ?? TtsConfig.DefaultSpeed }); - var response = await _restClient.ExecuteAsync(request, cancellationToken); + var response = await restClient.ExecuteAsync(request, cancellationToken); if (!response.IsSuccessful || response.Data == null || string.IsNullOrWhiteSpace(response.Data.Url)) throw CreateRequestFailure("generate speech audio", response); @@ -59,6 +65,8 @@ public async Task RegenerateStaticPromptsAsync(IEnumerable prompts, Canc if (!promptRequests.Any()) throw new ArgumentException("At least one static prompt is required.", nameof(prompts)); + var restClient = _restClientFactory(); + var request = new RestRequest("tts/admin/static-prompts", Method.Post); request.AddHeader(AdminKeyHeaderName, TtsConfig.StaticPromptAdminKey); request.AddJsonBody(new RegenerateStaticPromptsRequest @@ -66,7 +74,7 @@ public async Task RegenerateStaticPromptsAsync(IEnumerable prompts, Canc Prompts = promptRequests }); - var response = await _restClient.ExecuteAsync(request, cancellationToken); + var response = await restClient.ExecuteAsync(request, cancellationToken); if (!response.IsSuccessful) throw CreateRequestFailure("regenerate static prompts", response); diff --git a/Tests/Resgrid.Tests/Web/Services/TtsAudioServiceTests.cs b/Tests/Resgrid.Tests/Web/Services/TtsAudioServiceTests.cs new file mode 100644 index 000000000..479ddda09 --- /dev/null +++ b/Tests/Resgrid.Tests/Web/Services/TtsAudioServiceTests.cs @@ -0,0 +1,58 @@ +using System; +using System.Threading.Tasks; +using FluentAssertions; +using NUnit.Framework; +using Resgrid.Services; +using RestSharp; + +namespace Resgrid.Tests.Web.Services +{ + [TestFixture] + public class TtsAudioServiceTests + { + [Test] + public void constructor_should_not_resolve_rest_client() + { + // Regression: constructing TtsAudioService (and therefore any controller that transitively + // depends on it, e.g. the Twilio controller that serves incoming SMS) must never build the + // TTS RestClient. Previously the RestClient was injected directly and resolved at activation, + // so an unconfigured TtsConfig.ServiceBaseUrl 500'd unrelated endpoints such as incoming SMS. + var factoryInvoked = false; + Func factory = () => + { + factoryInvoked = true; + throw new InvalidOperationException("RestClient must not be created during construction."); + }; + + var service = new TtsAudioService(factory); + + service.Should().NotBeNull(); + factoryInvoked.Should().BeFalse(); + } + + [Test] + public async Task generate_speech_url_should_defer_rest_client_resolution_until_invoked() + { + // The "ServiceBaseUrl must be configured" failure should still surface — but only when TTS + // audio is actually generated, not at construction time. + Func factory = () => + throw new InvalidOperationException("TtsConfig.ServiceBaseUrl must be configured before using the TTS service."); + + var service = new TtsAudioService(factory); + + await FluentActions + .Awaiting(() => service.GenerateSpeechUrlAsync("Hello from Resgrid")) + .Should() + .ThrowAsync() + .WithMessage("*ServiceBaseUrl must be configured*"); + } + + [Test] + public void constructor_should_reject_null_factory() + { + Action act = () => new TtsAudioService(null); + + act.Should().Throw(); + } + } +} diff --git a/Web/Resgrid.Web.Services/Controllers/TwilioController.cs b/Web/Resgrid.Web.Services/Controllers/TwilioController.cs index 1da24d86e..57ef81e2c 100644 --- a/Web/Resgrid.Web.Services/Controllers/TwilioController.cs +++ b/Web/Resgrid.Web.Services/Controllers/TwilioController.cs @@ -158,12 +158,18 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques if (departmentId.HasValue) messageEvent.CustomerId = departmentId.Value.ToString(); + // Diagnostic: did we resolve a department for this inbound text? If not, no reply is sent. + Framework.Logging.LogInfo($"[Twilio SMS] MessageSid={request.MessageSid} To={textMessage.To} From={textMessage.Msisdn} resolved DepartmentId={(departmentId.HasValue ? departmentId.Value.ToString() : "none")}"); + // Feature-flagged rollout: the chatbot ingress is the new path. When the flag is off // (globally or for this department) fall back to the original text-command handling so // existing behavior is preserved. var chatbotEnabled = await _featureToggleService.IsEnabledAsync( FeatureFlagKeys.ChatbotTwilioTextIntegration, departmentId ?? 0, false); + // Diagnostic: which path handled the message — chatbot ingress or legacy text commands? + Framework.Logging.LogInfo($"[Twilio SMS] MessageSid={request.MessageSid} DepartmentId={(departmentId.HasValue ? departmentId.Value.ToString() : "none")} ChatbotEnabled={chatbotEnabled} (path={(chatbotEnabled ? "chatbot" : "text-command")})"); + if (chatbotEnabled) { var chatbotMessage = new ChatbotMessage @@ -181,6 +187,9 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques if (chatbotResponse.Processed) messageEvent.Processed = true; + // Diagnostic: an empty reply text here yields a TwiML response with no usable message. + Framework.Logging.LogInfo($"[Twilio SMS] MessageSid={request.MessageSid} chatbot Processed={chatbotResponse.Processed} ReplyLength={(chatbotResponse.Text?.Length ?? 0)}"); + response.Message(chatbotResponse.Text); } else @@ -198,9 +207,15 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques await _numbersService.SaveInboundMessageEventAsync(messageEvent); } + // Diagnostic: a body length of ~60 bytes is an empty (no ), + // which means no reply will be delivered to the sender. Processed reflects whether a handler + // claimed the message. + var twiml = response.ToString(); + Framework.Logging.LogInfo($"[Twilio SMS] MessageSid={request.MessageSid} responding with {twiml.Length} bytes, Processed={messageEvent.Processed}"); + return new ContentResult { - Content = response.ToString(), + Content = twiml, ContentType = "application/xml", StatusCode = 200 }; @@ -212,6 +227,10 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques private async System.Threading.Tasks.Task ProcessTextCommandsAsync(TextMessage textMessage, InboundMessageEvent messageEvent, MessagingResponse response, int? departmentId, UserProfile userProfile) { + // Diagnostic: without a department the legacy path adds no message, so the sender gets no reply. + if (!departmentId.HasValue) + Framework.Logging.LogInfo($"[Twilio SMS] Text-command path: no department resolved for sender {textMessage.Msisdn} → number {textMessage.To}; no reply will be sent."); + if (departmentId.HasValue) { // Run all department-level lookups in parallel — they are independent of each other. @@ -229,6 +248,10 @@ private async System.Threading.Tasks.Task ProcessTextCommandsAsync(TextMessage t messageEvent.CustomerId = departmentId.Value.ToString(); + // Diagnostic: when Authorized=false the whole reply block below is skipped (no ). + // The matching PlanId is logged by LimitsService.CanDepartmentProvisionNumberAsync. + Framework.Logging.LogInfo($"[Twilio SMS] DepartmentId={departmentId.Value} Authorized(CanProvisionNumber)={authroized}"); + if (authroized) { bool isDispatchSource = false; @@ -277,9 +300,16 @@ private async System.Threading.Tasks.Task ProcessTextCommandsAsync(TextMessage t // only hit the DB again if the department came from the phone-number lookup path. var profile = userProfile ?? await _userProfileService.GetProfileByMobileNumberAsync(textMessage.Msisdn); + // Diagnostic: no matching user profile for the sender's number means no reply is added. + if (profile == null) + Framework.Logging.LogInfo($"[Twilio SMS] DepartmentId={departmentId.Value} sender {textMessage.Msisdn} has no matching user profile; no reply will be sent."); + if (profile != null) { var payload = _textCommandService.DetermineType(textMessage.Text); + + // Diagnostic: which command the inbound text resolved to (None still replies with a hint). + Framework.Logging.LogInfo($"[Twilio SMS] DepartmentId={departmentId.Value} UserId={profile.UserId} resolved text command={payload.Type}"); var customActions = customStates.FirstOrDefault(x => x.Type == (int)CustomStateTypes.Personnel); var customStaffing = customStates.FirstOrDefault(x => x.Type == (int)CustomStateTypes.Staffing); diff --git a/Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs b/Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs index 58590210d..660e85813 100644 --- a/Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs +++ b/Web/Resgrid.Web/Areas/User/Controllers/SubscriptionController.cs @@ -185,6 +185,13 @@ public async Task Index() model.Expires = "Never"; } + // The Subscription/Index view dereferences Model.Payment (e.g. Payment.Cancelled) without a + // null check. Departments with no current payment (free/unpaid, or when the billing API returns + // no payment) would otherwise throw a NullReferenceException while rendering the view. Mirror the + // Plan fallback below so the page renders as an active, never-expiring plan instead of erroring. + if (model.Payment == null) + model.Payment = new Resgrid.Model.Payment { Cancelled = false, Amount = 0, Quantity = 1, EndingOn = DateTime.MaxValue }; + if (model.Plan != null) { model.PossibleUpgrades = _subscriptionsService.GetPossibleUpgradesForPlan(model.Plan.PlanId);