diff --git a/frontend/src/Settings/MediaManagement/MediaManagement.js b/frontend/src/Settings/MediaManagement/MediaManagement.js index 884b4b095..8359a2cbf 100644 --- a/frontend/src/Settings/MediaManagement/MediaManagement.js +++ b/frontend/src/Settings/MediaManagement/MediaManagement.js @@ -371,7 +371,7 @@ class MediaManagement extends Component { - - - Folder chmod mode - - - - - - chown User - - - - - - chown Group - - - } diff --git a/src/NzbDrone.Api/Config/MediaManagementConfigModule.cs b/src/NzbDrone.Api/Config/MediaManagementConfigModule.cs index 8b35e53ed..17894db46 100644 --- a/src/NzbDrone.Api/Config/MediaManagementConfigModule.cs +++ b/src/NzbDrone.Api/Config/MediaManagementConfigModule.cs @@ -1,16 +1,17 @@ using FluentValidation; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Validation; using NzbDrone.Core.Validation.Paths; namespace NzbDrone.Api.Config { public class MediaManagementConfigModule : NzbDroneConfigModule { - public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator) + public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator, FileChmodValidator fileChmodValidator) : base(configService) { - SharedValidator.RuleFor(c => c.FileChmod).NotEmpty(); - SharedValidator.RuleFor(c => c.FolderChmod).NotEmpty(); + SharedValidator.RuleFor(c => c.FileChmod).SetValidator(fileChmodValidator).When(c => !string.IsNullOrEmpty(c.FileChmod) && PlatformInfo.IsMono); SharedValidator.RuleFor(c => c.RecycleBin).IsValidPath().SetValidator(pathExistsValidator).When(c => !string.IsNullOrWhiteSpace(c.RecycleBin)); } diff --git a/src/NzbDrone.Api/Config/MediaManagementConfigResource.cs b/src/NzbDrone.Api/Config/MediaManagementConfigResource.cs index 1992f8d5e..740f81454 100644 --- a/src/NzbDrone.Api/Config/MediaManagementConfigResource.cs +++ b/src/NzbDrone.Api/Config/MediaManagementConfigResource.cs @@ -16,9 +16,6 @@ namespace NzbDrone.Api.Config public bool SetPermissionsLinux { get; set; } public string FileChmod { get; set; } - public string FolderChmod { get; set; } - public string ChownUser { get; set; } - public string ChownGroup { get; set; } public bool SkipFreeSpaceCheckWhenImporting { get; set; } public bool CopyUsingHardlinks { get; set; } @@ -42,9 +39,6 @@ namespace NzbDrone.Api.Config SetPermissionsLinux = model.SetPermissionsLinux, FileChmod = model.FileChmod, - FolderChmod = model.FolderChmod, - ChownUser = model.ChownUser, - ChownGroup = model.ChownGroup, SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting, CopyUsingHardlinks = model.CopyUsingHardlinks, diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 5744294f5..b989e348c 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -1037,7 +1037,7 @@ namespace NzbDrone.Common.Test.DiskTests .Returns(new List()); Mocker.GetMock() - .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny(), false)); + .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny())); } private void WithRealDiskProvider() @@ -1094,7 +1094,7 @@ namespace NzbDrone.Common.Test.DiskTests .Returns(s => new FileStream(s, FileMode.Open, FileAccess.Read)); Mocker.GetMock() - .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny(), false)); + .Setup(v => v.CopyPermissions(It.IsAny(), It.IsAny())); } private void WithMockMount(string root) diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index 39a1dfeea..866f1a0fd 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -31,8 +31,8 @@ namespace NzbDrone.Common.Disk public abstract long? GetAvailableSpace(string path); public abstract void InheritFolderPermissions(string filename); - public abstract void SetPermissions(string path, string mask, string user, string group); - public abstract void CopyPermissions(string sourcePath, string targetPath, bool includeOwner); + public abstract void SetPermissions(string path, string mask); + public abstract void CopyPermissions(string sourcePath, string targetPath); public abstract long? GetTotalSize(string path); public DateTime FolderGetCreationTime(string path) @@ -515,5 +515,10 @@ namespace NzbDrone.Common.Disk stream.CopyTo(fileStream); } } + + public virtual bool IsValidFilePermissionMask(string mask) + { + throw new NotSupportedException(); + } } } diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index 030315721..b5df2d460 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -10,8 +10,8 @@ namespace NzbDrone.Common.Disk { long? GetAvailableSpace(string path); void InheritFolderPermissions(string filename); - void SetPermissions(string path, string mask, string user, string group); - void CopyPermissions(string sourcePath, string targetPath, bool includeOwner = false); + void SetPermissions(string path, string mask); + void CopyPermissions(string sourcePath, string targetPath); long? GetTotalSize(string path); DateTime FolderGetCreationTime(string path); DateTime FolderGetLastWrite(string path); @@ -52,5 +52,6 @@ namespace NzbDrone.Common.Disk List GetFileInfos(string path); void RemoveEmptySubfolders(string path); void SaveStream(Stream stream, string path); + bool IsValidFilePermissionMask(string mask); } } diff --git a/src/NzbDrone.Core/Configuration/ConfigService.cs b/src/NzbDrone.Core/Configuration/ConfigService.cs index d2562ff4b..aae622625 100644 --- a/src/NzbDrone.Core/Configuration/ConfigService.cs +++ b/src/NzbDrone.Core/Configuration/ConfigService.cs @@ -259,27 +259,6 @@ namespace NzbDrone.Core.Configuration set { SetValue("FileChmod", value); } } - public string FolderChmod - { - get { return GetValue("FolderChmod", "0755"); } - - set { SetValue("FolderChmod", value); } - } - - public string ChownUser - { - get { return GetValue("ChownUser", ""); } - - set { SetValue("ChownUser", value); } - } - - public string ChownGroup - { - get { return GetValue("ChownGroup", ""); } - - set { SetValue("ChownGroup", value); } - } - public int FirstDayOfWeek { get { return GetValueInt("FirstDayOfWeek", (int)CultureInfo.CurrentCulture.DateTimeFormat.FirstDayOfWeek); } diff --git a/src/NzbDrone.Core/Configuration/IConfigService.cs b/src/NzbDrone.Core/Configuration/IConfigService.cs index c736e0900..bdad8d54e 100644 --- a/src/NzbDrone.Core/Configuration/IConfigService.cs +++ b/src/NzbDrone.Core/Configuration/IConfigService.cs @@ -44,9 +44,6 @@ namespace NzbDrone.Core.Configuration //Permissions (Media Management) bool SetPermissionsLinux { get; set; } string FileChmod { get; set; } - string FolderChmod { get; set; } - string ChownUser { get; set; } - string ChownGroup { get; set; } //Indexers int Retention { get; set; } diff --git a/src/NzbDrone.Core/Datastore/Migration/140_remove_chown_and_folderchmod_config.cs b/src/NzbDrone.Core/Datastore/Migration/140_remove_chown_and_folderchmod_config.cs new file mode 100644 index 000000000..96997f6bf --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/140_remove_chown_and_folderchmod_config.cs @@ -0,0 +1,15 @@ +using System.Data; +using FluentMigrator; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(140)] + public class remove_chown_and_folderchmod_config : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + Execute.Sql("DELETE FROM config WHERE Key IN ('folderchmod', 'chownuser', 'chowngroup')"); + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs index 1c2c67225..984000996 100644 --- a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs +++ b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs @@ -186,8 +186,8 @@ namespace NzbDrone.Core.MediaFiles try { - var permissions = _configService.FolderChmod; - _diskProvider.SetPermissions(path, permissions, _configService.ChownUser, _configService.ChownGroup); + var permissions = _configService.FileChmod; + _diskProvider.SetPermissions(path, permissions); } catch (Exception ex) diff --git a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs index a874be0f6..52a8b37aa 100644 --- a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs +++ b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs @@ -63,7 +63,7 @@ namespace NzbDrone.Core.MediaFiles { if (OsInfo.IsNotWindows) { - SetMonoPermissions(path, _configService.FolderChmod); + SetMonoPermissions(path, _configService.FileChmod); } } @@ -85,7 +85,7 @@ namespace NzbDrone.Core.MediaFiles try { - _diskProvider.SetPermissions(path, permissions, _configService.ChownUser, _configService.ChownGroup); + _diskProvider.SetPermissions(path, permissions); } catch (Exception ex) diff --git a/src/NzbDrone.Core/Validation/FileChmodValidator.cs b/src/NzbDrone.Core/Validation/FileChmodValidator.cs new file mode 100644 index 000000000..c1a8dba44 --- /dev/null +++ b/src/NzbDrone.Core/Validation/FileChmodValidator.cs @@ -0,0 +1,23 @@ +using FluentValidation.Validators; +using NzbDrone.Common.Disk; + +namespace NzbDrone.Core.Validation +{ + public class FileChmodValidator : PropertyValidator + { + private readonly IDiskProvider _diskProvider; + + public FileChmodValidator(IDiskProvider diskProvider) + : base("Must contain a valid Unix permissions octal") + { + _diskProvider = diskProvider; + } + + protected override bool IsValid(PropertyValidatorContext context) + { + if (context.PropertyValue == null) return false; + + return _diskProvider.IsValidFilePermissionMask(context.PropertyValue.ToString()); + } + } +} \ No newline at end of file diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 99cbdab73..445224826 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -151,11 +151,95 @@ namespace NzbDrone.Mono.Test.DiskProviderTests Syscall.stat(dst, out var dstStat); dstStat.st_mode.Should().Be(origStat.st_mode); - Subject.CopyPermissions(src, dst, false); + Subject.CopyPermissions(src, dst); // Verify CopyPermissions Syscall.stat(dst, out dstStat); dstStat.st_mode.Should().Be(srcStat.st_mode); } + + [Test] + public void should_set_file_permissions() + { + var tempFile = GetTempFilePath(); + + File.WriteAllText(tempFile, "File1"); + SetWritePermissions(tempFile, false); + + // Verify test setup + Syscall.stat(tempFile, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0444"); + + Subject.SetPermissions(tempFile, "644"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); + + Subject.SetPermissions(tempFile, "0644"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0644"); + + Subject.SetPermissions(tempFile, "1664"); + Syscall.stat(tempFile, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1664"); + } + + [Test] + public void should_set_folder_permissions() + { + var tempPath = GetTempFilePath(); + + Directory.CreateDirectory(tempPath); + SetWritePermissions(tempPath, false); + + // Verify test setup + Syscall.stat(tempPath, out var fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0555"); + + Subject.SetPermissions(tempPath, "644"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "0644"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0755"); + + Subject.SetPermissions(tempPath, "1664"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("1775"); + + Subject.SetPermissions(tempPath, "775"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + + Subject.SetPermissions(tempPath, "640"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0750"); + + Subject.SetPermissions(tempPath, "0041"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0051"); + } + + [Test] + public void IsValidFilePermissionMask_should_return_correct() + { + // Files may not be executable + Subject.IsValidFilePermissionMask("0777").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0544").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0454").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0445").Should().BeFalse(); + + // No special bits should be set + Subject.IsValidFilePermissionMask("1644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("2644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("4644").Should().BeFalse(); + Subject.IsValidFilePermissionMask("7644").Should().BeFalse(); + + // Files should be readable and writeable by owner + Subject.IsValidFilePermissionMask("0400").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0000").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0200").Should().BeFalse(); + Subject.IsValidFilePermissionMask("0600").Should().BeTrue(); + } } } diff --git a/src/NzbDrone.Mono/Disk/DiskProvider.cs b/src/NzbDrone.Mono/Disk/DiskProvider.cs index 0e0b43077..9a832d5d1 100644 --- a/src/NzbDrone.Mono/Disk/DiskProvider.cs +++ b/src/NzbDrone.Mono/Disk/DiskProvider.cs @@ -72,13 +72,62 @@ namespace NzbDrone.Mono.Disk } } - public override void SetPermissions(string path, string mask, string user, string group) + public override void SetPermissions(string path, string mask) { - SetPermissions(path, mask); - SetOwner(path, user, group); + Logger.Debug("Setting permissions: {0} on {1}", mask, path); + + var permissions = NativeConvert.FromOctalPermissionString(mask); + + if (Directory.Exists(path)) + { + permissions = GetFolderPermissions(permissions); + } + + if (Syscall.chmod(path, permissions) < 0) + { + var error = Stdlib.GetLastError(); + + throw new LinuxPermissionsException("Error setting permissions: " + error); + } } - public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner) + private static FilePermissions GetFolderPermissions(FilePermissions permissions) + { + permissions |= (FilePermissions) ((int) (permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IRGRP | FilePermissions.S_IROTH)) >> 2); + + return permissions; + } + + public override bool IsValidFilePermissionMask(string mask) + { + try + { + var permissions = NativeConvert.FromOctalPermissionString(mask); + + if ((permissions & (FilePermissions.S_ISUID | FilePermissions.S_ISGID | FilePermissions.S_ISVTX)) != 0) + { + return false; + } + + if ((permissions & (FilePermissions.S_IXUSR | FilePermissions.S_IXGRP | FilePermissions.S_IXOTH)) != 0) + { + return false; + } + + if ((permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) != (FilePermissions.S_IRUSR | FilePermissions.S_IWUSR)) + { + return false; + } + + return true; + } + catch (FormatException) + { + return false; + } + } + + public override void CopyPermissions(string sourcePath, string targetPath) { try { @@ -89,11 +138,6 @@ namespace NzbDrone.Mono.Disk { Syscall.chmod(targetPath, srcStat.st_mode); } - - if (includeOwner && (srcStat.st_uid != tgtStat.st_uid || srcStat.st_gid != tgtStat.st_gid)) - { - Syscall.chown(targetPath, srcStat.st_uid, srcStat.st_gid); - } } catch (Exception ex) { @@ -361,39 +405,6 @@ namespace NzbDrone.Mono.Disk } } - private void SetPermissions(string path, string mask) - { - Logger.Debug("Setting permissions: {0} on {1}", mask, path); - - var filePermissions = NativeConvert.FromOctalPermissionString(mask); - - if (Syscall.chmod(path, filePermissions) < 0) - { - var error = Stdlib.GetLastError(); - - throw new LinuxPermissionsException("Error setting file permissions: " + error); - } - } - - private void SetOwner(string path, string user, string group) - { - if (string.IsNullOrWhiteSpace(user) && string.IsNullOrWhiteSpace(group)) - { - Logger.Debug("User and Group for chown not configured, skipping chown."); - return; - } - - var userId = GetUserId(user); - var groupId = GetGroupId(group); - - if (Syscall.chown(path, userId, groupId) < 0) - { - var error = Stdlib.GetLastError(); - - throw new LinuxPermissionsException("Error setting file owner and/or group: " + error); - } - } - private uint GetUserId(string user) { if (user.IsNullOrWhiteSpace()) diff --git a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs index b3a395e7a..508556e80 100644 --- a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs +++ b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs @@ -129,7 +129,7 @@ namespace NzbDrone.Update.UpdateEngine { // Old MacOS App stores Sonarr binaries in MacOS together with shell script // Make shim executable - _diskProvider.SetPermissions(shimPath, "0755", null, null); + _diskProvider.SetPermissions(shimPath, "0755"); } } } diff --git a/src/NzbDrone.Windows/Disk/DiskProvider.cs b/src/NzbDrone.Windows/Disk/DiskProvider.cs index 54cfb285f..f2bf301b4 100644 --- a/src/NzbDrone.Windows/Disk/DiskProvider.cs +++ b/src/NzbDrone.Windows/Disk/DiskProvider.cs @@ -44,12 +44,12 @@ namespace NzbDrone.Windows.Disk File.SetAccessControl(filename, fs); } - public override void SetPermissions(string path, string mask, string user, string group) + public override void SetPermissions(string path, string mask) { } - public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner) + public override void CopyPermissions(string sourcePath, string targetPath) { } diff --git a/src/Sonarr.Api.V3/Config/MediaManagementConfigModule.cs b/src/Sonarr.Api.V3/Config/MediaManagementConfigModule.cs index 75a9ca514..5a09620a3 100644 --- a/src/Sonarr.Api.V3/Config/MediaManagementConfigModule.cs +++ b/src/Sonarr.Api.V3/Config/MediaManagementConfigModule.cs @@ -1,17 +1,18 @@ using FluentValidation; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Validation; using NzbDrone.Core.Validation.Paths; namespace Sonarr.Api.V3.Config { public class MediaManagementConfigModule : SonarrConfigModule { - public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator) + public MediaManagementConfigModule(IConfigService configService, PathExistsValidator pathExistsValidator, FileChmodValidator fileChmodValidator) : base(configService) { SharedValidator.RuleFor(c => c.RecycleBinCleanupDays).GreaterThanOrEqualTo(0); - SharedValidator.RuleFor(c => c.FileChmod).NotEmpty(); - SharedValidator.RuleFor(c => c.FolderChmod).NotEmpty(); + SharedValidator.RuleFor(c => c.FileChmod).SetValidator(fileChmodValidator).When(c => !string.IsNullOrEmpty(c.FileChmod) && PlatformInfo.IsMono); SharedValidator.RuleFor(c => c.RecycleBin).IsValidPath().SetValidator(pathExistsValidator).When(c => !string.IsNullOrWhiteSpace(c.RecycleBin)); SharedValidator.RuleFor(c => c.MinimumFreeSpaceWhenImporting).GreaterThanOrEqualTo(100); } diff --git a/src/Sonarr.Api.V3/Config/MediaManagementConfigResource.cs b/src/Sonarr.Api.V3/Config/MediaManagementConfigResource.cs index bc53cf064..4039530e0 100644 --- a/src/Sonarr.Api.V3/Config/MediaManagementConfigResource.cs +++ b/src/Sonarr.Api.V3/Config/MediaManagementConfigResource.cs @@ -19,9 +19,6 @@ namespace Sonarr.Api.V3.Config public bool SetPermissionsLinux { get; set; } public string FileChmod { get; set; } - public string FolderChmod { get; set; } - public string ChownUser { get; set; } - public string ChownGroup { get; set; } public EpisodeTitleRequiredType EpisodeTitleRequired { get; set; } public bool SkipFreeSpaceCheckWhenImporting { get; set; } @@ -49,9 +46,6 @@ namespace Sonarr.Api.V3.Config SetPermissionsLinux = model.SetPermissionsLinux, FileChmod = model.FileChmod, - FolderChmod = model.FolderChmod, - ChownUser = model.ChownUser, - ChownGroup = model.ChownGroup, EpisodeTitleRequired = model.EpisodeTitleRequired, SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting,