Skip to content

Commit

Permalink
fix(notifications): Remove generic admin email in favour of admins' e…
Browse files Browse the repository at this point in the history 
…mail (#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.
  • Loading branch information
@sephrat
sephrat committed Feb 25, 2022
1 parent a3e97b3 commit b90fc5f
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 56 deletions.
54 changes: 20 additions & 34 deletions src/Ombi.Notifications/Agents/EmailNotification.cs
View file
Expand Up @@ -47,7 +47,7 @@ protected override bool ValidateConfiguration(EmailNotificationSettings settings
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;
}
Expand All @@ -73,26 +73,6 @@ private async Task<NotificationMessage> LoadTemplate(NotificationType type, Noti
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;
}
Expand Down Expand Up @@ -138,9 +118,7 @@ protected override async Task NewIssue(NotificationOptions model, EmailNotificat
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)
Expand All @@ -154,18 +132,16 @@ protected override async Task IssueComment(NotificationOptions model, EmailNotif
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)
Expand Down Expand Up @@ -204,9 +180,7 @@ protected override async Task AddedToRequestQueue(NotificationOptions model, Ema
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)
Expand Down Expand Up @@ -305,6 +279,19 @@ protected override async Task Send(NotificationMessage model, EmailNotificationS
{
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)
{
Expand All @@ -316,12 +303,11 @@ protected override async Task Test(NotificationOptions model, EmailNotificationS
{
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);
}
}
}
7 changes: 6 additions & 1 deletion src/Ombi.Notifications/BaseNotification.cs
View file
Expand Up @@ -216,9 +216,14 @@ protected UserNotificationPreferences GetUserPreference(string userId, Notificat
.FirstOrDefault(x => x.Agent == agent && x.UserId == userId);
}

protected async Task<IEnumerable<OmbiUser>> GetPrivilegedUsers()
protected async Task<IEnumerable<OmbiUser>> GetAdminUsers()
{
IEnumerable<OmbiUser> recipients = await _userManager.GetUsersInRoleAsync(OmbiRoles.Admin);
return recipients;
}
protected async Task<IEnumerable<OmbiUser>> GetPrivilegedUsers()
{
IEnumerable<OmbiUser> recipients = await GetAdminUsers();
recipients = recipients.Concat(await _userManager.GetUsersInRoleAsync(OmbiRoles.PowerUser));
return recipients;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Ombi.Schedule/Jobs/Ombi/NewsletterJob.cs
View file
Expand Up @@ -844,7 +844,7 @@ protected bool ValidateConfiguration(EmailNotificationSettings settings)
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;
}
Expand Down
1 change: 0 additions & 1 deletion src/Ombi.Settings/Settings/Models/Notifications/EmailNotificationSettings.cs
View file
Expand Up @@ -10,7 +10,6 @@ public class EmailNotificationSettings : Settings
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; }
}
Expand Down
1 change: 0 additions & 1 deletion src/Ombi/ClientApp/src/app/interfaces/INotificationSettings.ts
View file
Expand Up @@ -12,7 +12,6 @@ export interface IEmailNotificationSettings extends INotificationSettings {
senderName: string;
username: string;
authentication: boolean;
adminEmail: string;
disableTLS: boolean;
disableCertificateChecking: boolean;
notificationTemplates: INotificationTemplates[];
Expand Down
15 changes: 0 additions & 15 deletions src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.html
View file
Expand Up @@ -57,21 +57,6 @@
<input matInput formControlName="senderName" matTooltip="The 'Friendly' name that will appear in the 'FROM:' part of the email">
</mat-form-field>
</div>

<div class="md-form-field">
<mat-form-field appearance="outline">
<mat-label>Admin Email</mat-label>
<input matInput formControlName="adminEmail" matTooltip="The administrator email will be used to send emails for admin only notifications (e.g. raised issues)">
<mat-error *ngIf="emailForm.get('adminEmail').hasError('required')">
Admin Email is <strong>required</strong>
</mat-error>
<mat-error *ngIf="emailForm.get('adminEmail').hasError('email')">
Admin Email needs to be a valid email address
</mat-error>
</mat-form-field>
</div>


<div class="md-form-field" *ngIf="emailForm.controls['username'].validator">
<mat-form-field appearance="outline">
<mat-label>Username</mat-label>
Expand Down
1 change: 0 additions & 1 deletion src/Ombi/ClientApp/src/app/settings/notifications/emailnotification.component.ts
View file
Expand Up @@ -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],
});
Expand Down
12 changes: 10 additions & 2 deletions src/Ombi/Controllers/V1/External/TesterController.cs
View file
@@ -1,5 +1,6 @@
using System;
using System.Linq;
using System.Security.Principal;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Mvc;
using Microsoft.EntityFrameworkCore;
Expand Down Expand Up @@ -48,7 +49,7 @@ public class TesterController : Controller
IPlexApi plex, IEmbyApiFactory emby, IRadarrV3Api radarr, ISonarrApi sonarr, ILogger<TesterController> 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;
Expand All @@ -74,6 +75,7 @@ public class TesterController : Controller
UserManager = um;
WebhookNotification = webhookNotification;
_jellyfinApi = jellyfinApi;
UserPrinciple = user;
}

private INotificationService Service { get; }
Expand All @@ -100,6 +102,7 @@ public class TesterController : Controller
private IWhatsAppApi WhatsAppApi { get; }
private OmbiUserManager UserManager {get; }
private readonly IJellyfinApi _jellyfinApi;
private IPrincipal UserPrinciple { get; }

/// <summary>
/// Sends a test message to discord using the provided settings
Expand Down Expand Up @@ -283,7 +286,7 @@ public async Task<bool> Email([FromBody] EmailNotificationSettings settings)
{
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!");
Expand All @@ -298,6 +301,11 @@ public async Task<bool> Email([FromBody] EmailNotificationSettings settings)
return true;
}

private async Task<OmbiUser> GetCurrentUserAsync()
{
return await UserManager.Users.FirstOrDefaultAsync(x => x.NormalizedUserName == UserPrinciple.Identity.Name.ToUpper());
}

/// <summary>
/// Checks if we can connect to Plex with the provided settings
/// </summary>
Expand Down

0 comments on commit b90fc5f

Please sign in to comment.