From b90fc5fea771a83e6cf576c71a307066efd59ea4 Mon Sep 17 00:00:00 2001 From: sephrat <34862846+sephrat@users.noreply.github.com> Date: Fri, 25 Feb 2022 14:18:06 +0100 Subject: [PATCH] fix(notifications): Remove generic admin email in favour of admins' email (#4519) This removes the generic admin email setting. Instead we use the email addresses set on the users' profile. Allows for notifications to many recipients in case of multiple admins. Email testing now sends the test email to the currently logged in user. --- .../Agents/EmailNotification.cs | 54 +++++++------------ src/Ombi.Notifications/BaseNotification.cs | 7 ++- src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs | 2 +- .../EmailNotificationSettings.cs | 1 - .../app/interfaces/INotificationSettings.ts | 1 - .../emailnotification.component.html | 15 ------ .../emailnotification.component.ts | 1 - .../V1/External/TesterController.cs | 12 ++++- 8 files changed, 37 insertions(+), 56 deletions(-) diff --git a/src/Ombi.Notifications/Agents/EmailNotification.cs b/src/Ombi.Notifications/Agents/EmailNotification.cs index 92ec766c3..8f6de1aa3 100644 --- a/src/Ombi.Notifications/Agents/EmailNotification.cs +++ b/src/Ombi.Notifications/Agents/EmailNotification.cs @@ -47,7 +47,7 @@ namespace Ombi.Notifications.Agents return false; } } - if (string.IsNullOrEmpty(settings.Host) || string.IsNullOrEmpty(settings.AdminEmail) || string.IsNullOrEmpty(settings.Port.ToString())) + if (string.IsNullOrEmpty(settings.Host) || string.IsNullOrEmpty(settings.Port.ToString())) { return false; } @@ -73,26 +73,6 @@ namespace Ombi.Notifications.Agents Subject = parsed.Subject, }; - if (model.Substitutes.TryGetValue("AdminComment", out var isAdminString)) - { - var isAdmin = bool.Parse(isAdminString); - if (isAdmin) - { - var user = _userManager.Users.FirstOrDefault(x => x.Id == model.UserId); - // Send to user - message.To = user.Email; - } - else - { - // Send to admin - message.To = settings.AdminEmail; - } - } - else - { - // Send to admin - message.To = settings.AdminEmail; - } return message; } @@ -138,9 +118,7 @@ namespace Ombi.Notifications.Agents message.Other.Add("PlainTextBody", plaintext); // Issues should be sent to admin - message.To = settings.AdminEmail; - - await Send(message, settings); + await SendToAdmins(message, settings); } protected override async Task IssueComment(NotificationOptions model, EmailNotificationSettings settings) @@ -154,18 +132,16 @@ namespace Ombi.Notifications.Agents var plaintext = await LoadPlainTextMessage(NotificationType.IssueComment, model, settings); message.Other.Add("PlainTextBody", plaintext); - if (model.Substitutes.TryGetValue("AdminComment", out var isAdminString)) + if (model.Substitutes.TryGetValue("AdminComment", out var isAdminString) && !bool.Parse(isAdminString)) { - var isAdmin = bool.Parse(isAdminString); - message.To = isAdmin ? model.Recipient : settings.AdminEmail; + await SendToAdmins(message, settings); } else { message.To = model.Recipient; + await Send(message, settings); } - - await Send(message, settings); } protected override async Task IssueResolved(NotificationOptions model, EmailNotificationSettings settings) @@ -204,9 +180,7 @@ namespace Ombi.Notifications.Agents var plaintext = await LoadPlainTextMessage(NotificationType.ItemAddedToFaultQueue, model, settings); message.Other.Add("PlainTextBody", plaintext); - // Issues resolved should be sent to the user - message.To = settings.AdminEmail; - await Send(message, settings); + await SendToAdmins(message, settings); } protected override async Task RequestDeclined(NotificationOptions model, EmailNotificationSettings settings) @@ -305,6 +279,19 @@ namespace Ombi.Notifications.Agents { await EmailProvider.Send(model, settings); } + + protected async Task SendToAdmins(NotificationMessage message, EmailNotificationSettings settings) + { + foreach (var recipient in (await GetAdminUsers()).DistinctBy(x => x.Email)) + { + if (recipient.Email.IsNullOrEmpty()) + { + continue; + } + message.To = recipient.Email; + await Send(message, settings); + } + } protected override async Task Test(NotificationOptions model, EmailNotificationSettings settings) { @@ -316,12 +303,11 @@ namespace Ombi.Notifications.Agents { Message = html, Subject = $"Ombi: Test", - To = settings.AdminEmail, }; message.Other.Add("PlainTextBody", "This is just a test! Success!"); - await Send(message, settings); + await SendToAdmins(message, settings); } } } diff --git a/src/Ombi.Notifications/BaseNotification.cs b/src/Ombi.Notifications/BaseNotification.cs index b52f0a0cd..97e2a676d 100644 --- a/src/Ombi.Notifications/BaseNotification.cs +++ b/src/Ombi.Notifications/BaseNotification.cs @@ -216,9 +216,14 @@ namespace Ombi.Notifications .FirstOrDefault(x => x.Agent == agent && x.UserId == userId); } - protected async Task> GetPrivilegedUsers() + protected async Task> GetAdminUsers() { IEnumerable recipients = await _userManager.GetUsersInRoleAsync(OmbiRoles.Admin); + return recipients; + } + protected async Task> GetPrivilegedUsers() + { + IEnumerable recipients = await GetAdminUsers(); recipients = recipients.Concat(await _userManager.GetUsersInRoleAsync(OmbiRoles.PowerUser)); return recipients; } diff --git a/src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs b/src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs index dbb343126..a11a9ff80 100644 --- a/src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs +++ b/src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs @@ -844,7 +844,7 @@ namespace Ombi.Schedule.Jobs.Ombi return false; } } - if (string.IsNullOrEmpty(settings.Host) || string.IsNullOrEmpty(settings.AdminEmail) || string.IsNullOrEmpty(settings.Port.ToString())) + if (string.IsNullOrEmpty(settings.Host) || string.IsNullOrEmpty(settings.Port.ToString())) { return false; } diff --git a/src/Ombi.Settings/Settings/Models/Notifications/EmailNotificationSettings.cs b/src/Ombi.Settings/Settings/Models/Notifications/EmailNotificationSettings.cs index 9ea8cc492..f63b77ae5 100644 --- a/src/Ombi.Settings/Settings/Models/Notifications/EmailNotificationSettings.cs +++ b/src/Ombi.Settings/Settings/Models/Notifications/EmailNotificationSettings.cs @@ -10,7 +10,6 @@ public string SenderAddress { get; set; } public string Username { get; set; } public bool Authentication { get; set; } - public string AdminEmail { get; set; } public bool DisableTLS { get; set; } public bool DisableCertificateChecking { get; set; } } diff --git a/src/Ombi/ClientApp/src/app/interfaces/INotificationSettings.ts b/src/Ombi/ClientApp/src/app/interfaces/INotificationSettings.ts index f2efc3f6f..460a970fe 100644 --- a/src/Ombi/ClientApp/src/app/interfaces/INotificationSettings.ts +++ b/src/Ombi/ClientApp/src/app/interfaces/INotificationSettings.ts @@ -12,7 +12,6 @@ export interface IEmailNotificationSettings extends INotificationSettings { senderName: string; username: string; authentication: boolean; - adminEmail: string; disableTLS: boolean; disableCertificateChecking: boolean; notificationTemplates: INotificationTemplates[]; diff --git a/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.html b/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.html index 5c24d21f2..7e95b0dfd 100644 --- a/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.html +++ b/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.html @@ -57,21 +57,6 @@ - -
- - Admin Email - - - Admin Email is required - - - Admin Email needs to be a valid email address - - -
- -
Username diff --git a/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.ts b/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.ts index f880891a5..7673e2e1d 100644 --- a/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.ts +++ b/src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.ts @@ -35,7 +35,6 @@ export class EmailNotificationComponent implements OnInit { senderAddress: [x.senderAddress, [Validators.required, Validators.email]], senderName: [x.senderName], username: [x.username], - adminEmail: [x.adminEmail, [Validators.required, Validators.email]], disableTLS: [x.disableTLS], disableCertificateChecking: [x.disableCertificateChecking], }); diff --git a/src/Ombi/Controllers/V1/External/TesterController.cs b/src/Ombi/Controllers/V1/External/TesterController.cs index 13bf44a1d..f18547e0f 100644 --- a/src/Ombi/Controllers/V1/External/TesterController.cs +++ b/src/Ombi/Controllers/V1/External/TesterController.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Security.Principal; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc; using Microsoft.EntityFrameworkCore; @@ -48,7 +49,7 @@ namespace Ombi.Controllers.V1.External IPlexApi plex, IEmbyApiFactory emby, IRadarrV3Api radarr, ISonarrApi sonarr, ILogger log, IEmailProvider provider, ICouchPotatoApi cpApi, ITelegramNotification telegram, ISickRageApi srApi, INewsletterJob newsletter, ILegacyMobileNotification mobileNotification, ILidarrApi lidarrApi, IGotifyNotification gotifyNotification, IWhatsAppApi whatsAppApi, OmbiUserManager um, IWebhookNotification webhookNotification, - IJellyfinApi jellyfinApi) + IJellyfinApi jellyfinApi, IPrincipal user) { Service = service; DiscordNotification = notification; @@ -74,6 +75,7 @@ namespace Ombi.Controllers.V1.External UserManager = um; WebhookNotification = webhookNotification; _jellyfinApi = jellyfinApi; + UserPrinciple = user; } private INotificationService Service { get; } @@ -100,6 +102,7 @@ namespace Ombi.Controllers.V1.External private IWhatsAppApi WhatsAppApi { get; } private OmbiUserManager UserManager {get; } private readonly IJellyfinApi _jellyfinApi; + private IPrincipal UserPrinciple { get; } /// /// Sends a test message to discord using the provided settings @@ -283,7 +286,7 @@ namespace Ombi.Controllers.V1.External { Message = "This is just a test! Success!", Subject = $"Ombi: Test", - To = settings.AdminEmail, + To = (await GetCurrentUserAsync()).Email, }; message.Other.Add("PlainTextBody", "This is just a test! Success!"); @@ -298,6 +301,11 @@ namespace Ombi.Controllers.V1.External return true; } + private async Task GetCurrentUserAsync() + { + return await UserManager.Users.FirstOrDefaultAsync(x => x.NormalizedUserName == UserPrinciple.Identity.Name.ToUpper()); + } + /// /// Checks if we can connect to Plex with the provided settings ///