Fixed a critical bug where registration was broken for first time flow. Refactored how backup before migrations occured such that it now puts the db in temp. The db will be deleted automatically that night. (#900)

API/Controllers/AccountController.cs

@@ -78,7 +78,6 @@ namespace API.Controllers
         /// </summary>
         /// <param name="registerDto"></param>
         /// <returns></returns>
-        [Authorize(Policy = "RequireAdminRole")]
         [HttpPost("register")]
         public async Task<ActionResult<UserDto>> Register(RegisterDto registerDto)
         {
@@ -89,6 +88,17 @@ namespace API.Controllers
                     return BadRequest("Username is taken.");
                 }
 
+                // If we are registering an admin account, ensure there are no existing admins or user registering is an admin
+                if (registerDto.IsAdmin)
+                {
+                    var firstTimeFlow = !(await _userManager.GetUsersInRoleAsync("Admin")).Any();
+                    if (!firstTimeFlow && !await _unitOfWork.UserRepository.IsUserAdminAsync(
+                            await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername())))
+                    {
+                        return BadRequest("You are not permitted to create an admin account");
+                    }
+                }
+
                 var user = _mapper.Map<AppUser>(registerDto);
                 user.UserPreferences ??= new AppUserPreferences();
                 user.ApiKey = HashUtil.ApiKey();
@@ -104,6 +114,7 @@ namespace API.Controllers
 
                 if (!result.Succeeded) return BadRequest(result.Errors);
 
+
                 var role = registerDto.IsAdmin ? PolicyConstants.AdminRole : PolicyConstants.PlebRole;
                 var roleResult = await _userManager.AddToRoleAsync(user, role);
 
@@ -156,7 +167,7 @@ namespace API.Controllers
 
             if (user == null) return Unauthorized("Invalid username");
 
-            var isAdmin = await _unitOfWork.UserRepository.IsUserAdmin(user);
+            var isAdmin = await _unitOfWork.UserRepository.IsUserAdminAsync(user);
             var settings = await _unitOfWork.SettingsRepository.GetSettingsDtoAsync();
             if (!settings.EnableAuthentication && !isAdmin)
             {

API/Controllers/CollectionController.cs

@@ -32,7 +32,7 @@ namespace API.Controllers
         public async Task<IEnumerable<CollectionTagDto>> GetAllTags()
         {
             var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername());
-            var isAdmin = await _unitOfWork.UserRepository.IsUserAdmin(user);
+            var isAdmin = await _unitOfWork.UserRepository.IsUserAdminAsync(user);
             if (isAdmin)
             {
                 return await _unitOfWork.CollectionTagRepository.GetAllTagDtosAsync();

API/Controllers/OPDSController.cs

@@ -184,7 +184,7 @@ public class OpdsController : BaseApiController
             return BadRequest("OPDS is not enabled on this server");
         var userId = await GetUser(apiKey);
         var user = await _unitOfWork.UserRepository.GetUserByIdAsync(userId);
-        var isAdmin = await _unitOfWork.UserRepository.IsUserAdmin(user);
+        var isAdmin = await _unitOfWork.UserRepository.IsUserAdminAsync(user);
 
         IList<CollectionTagDto> tags = isAdmin ? (await _unitOfWork.CollectionTagRepository.GetAllTagDtosAsync()).ToList()
             : (await _unitOfWork.CollectionTagRepository.GetAllPromotedTagDtosAsync()).ToList();
@@ -220,7 +220,7 @@ public class OpdsController : BaseApiController
             return BadRequest("OPDS is not enabled on this server");
         var userId = await GetUser(apiKey);
         var user = await _unitOfWork.UserRepository.GetUserByIdAsync(userId);
-        var isAdmin = await _unitOfWork.UserRepository.IsUserAdmin(user);
+        var isAdmin = await _unitOfWork.UserRepository.IsUserAdminAsync(user);
 
         IEnumerable <CollectionTagDto> tags;
         if (isAdmin)

API/Controllers/SeriesController.cs

@@ -154,7 +154,7 @@ namespace API.Controllers
         public async Task<ActionResult> UpdateSeriesRating(UpdateSeriesRatingDto updateSeriesRatingDto)
         {
             var user = await _unitOfWork.UserRepository.GetUserByUsernameAsync(User.GetUsername(), AppUserIncludes.Ratings);
-            var userRating = await _unitOfWork.UserRepository.GetUserRating(updateSeriesRatingDto.SeriesId, user.Id) ??
+            var userRating = await _unitOfWork.UserRepository.GetUserRatingAsync(updateSeriesRatingDto.SeriesId, user.Id) ??
                              new AppUserRating();
 
             userRating.Rating = updateSeriesRatingDto.UserRating;

API/Data/Repositories/UserRepository.cs

@@ -32,8 +32,8 @@ public interface IUserRepository
     Task<IEnumerable<MemberDto>>  GetMembersAsync();
     Task<IEnumerable<AppUser>> GetAdminUsersAsync();
     Task<IEnumerable<AppUser>> GetNonAdminUsersAsync();
-    Task<bool> IsUserAdmin(AppUser user);
+    Task<bool> IsUserAdminAsync(AppUser user);
-    Task<AppUserRating> GetUserRating(int seriesId, int userId);
+    Task<AppUserRating> GetUserRatingAsync(int seriesId, int userId);
     Task<AppUserPreferences> GetPreferencesAsync(string username);
     Task<IEnumerable<BookmarkDto>> GetBookmarkDtosForSeries(int userId, int seriesId);
     Task<IEnumerable<BookmarkDto>> GetBookmarkDtosForVolume(int userId, int volumeId);
@@ -208,12 +208,12 @@ public class UserRepository : IUserRepository
         return await _userManager.GetUsersInRoleAsync(PolicyConstants.PlebRole);
     }
 
-    public async Task<bool> IsUserAdmin(AppUser user)
+    public async Task<bool> IsUserAdminAsync(AppUser user)
     {
         return await _userManager.IsInRoleAsync(user, PolicyConstants.AdminRole);
     }
 
-    public async Task<AppUserRating> GetUserRating(int seriesId, int userId)
+    public async Task<AppUserRating> GetUserRatingAsync(int seriesId, int userId)
     {
         return await _context.AppUserRating.Where(r => r.SeriesId == seriesId && r.AppUserId == userId)
             .SingleOrDefaultAsync();

API/Program.cs

@@ -56,51 +56,29 @@ namespace API
 
             try
             {
+                var logger = services.GetRequiredService<ILogger<Program>>();
                 var context = services.GetRequiredService<DataContext>();
+                var pendingMigrations = await context.Database.GetPendingMigrationsAsync();
+                if (pendingMigrations.Any())
+                {
+                    logger.LogInformation("Performing backup as migrations are needed. Backup will be kavita.db in temp folder");
+                    directoryService.CopyFileToDirectory(directoryService.FileSystem.Path.Join(directoryService.ConfigDirectory, "kavita.db"), directoryService.TempDirectory);
+                }
+
+                await context.Database.MigrateAsync();
+                var roleManager = services.GetRequiredService<RoleManager<AppRole>>();
+
+                await Seed.SeedRoles(roleManager);
+                await Seed.SeedSettings(context, directoryService);
+                await Seed.SeedUserApiKeys(context);
 
 
                 if (isDocker && new FileInfo("data/appsettings.json").Exists)
                 {
-                    var logger = services.GetRequiredService<ILogger<Startup>>();
+                    //var logger = services.GetRequiredService<ILogger<Startup>>();
                     logger.LogCritical("WARNING! Mount point is incorrect, nothing here will persist. Please change your container mount from /kavita/data to /kavita/config");
                     return;
                 }
-
-                var requiresCoverImageMigration = !Directory.Exists(directoryService.CoverImageDirectory);
-                try
-                {
-                    // If this is a new install, tables wont exist yet
-                    if (requiresCoverImageMigration)
-                    {
-                        MigrateCoverImages.ExtractToImages(context, directoryService, services.GetRequiredService<ImageService>());
-                    }
-                }
-                catch (Exception)
-                {
-                    requiresCoverImageMigration = false;
-                }
-
-                if (requiresCoverImageMigration)
-                {
-                    await MigrateCoverImages.UpdateDatabaseWithImages(context, directoryService);
-                }
-
-                // // Apply all migrations on startup
-                // // If we have pending migrations, make a backup first
-                // var pendingMigrations = await context.Database.GetPendingMigrationsAsync();
-                // if (pendingMigrations.Any())
-                // {
-                //     var logger = services.GetRequiredService<ILogger<Program>>();
-                //     logger.LogInformation("Performing backup as migrations are needed");
-                //     // var backupService = services.GetRequiredService<BackupService>();
-                //     // await backupService.BackupDatabase();
-                // }
-                //
-                // await context.Database.MigrateAsync();
-                //
-                // await Seed.SeedRoles(roleManager);
-                // await Seed.SeedSettings(context, directoryService);
-                // await Seed.SeedUserApiKeys(context);
             }
             catch (Exception ex)
             {

API/Startup.cs

@@ -134,7 +134,7 @@ namespace API
         // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
         public void Configure(IApplicationBuilder app, IBackgroundJobClient backgroundJobs, IWebHostEnvironment env,
             IHostApplicationLifetime applicationLifetime, IServiceProvider serviceProvider, ICacheService cacheService,
-            IDirectoryService directoryService, IUnitOfWork unitOfWork, IBackupService backupService)
+            IDirectoryService directoryService, IUnitOfWork unitOfWork, IBackupService backupService, IImageService imageService)
         {
 
             // Apply Migrations
@@ -144,26 +144,44 @@ namespace API
                 {
                     // Apply all migrations on startup
                     // If we have pending migrations, make a backup first
+                    var isDocker = new OsInfo(Array.Empty<IOsVersionAdapter>()).IsDocker;
                     var logger = serviceProvider.GetRequiredService<ILogger<Program>>();
                     var context = serviceProvider.GetRequiredService<DataContext>();
-                    var pendingMigrations = await context.Database.GetPendingMigrationsAsync();
+                    // var pendingMigrations = await context.Database.GetPendingMigrationsAsync();
-                    if (pendingMigrations.Any())
+                    // if (pendingMigrations.Any())
-                    {
+                    // {
-                        logger.LogInformation("Performing backup as migrations are needed");
+                    //     logger.LogInformation("Performing backup as migrations are needed");
-                        await backupService.BackupDatabase();
+                    //     await backupService.BackupDatabase();
-                    }
+                    // }
-
+                    //
-                    await context.Database.MigrateAsync();
+                    // await context.Database.MigrateAsync();
+                    // var roleManager = serviceProvider.GetRequiredService<RoleManager<AppRole>>();
+                    //
+                    // await Seed.SeedRoles(roleManager);
+                    // await Seed.SeedSettings(context, directoryService);
+                    // await Seed.SeedUserApiKeys(context);
 
                     await MigrateBookmarks.Migrate(directoryService, unitOfWork,
                         logger, cacheService);
 
-                    var roleManager = serviceProvider.GetRequiredService<RoleManager<AppRole>>();
+                    var requiresCoverImageMigration = !Directory.Exists(directoryService.CoverImageDirectory);
-
+                    try
-                    await Seed.SeedRoles(roleManager);
+                    {
-                    await Seed.SeedSettings(context, directoryService);
+                        // If this is a new install, tables wont exist yet
-                    await Seed.SeedUserApiKeys(context);
+                        if (requiresCoverImageMigration)
+                        {
+                            MigrateCoverImages.ExtractToImages(context, directoryService, imageService);
+                        }
+                    }
+                    catch (Exception)
+                    {
+                        requiresCoverImageMigration = false;
+                    }
 
+                    if (requiresCoverImageMigration)
+                    {
+                        await MigrateCoverImages.UpdateDatabaseWithImages(context, directoryService);
+                    }
                 }).GetAwaiter()
                     .GetResult();
             }