Auth Email Rework (#1567)

* Hooked up Send to for Series and volumes and fixed a bug where Email Service errors weren't propagating to the UI layer.

When performing actions on series detail, don't disable the button anymore.

* Added send to action to volumes

* Fixed a bug where .kavitaignore wasn't being applied at library root level

* Added a notification for when a device is being sent a file.

* Added a check in forgot password for users that do not have an email set or aren't confirmed.

* Added a new api for change email and moved change password directly into new Account tab (styling and logic needs testing)

* Save approx scroll position like with jump key, but on normal click of card.

* Implemented the ability to change your email address or set one. This requires a 2 step process using a confirmation token. This needs polishing and css.

* Removed an unused directive from codebase

* Fixed up some typos on publicly

* Updated query for Pending Invites to also check if the user account has not logged in at least once.

* Cleaned up the css for validate email change

* Hooked in an indicator to tell user that a user has an unconfirmed email

* Cleaned up code smells
This commit is contained in:
Joe Milazzo 2022-10-01 08:23:35 -05:00 committed by GitHub
parent 3792ac3421
commit 5f17c2fb73
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
49 changed files with 816 additions and 274 deletions

View file

@ -19,6 +19,7 @@ using API.Services;
using API.SignalR;
using AutoMapper;
using Kavita.Common;
using Kavita.Common.EnvironmentInfo;
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Identity;
using Microsoft.AspNetCore.Mvc;
@ -188,15 +189,6 @@ public class AccountController : BaseApiController
if (user == null) return Unauthorized("Invalid username");
// Check if the user has an email, if not, inform them so they can migrate
var validPassword = await _signInManager.UserManager.CheckPasswordAsync(user, loginDto.Password);
if (string.IsNullOrEmpty(user.Email) && !user.EmailConfirmed && validPassword)
{
_logger.LogCritical("User {UserName} does not have an email. Providing a one time migration", user.UserName);
return Unauthorized(
"You are missing an email on your account. Please wait while we migrate your account.");
}
var result = await _signInManager
.CheckPasswordSignInAsync(user, loginDto.Password, true);
@ -256,6 +248,7 @@ public class AccountController : BaseApiController
[HttpGet("roles")]
public ActionResult<IList<string>> GetRoles()
{
// TODO: This should be moved to ServerController
return typeof(PolicyConstants)
.GetFields(BindingFlags.Public | BindingFlags.Static)
.Where(f => f.FieldType == typeof(string))
@ -285,6 +278,86 @@ public class AccountController : BaseApiController
}
/// <summary>
/// Initiates the flow to update a user's email address. The email address is not changed in this API. A confirmation link is sent/dumped which will
/// validate the email. It must be confirmed for the email to update.
/// </summary>
/// <param name="dto"></param>
/// <returns>Returns just if the email was sent or server isn't reachable</returns>
[HttpPost("update/email")]
public async Task<ActionResult> UpdateEmail(UpdateEmailDto dto)
{
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
if (user == null) return Unauthorized("You do not have permission");
if (dto == null || string.IsNullOrEmpty(dto.Email)) return BadRequest("Invalid payload");
// Validate no other users exist with this email
if (user.Email.Equals(dto.Email)) return Ok("Nothing to do");
// Check if email is used by another user
var existingUserEmail = await _unitOfWork.UserRepository.GetUserByEmailAsync(dto.Email);
if (existingUserEmail != null)
{
return BadRequest("You cannot share emails across multiple accounts");
}
// All validations complete, generate a new token and email it to the user at the new address. Confirm email link will update the email
var token = await _userManager.GenerateEmailConfirmationTokenAsync(user);
if (string.IsNullOrEmpty(token))
{
_logger.LogError("There was an issue generating a token for the email");
return BadRequest("There was an issue creating a confirmation email token. See logs.");
}
user.EmailConfirmed = false;
user.ConfirmationToken = token;
await _userManager.UpdateAsync(user);
// Send a confirmation email
try
{
var emailLink = GenerateEmailLink(user.ConfirmationToken, "confirm-email-update", dto.Email);
_logger.LogCritical("[Update Email]: Email Link for {UserName}: {Link}", user.UserName, emailLink);
var host = _environment.IsDevelopment() ? "localhost:4200" : Request.Host.ToString();
var accessible = await _emailService.CheckIfAccessible(host);
if (accessible)
{
try
{
// Email the old address of the update change
await _emailService.SendEmailChangeEmail(new ConfirmationEmailDto()
{
EmailAddress = string.IsNullOrEmpty(user.Email) ? dto.Email : user.Email,
InstallId = BuildInfo.Version.ToString(),
InvitingUser = (await _unitOfWork.UserRepository.GetAdminUsersAsync()).First().UserName,
ServerConfirmationLink = emailLink
});
}
catch (Exception)
{
/* Swallow exception */
}
}
return Ok(new InviteUserResponse
{
EmailLink = string.Empty,
EmailSent = accessible
});
}
catch (Exception ex)
{
_logger.LogError(ex, "There was an error during invite user flow, unable to send an email");
}
await _eventHub.SendMessageToAsync(MessageFactory.UserUpdate, MessageFactory.UserUpdateEvent(user.Id, user.UserName), user.Id);
return Ok();
}
/// <summary>
/// Update the user account. This can only affect Username, Email (will require confirming), Roles, and Library access.
/// </summary>
@ -310,14 +383,6 @@ public class AccountController : BaseApiController
_unitOfWork.UserRepository.Update(user);
}
if (!user.Email.Equals(dto.Email))
{
// Validate username change
var errors = await _accountService.ValidateEmail(dto.Email);
if (errors.Any()) return BadRequest("Email already registered");
// NOTE: This needs to be handled differently, like save it in a temp variable in DB until email is validated. For now, I wont allow it
}
// Update roles
var existingRoles = await _userManager.GetRolesAsync(user);
var hasAdminRole = dto.Roles.Contains(PolicyConstants.AdminRole);
@ -404,18 +469,22 @@ public class AccountController : BaseApiController
public async Task<ActionResult<string>> InviteUser(InviteUserDto dto)
{
var adminUser = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
if (adminUser == null) return Unauthorized("You need to login");
if (adminUser == null) return Unauthorized("You are not permitted");
_logger.LogInformation("{User} is inviting {Email} to the server", adminUser.UserName, dto.Email);
// Check if there is an existing invite
dto.Email = dto.Email.Trim();
var emailValidationErrors = await _accountService.ValidateEmail(dto.Email);
if (emailValidationErrors.Any())
if (!string.IsNullOrEmpty(dto.Email))
{
var invitedUser = await _unitOfWork.UserRepository.GetUserByEmailAsync(dto.Email);
if (await _userManager.IsEmailConfirmedAsync(invitedUser))
return BadRequest($"User is already registered as {invitedUser.UserName}");
return BadRequest("User is already invited under this email and has yet to accepted invite.");
dto.Email = dto.Email.Trim();
var emailValidationErrors = await _accountService.ValidateEmail(dto.Email);
if (emailValidationErrors.Any())
{
var invitedUser = await _unitOfWork.UserRepository.GetUserByEmailAsync(dto.Email);
if (await _userManager.IsEmailConfirmedAsync(invitedUser))
return BadRequest($"User is already registered as {invitedUser.UserName}");
return BadRequest("User is already invited under this email and has yet to accepted invite.");
}
}
// Create a new user
@ -575,6 +644,44 @@ public class AccountController : BaseApiController
};
}
/// <summary>
/// Final step in email update change. Given a confirmation token and the email, this will finish the email change.
/// </summary>
/// <remarks>This will force connected clients to re-authenticate</remarks>
/// <param name="dto"></param>
/// <returns></returns>
[AllowAnonymous]
[HttpPost("confirm-email-update")]
public async Task<ActionResult> ConfirmEmailUpdate(ConfirmEmailUpdateDto dto)
{
var user = await _unitOfWork.UserRepository.GetUserByConfirmationToken(dto.Token);
if (user == null)
{
return BadRequest("Invalid Email Token");
}
if (!await ConfirmEmailToken(dto.Token, user)) return BadRequest("Invalid Email Token");
_logger.LogInformation("User is updating email from {OldEmail} to {NewEmail}", user.Email, dto.Email);
var result = await _userManager.SetEmailAsync(user, dto.Email);
if (!result.Succeeded)
{
_logger.LogError("Unable to update email for users: {Errors}", result.Errors.Select(e => e.Description));
return BadRequest("Unable to update email for user. Check logs");
}
user.ConfirmationToken = null;
await _unitOfWork.CommitAsync();
// For the user's connected devices to pull the new information in
await _eventHub.SendMessageToAsync(MessageFactory.UserUpdate,
MessageFactory.UserUpdateEvent(user.Id, user.UserName), user.Id);
// Perform Login code
return Ok();
}
[AllowAnonymous]
[HttpPost("confirm-password-reset")]
public async Task<ActionResult<string>> ConfirmForgotPassword(ConfirmPasswordResetDto dto)
@ -619,15 +726,15 @@ public class AccountController : BaseApiController
}
var roles = await _userManager.GetRolesAsync(user);
if (!roles.Any(r => r is PolicyConstants.AdminRole or PolicyConstants.ChangePasswordRole))
return Unauthorized("You are not permitted to this operation.");
if (string.IsNullOrEmpty(user.Email) || !user.EmailConfirmed)
return BadRequest("You do not have an email on account or it has not been confirmed");
var token = await _userManager.GeneratePasswordResetTokenAsync(user);
var emailLink = GenerateEmailLink(token, "confirm-reset-password", user.Email);
_logger.LogCritical("[Forgot Password]: Email Link for {UserName}: {Link}", user.UserName, emailLink);
_logger.LogCritical("[Forgot Password]: Token {UserName}: {Token}", user.UserName, token);
var host = _environment.IsDevelopment() ? "localhost:4200" : Request.Host.ToString();
if (await _emailService.CheckIfAccessible(host))
{
@ -643,6 +750,15 @@ public class AccountController : BaseApiController
return Ok("Your server is not accessible. The Link to reset your password is in the logs.");
}
[HttpGet("email-confirmed")]
public async Task<ActionResult<bool>> IsEmailConfirmed()
{
var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
if (user == null) return Unauthorized();
return Ok(user.EmailConfirmed);
}
[AllowAnonymous]
[HttpPost("confirm-migration-email")]
public async Task<ActionResult<UserDto>> ConfirmMigrationEmail(ConfirmMigrationEmailDto dto)

View file

@ -1,11 +1,15 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using API.Data;
using API.Data.Repositories;
using API.DTOs.Device;
using API.Extensions;
using API.Services;
using API.SignalR;
using ExCSS;
using Kavita.Common;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
@ -20,12 +24,14 @@ public class DeviceController : BaseApiController
private readonly IUnitOfWork _unitOfWork;
private readonly IDeviceService _deviceService;
private readonly IEmailService _emailService;
private readonly IEventHub _eventHub;
public DeviceController(IUnitOfWork unitOfWork, IDeviceService deviceService, IEmailService emailService)
public DeviceController(IUnitOfWork unitOfWork, IDeviceService deviceService, IEmailService emailService, IEventHub eventHub)
{
_unitOfWork = unitOfWork;
_deviceService = deviceService;
_emailService = emailService;
_eventHub = eventHub;
}
@ -76,26 +82,33 @@ public class DeviceController : BaseApiController
[HttpPost("send-to")]
public async Task<ActionResult> SendToDevice(SendToDeviceDto dto)
{
if (dto.ChapterId < 0) return BadRequest("ChapterId must be greater than 0");
if (dto.ChapterIds.Any(i => i < 0)) return BadRequest("ChapterIds must be greater than 0");
if (dto.DeviceId < 0) return BadRequest("DeviceId must be greater than 0");
if (await _emailService.IsDefaultEmailService())
return BadRequest("Send to device cannot be used with Kavita's email service. Please configure your own.");
var userId = await _unitOfWork.UserRepository.GetUserIdByUsernameAsync(User.GetUsername());
await _eventHub.SendMessageToAsync(MessageFactory.NotificationProgress, MessageFactory.SendingToDeviceEvent($"Transferring files to your device", "started"), userId);
try
{
var success = await _deviceService.SendTo(dto.ChapterId, dto.DeviceId);
var success = await _deviceService.SendTo(dto.ChapterIds, dto.DeviceId);
if (success) return Ok();
}
catch (KavitaException ex)
{
return BadRequest(ex.Message);
}
finally
{
await _eventHub.SendMessageToAsync(MessageFactory.SendingToDevice, MessageFactory.SendingToDeviceEvent($"Transferring files to your device", "ended"), userId);
}
return BadRequest("There was an error sending the file to the device");
}
}

View file

@ -0,0 +1,11 @@
using System.ComponentModel.DataAnnotations;
namespace API.DTOs.Account;
public class ConfirmEmailUpdateDto
{
[Required]
public string Email { get; set; }
[Required]
public string Token { get; set; }
}

View file

@ -0,0 +1,6 @@
namespace API.DTOs.Account;
public class UpdateEmailDto
{
public string Email { get; set; }
}

View file

@ -0,0 +1,14 @@
namespace API.DTOs.Account;
public class UpdateEmailResponse
{
/// <summary>
/// Did the user not have an existing email
/// </summary>
/// <remarks>This informs the user to check the new email address</remarks>
public bool HadNoExistingEmail { get; set; }
/// <summary>
/// Was an email sent (ie is this server accessible)
/// </summary>
public bool EmailSent { get; set; }
}

View file

@ -6,11 +6,6 @@ public record UpdateUserDto
{
public int UserId { get; set; }
public string Username { get; set; }
/// <summary>
/// This field will not result in any change to the User model. Changing email is not supported.
/// </summary>
public string Email { get; set; }
/// <summary>
/// List of Roles to assign to user. If admin not present, Pleb will be applied.
/// If admin present, all libraries will be granted access and will ignore those from DTO.
/// </summary>

View file

@ -1,7 +1,9 @@
namespace API.DTOs.Device;
using System.Collections.Generic;
namespace API.DTOs.Device;
public class SendToDeviceDto
{
public int DeviceId { get; set; }
public int ChapterId { get; set; }
public IReadOnlyList<int> ChapterIds { get; set; }
}

View file

@ -6,7 +6,9 @@ public class RegisterDto
{
[Required]
public string Username { get; init; }
[Required]
/// <summary>
/// An email to register with. Optional. Provides Forgot Password functionality
/// </summary>
public string Email { get; init; }
[Required]
[StringLength(32, MinimumLength = 6)]

View file

@ -62,6 +62,7 @@ public interface IUserRepository
Task<IEnumerable<AppUserPreferences>> GetAllPreferencesByThemeAsync(int themeId);
Task<bool> HasAccessToLibrary(int libraryId, int userId);
Task<IEnumerable<AppUser>> GetAllUsersAsync(AppUserIncludes includeFlags);
Task<AppUser> GetUserByConfirmationToken(string token);
}
public class UserRepository : IUserRepository
@ -268,6 +269,11 @@ public class UserRepository : IUserRepository
return await query.ToListAsync();
}
public async Task<AppUser> GetUserByConfirmationToken(string token)
{
return await _context.AppUser.SingleOrDefaultAsync(u => u.ConfirmationToken.Equals(token));
}
public async Task<IEnumerable<AppUser>> GetAdminUsersAsync()
{
return await _userManager.GetUsersInRoleAsync(PolicyConstants.AdminRole);
@ -403,10 +409,14 @@ public class UserRepository : IUserRepository
.ToListAsync();
}
/// <summary>
/// Returns a list of users that are considered Pending by invite. This means email is unconfirmed and they have never logged in
/// </summary>
/// <returns></returns>
public async Task<IEnumerable<MemberDto>> GetPendingMemberDtosAsync()
{
return await _context.Users
.Where(u => !u.EmailConfirmed)
.Where(u => !u.EmailConfirmed && u.LastActive == DateTime.MinValue)
.Include(x => x.Libraries)
.Include(r => r.UserRoles)
.ThenInclude(r => r.Role)

View file

@ -32,7 +32,7 @@ public static class IdentityServiceExtensions
opt.Password.RequireNonAlphanumeric = false;
opt.Password.RequiredLength = 6;
opt.SignIn.RequireConfirmedEmail = true;
opt.SignIn.RequireConfirmedEmail = false;
opt.Lockout.AllowedForNewUsers = true;
opt.Lockout.DefaultLockoutTimeSpan = TimeSpan.FromMinutes(10);

View file

@ -7,6 +7,7 @@ using API.DTOs.Device;
using API.DTOs.Email;
using API.Entities;
using API.Entities.Enums;
using API.SignalR;
using Kavita.Common;
using Microsoft.Extensions.Logging;
@ -17,7 +18,7 @@ public interface IDeviceService
Task<Device> Create(CreateDeviceDto dto, AppUser userWithDevices);
Task<Device> Update(UpdateDeviceDto dto, AppUser userWithDevices);
Task<bool> Delete(AppUser userWithDevices, int deviceId);
Task<bool> SendTo(int chapterId, int deviceId);
Task<bool> SendTo(IReadOnlyList<int> chapterIds, int deviceId);
}
public class DeviceService : IDeviceService
@ -102,9 +103,9 @@ public class DeviceService : IDeviceService
return false;
}
public async Task<bool> SendTo(int chapterId, int deviceId)
public async Task<bool> SendTo(IReadOnlyList<int> chapterIds, int deviceId)
{
var files = await _unitOfWork.ChapterRepository.GetFilesForChapterAsync(chapterId);
var files = await _unitOfWork.ChapterRepository.GetFilesForChaptersAsync(chapterIds);
if (files.Any(f => f.Format is not (MangaFormat.Epub or MangaFormat.Pdf)))
throw new KavitaException("Cannot Send non Epub or Pdf to devices as not supported");
@ -118,6 +119,7 @@ public class DeviceService : IDeviceService
DestinationEmail = device.EmailAddress,
FilePaths = files.Select(m => m.FilePath)
});
return success;
}
}

View file

@ -1,9 +1,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using API.Data;
using API.DTOs.Email;
@ -14,7 +12,6 @@ using Kavita.Common.EnvironmentInfo;
using Kavita.Common.Helpers;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Microsoft.Net.Http.Headers;
namespace API.Services;
@ -27,6 +24,7 @@ public interface IEmailService
Task<bool> SendFilesToEmail(SendToDto data);
Task<EmailTestResultDto> TestConnectivity(string emailUrl);
Task<bool> IsDefaultEmailService();
Task SendEmailChangeEmail(ConfirmationEmailDto data);
}
public class EmailService : IEmailService
@ -84,6 +82,16 @@ public class EmailService : IEmailService
.Equals(DefaultApiUrl);
}
public async Task SendEmailChangeEmail(ConfirmationEmailDto data)
{
var emailLink = (await _unitOfWork.SettingsRepository.GetSettingAsync(ServerSettingKey.EmailServiceUrl)).Value;
var success = await SendEmailWithPost(emailLink + "/api/account/email-change", data);
if (!success)
{
_logger.LogError("There was a critical error sending Confirmation email");
}
}
public async Task SendConfirmationEmail(ConfirmationEmailDto data)
{
var emailLink = (await _unitOfWork.SettingsRepository.GetSettingAsync(ServerSettingKey.EmailServiceUrl)).Value;
@ -172,18 +180,20 @@ public class EmailService : IEmailService
if (response.StatusCode != StatusCodes.Status200OK)
{
return false;
var errorMessage = await response.GetStringAsync();
throw new KavitaException(errorMessage);
}
}
catch (Exception)
catch (FlurlHttpException ex)
{
_logger.LogError(ex, "There was an exception when interacting with Email Service");
return false;
}
return true;
}
private async Task<bool> SendEmailWithFiles(string url, IEnumerable<string> filePaths, string destEmail, int timeoutSecs = 30)
private async Task<bool> SendEmailWithFiles(string url, IEnumerable<string> filePaths, string destEmail, int timeoutSecs = 300)
{
try
{
@ -193,7 +203,8 @@ public class EmailService : IEmailService
.WithHeader("x-api-key", "MsnvA2DfQqxSK5jh")
.WithHeader("x-kavita-version", BuildInfo.Version)
.WithHeader("x-kavita-installId", settings.InstallId)
.WithTimeout(TimeSpan.FromSeconds(timeoutSecs))
.WithTimeout(timeoutSecs)
.AllowHttpStatus("4xx")
.PostMultipartAsync(mp =>
{
mp.AddString("email", destEmail);
@ -208,10 +219,11 @@ public class EmailService : IEmailService
if (response.StatusCode != StatusCodes.Status200OK)
{
return false;
var errorMessage = await response.GetStringAsync();
throw new KavitaException(errorMessage);
}
}
catch (Exception ex)
catch (FlurlHttpException ex)
{
_logger.LogError(ex, "There was an exception when sending Email for SendTo");
return false;

View file

@ -82,7 +82,8 @@ public class ParseScannedFiles
{
// This is used in library scan, so we should check first for a ignore file and use that here as well
var potentialIgnoreFile = _directoryService.FileSystem.Path.Join(folderPath, DirectoryService.KavitaIgnoreFile);
var directories = _directoryService.GetDirectories(folderPath, _directoryService.CreateMatcherFromFile(potentialIgnoreFile)).ToList();
var matcher = _directoryService.CreateMatcherFromFile(potentialIgnoreFile);
var directories = _directoryService.GetDirectories(folderPath, matcher).ToList();
foreach (var directory in directories)
{
@ -94,7 +95,7 @@ public class ParseScannedFiles
else
{
// For a scan, this is doing everything in the directory loop before the folder Action is called...which leads to no progress indication
await folderAction(_directoryService.ScanFiles(directory), directory);
await folderAction(_directoryService.ScanFiles(directory, matcher), directory);
}
}

View file

@ -112,6 +112,10 @@ public static class MessageFactory
/// A generic message that can occur in background processing to inform user, but no direct action is needed
/// </summary>
public const string Info = "Info";
/// <summary>
/// When files are being emailed to a device
/// </summary>
public const string SendingToDevice = "SendingToDevice";
public static SignalRMessage ScanSeriesEvent(int libraryId, int seriesId, string seriesName)
@ -249,6 +253,19 @@ public static class MessageFactory
};
}
public static SignalRMessage SendingToDeviceEvent(string subtitle, string eventType)
{
return new SignalRMessage
{
Name = SendingToDevice,
Title = "Sending files to Device",
SubTitle = subtitle,
EventType = eventType,
Progress = ProgressType.Indeterminate,
Body = new { }
};
}
public static SignalRMessage SeriesAddedToCollectionEvent(int tagId, int seriesId)
{
return new SignalRMessage