From cc4e3c877699c8b7061994f235966eb65cdedfbc Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Wed, 15 Jul 2026 22:57:41 -0700 Subject: [PATCH 1/2] RG-T117 Chatbot fix --- .../Resgrid.Chatbot/Models/ChatbotPlatform.cs | 2 + Core/Resgrid.Chatbot/Models/ChatbotSession.cs | 8 +- .../Services/ChatbotIngressService.cs | 213 ++++++++++----- Core/Resgrid.Framework/SecurityPinUtility.cs | 71 +++++ Core/Resgrid.Model/ChatbotIdentity.cs | 8 + Core/Resgrid.Model/DepartmentSettingTypes.cs | 1 + .../Services/IDepartmentSettingsService.cs | 6 + .../Services/ISecurityPinService.cs | 41 +++ Core/Resgrid.Model/UserProfile.cs | 14 + .../DepartmentSettingsService.cs | 21 ++ Core/Resgrid.Services/SecurityPinService.cs | 100 +++++++ Core/Resgrid.Services/ServicesModule.cs | 1 + Core/Resgrid.Services/UserProfileService.cs | 29 +- .../Migrations/M0089_AddingUserSecurityPin.cs | 21 ++ .../M0089_AddingUserSecurityPinPg.cs | 21 ++ .../Modules/TestingDataModule.cs | 1 + .../Services/SecurityPinServiceTests.cs | 255 ++++++++++++++++++ .../Controllers/SignalWireController.cs | 8 + .../Controllers/TwilioController.cs | 31 ++- .../User/Controllers/DepartmentController.cs | 15 +- .../Areas/User/Controllers/HomeController.cs | 35 ++- .../User/Models/DepartmentSettingsModel.cs | 3 + .../Areas/User/Models/EditProfileModel.cs | 12 + .../User/Views/Department/Settings.cshtml | 11 + .../User/Views/Home/EditUserProfile.cshtml | 29 ++ 25 files changed, 876 insertions(+), 81 deletions(-) create mode 100644 Core/Resgrid.Framework/SecurityPinUtility.cs create mode 100644 Core/Resgrid.Model/Services/ISecurityPinService.cs create mode 100644 Core/Resgrid.Services/SecurityPinService.cs create mode 100644 Providers/Resgrid.Providers.Migrations/Migrations/M0089_AddingUserSecurityPin.cs create mode 100644 Providers/Resgrid.Providers.MigrationsPg/Migrations/M0089_AddingUserSecurityPinPg.cs create mode 100644 Tests/Resgrid.Tests/Services/SecurityPinServiceTests.cs diff --git a/Core/Resgrid.Chatbot/Models/ChatbotPlatform.cs b/Core/Resgrid.Chatbot/Models/ChatbotPlatform.cs index 385770d7c..89a89a861 100644 --- a/Core/Resgrid.Chatbot/Models/ChatbotPlatform.cs +++ b/Core/Resgrid.Chatbot/Models/ChatbotPlatform.cs @@ -1,5 +1,7 @@ namespace Resgrid.Chatbot.Models { + // The SMS values (SmsTwilio, SmsSignalWire) are mirrored as constants on + // Resgrid.Model.ChatbotIdentity for consumers that cannot reference this project. public enum ChatbotPlatform { Unknown = 0, diff --git a/Core/Resgrid.Chatbot/Models/ChatbotSession.cs b/Core/Resgrid.Chatbot/Models/ChatbotSession.cs index e336ee5a2..dad0aac76 100644 --- a/Core/Resgrid.Chatbot/Models/ChatbotSession.cs +++ b/Core/Resgrid.Chatbot/Models/ChatbotSession.cs @@ -12,7 +12,13 @@ public enum ChatbotDialogState AwaitingConfirmation = 4, AwaitingParameter = 5, AwaitingAuth = 6, - Completed = 7 + Completed = 7, + + /// + /// Step-up security-PIN prompt for a confirmed destructive action (the user already replied + /// YES; the pending intent executes once the correct PIN is supplied). + /// + AwaitingPin = 8 } public class ChatbotSession diff --git a/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs b/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs index d44d6bcc2..6adc0d311 100644 --- a/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs +++ b/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs @@ -25,6 +25,9 @@ public class ChatbotIngressService : IChatbotIngressService private readonly IAuthorizationService _authorizationService; private readonly IChatbotDepartmentConfigService _departmentConfigService; private readonly IChatbotRateLimiter _rateLimiter; + private readonly ISecurityPinService _securityPinService; + + private const int MaxPinAttempts = 3; public ChatbotIngressService( IChatbotUserIdentityService userIdentityService, @@ -39,7 +42,8 @@ public ChatbotIngressService( ILimitsService limitsService, IAuthorizationService authorizationService, IChatbotDepartmentConfigService departmentConfigService, - IChatbotRateLimiter rateLimiter) + IChatbotRateLimiter rateLimiter, + ISecurityPinService securityPinService) { _userIdentityService = userIdentityService; _sessionManager = sessionManager; @@ -54,6 +58,7 @@ public ChatbotIngressService( _authorizationService = authorizationService; _departmentConfigService = departmentConfigService; _rateLimiter = rateLimiter; + _securityPinService = securityPinService; } public async Task ProcessMessageAsync(ChatbotMessage message) @@ -66,9 +71,30 @@ public async Task ProcessMessageAsync(ChatbotMessage message) // 1. Identify user from the platform-specific identifier (already-linked only). var identity = await ResolveUserIdentityAsync(message); - // 1b. SMS: a phone number that matches a Resgrid profile but isn't linked yet. Depending - // on the department's RequireLinkingConfirmation setting, either auto-link or run a - // one-time "Reply YES to link" confirmation before trusting the number. + // 1b. SMS identities are only trusted while the linked profile's mobile number is + // verified AND still matches the sending number. Links created before verification was + // required, or made stale by a number change, must not authenticate the sender — drop + // them and fall through to the fresh phone-match below (the number may now belong to a + // different, verified profile). + if (identity != null && IsSmsPlatform(message.Platform)) + { + var linkedProfile = await _userProfileService.GetProfileByUserIdAsync(identity.UserId); + if (linkedProfile == null + || linkedProfile.MobileNumberVerified != true + || !PhoneNumbersMatch(linkedProfile.GetPhoneNumber(), message.From)) + { + // Only physically remove SMS links. The phone fallback can also surface a + // cross-platform identity (e.g. WhatsApp ids are phone numbers); those are + // merely not trusted for this SMS message, not deleted. + if (IsSmsPlatform(identity.Platform)) + await _userIdentityService.UnlinkUserAsync(identity.Id); + identity = null; + } + } + + // 1c. SMS: a phone number that matches a Resgrid profile but isn't linked yet. Only a + // VERIFIED mobile number may authenticate a sender; when it is verified the identity link + // is created silently (it's an internal optimization, not something the user is asked about). if (identity == null && IsSmsPlatform(message.Platform)) { var cleanPhone = message.From?.Replace("+", "").Trim(); @@ -77,19 +103,18 @@ public async Task ProcessMessageAsync(ChatbotMessage message) var profile = await _userProfileService.GetProfileByMobileNumberAsync(cleanPhone); if (profile != null) { - var candidateDepartment = await ResolveActiveDepartmentAsync(profile.UserId); - var requireConfirmation = candidateDepartment == null - || ((await _departmentConfigService.GetConfigAsync(candidateDepartment.DepartmentId))?.RequireLinkingConfirmation ?? true); - - if (!requireConfirmation) + if (profile.MobileNumberVerified == true) { identity = await _userIdentityService.LinkUserAsync( - profile.UserId, message.Platform, message.From, profile.FullName.AsFirstNameLastName, "phone_match"); + profile.UserId, message.Platform, message.From, profile.FullName.AsFirstNameLastName, "phone_match_verified"); } else { - // Returns the prompt / acknowledgement; links the number on a YES reply. - return await HandlePhoneLinkConfirmationAsync(message, profile, candidateDepartment); + return new ChatbotResponse + { + Text = "This mobile number matches a Resgrid profile but hasn't been verified yet. Please verify your mobile number on your Resgrid profile page, then text again.", + Processed = true + }; } } } @@ -206,24 +231,33 @@ public async Task ProcessMessageAsync(ChatbotMessage message) var reply = message.Text?.Trim().ToUpperInvariant(); if (reply == "YES" || reply == "Y" || reply == "CONFIRM" || reply == "OK") { - var pendingType = session.PendingIntent.Value; - var confirmedIntent = new ChatbotIntent - { - Type = pendingType, - Parameters = new Dictionary(session.Context) { ["__confirmed"] = "true" } - }; - session.State = ChatbotDialogState.Idle; - session.PendingIntent = null; - - var confirmHandler = _actionHandlers.FirstOrDefault(h => h.CanHandle(pendingType)); - if (confirmHandler != null) + // Step-up auth: when the department forces security-PIN usage (or the user opted + // in), a confirmed destructive action additionally requires the user's 4-digit PIN + // before it executes. + if (await _securityPinService.IsPinRequiredAsync(identity.UserId, department.DepartmentId)) { - var confirmResponse = await confirmHandler.HandleAsync(message, confirmedIntent, session); - confirmResponse.Intent = confirmedIntent; - session.Context.Clear(); + session.State = ChatbotDialogState.AwaitingPin; + session.Context["__pinAttempts"] = "0"; await _sessionManager.SaveSessionAsync(session); - return confirmResponse; + + if (!await _securityPinService.HasPinAsync(identity.UserId)) + { + // Department forces PINs but this user doesn't have one yet — generate one + // so they can look it up on their profile page and complete the action. + await _securityPinService.EnsurePinAsync(identity.UserId, department.DepartmentId); + return new ChatbotResponse + { + Text = "This action requires your security PIN. A PIN has been generated for you — view or change it on your Resgrid profile page, then reply with the 4-digit PIN to continue, or NO to cancel.", + Processed = true + }; + } + + return new ChatbotResponse { Text = "For security, reply with your 4-digit security PIN to complete this action, or NO to cancel.", Processed = true }; } + + var confirmResponse = await DispatchConfirmedIntentAsync(message, session); + if (confirmResponse != null) + return confirmResponse; } else if (reply == "NO" || reply == "N" || reply == "CANCEL") { @@ -237,6 +271,48 @@ public async Task ProcessMessageAsync(ChatbotMessage message) } } + // 5b. PIN step-up for an already-confirmed destructive action: the user replied YES and a + // security PIN is required, so the pending intent only executes on a correct PIN. + if (session.State == ChatbotDialogState.AwaitingPin && session.PendingIntent.HasValue) + { + var pinReply = message.Text?.Trim(); + + if (IsNegative(pinReply)) + { + session.Reset(); + await _sessionManager.SaveSessionAsync(session); + return new ChatbotResponse { Text = "Cancelled.", Processed = true }; + } + + if (!SecurityPinUtility.IsValidFormat(pinReply)) + return new ChatbotResponse { Text = "Please reply with your 4-digit security PIN, or NO to cancel.", Processed = true }; + + if (await _securityPinService.ValidatePinAsync(identity.UserId, pinReply)) + { + session.Context.Remove("__pinAttempts"); + var confirmResponse = await DispatchConfirmedIntentAsync(message, session); + if (confirmResponse != null) + return confirmResponse; + } + else + { + session.Context.TryGetValue("__pinAttempts", out var attemptsRaw); + int.TryParse(attemptsRaw, out var attempts); + attempts++; + + if (attempts >= MaxPinAttempts) + { + session.Reset(); + await _sessionManager.SaveSessionAsync(session); + return new ChatbotResponse { Text = "Too many incorrect PIN attempts. The action has been cancelled.", Processed = true }; + } + + session.Context["__pinAttempts"] = attempts.ToString(); + await _sessionManager.SaveSessionAsync(session); + return new ChatbotResponse { Text = "Incorrect PIN. Reply with your 4-digit security PIN, or NO to cancel.", Processed = true }; + } + } + if (session.State != ChatbotDialogState.Idle) { var continuationResult = await _conversationEngine.HandleContinuationAsync(message, session); @@ -331,12 +407,6 @@ private static bool IsEmergencyText(string text) return t.Contains("mayday") || t.Contains("emergency") || t.Contains("officer down") || t.Contains("firefighter down"); } - private static bool IsAffirmative(string text) - { - var t = text?.Trim().ToLowerInvariant(); - return t == "yes" || t == "y" || t == "confirm"; - } - private static bool IsNegative(string text) { var t = text?.Trim().ToLowerInvariant(); @@ -344,55 +414,52 @@ private static bool IsNegative(string text) } /// - /// One-time confirmation before linking an unrecognized SMS number to the matching Resgrid - /// account. Prompts on first contact, links on YES, cancels on NO. + /// Executes the pending (already confirmed, and PIN-verified when required) destructive intent. + /// Returns the handler's response, or null when no handler claims the intent (the caller then + /// falls through to normal classification, matching the original confirmation-gate behavior). /// - private async Task HandlePhoneLinkConfirmationAsync(ChatbotMessage message, Model.UserProfile profile, Model.Department department) + private async Task DispatchConfirmedIntentAsync(ChatbotMessage message, ChatbotSession session) { - // A matched profile may have no active department membership, in which case the caller - // passes a null department. The phone-link confirmation only tracks trust of the number - // (linking doesn't depend on a department), so fall back to department id 0 for the - // temporary session rather than dereferencing a null department. - var session = await _sessionManager.GetOrCreateSessionAsync(profile.UserId, department?.DepartmentId ?? 0, message.Platform, message.From); - - var awaitingConfirmation = session.State == ChatbotDialogState.AwaitingConfirmation - && session.Context != null - && session.Context.TryGetValue("pendingAction", out var pendingAction) - && pendingAction == "confirm_phone_link"; - - if (awaitingConfirmation) + var pendingType = session.PendingIntent.Value; + var confirmedIntent = new ChatbotIntent { - if (IsAffirmative(message.Text)) - { - await _userIdentityService.LinkUserAsync( - profile.UserId, message.Platform, message.From, profile.FullName.AsFirstNameLastName, "phone_match_confirmed"); - session.State = ChatbotDialogState.Idle; - session.Context.Remove("pendingAction"); - await _sessionManager.SaveSessionAsync(session); - return new ChatbotResponse { Text = "Your number is now linked to your Resgrid account. Text HELP to see what you can do.", Processed = true }; - } - - if (IsNegative(message.Text)) - { - await _sessionManager.EndSessionAsync(session.SessionId); - return new ChatbotResponse { Text = "No problem — I won't link this number. Reply LINK anytime to connect your account.", Processed = true }; - } + Type = pendingType, + Parameters = new Dictionary(session.Context) { ["__confirmed"] = "true" } + }; + session.State = ChatbotDialogState.Idle; + session.PendingIntent = null; - return new ChatbotResponse { Text = "Please reply YES to link this number to your Resgrid account, or NO to cancel.", Processed = false }; - } + var confirmHandler = _actionHandlers.FirstOrDefault(h => h.CanHandle(pendingType)); + if (confirmHandler == null) + return null; - // First contact for this number: ask for confirmation before linking. - session.State = ChatbotDialogState.AwaitingConfirmation; - session.Context["pendingAction"] = "confirm_phone_link"; + var confirmResponse = await confirmHandler.HandleAsync(message, confirmedIntent, session); + confirmResponse.Intent = confirmedIntent; + session.Context.Clear(); await _sessionManager.SaveSessionAsync(session); + return confirmResponse; + } - return new ChatbotResponse - { - Text = $"We found a Resgrid account for {profile.FirstName}. Reply YES to link this number, or NO to cancel.", - Processed = true - }; + /// + /// Compares a profile phone number against a sender number, ignoring formatting and tolerating + /// a leading US country code on either side (mirrors GetProfileByMobileNumberAsync). + /// + private static bool PhoneNumbersMatch(string profileNumber, string senderNumber) + { + var a = NormalizePhone(profileNumber); + var b = NormalizePhone(senderNumber); + + if (string.IsNullOrWhiteSpace(a) || string.IsNullOrWhiteSpace(b)) + return false; + + return a == b + || (a.Length == 11 && a[0] == '1' && a.Substring(1) == b) + || (b.Length == 11 && b[0] == '1' && b.Substring(1) == a); } + private static string NormalizePhone(string number) + => number?.Replace(" ", "").Replace("(", "").Replace(")", "").Replace("+", "").Replace("-", "").Replace(".", "").Trim(); + private async Task ResolveActiveDepartmentAsync(string userId) { // Shared resolution: the user's active (then default, then first) membership, preferring one whose diff --git a/Core/Resgrid.Framework/SecurityPinUtility.cs b/Core/Resgrid.Framework/SecurityPinUtility.cs new file mode 100644 index 000000000..36ccd0aff --- /dev/null +++ b/Core/Resgrid.Framework/SecurityPinUtility.cs @@ -0,0 +1,71 @@ +using System.Security.Cryptography; + +namespace Resgrid.Framework +{ + /// + /// Generation and validation rules for the 4-digit user security PIN used as a step-up + /// (2FA-style) check for dangerous/department-wide chatbot and SMS actions. + /// + public static class SecurityPinUtility + { + public const int PinLength = 4; + + /// True when the value is exactly ASCII digits. + public static bool IsValidFormat(string pin) + { + if (string.IsNullOrWhiteSpace(pin) || pin.Length != PinLength) + return false; + + foreach (var c in pin) + { + if (c < '0' || c > '9') + return false; + } + + return true; + } + + /// + /// True for trivially guessable PINs: all-same-digit (0000, 1111, ...) and consecutive + /// ascending/descending runs (1234, 0123, 4321, 9876, ...). + /// + public static bool IsWeak(string pin) + { + if (!IsValidFormat(pin)) + return true; + + bool allSame = true, ascending = true, descending = true; + for (int i = 1; i < pin.Length; i++) + { + if (pin[i] != pin[0]) + allSame = false; + if (pin[i] != pin[i - 1] + 1) + ascending = false; + if (pin[i] != pin[i - 1] - 1) + descending = false; + } + + return allSame || ascending || descending; + } + + /// Valid format and not weak. + public static bool IsAcceptable(string pin) => IsValidFormat(pin) && !IsWeak(pin); + + /// Generates a random PIN that passes using a crypto RNG. + public static string Generate() + { + using var rng = RandomNumberGenerator.Create(); + var bytes = new byte[4]; + + while (true) + { + rng.GetBytes(bytes); + uint value = (uint)(bytes[0] | bytes[1] << 8 | bytes[2] << 16 | bytes[3] << 24); + string pin = (value % 10000).ToString().PadLeft(PinLength, '0'); + + if (IsAcceptable(pin)) + return pin; + } + } + } +} diff --git a/Core/Resgrid.Model/ChatbotIdentity.cs b/Core/Resgrid.Model/ChatbotIdentity.cs index 538d16d96..1dd1c9570 100644 --- a/Core/Resgrid.Model/ChatbotIdentity.cs +++ b/Core/Resgrid.Model/ChatbotIdentity.cs @@ -14,6 +14,14 @@ namespace Resgrid.Model [Table("ChatbotUserIdentities")] public class ChatbotIdentity : IEntity { + /// + /// SMS platform values, kept in sync with Resgrid.Chatbot.Models.ChatbotPlatform + /// (which Resgrid.Services cannot reference). Used when services outside the chatbot + /// project need to invalidate SMS-based identity links. + /// + public const int PlatformSmsTwilio = 1; + public const int PlatformSmsSignalWire = 2; + public string Id { get; set; } [Required] diff --git a/Core/Resgrid.Model/DepartmentSettingTypes.cs b/Core/Resgrid.Model/DepartmentSettingTypes.cs index d219ed065..d747393f8 100644 --- a/Core/Resgrid.Model/DepartmentSettingTypes.cs +++ b/Core/Resgrid.Model/DepartmentSettingTypes.cs @@ -55,5 +55,6 @@ public enum DepartmentSettingTypes UnitCallReleaseStatusToSet = 51, UnitCallStatusOverridesByUnitType = 52, EnableModernNotifications = 53, + ForceChatbotSecurityPin = 54, } } diff --git a/Core/Resgrid.Model/Services/IDepartmentSettingsService.cs b/Core/Resgrid.Model/Services/IDepartmentSettingsService.cs index 221dd84cc..d0054b6f4 100644 --- a/Core/Resgrid.Model/Services/IDepartmentSettingsService.cs +++ b/Core/Resgrid.Model/Services/IDepartmentSettingsService.cs @@ -314,5 +314,11 @@ Task SetUnitCallStatusOverridesByUnitTypeAsync(int department Task GetSettingByTypeAsync(int departmentId, DepartmentSettingTypes type); Task GetModernNotificationsEnabledAsync(int departmentId, bool bypassCache = false); + + /// + /// True when the department forces every member to use their security PIN for dangerous + /// chatbot/SMS actions (overrides the per-user opt-in). + /// + Task GetForceChatbotSecurityPinAsync(int departmentId, bool bypassCache = false); } } diff --git a/Core/Resgrid.Model/Services/ISecurityPinService.cs b/Core/Resgrid.Model/Services/ISecurityPinService.cs new file mode 100644 index 000000000..c2c5a0e11 --- /dev/null +++ b/Core/Resgrid.Model/Services/ISecurityPinService.cs @@ -0,0 +1,41 @@ +using System.Threading; +using System.Threading.Tasks; + +namespace Resgrid.Model.Services +{ + /// + /// Manages the per-user 4-digit security PIN used as a step-up (2FA-style) check for + /// dangerous/department-wide chatbot and SMS actions. PINs are stored AES-encrypted on the + /// user profile. A user can opt in personally (UserProfile.SecurityPinEnabled) or a + /// department can force PIN usage for all members (DepartmentSettingTypes.ForceChatbotSecurityPin). + /// + public interface ISecurityPinService + { + /// + /// True when the user must supply their security PIN for dangerous actions: either the + /// department forces PIN usage or the user opted in on their profile. + /// + Task IsPinRequiredAsync(string userId, int departmentId); + + /// True when the user has a security PIN set on their profile. + Task HasPinAsync(string userId); + + /// Validates the supplied PIN against the user's stored (encrypted) PIN. + Task ValidatePinAsync(string userId, string pin); + + /// Gets the user's decrypted PIN (for own-profile display), or null when none is set. + Task GetPinAsync(string userId); + + /// + /// Generates and saves a random PIN for the user when they don't have one yet. + /// Returns the profile (with a PIN guaranteed set) or null when no profile exists. + /// + Task EnsurePinAsync(string userId, int departmentId, CancellationToken cancellationToken = default); + + /// + /// Generates a random PIN for every member of the department that doesn't have one yet. + /// Used when a department enables the ForceChatbotSecurityPin setting. + /// + Task EnsurePinsForDepartmentAsync(int departmentId, CancellationToken cancellationToken = default); + } +} diff --git a/Core/Resgrid.Model/UserProfile.cs b/Core/Resgrid.Model/UserProfile.cs index e40481c1d..0866b3aaf 100644 --- a/Core/Resgrid.Model/UserProfile.cs +++ b/Core/Resgrid.Model/UserProfile.cs @@ -184,6 +184,20 @@ public class UserProfile: IEntity [ProtoMember(50)] public bool HomeVerificationVoiceCodeConsumed { get; set; } + /// + /// AES-encrypted (via IEncryptionService) 4-digit security PIN used as a step-up check for + /// dangerous/department-wide chatbot and SMS actions. Null = no PIN set yet. + /// + [ProtoMember(51)] + public string SecurityPin { get; set; } + + /// + /// The user opted in to requiring their security PIN for dangerous chatbot/SMS actions. + /// The department-level ForceChatbotSecurityPin setting overrides this to on for everyone. + /// + [ProtoMember(52)] + public bool SecurityPinEnabled { get; set; } + [NotMapped] [JsonIgnore] public object IdValue diff --git a/Core/Resgrid.Services/DepartmentSettingsService.cs b/Core/Resgrid.Services/DepartmentSettingsService.cs index 194521ddc..5103f1638 100644 --- a/Core/Resgrid.Services/DepartmentSettingsService.cs +++ b/Core/Resgrid.Services/DepartmentSettingsService.cs @@ -23,6 +23,7 @@ public class DepartmentSettingsService : IDepartmentSettingsService private static string TtsLanguageCacheKey = "DSetTtsLanguage_{0}"; private static string PersonnelOnUnitSetUnitStatusCacheKey = "DSetPersonnelOnUnitSetUnitStatus_{0}"; private static string ModernNotificationsCacheKey = "DSetModernNotifications_{0}"; + private static string ForceChatbotSecurityPinCacheKey = "DSetForceChatbotSecurityPin_{0}"; private static TimeSpan LongCacheLength = TimeSpan.FromDays(14); private static TimeSpan ThatsNotLongThisIsLongCacheLength = TimeSpan.FromDays(365); private static TimeSpan TwoYearCacheLength = TimeSpan.FromDays(730); @@ -85,6 +86,9 @@ public DepartmentSettingsService(IDepartmentSettingsRepository departmentSetting case DepartmentSettingTypes.EnableModernNotifications: await _cacheProvider.RemoveAsync(string.Format(ModernNotificationsCacheKey, departmentId)); break; + case DepartmentSettingTypes.ForceChatbotSecurityPin: + await _cacheProvider.RemoveAsync(string.Format(ForceChatbotSecurityPinCacheKey, departmentId)); + break; } savedSetting.Setting = setting; @@ -954,6 +958,23 @@ async Task getSetting() return bool.Parse(await getSetting()); } + public async Task GetForceChatbotSecurityPinAsync(int departmentId, bool bypassCache = false) + { + async Task getSetting() + { + var s = await GetSettingByDepartmentIdType(departmentId, DepartmentSettingTypes.ForceChatbotSecurityPin); + return s?.Setting ?? "false"; + } + + if (Config.SystemBehaviorConfig.CacheEnabled && !bypassCache) + { + var cachedValue = await _cacheProvider.RetrieveAsync(string.Format(ForceChatbotSecurityPinCacheKey, departmentId), getSetting, LongCacheLength); + return bool.Parse(cachedValue); + } + + return bool.Parse(await getSetting()); + } + private static string GetDefaultTtsLanguage() { if (EspeakVoiceCatalog.TryNormalizeIdentifier(TtsConfig.DefaultVoice, out var normalizedVoice)) diff --git a/Core/Resgrid.Services/SecurityPinService.cs b/Core/Resgrid.Services/SecurityPinService.cs new file mode 100644 index 000000000..a11f82e30 --- /dev/null +++ b/Core/Resgrid.Services/SecurityPinService.cs @@ -0,0 +1,100 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Resgrid.Framework; +using Resgrid.Model; +using Resgrid.Model.Services; + +namespace Resgrid.Services +{ + /// + /// Implements the per-user security PIN used as a step-up check for dangerous/department-wide + /// chatbot and SMS actions. PINs are stored AES-encrypted (via ) + /// on the user profile, mirroring how contact verification codes are stored. + /// + public class SecurityPinService : ISecurityPinService + { + private readonly IUserProfileService _userProfileService; + private readonly IDepartmentSettingsService _departmentSettingsService; + private readonly IEncryptionService _encryptionService; + + public SecurityPinService(IUserProfileService userProfileService, IDepartmentSettingsService departmentSettingsService, + IEncryptionService encryptionService) + { + _userProfileService = userProfileService; + _departmentSettingsService = departmentSettingsService; + _encryptionService = encryptionService; + } + + public async Task IsPinRequiredAsync(string userId, int departmentId) + { + if (await _departmentSettingsService.GetForceChatbotSecurityPinAsync(departmentId)) + return true; + + var profile = await _userProfileService.GetProfileByUserIdAsync(userId); + return profile != null && profile.SecurityPinEnabled; + } + + public async Task HasPinAsync(string userId) + { + var profile = await _userProfileService.GetProfileByUserIdAsync(userId); + return !string.IsNullOrWhiteSpace(profile?.SecurityPin); + } + + public async Task ValidatePinAsync(string userId, string pin) + { + if (!SecurityPinUtility.IsValidFormat(pin)) + return false; + + var storedPin = await GetPinAsync(userId); + return storedPin != null && string.Equals(storedPin, pin, StringComparison.Ordinal); + } + + public async Task GetPinAsync(string userId) + { + var profile = await _userProfileService.GetProfileByUserIdAsync(userId); + if (string.IsNullOrWhiteSpace(profile?.SecurityPin)) + return null; + + try + { + return _encryptionService.Decrypt(profile.SecurityPin); + } + catch (Exception ex) + { + // An undecryptable value (corrupted or written with a different key) is treated as no PIN. + Logging.LogException(ex); + return null; + } + } + + public async Task EnsurePinAsync(string userId, int departmentId, CancellationToken cancellationToken = default) + { + var profile = await _userProfileService.GetProfileByUserIdAsync(userId, bypassCache: true); + if (profile == null) + return null; + + if (!string.IsNullOrWhiteSpace(profile.SecurityPin)) + return profile; + + profile.SecurityPin = _encryptionService.Encrypt(SecurityPinUtility.Generate()); + return await _userProfileService.SaveProfileAsync(departmentId, profile, cancellationToken); + } + + public async Task EnsurePinsForDepartmentAsync(int departmentId, CancellationToken cancellationToken = default) + { + var profiles = await _userProfileService.GetAllProfilesForDepartmentAsync(departmentId, bypassCache: true); + if (profiles == null) + return; + + foreach (var profile in profiles.Values) + { + if (!string.IsNullOrWhiteSpace(profile.SecurityPin)) + continue; + + profile.SecurityPin = _encryptionService.Encrypt(SecurityPinUtility.Generate()); + await _userProfileService.SaveProfileAsync(departmentId, profile, cancellationToken); + } + } + } +} diff --git a/Core/Resgrid.Services/ServicesModule.cs b/Core/Resgrid.Services/ServicesModule.cs index f198cab6e..44dc5fbf9 100644 --- a/Core/Resgrid.Services/ServicesModule.cs +++ b/Core/Resgrid.Services/ServicesModule.cs @@ -93,6 +93,7 @@ protected override void Load(ContainerBuilder builder) builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); + builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); builder.Register(_ => diff --git a/Core/Resgrid.Services/UserProfileService.cs b/Core/Resgrid.Services/UserProfileService.cs index 147768847..f9a5bdc94 100644 --- a/Core/Resgrid.Services/UserProfileService.cs +++ b/Core/Resgrid.Services/UserProfileService.cs @@ -18,11 +18,14 @@ public class UserProfileService : IUserProfileService private readonly IUserProfilesRepository _userProfileRepository; private readonly ICacheProvider _cacheProvider; + private readonly IChatbotIdentityRepository _chatbotIdentityRepository; - public UserProfileService(IUserProfilesRepository userProfileRepository, ICacheProvider cacheProvider) + public UserProfileService(IUserProfilesRepository userProfileRepository, ICacheProvider cacheProvider, + IChatbotIdentityRepository chatbotIdentityRepository) { _userProfileRepository = userProfileRepository; _cacheProvider = cacheProvider; + _chatbotIdentityRepository = chatbotIdentityRepository; } public async Task GetProfileByUserIdAsync(string userId, bool bypassCache = false) @@ -75,6 +78,12 @@ public async Task> GetAllProfilesForDepartmentIn // Load existing profile directly from repository (bypass cache) to detect contact changes var existing = await _userProfileRepository.GetProfileByUserIdAsync(profile.UserId); + // A null SecurityPin means "not supplied by this caller" — keep the stored (encrypted) PIN + // so profile saves from flows that don't know about PINs can't silently wipe it. Clearing a + // PIN intentionally is done by saving an empty string. + if (existing != null && profile.SecurityPin == null && existing.SecurityPin != null) + profile.SecurityPin = existing.SecurityPin; + if (existing == null) { // Brand-new profile (admin-created user) — mark all contact methods as pending @@ -96,6 +105,11 @@ public async Task> GetAllProfilesForDepartmentIn profile.MobileVerificationVoiceCodeConsumed = false; profile.MobileVerificationAttempts = 0; profile.MobileVerificationAttemptsResetDate = null; + + // The old number no longer identifies this user: remove any SMS chatbot identity + // links so inbound texts can't act as this account until the new number is verified + // and re-linked. + await RemoveSmsChatbotIdentitiesAsync(profile.UserId, cancellationToken); } if (!string.Equals(existing.HomeNumber ?? string.Empty, profile.HomeNumber ?? string.Empty, StringComparison.OrdinalIgnoreCase)) { @@ -125,6 +139,19 @@ public async Task> GetAllProfilesForDepartmentIn return savedProfile; } + private async Task RemoveSmsChatbotIdentitiesAsync(string userId, CancellationToken cancellationToken) + { + var identities = await _chatbotIdentityRepository.GetAllByUserIdAsync(userId); + if (identities == null) + return; + + foreach (var identity in identities.Where(i => + i.Platform == ChatbotIdentity.PlatformSmsTwilio || i.Platform == ChatbotIdentity.PlatformSmsSignalWire)) + { + await _chatbotIdentityRepository.DeleteAsync(identity, cancellationToken); + } + } + public void ClearUserProfileFromCache(string userId) { _cacheProvider.Remove(string.Format(CacheKey, userId)); diff --git a/Providers/Resgrid.Providers.Migrations/Migrations/M0089_AddingUserSecurityPin.cs b/Providers/Resgrid.Providers.Migrations/Migrations/M0089_AddingUserSecurityPin.cs new file mode 100644 index 000000000..f58aec67f --- /dev/null +++ b/Providers/Resgrid.Providers.Migrations/Migrations/M0089_AddingUserSecurityPin.cs @@ -0,0 +1,21 @@ +using FluentMigrator; + +namespace Resgrid.Providers.Migrations.Migrations +{ + [Migration(89)] + public class M0089_AddingUserSecurityPin : Migration + { + public override void Up() + { + Alter.Table("UserProfiles") + .AddColumn("SecurityPin").AsString(512).Nullable() + .AddColumn("SecurityPinEnabled").AsBoolean().NotNullable().WithDefaultValue(false); + } + + public override void Down() + { + Delete.Column("SecurityPin").FromTable("UserProfiles"); + Delete.Column("SecurityPinEnabled").FromTable("UserProfiles"); + } + } +} diff --git a/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0089_AddingUserSecurityPinPg.cs b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0089_AddingUserSecurityPinPg.cs new file mode 100644 index 000000000..e97a36ba1 --- /dev/null +++ b/Providers/Resgrid.Providers.MigrationsPg/Migrations/M0089_AddingUserSecurityPinPg.cs @@ -0,0 +1,21 @@ +using FluentMigrator; + +namespace Resgrid.Providers.MigrationsPg.Migrations +{ + [Migration(89)] + public class M0089_AddingUserSecurityPinPg : Migration + { + public override void Up() + { + Alter.Table("UserProfiles".ToLower()) + .AddColumn("SecurityPin".ToLower()).AsCustom("citext").Nullable() + .AddColumn("SecurityPinEnabled".ToLower()).AsBoolean().NotNullable().WithDefaultValue(false); + } + + public override void Down() + { + Delete.Column("SecurityPin".ToLower()).FromTable("UserProfiles".ToLower()); + Delete.Column("SecurityPinEnabled".ToLower()).FromTable("UserProfiles".ToLower()); + } + } +} diff --git a/Repositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.cs b/Repositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.cs index 18ed1e97d..e085e84c0 100644 --- a/Repositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.cs +++ b/Repositories/Resgrid.Repositories.DataRepository/Modules/TestingDataModule.cs @@ -189,6 +189,7 @@ protected override void Load(ContainerBuilder builder) builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); builder.RegisterType().As().InstancePerLifetimeScope(); + builder.RegisterType().As().InstancePerLifetimeScope(); // Indoor Maps Repositories builder.RegisterType().As().InstancePerLifetimeScope(); diff --git a/Tests/Resgrid.Tests/Services/SecurityPinServiceTests.cs b/Tests/Resgrid.Tests/Services/SecurityPinServiceTests.cs new file mode 100644 index 000000000..907d26376 --- /dev/null +++ b/Tests/Resgrid.Tests/Services/SecurityPinServiceTests.cs @@ -0,0 +1,255 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Moq; +using NUnit.Framework; +using Resgrid.Framework; +using Resgrid.Model; +using Resgrid.Model.Services; +using Resgrid.Services; + +namespace Resgrid.Tests.Services +{ + namespace SecurityPinUtilityTests + { + [TestFixture] + public class when_validating_pin_format + { + [TestCase("2580", ExpectedResult = true)] + [TestCase("7392", ExpectedResult = true)] + [TestCase("123", ExpectedResult = false)] + [TestCase("12345", ExpectedResult = false)] + [TestCase("12a4", ExpectedResult = false)] + [TestCase("", ExpectedResult = false)] + [TestCase(null, ExpectedResult = false)] + [TestCase(" 123", ExpectedResult = false)] + public bool validates_format(string pin) => SecurityPinUtility.IsValidFormat(pin); + } + + [TestFixture] + public class when_checking_pin_strength + { + [TestCase("0000")] + [TestCase("1111")] + [TestCase("9999")] + [TestCase("1234")] + [TestCase("0123")] + [TestCase("6789")] + [TestCase("4321")] + [TestCase("9876")] + [TestCase("3210")] + public void rejects_weak_pins(string pin) + { + SecurityPinUtility.IsWeak(pin).Should().BeTrue(); + SecurityPinUtility.IsAcceptable(pin).Should().BeFalse(); + } + + [TestCase("2580")] + [TestCase("1357")] + [TestCase("7392")] + [TestCase("1233")] + [TestCase("1235")] + public void accepts_normal_pins(string pin) + { + SecurityPinUtility.IsWeak(pin).Should().BeFalse(); + SecurityPinUtility.IsAcceptable(pin).Should().BeTrue(); + } + } + + [TestFixture] + public class when_generating_a_pin + { + [Test] + public void generated_pins_are_always_acceptable() + { + for (int i = 0; i < 250; i++) + { + var pin = SecurityPinUtility.Generate(); + SecurityPinUtility.IsAcceptable(pin).Should().BeTrue($"generated pin '{pin}' should be acceptable"); + } + } + } + } + + namespace SecurityPinServiceTests + { + public class with_the_security_pin_service : TestBase + { + protected Mock _userProfileServiceMock; + protected Mock _departmentSettingsServiceMock; + protected Mock _encryptionServiceMock; + protected ISecurityPinService _securityPinService; + + protected const string UserId = "user1"; + protected const int DepartmentId = 42; + + [SetUp] + public void SetUpSecurityPinService() + { + _userProfileServiceMock = new Mock(); + _departmentSettingsServiceMock = new Mock(); + + // Passthrough encryption mock: Encrypt wraps with "ENC:" so tests can verify the + // persisted value is transformed, and Decrypt strips it back. + _encryptionServiceMock = new Mock(); + _encryptionServiceMock + .Setup(e => e.Encrypt(It.IsAny())) + .Returns(plain => "ENC:" + plain); + _encryptionServiceMock + .Setup(e => e.Decrypt(It.IsAny())) + .Returns(cipher => cipher.StartsWith("ENC:") ? cipher.Substring(4) : cipher); + + _securityPinService = new SecurityPinService( + _userProfileServiceMock.Object, + _departmentSettingsServiceMock.Object, + _encryptionServiceMock.Object); + } + + protected void SetupProfile(UserProfile profile) + { + _userProfileServiceMock + .Setup(s => s.GetProfileByUserIdAsync(UserId, It.IsAny())) + .ReturnsAsync(profile); + } + + protected void SetupForcePin(bool forced) + { + _departmentSettingsServiceMock + .Setup(s => s.GetForceChatbotSecurityPinAsync(DepartmentId, It.IsAny())) + .ReturnsAsync(forced); + } + } + + [TestFixture] + public class when_checking_if_a_pin_is_required : with_the_security_pin_service + { + [Test] + public async Task required_when_department_forces_pins() + { + SetupForcePin(true); + SetupProfile(new UserProfile { UserId = UserId, SecurityPinEnabled = false }); + + (await _securityPinService.IsPinRequiredAsync(UserId, DepartmentId)).Should().BeTrue(); + } + + [Test] + public async Task required_when_the_user_opted_in() + { + SetupForcePin(false); + SetupProfile(new UserProfile { UserId = UserId, SecurityPinEnabled = true }); + + (await _securityPinService.IsPinRequiredAsync(UserId, DepartmentId)).Should().BeTrue(); + } + + [Test] + public async Task not_required_when_neither_forced_nor_opted_in() + { + SetupForcePin(false); + SetupProfile(new UserProfile { UserId = UserId, SecurityPinEnabled = false }); + + (await _securityPinService.IsPinRequiredAsync(UserId, DepartmentId)).Should().BeFalse(); + } + + [Test] + public async Task not_required_when_no_profile_exists_and_not_forced() + { + SetupForcePin(false); + SetupProfile(null); + + (await _securityPinService.IsPinRequiredAsync(UserId, DepartmentId)).Should().BeFalse(); + } + } + + [TestFixture] + public class when_validating_a_pin : with_the_security_pin_service + { + [Test] + public async Task correct_pin_validates() + { + SetupProfile(new UserProfile { UserId = UserId, SecurityPin = "ENC:2580" }); + + (await _securityPinService.ValidatePinAsync(UserId, "2580")).Should().BeTrue(); + } + + [Test] + public async Task wrong_pin_fails() + { + SetupProfile(new UserProfile { UserId = UserId, SecurityPin = "ENC:2580" }); + + (await _securityPinService.ValidatePinAsync(UserId, "2581")).Should().BeFalse(); + } + + [Test] + public async Task malformed_pin_fails_without_touching_the_store() + { + (await _securityPinService.ValidatePinAsync(UserId, "not-a-pin")).Should().BeFalse(); + _userProfileServiceMock.Verify(s => s.GetProfileByUserIdAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + [Test] + public async Task fails_when_no_pin_is_set() + { + SetupProfile(new UserProfile { UserId = UserId, SecurityPin = null }); + + (await _securityPinService.ValidatePinAsync(UserId, "2580")).Should().BeFalse(); + } + } + + [TestFixture] + public class when_ensuring_a_pin : with_the_security_pin_service + { + [Test] + public async Task generates_and_saves_a_pin_when_missing() + { + var profile = new UserProfile { UserId = UserId }; + SetupProfile(profile); + _userProfileServiceMock + .Setup(s => s.SaveProfileAsync(DepartmentId, It.IsAny(), It.IsAny())) + .ReturnsAsync((int d, UserProfile p, CancellationToken c) => p); + + var saved = await _securityPinService.EnsurePinAsync(UserId, DepartmentId); + + saved.SecurityPin.Should().StartWith("ENC:"); + SecurityPinUtility.IsAcceptable(saved.SecurityPin.Substring(4)).Should().BeTrue(); + _userProfileServiceMock.Verify(s => s.SaveProfileAsync(DepartmentId, profile, It.IsAny()), Times.Once); + } + + [Test] + public async Task keeps_the_existing_pin() + { + SetupProfile(new UserProfile { UserId = UserId, SecurityPin = "ENC:2580" }); + + var saved = await _securityPinService.EnsurePinAsync(UserId, DepartmentId); + + saved.SecurityPin.Should().Be("ENC:2580"); + _userProfileServiceMock.Verify(s => s.SaveProfileAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + } + + [TestFixture] + public class when_ensuring_pins_for_a_department : with_the_security_pin_service + { + [Test] + public async Task only_members_without_a_pin_get_one() + { + var withPin = new UserProfile { UserId = "hasPin", SecurityPin = "ENC:2580" }; + var withoutPin = new UserProfile { UserId = "noPin" }; + + _userProfileServiceMock + .Setup(s => s.GetAllProfilesForDepartmentAsync(DepartmentId, It.IsAny())) + .ReturnsAsync(new Dictionary { { withPin.UserId, withPin }, { withoutPin.UserId, withoutPin } }); + _userProfileServiceMock + .Setup(s => s.SaveProfileAsync(DepartmentId, It.IsAny(), It.IsAny())) + .ReturnsAsync((int d, UserProfile p, CancellationToken c) => p); + + await _securityPinService.EnsurePinsForDepartmentAsync(DepartmentId); + + withPin.SecurityPin.Should().Be("ENC:2580"); + withoutPin.SecurityPin.Should().StartWith("ENC:"); + _userProfileServiceMock.Verify(s => s.SaveProfileAsync(DepartmentId, withoutPin, It.IsAny()), Times.Once); + _userProfileServiceMock.Verify(s => s.SaveProfileAsync(DepartmentId, withPin, It.IsAny()), Times.Never); + } + } + } +} diff --git a/Web/Resgrid.Web.Services/Controllers/SignalWireController.cs b/Web/Resgrid.Web.Services/Controllers/SignalWireController.cs index e0f593e7e..c52a30885 100644 --- a/Web/Resgrid.Web.Services/Controllers/SignalWireController.cs +++ b/Web/Resgrid.Web.Services/Controllers/SignalWireController.cs @@ -175,6 +175,14 @@ public async Task Receive(CancellationToken cancellationToken) { profile = await _userProfileService.GetProfileByMobileNumberAsync(textMessage.Msisdn); + // SECURITY: an unverified mobile number must never identify/act as the matched user. + // Verification (MobileNumberVerified == true) is the bar for trusting an inbound sender. + if (profile != null && profile.MobileNumberVerified != true) + { + Framework.Logging.LogInfo($"[SignalWire SMS] MessageSid={textMessage.MessageId} From={textMessage.Msisdn} matched a profile but the mobile number is not verified; not linking sender to user."); + profile = null; + } + if (profile != null) { var department = await _departmentsService.GetDepartmentByUserIdAsync(profile.UserId); diff --git a/Web/Resgrid.Web.Services/Controllers/TwilioController.cs b/Web/Resgrid.Web.Services/Controllers/TwilioController.cs index 2eb185ce0..10c22cd1b 100644 --- a/Web/Resgrid.Web.Services/Controllers/TwilioController.cs +++ b/Web/Resgrid.Web.Services/Controllers/TwilioController.cs @@ -142,6 +142,17 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques // department the sender operates in. Legacy clients that still text a per-department provisioned // inbound number fall back to resolving the department from that number. UserProfile userProfile = await _userProfileService.GetProfileByMobileNumberAsync(textMessage.Msisdn); + + // SECURITY: an unverified mobile number must never identify/act as the matched user — + // anyone can type someone else's number into a profile. Verification (MobileNumberVerified + // == true) is the bar for trusting an inbound sender. + bool senderNumberUnverified = userProfile != null && userProfile.MobileNumberVerified != true; + if (senderNumberUnverified) + { + Framework.Logging.LogInfo($"[Twilio SMS] MessageSid={request.MessageSid} From={textMessage.Msisdn} matched a profile but the mobile number is not verified; not linking sender to user."); + userProfile = null; + } + int? departmentId = null; if (userProfile != null) { @@ -194,7 +205,7 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques } else { - await ProcessTextCommandsAsync(textMessage, messageEvent, response, departmentId, userProfile); + await ProcessTextCommandsAsync(textMessage, messageEvent, response, departmentId, userProfile, senderNumberUnverified); } } catch (Exception ex) @@ -225,7 +236,7 @@ public async Task IncomingMessage([FromQuery] TwilioMessage reques // enabled the Chatbot Twilio integration feature flag keep their existing behavior. departmentId // and userProfile are resolved by the caller so the flag can be evaluated before dispatching here. private async System.Threading.Tasks.Task ProcessTextCommandsAsync(TextMessage textMessage, InboundMessageEvent messageEvent, - MessagingResponse response, int? departmentId, UserProfile userProfile) + MessagingResponse response, int? departmentId, UserProfile userProfile, bool senderNumberUnverified = false) { // Diagnostic: without a department the legacy path adds no message, so the sender gets no reply. if (!departmentId.HasValue) @@ -300,13 +311,25 @@ 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); + // SECURITY: same verified-number bar as the caller — an unverified number must not + // act as the matched user. + if (profile != null && profile.MobileNumberVerified != true) + { + senderNumberUnverified = true; + profile = null; + } + // No matching user profile for the sender's number: reply so the user knows, rather than // returning an empty response (pre-refactor behavior). if (profile == null) { - Framework.Logging.LogInfo($"[Twilio SMS] DepartmentId={departmentId.Value} sender {textMessage.Msisdn} has no matching user profile; replying with not-found message."); + Framework.Logging.LogInfo($"[Twilio SMS] DepartmentId={departmentId.Value} sender {textMessage.Msisdn} has no usable user profile (unverified={senderNumberUnverified}); replying with not-found message."); messageEvent.Processed = true; - response.Message("Resgrid: We couldn't find a Resgrid user linked to this mobile number. Please add this number to your Resgrid profile to use text commands."); + + if (senderNumberUnverified) + response.Message("Resgrid: This mobile number matches a Resgrid profile but hasn't been verified. Please verify your mobile number on your Resgrid profile page to use text commands."); + else + response.Message("Resgrid: We couldn't find a Resgrid user linked to this mobile number. Please add this number to your Resgrid profile to use text commands."); } if (profile != null) diff --git a/Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs b/Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs index fae2037d4..ce600be6a 100644 --- a/Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs +++ b/Web/Resgrid.Web/Areas/User/Controllers/DepartmentController.cs @@ -63,6 +63,7 @@ public class DepartmentController : SecureBaseController private readonly INotesService _notesService; private readonly IContactsService _contactsService; private readonly ICheckInTimerService _checkInTimerService; + private readonly ISecurityPinService _securityPinService; public DepartmentController(IDepartmentsService departmentsService, IUsersService usersService, IActionLogsService actionLogsService, IEmailService emailService, IDepartmentGroupsService departmentGroupsService, IUserProfileService userProfileService, IDeleteService deleteService, @@ -70,7 +71,8 @@ public DepartmentController(IDepartmentsService departmentsService, IUsersServic ILimitsService limitsService, ICallsService callsService, IDepartmentSettingsService departmentSettingsService, IUnitsService unitsService, ICertificationService certificationService, INumbersService numbersService, IScheduledTasksService scheduledTasksService, IPersonnelRolesService personnelRolesService, IEventAggregator eventAggregator, ICustomStateService customStateService, ICqrsProvider cqrsProvider, IPrinterProvider printerProvider, IQueueService queueService, - IDocumentsService documentsService, INotesService notesService, IContactsService contactsService, ICheckInTimerService checkInTimerService) + IDocumentsService documentsService, INotesService notesService, IContactsService contactsService, ICheckInTimerService checkInTimerService, + ISecurityPinService securityPinService) { _departmentsService = departmentsService; _usersService = usersService; @@ -100,6 +102,7 @@ public DepartmentController(IDepartmentsService departmentsService, IUsersServic _notesService = notesService; _contactsService = contactsService; _checkInTimerService = checkInTimerService; + _securityPinService = securityPinService; } #endregion Private Members and Constructors @@ -244,6 +247,7 @@ public async Task Settings() var activeCallRssKey = await _departmentSettingsService.GetRssKeyForDepartmentAsync(DepartmentId); model.DisableAutoAvailable = await _departmentSettingsService.GetDisableAutoAvailableForDepartmentAsync(DepartmentId); model.EnableModernNotifications = await _departmentSettingsService.GetModernNotificationsEnabledAsync(DepartmentId); + model.ForceChatbotSecurityPin = await _departmentSettingsService.GetForceChatbotSecurityPinAsync(DepartmentId); model.TtsLanguage = await _departmentSettingsService.GetTtsLanguageForDepartmentAsync(DepartmentId); model.TtsLanguages = BuildTtsLanguageSelectList(model.TtsLanguage); @@ -541,6 +545,15 @@ await _departmentSettingsService.SaveOrUpdateSettingAsync(DepartmentId, model.Di cancellationToken); await _departmentSettingsService.SaveOrUpdateSettingAsync(DepartmentId, model.EnableModernNotifications.ToString(), DepartmentSettingTypes.EnableModernNotifications, cancellationToken); + + var forcePinWasEnabled = await _departmentSettingsService.GetForceChatbotSecurityPinAsync(DepartmentId, bypassCache: true); + await _departmentSettingsService.SaveOrUpdateSettingAsync(DepartmentId, model.ForceChatbotSecurityPin.ToString(), DepartmentSettingTypes.ForceChatbotSecurityPin, + cancellationToken); + + // Turning the forced-PIN requirement on: give every member without a PIN a random one so + // they can complete PIN-protected chatbot/text actions immediately. + if (model.ForceChatbotSecurityPin && !forcePinWasEnabled) + await _securityPinService.EnsurePinsForDepartmentAsync(DepartmentId, cancellationToken); await _departmentSettingsService.SaveOrUpdateSettingAsync(DepartmentId, model.TtsLanguage, DepartmentSettingTypes.TtsLanguage, cancellationToken); diff --git a/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs b/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs index 2c6812ce0..e3da5b95d 100644 --- a/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs +++ b/Web/Resgrid.Web/Areas/User/Controllers/HomeController.cs @@ -71,6 +71,8 @@ public class HomeController : SecureBaseController private readonly IGdprDataExportService _gdprDataExportService; private readonly ISystemAuditsService _systemAuditsService; private readonly IPhoneNumberProcesserProvider _phoneNumberProcesser; + private readonly ISecurityPinService _securityPinService; + private readonly IEncryptionService _encryptionService; public HomeController(IDepartmentsService departmentsService, IUsersService usersService, IActionLogsService actionLogsService, IUserStateService userStateService, IDepartmentGroupsService departmentGroupsService, Resgrid.Model.Services.IAuthorizationService authorizationService, @@ -80,7 +82,8 @@ public HomeController(IDepartmentsService departmentsService, IUsersService user IStringLocalizerFactory factory, ISubscriptionsService subscriptionsService, IContactVerificationService contactVerificationService, IUserDefinedFieldsService userDefinedFieldsService, IUdfRenderingService udfRenderingService, IDepartmentSsoService departmentSsoService, IStringLocalizer secLocalizer, IGdprDataExportService gdprDataExportService, - ISystemAuditsService systemAuditsService, IPhoneNumberProcesserProvider phoneNumberProcesser) + ISystemAuditsService systemAuditsService, IPhoneNumberProcesserProvider phoneNumberProcesser, + ISecurityPinService securityPinService, IEncryptionService encryptionService) { _departmentsService = departmentsService; _usersService = usersService; @@ -110,6 +113,8 @@ public HomeController(IDepartmentsService departmentsService, IUsersService user _gdprDataExportService = gdprDataExportService; _systemAuditsService = systemAuditsService; _phoneNumberProcesser = phoneNumberProcesser; + _securityPinService = securityPinService; + _encryptionService = encryptionService; _localizer = factory.Create("Home.Dashboard", new AssemblyName(typeof(SupportedLocales).GetTypeInfo().Assembly.FullName).Name); } @@ -411,6 +416,14 @@ public async Task EditUserProfile(string userId) if (model.Profile == null) model.Profile = new UserProfile(); + // Security PIN is only shown to the profile's owner (never to admins editing another user). + model.DepartmentForcesSecurityPin = await _departmentSettingsService.GetForceChatbotSecurityPinAsync(DepartmentId); + if (model.IsOwnProfile) + { + model.SecurityPinEnabled = model.Profile.SecurityPinEnabled; + model.SecurityPin = await _securityPinService.GetPinAsync(userId); + } + if (model.Profile.Image == null) model.HasCustomIamge = false; else @@ -680,6 +693,14 @@ public async Task EditUserProfile(EditProfileModel model, IFormCo if (newUser != null) ModelState.AddModelError("", "The NEW username you have supplied is already in use, please try another one. If you didn't mean to update your username please leave that field blank."); } + + if (!String.IsNullOrWhiteSpace(model.SecurityPin)) + { + if (!SecurityPinUtility.IsValidFormat(model.SecurityPin)) + ModelState.AddModelError("SecurityPin", "The security PIN must be exactly 4 digits."); + else if (SecurityPinUtility.IsWeak(model.SecurityPin)) + ModelState.AddModelError("SecurityPin", "That security PIN is too easy to guess. Avoid repeated digits (like 0000) and sequences (like 1234 or 4321)."); + } } if (ModelState.IsValid) @@ -727,6 +748,18 @@ public async Task EditUserProfile(EditProfileModel model, IFormCo savedProfile.TimeZone = model.Profile.TimeZone; savedProfile.Language = model.Profile.Language; + // Security PIN: only the profile owner can manage it. A blank input keeps the current + // PIN; enabling the option with no PIN on file generates a random one. + if (model.IsOwnProfile) + { + savedProfile.SecurityPinEnabled = model.SecurityPinEnabled; + + if (!String.IsNullOrWhiteSpace(model.SecurityPin)) + savedProfile.SecurityPin = _encryptionService.Encrypt(model.SecurityPin.Trim()); + else if (model.SecurityPinEnabled && String.IsNullOrWhiteSpace(savedProfile.SecurityPin)) + savedProfile.SecurityPin = _encryptionService.Encrypt(SecurityPinUtility.Generate()); + } + if (model.CanEnableVoice) { savedProfile.VoiceForCall = model.Profile.VoiceForCall; diff --git a/Web/Resgrid.Web/Areas/User/Models/DepartmentSettingsModel.cs b/Web/Resgrid.Web/Areas/User/Models/DepartmentSettingsModel.cs index 0d87a7195..b58f8f9b5 100644 --- a/Web/Resgrid.Web/Areas/User/Models/DepartmentSettingsModel.cs +++ b/Web/Resgrid.Web/Areas/User/Models/DepartmentSettingsModel.cs @@ -68,6 +68,9 @@ public class DepartmentSettingsModel : BaseUserModel public bool EnableModernNotifications { get; set; } + [Display(Name = "Require security PIN for dangerous chatbot/text actions")] + public bool ForceChatbotSecurityPin { get; set; } + public DepartmentSettingsModel() { Users = new Dictionary(); diff --git a/Web/Resgrid.Web/Areas/User/Models/EditProfileModel.cs b/Web/Resgrid.Web/Areas/User/Models/EditProfileModel.cs index 643ee976b..34c98245d 100644 --- a/Web/Resgrid.Web/Areas/User/Models/EditProfileModel.cs +++ b/Web/Resgrid.Web/Areas/User/Models/EditProfileModel.cs @@ -111,6 +111,18 @@ public class EditProfileModel: BaseUserModel public bool EnableSms { get; set; } + // ── Security PIN (step-up auth for dangerous chatbot/SMS actions; own profile only) ── + + /// The user's decrypted 4-digit security PIN (only populated for the own-profile view). + [Display(Name = "Security PIN")] + public string SecurityPin { get; set; } + + [Display(Name = "Require my security PIN for dangerous chatbot/text actions")] + public bool SecurityPinEnabled { get; set; } + + /// True when the department forces PIN usage for all members (personal opt-in is then moot). + public bool DepartmentForcesSecurityPin { get; set; } + // ── Contact verification status (tri-state: null = grandfathered, false = pending, true = verified) ── public bool? EmailVerified => Profile?.EmailVerified; public bool? MobileNumberVerified => Profile?.MobileNumberVerified; diff --git a/Web/Resgrid.Web/Areas/User/Views/Department/Settings.cshtml b/Web/Resgrid.Web/Areas/User/Views/Department/Settings.cshtml index 2f9de053b..d0dcacbc6 100644 --- a/Web/Resgrid.Web/Areas/User/Views/Department/Settings.cshtml +++ b/Web/Resgrid.Web/Areas/User/Views/Department/Settings.cshtml @@ -91,6 +91,17 @@ @localizer["EnableModernNotificationsHelp"] +
+ +
+
+
+ +
+
+ Require every member to confirm dangerous or department-wide chatbot/text actions with their personal 4-digit security PIN. Members without a PIN get a randomly generated one (viewable on their profile page). +
+
diff --git a/Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml b/Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml index 2957b9b15..d7d30fdb4 100644 --- a/Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml +++ b/Web/Resgrid.Web/Areas/User/Views/Home/EditUserProfile.cshtml @@ -296,6 +296,35 @@
+ @if (Model.IsOwnProfile) + { +
+ +
+ + + Your 4-digit security PIN confirms dangerous or department-wide actions performed over text message or chatbot. Avoid repeated digits (0000) and sequences (1234, 4321). Leave blank to keep your current PIN. +
+
+
+ +
+ @if (Model.DepartmentForcesSecurityPin) + { + + + Your department requires a security PIN for dangerous chatbot/text actions, so this option is always on. + } + else + { + + When enabled, dangerous chatbot/text actions ask for your PIN as an extra security step. If you enable this without a PIN a random one is generated for you. + } +
+
+
+ } + @if (ClaimsAuthorizationHelper.IsUserDepartmentAdmin()) {
From 4fe1e17d783ec8a5d0c775ffb57022169451a8ab Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Thu, 16 Jul 2026 08:19:50 -0700 Subject: [PATCH 2/2] RG-T117 PR#426 fixes --- .../Services/ChatbotIngressService.cs | 86 +++++++++---------- Core/Resgrid.Framework/StringHelpers.cs | 16 ++++ .../Areas/User/Department/Department.ar.resx | 10 ++- .../Areas/User/Department/Department.de.resx | 10 ++- .../Areas/User/Department/Department.en.resx | 8 +- .../Areas/User/Department/Department.es.resx | 6 ++ .../Areas/User/Department/Department.fr.resx | 10 ++- .../Areas/User/Department/Department.it.resx | 10 ++- .../Areas/User/Department/Department.pl.resx | 10 ++- .../Areas/User/Department/Department.resx | 8 +- .../Areas/User/Department/Department.sv.resx | 10 ++- .../Areas/User/Department/Department.uk.resx | 10 ++- .../Areas/User/Home/EditProfile.ar.resx | 22 ++++- .../Areas/User/Home/EditProfile.de.resx | 20 ++++- .../Areas/User/Home/EditProfile.en.resx | 20 ++++- .../Areas/User/Home/EditProfile.es.resx | 20 ++++- .../Areas/User/Home/EditProfile.fr.resx | 20 ++++- .../Areas/User/Home/EditProfile.it.resx | 20 ++++- .../Areas/User/Home/EditProfile.pl.resx | 20 ++++- .../Areas/User/Home/EditProfile.sv.resx | 20 ++++- .../Areas/User/Home/EditProfile.uk.resx | 20 ++++- .../Repositories/IUserProfilesRepository.cs | 10 +++ Core/Resgrid.Services/SecurityPinService.cs | 32 +++++-- .../UserProfilesRepository.cs | 51 +++++++++++ .../Services/SecurityPinServiceTests.cs | 46 ++++++++-- .../Controllers/SignalWireController.cs | 2 +- .../Controllers/TwilioController.cs | 8 +- .../User/Controllers/DepartmentController.cs | 9 +- .../Areas/User/Controllers/HomeController.cs | 3 + .../User/Views/Department/Settings.cshtml | 4 +- .../User/Views/Home/EditUserProfile.cshtml | 12 +-- 31 files changed, 452 insertions(+), 101 deletions(-) diff --git a/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs b/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs index 855dfc731..4d134c444 100644 --- a/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs +++ b/Core/Resgrid.Chatbot/Services/ChatbotIngressService.cs @@ -241,15 +241,33 @@ await _userIdentityService.LinkUserAsync( // before it executes. if (await _securityPinService.IsPinRequiredAsync(identity.UserId, department.DepartmentId)) { + // Provision a missing PIN BEFORE persisting the awaiting-PIN state: if + // provisioning fails (no profile, or a thrown error caught by the outer + // handler) the session stays parked in AwaitingConfirmation instead of + // being stranded awaiting a PIN that doesn't exist. + bool pinGenerated = false; + if (!await _securityPinService.HasPinAsync(identity.UserId)) + { + var provisioned = await _securityPinService.EnsurePinAsync(identity.UserId, department.DepartmentId); + if (provisioned == null || string.IsNullOrWhiteSpace(provisioned.SecurityPin)) + { + return new ChatbotResponse + { + Text = "This action requires a security PIN, but one couldn't be set up for your account. Please set a PIN on your Resgrid profile page, then reply YES again.", + Processed = true + }; + } + pinGenerated = true; + } + session.State = ChatbotDialogState.AwaitingPin; session.Context["__pinAttempts"] = "0"; await _sessionManager.SaveSessionAsync(session); - if (!await _securityPinService.HasPinAsync(identity.UserId)) + if (pinGenerated) { - // Department forces PINs but this user doesn't have one yet — generate one - // so they can look it up on their profile page and complete the action. - await _securityPinService.EnsurePinAsync(identity.UserId, department.DepartmentId); + // Department forces PINs but this user didn't have one yet — one was + // generated so they can look it up on their profile page and continue. return new ChatbotResponse { Text = "This action requires your security PIN. A PIN has been generated for you — view or change it on your Resgrid profile page, then reply with the 4-digit PIN to continue, or NO to cancel.", @@ -260,37 +278,7 @@ await _userIdentityService.LinkUserAsync( return new ChatbotResponse { Text = "For security, reply with your 4-digit security PIN to complete this action, or NO to cancel.", Processed = true }; } - var confirmResponse = await DispatchConfirmedIntentAsync(message, session); - if (confirmResponse != null) - return confirmResponse; - var pendingType = session.PendingIntent.Value; - - // Locate the owning handler BEFORE mutating session state. If none can handle - // the pending intent, leave the session parked in AwaitingConfirmation and return - // an explicit error rather than silently dropping the confirmation. - var confirmHandler = _actionHandlers.FirstOrDefault(h => h.CanHandle(pendingType)); - if (confirmHandler == null) - { - return new ChatbotResponse - { - Text = "Unable to complete confirmation — please try again or contact support.", - Processed = false - }; - } - - var confirmedIntent = new ChatbotIntent - { - Type = pendingType, - Parameters = new Dictionary(session.Context) { ["__confirmed"] = "true" } - }; - - var confirmResponse = await confirmHandler.HandleAsync(message, confirmedIntent, session); - confirmResponse.Intent = confirmedIntent; - session.Context.Clear(); - session.State = ChatbotDialogState.Idle; - session.PendingIntent = null; - await _sessionManager.SaveSessionAsync(session); - return confirmResponse; + return await DispatchConfirmedIntentAsync(message, session); } else if (reply == "NO" || reply == "N" || reply == "CANCEL") { @@ -323,9 +311,7 @@ await _userIdentityService.LinkUserAsync( if (await _securityPinService.ValidatePinAsync(identity.UserId, pinReply)) { session.Context.Remove("__pinAttempts"); - var confirmResponse = await DispatchConfirmedIntentAsync(message, session); - if (confirmResponse != null) - return confirmResponse; + return await DispatchConfirmedIntentAsync(message, session); } else { @@ -448,27 +434,35 @@ private static bool IsNegative(string text) /// /// Executes the pending (already confirmed, and PIN-verified when required) destructive intent. - /// Returns the handler's response, or null when no handler claims the intent (the caller then - /// falls through to normal classification, matching the original confirmation-gate behavior). + /// The owning handler is located BEFORE any session state is mutated: if none can handle the + /// pending intent the session stays parked and an explicit error is returned rather than + /// silently dropping the confirmation. /// private async Task DispatchConfirmedIntentAsync(ChatbotMessage message, ChatbotSession session) { var pendingType = session.PendingIntent.Value; + + var confirmHandler = _actionHandlers.FirstOrDefault(h => h.CanHandle(pendingType)); + if (confirmHandler == null) + { + return new ChatbotResponse + { + Text = "Unable to complete confirmation — please try again or contact support.", + Processed = false + }; + } + var confirmedIntent = new ChatbotIntent { Type = pendingType, Parameters = new Dictionary(session.Context) { ["__confirmed"] = "true" } }; - session.State = ChatbotDialogState.Idle; - session.PendingIntent = null; - - var confirmHandler = _actionHandlers.FirstOrDefault(h => h.CanHandle(pendingType)); - if (confirmHandler == null) - return null; var confirmResponse = await confirmHandler.HandleAsync(message, confirmedIntent, session); confirmResponse.Intent = confirmedIntent; session.Context.Clear(); + session.State = ChatbotDialogState.Idle; + session.PendingIntent = null; await _sessionManager.SaveSessionAsync(session); return confirmResponse; } diff --git a/Core/Resgrid.Framework/StringHelpers.cs b/Core/Resgrid.Framework/StringHelpers.cs index 872be8ce6..5caee30e6 100644 --- a/Core/Resgrid.Framework/StringHelpers.cs +++ b/Core/Resgrid.Framework/StringHelpers.cs @@ -21,6 +21,22 @@ public sealed record PasswordComplexityResult( public static class StringHelpers { + /// + /// Masks a phone number for diagnostic logging (PII redaction), keeping only the last + /// four digits (e.g. "***4567"). Values with four or fewer characters are fully masked. + /// + public static string MaskPhoneNumber(string number) + { + if (string.IsNullOrWhiteSpace(number)) + return "***"; + + var trimmed = number.Trim(); + if (trimmed.Length <= 4) + return "***"; + + return "***" + trimmed.Substring(trimmed.Length - 4); + } + /// /// Remove HTML tags from string using char array. /// diff --git a/Core/Resgrid.Localization/Areas/User/Department/Department.ar.resx b/Core/Resgrid.Localization/Areas/User/Department/Department.ar.resx index d931429c1..b101355df 100644 --- a/Core/Resgrid.Localization/Areas/User/Department/Department.ar.resx +++ b/Core/Resgrid.Localization/Areas/User/Department/Department.ar.resx @@ -1,4 +1,4 @@ - +