diff --git a/src/AssemblyInfo.cs b/src/AssemblyInfo.cs index ee99582..e5059fe 100644 --- a/src/AssemblyInfo.cs +++ b/src/AssemblyInfo.cs @@ -14,7 +14,7 @@ * limitations under the License. */ -//------------------------------------------------------------------------------ +//------------------------------------------------------------------------------ // // This code was generated by GitVersion. // diff --git a/src/Plugins/AWSS3/StorageAdminService.cs b/src/Plugins/AWSS3/StorageAdminService.cs index 181836e..41f2ab0 100644 --- a/src/Plugins/AWSS3/StorageAdminService.cs +++ b/src/Plugins/AWSS3/StorageAdminService.cs @@ -18,11 +18,13 @@ using System.Threading.Tasks; using Amazon.SecurityToken.Model; using Monai.Deploy.Storage.API; +using Monai.Deploy.Storage.S3Policy.Policies; namespace Monai.Deploy.Storage.AWSS3 { public class StorageAdminService : IStorageAdminService { public Task CreateUserAsync(string username, AccessPermissions permissions, string[] bucketNames) => throw new NotImplementedException(); + public Task CreateUserAsync(string username, PolicyRequest[] policyRequests) => throw new NotImplementedException(); } } diff --git a/src/Plugins/MinIO/Mc/mc.exe b/src/Plugins/MinIO/Mc/mc.exe new file mode 100644 index 0000000..1f83bb1 Binary files /dev/null and b/src/Plugins/MinIO/Mc/mc.exe differ diff --git a/src/Plugins/MinIO/Monai.Deploy.Storage.MinIO.csproj b/src/Plugins/MinIO/Monai.Deploy.Storage.MinIO.csproj index 11d881b..2882084 100644 --- a/src/Plugins/MinIO/Monai.Deploy.Storage.MinIO.csproj +++ b/src/Plugins/MinIO/Monai.Deploy.Storage.MinIO.csproj @@ -63,4 +63,5 @@ + diff --git a/src/Plugins/MinIO/StorageAdminService.cs b/src/Plugins/MinIO/StorageAdminService.cs index b323067..55ce093 100644 --- a/src/Plugins/MinIO/StorageAdminService.cs +++ b/src/Plugins/MinIO/StorageAdminService.cs @@ -22,7 +22,6 @@ using Microsoft.Extensions.Options; using Monai.Deploy.Storage.API; using Monai.Deploy.Storage.Configuration; -using Monai.Deploy.Storage.Minio.Extensions; using Monai.Deploy.Storage.S3Policy; using Monai.Deploy.Storage.S3Policy.Policies; @@ -30,11 +29,16 @@ namespace Monai.Deploy.Storage.MinIO { public class StorageAdminService : IStorageAdminService { - private const string UserCommand = "admin user list minio"; private readonly string _executableLocation; private readonly string _serviceName; private readonly string _temporaryFilePath; + private readonly string _endpoint; + private readonly string _accessKey; + private readonly string _secretKey; private readonly IFileSystem _fileSystem; + private readonly string _set_connection_cmd; + private readonly string _get_connections_cmd; + private readonly string _get_users_cmd; public StorageAdminService(IOptions options, ILogger logger, IFileSystem fileSystem) { @@ -48,56 +52,34 @@ public StorageAdminService(IOptions options, ILogge _executableLocation = options.Value.Settings[ConfigurationKeys.McExecutablePath]; _serviceName = options.Value.Settings[ConfigurationKeys.McServiceName]; _temporaryFilePath = _fileSystem.Path.GetTempPath(); + _endpoint = options.Value.Settings[ConfigurationKeys.EndPoint]; + _accessKey = options.Value.Settings[ConfigurationKeys.AccessKey]; + _secretKey = options.Value.Settings[ConfigurationKeys.AccessToken]; + _set_connection_cmd = $"alias set {_serviceName} http://{_endpoint} {_accessKey} {_secretKey}"; + _get_connections_cmd = "alias list"; + _get_users_cmd = $"admin user list {_serviceName}"; } - public async Task CreateUserAsync(string username, AccessPermissions permissions, string[] bucketNames) + private static void ValidateConfiguration(StorageServiceConfiguration configuration) { - Guard.Against.NullOrWhiteSpace(username, nameof(username)); - Guard.Against.NullOrEmpty(bucketNames, nameof(bucketNames)); - - if (await UserAlreadyExistsAsync(username).ConfigureAwait(false)) - { - throw new InvalidOperationException("User already exists"); - } - - var credentials = new Credentials(); - var userSecretKey = Convert.ToBase64String(Guid.NewGuid().ToByteArray()); - credentials.SecretAccessKey = userSecretKey; - credentials.AccessKeyId = username; - - var result = await Execute(CreateUserCmd(username, userSecretKey)).ConfigureAwait(false); - - if (!result.Any(r => r.Contains($"Added user `{username}` successfully."))) - { - await RemoveUserAsync(username).ConfigureAwait(false); - throw new InvalidOperationException($"Unknown Output {result.SelectMany(e => e)}"); - } - - var minioPolicies = new List() - { - permissions.GetString() - }; - - var policyRequests = bucketNames.Select( - bucket => new PolicyRequest(bucket, "") - ).ToArray(); + Guard.Against.Null(configuration, nameof(configuration)); - await CreatePolicyAsync(policyRequests, username).ConfigureAwait(false); - var setPolicyResult = await SetPolicyAsync(IdentityType.User, minioPolicies, credentials.AccessKeyId).ConfigureAwait(false); - if (!setPolicyResult) + foreach (var key in ConfigurationKeys.McRequiredKeys) { - await RemoveUserAsync(username).ConfigureAwait(false); - throw new InvalidOperationException("Failed to set policy, user has been removed"); + if (!configuration.Settings.ContainsKey(key)) + { + throw new ConfigurationException($"IMinioAdmin Shell is missing configuration for {key}."); + } } - - return credentials; } - private async Task SetPolicyAsync(IdentityType policyType, List policies, string itemName) + private string CreateUserCmd(string username, string secretKey) => $"admin user add {_serviceName} {username} {secretKey}"; + + public async Task SetPolicyAsync(IdentityType policyType, List policies, string itemName) { var policiesStr = string.Join(',', policies); var setPolicyCmd = $"admin policy set {_serviceName} {policiesStr} {policyType.ToString().ToLower()}={itemName}"; - var result = await Execute(setPolicyCmd).ConfigureAwait(false); + var result = await ExecuteAsync(setPolicyCmd).ConfigureAwait(false); var expectedResult = $"Policy `{policiesStr}` is set on {policyType.ToString().ToLower()} `{itemName}`"; if (!result.Any(r => r.Contains(expectedResult))) @@ -107,17 +89,7 @@ private async Task SetPolicyAsync(IdentityType policyType, List po return true; } - private async Task RemoveUserAsync(string username) - { - var result = await Execute($"admin user remove {_serviceName} {username}").ConfigureAwait(false); - - if (!result.Any(r => r.Contains($"Removed user `{username}` successfully."))) - { - throw new InvalidOperationException("Unable to remove user"); - } - } - - private async Task> Execute(string cmd) + private async Task> ExecuteAsync(string cmd) { if (cmd.StartsWith("mc")) { @@ -126,19 +98,41 @@ private async Task> Execute(string cmd) using (var process = CreateProcess(cmd)) { - var (lines, errors) = await RunProcess(process).ConfigureAwait(false); + var (lines, errors) = await RunProcessAsync(process); if (errors.Any()) { - throw new InvalidOperationException($"Unknown Error {errors.SelectMany(e => e)}"); + throw new InvalidOperationException($"Unknown Error {string.Join("\n", errors)}"); } return lines; } } + private static async Task<(List Output, List Errors)> RunProcessAsync(Process process) + { + var output = new List(); + var errors = new List(); + process.Start(); + while (!process.StandardOutput.EndOfStream) + { + var line = process.StandardOutput.ReadLine(); + if (line == null) continue; + output.Add(line); + } + while (!process.StandardError.EndOfStream) + { + var line = process.StandardError.ReadLine(); + if (line == null) continue; + errors.Add(line); + } + + await process.WaitForExitAsync().ConfigureAwait(false); + return (output, errors); + } + private Process CreateProcess(string cmd) { - var startinfo = new ProcessStartInfo() + ProcessStartInfo startinfo = new() { FileName = _executableLocation, Arguments = cmd, @@ -148,7 +142,7 @@ private Process CreateProcess(string cmd) RedirectStandardError = true }; - var process = new Process() + Process process = new() { StartInfo = startinfo }; @@ -156,6 +150,93 @@ private Process CreateProcess(string cmd) return process; } + public async Task HasConnectionAsync() + { + var result = await ExecuteAsync(_get_connections_cmd).ConfigureAwait(false); + return result.Any(r => r.Equals(_serviceName)); + } + + public async Task SetConnectionAsync() + { + if (await HasConnectionAsync()) + { + return true; + } + var result = await ExecuteAsync(_set_connection_cmd).ConfigureAwait(false); + if (result.Any(r => r.Contains($"Added `{_serviceName}` successfully."))) + { + return true; + } + return false; + } + + public async Task UserAlreadyExistsAsync(string username) + { + var result = await ExecuteAsync(_get_users_cmd).ConfigureAwait(false); + return result.Any(r => r.Contains(username)); + } + + public async Task RemoveUserAsync(string username) + { + var result = await ExecuteAsync($"admin user remove {_serviceName} {username}").ConfigureAwait(false); + + if (!result.Any(r => r.Contains($"Removed user `{username}` successfully."))) + { + throw new InvalidOperationException("Unable to remove user"); + } + } + + [Obsolete("CreateUserAsync with bucketNames is deprecated, please use CreateUserAsync with an array of PolicyRequest instead.")] + public async Task CreateUserAsync(string username, AccessPermissions permissions, string[] bucketNames) + { + var policyRequests = new List(); + + for (var i = 0; i < bucketNames.Length; i++) + { + policyRequests.Add(new PolicyRequest(bucketNames[i], "/*")); + } + + return await CreateUserAsync(username, policyRequests.ToArray()).ConfigureAwait(false); + } + + public async Task CreateUserAsync(string username, PolicyRequest[] policyRequests) + { + if (!await SetConnectionAsync()) + { + throw new InvalidOperationException("Unable to set connection for more information, attempt mc alias set {_serviceName} http://{_endpoint} {_accessKey} {_secretKey}"); + } + if (await UserAlreadyExistsAsync(username)) + { + throw new InvalidOperationException("User already exists"); + } + + Credentials credentials = new(); + var userSecretKey = Convert.ToBase64String(Guid.NewGuid().ToByteArray()); + credentials.SecretAccessKey = userSecretKey; + credentials.AccessKeyId = username; + + var result = await ExecuteAsync(CreateUserCmd(username, userSecretKey)).ConfigureAwait(false); + + if (result.Any(r => r.Contains($"Added user `{username}` successfully.")) is false) + { + await RemoveUserAsync(username); + throw new InvalidOperationException($"Unknown Output {string.Join("\n", result)}"); + } + + + var policyName = await CreatePolicyAsync(policyRequests.ToArray(), username).ConfigureAwait(false); + var minioPolicies = new List { policyName }; + + var setPolicyResult = await SetPolicyAsync(IdentityType.User, minioPolicies, credentials.AccessKeyId).ConfigureAwait(false); + if (setPolicyResult is false) + { + await RemoveUserAsync(username).ConfigureAwait(false); + throw new InvalidOperationException("Failed to set policy, user has been removed"); + } + + return credentials; + } + /// /// Admin policy command requires json file for policy so we create file /// and remove it after setting the admin policy for the user. @@ -164,20 +245,18 @@ private Process CreateProcess(string cmd) /// /// /// - private async Task CreatePolicyAsync(PolicyRequest[] policyRequests, string username) + private async Task CreatePolicyAsync(PolicyRequest[] policyRequests, string username) { - Guard.Against.NullOrEmpty(policyRequests, nameof(policyRequests)); - Guard.Against.NullOrWhiteSpace(username, nameof(username)); - - var userFile = await CreatePolicyFile(policyRequests, username).ConfigureAwait(false); - var result = await Execute($"admin policy {_serviceName} pol_{username} {username}.json").ConfigureAwait(false); - if (!result.Any(r => r.Contains($"Added policy `pol_{username}` successfully."))) + var policyFileName = await CreatePolicyFile(policyRequests, username).ConfigureAwait(false); + var result = await ExecuteAsync($"admin policy add {_serviceName} pol_{username} {policyFileName}").ConfigureAwait(false); + if (result.Any(r => r.Contains($"Added policy `pol_{username}` successfully.")) is false) { - await RemoveUserAsync(username).ConfigureAwait(false); + await RemoveUserAsync(username); File.Delete($"{username}.json"); throw new InvalidOperationException("Failed to create policy, user has been removed"); } - File.Delete(userFile); + File.Delete($"{username}.json"); + return $"pol_{username}"; } private async Task CreatePolicyFile(PolicyRequest[] policyRequests, string username) @@ -192,48 +271,5 @@ private async Task CreatePolicyFile(PolicyRequest[] policyRequests, stri await _fileSystem.File.WriteAllLinesAsync(filename, lines).ConfigureAwait(false); return filename; } - - private async Task UserAlreadyExistsAsync(string username) - { - var result = await Execute(UserCommand).ConfigureAwait(false); - return result.Any(r => r.Contains(username)); - } - - private static void ValidateConfiguration(StorageServiceConfiguration configuration) - { - Guard.Against.Null(configuration, nameof(configuration)); - - foreach (var key in ConfigurationKeys.McRequiredKeys) - { - if (!configuration.Settings.ContainsKey(key)) - { - throw new ConfigurationException($"IMinioAdmin Shell is missing configuration for {key}."); - } - } - } - - private string CreateUserCmd(string username, string secretKey) => $"admin user add {_serviceName} {username} {secretKey}"; - - private static async Task<(List Output, List Errors)> RunProcess(Process process) - { - var output = new List(); - var errors = new List(); - process.Start(); - while (!process.StandardOutput.EndOfStream) - { - var line = process.StandardOutput.ReadLine(); - if (line == null) continue; - output.Add(line); - } - while (!process.StandardError.EndOfStream) - { - var line = process.StandardError.ReadLine(); - if (line == null) continue; - errors.Add(line); - } - - await process.WaitForExitAsync().ConfigureAwait(false); - return (output, errors); - } } } diff --git a/src/Plugins/MinIO/Tests/MinioPolicyExtensionsTest.cs b/src/Plugins/MinIO/Tests/MinioPolicyExtensionsTest.cs index 617daa6..2494de9 100644 --- a/src/Plugins/MinIO/Tests/MinioPolicyExtensionsTest.cs +++ b/src/Plugins/MinIO/Tests/MinioPolicyExtensionsTest.cs @@ -14,9 +14,15 @@ * limitations under the License. */ +using Microsoft.Extensions.Options; using Monai.Deploy.Storage.API; +using Monai.Deploy.Storage.Configuration; using Monai.Deploy.Storage.Minio.Extensions; using Xunit; +using Moq; +using System.IO.Abstractions; +using Microsoft.Extensions.Logging; +using Monai.Deploy.Storage.S3Policy.Policies; namespace Monai.Deploy.Storage.MinIO.Tests { @@ -32,5 +38,42 @@ public void GetString_Test(AccessPermissions permissions, string expectedValue) { Assert.Equal(expectedValue, permissions.GetString()); } + + // integration test needs Minio setup + //[Fact] + public async Task Should_Set_Correct_Policy() + { + var optionSettings = new StorageServiceConfiguration + { + Settings = new Dictionary { + { "executableLocation", "mc.exe" }, + { "serviceName", "serviceName" }, + { "endpoint", "localhost:9000" }, + { "accessKey", "admin" }, + { "accessToken", "password" }, + } + }; + var options = Options.Create(optionSettings); + + var systemUnderTest = new StorageAdminService(options, new Mock>().Object, new FileSystem()); + const string bucketName = "test-bucket"; + const string payloadId = "00000000-1000-0000-0000-000000000000"; + const string userName = "nameUsedForTests"; + + var policys = new PolicyRequest[] { new PolicyRequest(bucketName, payloadId) }; + + try + { + var result = await systemUnderTest.CreateUserAsync(userName, policys); + } + catch (Exception ex) + { + var message = ex.Message; + } + finally + { + await systemUnderTest.RemoveUserAsync(userName); + } + } } } diff --git a/src/Plugins/MinIO/Tests/mc.exe b/src/Plugins/MinIO/Tests/mc.exe new file mode 100644 index 0000000..1f83bb1 Binary files /dev/null and b/src/Plugins/MinIO/Tests/mc.exe differ diff --git a/src/S3Policy/Policies/Condition.cs b/src/S3Policy/Policies/Condition.cs index 6400872..6a29ee6 100644 --- a/src/S3Policy/Policies/Condition.cs +++ b/src/S3Policy/Policies/Condition.cs @@ -14,12 +14,16 @@ * limitations under the License. */ +using Newtonsoft.Json; + namespace Monai.Deploy.Storage.S3Policy.Policies { public class Condition { + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public StringLike? StringLike { get; set; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public StringEquals? StringEquals { get; set; } } } diff --git a/src/S3Policy/Policies/Statement.cs b/src/S3Policy/Policies/Statement.cs index 7f8a21b..18b9df4 100644 --- a/src/S3Policy/Policies/Statement.cs +++ b/src/S3Policy/Policies/Statement.cs @@ -14,6 +14,8 @@ * limitations under the License. */ +using Newtonsoft.Json; + namespace Monai.Deploy.Storage.S3Policy.Policies { public class Statement @@ -26,6 +28,7 @@ public class Statement public string[]? Resource { get; set; } + [JsonProperty(NullValueHandling = NullValueHandling.Ignore)] public Condition? Condition { get; set; } } -} +} \ No newline at end of file diff --git a/src/S3Policy/Policies/StringEquals.cs b/src/S3Policy/Policies/StringEquals.cs index 3d11e6f..3cacd14 100644 --- a/src/S3Policy/Policies/StringEquals.cs +++ b/src/S3Policy/Policies/StringEquals.cs @@ -13,17 +13,17 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - + using Newtonsoft.Json; namespace Monai.Deploy.Storage.S3Policy.Policies { public class StringEquals { - [JsonProperty(PropertyName = "s3:prefix")] + [JsonProperty(PropertyName = "s3:prefix", NullValueHandling = NullValueHandling.Ignore)] public string[]? S3Prefix { get; set; } - [JsonProperty(PropertyName = "s3:delimiter")] + [JsonProperty(PropertyName = "s3:delimiter", NullValueHandling = NullValueHandling.Ignore)] public string[]? S3Delimiter { get; set; } } } diff --git a/src/S3Policy/PolicyExtensions.cs b/src/S3Policy/PolicyExtensions.cs index 89438c2..9912ae4 100644 --- a/src/S3Policy/PolicyExtensions.cs +++ b/src/S3Policy/PolicyExtensions.cs @@ -124,9 +124,10 @@ public static Policy ToPolicy(PolicyRequest[] policyRequests) Resource = policyRequests.Select(pr => pr.BucketName).ToArray(), Condition = new Condition { - StringEquals = new StringEquals + StringLike = new StringLike { - S3Prefix = policyRequests.Select(pr => $"{pr.FolderName}/*").ToArray(), + S3Prefix = policyRequests.Select(pr => $"{pr.FolderName}/*") + .Union( policyRequests.Select(pr => $"{pr.FolderName}")).ToArray() } } }, @@ -135,7 +136,7 @@ public static Policy ToPolicy(PolicyRequest[] policyRequests) Sid = "AllowAllS3ActionsInUserFolder", Action = new string[] { "s3:*" }, Effect = "Allow", - Resource = policyRequests.Select(pr => $"{pr.BucketName}/{pr.FolderName}").ToArray(), + Resource = policyRequests.Select(pr => $"{pr.BucketName}/{pr.FolderName}/*").ToArray(), }, } }; diff --git a/src/Storage/API/IStorageAdminService.cs b/src/Storage/API/IStorageAdminService.cs index 584721a..eca09bf 100644 --- a/src/Storage/API/IStorageAdminService.cs +++ b/src/Storage/API/IStorageAdminService.cs @@ -15,6 +15,7 @@ */ using Amazon.SecurityToken.Model; +using Monai.Deploy.Storage.S3Policy.Policies; namespace Monai.Deploy.Storage.API { @@ -27,6 +28,15 @@ public interface IStorageAdminService /// User permissions /// Name of the bucket that the user needs access to /// + [Obsolete("CreateUserAsync with bucketNames is deprecated, please use CreateUserAsync with an array of PolicyRequest instead.")] Task CreateUserAsync(string username, AccessPermissions permissions, string[] bucketNames); + + /// + /// Creates a user with read only permissions + /// + /// Username + /// Contains the buckets and folders that the user needs access to + /// + Task CreateUserAsync(string username, PolicyRequest[] policyRequests); } }