diff --git a/frontend/src/Settings/MediaManagement/MediaManagement.js b/frontend/src/Settings/MediaManagement/MediaManagement.js index 4c43fc3f4..11b41fcbd 100644 --- a/frontend/src/Settings/MediaManagement/MediaManagement.js +++ b/frontend/src/Settings/MediaManagement/MediaManagement.js @@ -370,7 +370,7 @@ class MediaManagement extends Component { - - - Folder chmod mode - - - - - - chown User - - - - - - chown Group - - - } diff --git a/src/Lidarr.Api.V1/Config/MediaManagementConfigModule.cs b/src/Lidarr.Api.V1/Config/MediaManagementConfigModule.cs index e9730ad57..236572a50 100644 --- a/src/Lidarr.Api.V1/Config/MediaManagementConfigModule.cs +++ b/src/Lidarr.Api.V1/Config/MediaManagementConfigModule.cs @@ -1,17 +1,18 @@ -using FluentValidation; +using FluentValidation; +using NzbDrone.Common.EnvironmentInfo; using NzbDrone.Core.Configuration; +using NzbDrone.Core.Validation; using NzbDrone.Core.Validation.Paths; namespace Lidarr.Api.V1.Config { public class MediaManagementConfigModule : LidarrConfigModule { - 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) && (OsInfo.IsLinux || OsInfo.IsOsx)); 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/Lidarr.Api.V1/Config/MediaManagementConfigResource.cs b/src/Lidarr.Api.V1/Config/MediaManagementConfigResource.cs index 99819ced9..e49186fb9 100644 --- a/src/Lidarr.Api.V1/Config/MediaManagementConfigResource.cs +++ b/src/Lidarr.Api.V1/Config/MediaManagementConfigResource.cs @@ -20,9 +20,6 @@ namespace Lidarr.Api.V1.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 int MinimumFreeSpaceWhenImporting { get; set; } @@ -50,9 +47,6 @@ namespace Lidarr.Api.V1.Config SetPermissionsLinux = model.SetPermissionsLinux, FileChmod = model.FileChmod, - FolderChmod = model.FolderChmod, - ChownUser = model.ChownUser, - ChownGroup = model.ChownGroup, SkipFreeSpaceCheckWhenImporting = model.SkipFreeSpaceCheckWhenImporting, MinimumFreeSpaceWhenImporting = model.MinimumFreeSpaceWhenImporting, diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 278d73ee1..d9cc1ae20 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -1048,7 +1048,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() @@ -1112,7 +1112,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 4627efef1..b2030fc35 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -38,8 +38,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) @@ -543,5 +543,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 2fe2f6359..ef1628888 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -11,8 +11,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); @@ -55,5 +55,6 @@ namespace NzbDrone.Common.Disk List GetFileInfos(string path, SearchOption searchOption = SearchOption.TopDirectoryOnly); 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 9cb64f62b..9cebfb8d9 100644 --- a/src/NzbDrone.Core/Configuration/ConfigService.cs +++ b/src/NzbDrone.Core/Configuration/ConfigService.cs @@ -262,27 +262,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 string MetadataSource { get { return GetValue("MetadataSource", ""); } diff --git a/src/NzbDrone.Core/Configuration/IConfigService.cs b/src/NzbDrone.Core/Configuration/IConfigService.cs index 3ec13925b..1b546cbbd 100644 --- a/src/NzbDrone.Core/Configuration/IConfigService.cs +++ b/src/NzbDrone.Core/Configuration/IConfigService.cs @@ -43,9 +43,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/045_remove_chown_and_folderchmod_config.cs b/src/NzbDrone.Core/Datastore/Migration/045_remove_chown_and_folderchmod_config.cs new file mode 100644 index 000000000..b1f37d2fe --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/045_remove_chown_and_folderchmod_config.cs @@ -0,0 +1,14 @@ +using FluentMigrator; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + [Migration(045)] + public class remove_chown_and_folderchmod_config : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + Execute.Sql("DELETE FROM config WHERE Key IN ('folderchmod', 'chownuser')"); + } + } +} diff --git a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs index 5165db163..0b2a8f13c 100644 --- a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs +++ b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs @@ -10,6 +10,7 @@ using NzbDrone.Common; using NzbDrone.Common.Disk; using NzbDrone.Common.Extensions; using NzbDrone.Common.Instrumentation.Extensions; +using NzbDrone.Core.Configuration; using NzbDrone.Core.MediaFiles.Commands; using NzbDrone.Core.MediaFiles.Events; using NzbDrone.Core.MediaFiles.TrackImport; @@ -36,6 +37,7 @@ namespace NzbDrone.Core.MediaFiles public static readonly Regex ExcludedSubFoldersRegex = new Regex(@"(?:\\|\/|^)(?:extras|@eadir|\.@__thumb|extrafanart|plex versions|\.[^\\/]+)(?:\\|\/)", RegexOptions.Compiled | RegexOptions.IgnoreCase); public static readonly Regex ExcludedFilesRegex = new Regex(@"^\._|^Thumbs\.db$|^\.DS_store$|\.partial~$", RegexOptions.Compiled | RegexOptions.IgnoreCase); + private readonly IConfigService _configService; private readonly IDiskProvider _diskProvider; private readonly IMediaFileService _mediaFileService; private readonly IMakeImportDecision _importDecisionMaker; @@ -46,7 +48,8 @@ namespace NzbDrone.Core.MediaFiles private readonly IEventAggregator _eventAggregator; private readonly Logger _logger; - public DiskScanService(IDiskProvider diskProvider, + public DiskScanService(IConfigService configService, + IDiskProvider diskProvider, IMediaFileService mediaFileService, IMakeImportDecision importDecisionMaker, IImportApprovedTracks importApprovedTracks, @@ -56,6 +59,7 @@ namespace NzbDrone.Core.MediaFiles IEventAggregator eventAggregator, Logger logger) { + _configService = configService; _diskProvider = diskProvider; _mediaFileService = mediaFileService; _importDecisionMaker = importDecisionMaker; diff --git a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs index cc7c400c1..203ba6b03 100644 --- a/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs +++ b/src/NzbDrone.Core/MediaFiles/MediaFileAttributeService.cs @@ -62,7 +62,7 @@ namespace NzbDrone.Core.MediaFiles { if (OsInfo.IsNotWindows) { - SetMonoPermissions(path, _configService.FolderChmod); + SetMonoPermissions(path, _configService.FileChmod); } } @@ -84,7 +84,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/Update/InstallUpdateService.cs b/src/NzbDrone.Core/Update/InstallUpdateService.cs index fbc04f6c2..0540db052 100644 --- a/src/NzbDrone.Core/Update/InstallUpdateService.cs +++ b/src/NzbDrone.Core/Update/InstallUpdateService.cs @@ -149,7 +149,7 @@ namespace NzbDrone.Core.Update // Set executable flag on update app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), "0755", null, null); + _diskProvider.SetPermissions(_appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime), "0755"); } _logger.Info("Starting update client {0}", _appFolderInfo.GetUpdateClientExePath(updatePackage.Runtime)); diff --git a/src/NzbDrone.Core/Validation/FileChmodValidator.cs b/src/NzbDrone.Core/Validation/FileChmodValidator.cs new file mode 100644 index 000000000..c9f0881a7 --- /dev/null +++ b/src/NzbDrone.Core/Validation/FileChmodValidator.cs @@ -0,0 +1,26 @@ +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()); + } + } +} diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs index 585c19eb4..19f008757 100644 --- a/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/DiskProviderFixture.cs @@ -151,11 +151,100 @@ 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"); + + // reinstate sane permissions so fokder can be cleaned up + Subject.SetPermissions(tempPath, "775"); + Syscall.stat(tempPath, out fileStat); + NativeConvert.ToOctalPermissionString(fileStat.st_mode).Should().Be("0775"); + } + + [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 623363b6a..e83da5ee8 100644 --- a/src/NzbDrone.Mono/Disk/DiskProvider.cs +++ b/src/NzbDrone.Mono/Disk/DiskProvider.cs @@ -83,13 +83,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); + } + } + + private static FilePermissions GetFolderPermissions(FilePermissions permissions) + { + permissions |= (FilePermissions)((int)(permissions & (FilePermissions.S_IRUSR | FilePermissions.S_IRGRP | FilePermissions.S_IROTH)) >> 2); + + return permissions; } - public override void CopyPermissions(string sourcePath, string targetPath, bool includeOwner) + 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 { @@ -100,11 +149,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) { @@ -385,39 +429,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 079169b86..03411e456 100644 --- a/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs +++ b/src/NzbDrone.Update/UpdateEngine/InstallUpdateService.cs @@ -122,7 +122,7 @@ namespace NzbDrone.Update.UpdateEngine // Set executable flag on Lidarr app if (OsInfo.IsOsx || (OsInfo.IsLinux && PlatformInfo.IsNetCore)) { - _diskProvider.SetPermissions(Path.Combine(installationFolder, "Lidarr"), "0755", null, null); + _diskProvider.SetPermissions(Path.Combine(installationFolder, "Lidarr"), "0755"); } } catch (Exception e) diff --git a/src/NzbDrone.Windows/Disk/DiskProvider.cs b/src/NzbDrone.Windows/Disk/DiskProvider.cs index a0a01e86e..eda3b2b8f 100644 --- a/src/NzbDrone.Windows/Disk/DiskProvider.cs +++ b/src/NzbDrone.Windows/Disk/DiskProvider.cs @@ -58,11 +58,11 @@ namespace NzbDrone.Windows.Disk file.SetAccessControl(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) { }