Amazon s3 media options update (#11871)

neglectedvalue · Piedone · web-flow · commit 9c8f94ff18a4 · 2022-06-27T21:07:42.000+02:00
Co-authored-by: ngv &lt;funkycatz@yandex.ru&gt;
Co-authored-by: Zoltán Lehóczky &lt;zoltan.lehoczky@lombiq.com&gt;
diff --git a/src/OrchardCore.Build/Dependencies.props b/src/OrchardCore.Build/Dependencies.props
@@ -11,6 +11,7 @@
     <PackageManagement Include="AngleSharp" Version="0.17.1" />
     <PackageManagement Include="AWSSDK.S3" Version="3.7.9.10" />
     <PackageManagement Include="AWSSDK.SecurityToken" Version="3.7.1.159" />
+    <PackageManagement Include="AWSSDK.Extensions.NETCore.Setup" Version="3.7.2" />
     <PackageManagement Include="Azure.Extensions.AspNetCore.Configuration.Secrets" Version="1.2.2" />
     <PackageManagement Include="Azure.Extensions.AspNetCore.DataProtection.Blobs" Version="1.2.1" />
     <PackageManagement Include="Azure.Identity" Version="1.6.0" />
diff --git a/src/OrchardCore.Cms.Web/appsettings.json b/src/OrchardCore.Cms.Web/appsettings.json
@@ -44,18 +44,16 @@
     //}
     // See https://docs.orchardcore.net/en/latest/docs/reference/modules/Media.AmazonS3/#configuration to configure media storage in Amazon S3 Storage.
     //"OrchardCore_Media_AmazonS3": {
-    //  "BucketName": "somebucketname",
-    // Credentials section needed only if Orchard will be hosted not in the AWS Cloud
-    // since in AWS you don't need to store this info in the appsettings file, you just
-    // need to set BucketName and BasePath.
-    //    "Credentials": {
-    //      "SecretKey": "",
-    //      "AccessKeyId": "",
-    //      "RegionEndpoint": "eu-central-1"
-    //    },
-    //    "BasePath": "/media",
-    //    "ProfileName": "",
-    //    "CreateBucket" : true
+    //  "Region": "eu-central-1",
+    //  "Profile": "default",
+    //  "ProfilesLocation": "",
+    //  "Credentials": {
+    //    "SecretKey": "",
+    //    "AccessKey": ""
+    //  },
+    //  "BasePath": "/media",
+    //  "CreateBucket": true,
+    //  "BucketName": ""
     //},
     // See https://docs.orchardcore.net/en/latest/docs/reference/modules/Media.Azure/#configuration to configure media storage in Azure Blob Storage.
     //"OrchardCore_Media_Azure":
diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsConfiguration.cs
@@ -29,7 +29,7 @@ public AwsStorageOptionsConfiguration(
 
     public void Configure(AwsStorageOptions options)
     {
-        options.BindConfiguration(_shellConfiguration);
+        options.BindConfiguration(_shellConfiguration, _logger);
 
         var templateOptions = new TemplateOptions();
         var templateContext = new TemplateContext(templateOptions);
diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsExtension.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/AwsStorageOptionsExtension.cs
@@ -2,8 +2,11 @@
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
 using Amazon;
+using Amazon.Extensions.NETCore.Setup;
+using Amazon.Runtime;
 using Amazon.Runtime.CredentialManagement;
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.Logging;
 using OrchardCore.Environment.Shell.Configuration;
 using OrchardCore.FileStorage.AmazonS3;
 
@@ -18,30 +21,20 @@ public static IEnumerable<ValidationResult> Validate(this AwsStorageOptions opti
             yield return new ValidationResult(Constants.ValidationMessages.BucketNameIsEmpty);
         }
 
-        if (options.Credentials != null)
+        if (options.AwsOptions is not null)
         {
-            if (String.IsNullOrWhiteSpace(options.Credentials.SecretKey))
-            {
-                yield return new ValidationResult(Constants.ValidationMessages.SecretKeyIsEmpty);
-            }
-
-            if (String.IsNullOrWhiteSpace(options.Credentials.AccessKeyId))
-            {
-                yield return new ValidationResult(Constants.ValidationMessages.AccessKeyIdIsEmpty);
-            }
-
-            if (String.IsNullOrWhiteSpace(options.Credentials.RegionEndpoint))
+            if (options.AwsOptions.Region is null)
             {
                 yield return new ValidationResult(Constants.ValidationMessages.RegionEndpointIsEmpty);
             }
         }
     }
 
-    public static AwsStorageOptions BindConfiguration(this AwsStorageOptions options, IShellConfiguration shellConfiguration)
+    public static AwsStorageOptions BindConfiguration(this AwsStorageOptions options, IShellConfiguration shellConfiguration, ILogger logger)
     {
         var section = shellConfiguration.GetSection("OrchardCore_Media_AmazonS3");
 
-        if (section == null)
+        if (!section.Exists())
         {
             return options;
         }
@@ -50,41 +43,33 @@ public static AwsStorageOptions BindConfiguration(this AwsStorageOptions options
         options.BasePath = section.GetValue(nameof(options.BasePath), String.Empty);
         options.CreateBucket = section.GetValue(nameof(options.CreateBucket), false);
 
-        var credentials = section.GetSection("Credentials");
-        if (credentials.Exists())
+        try
         {
-            options.Credentials = new AwsStorageCredentials
-            {
-                RegionEndpoint =
-                    credentials.GetValue(nameof(options.Credentials.RegionEndpoint), RegionEndpoint.USEast1.SystemName),
-                SecretKey = credentials.GetValue(nameof(options.Credentials.SecretKey), String.Empty),
-                AccessKeyId = credentials.GetValue(nameof(options.Credentials.AccessKeyId), String.Empty),
-            };
+            // Binding AWS Options
+            options.AwsOptions = shellConfiguration.GetAWSOptions("OrchardCore_Media_AmazonS3");
 
-        }
-        else
-        {
-            // Attempt to load Credentials from Profile.
-            var profileName = section.GetValue("ProfileName", String.Empty);
-            if (!String.IsNullOrEmpty(profileName))
+            // In case Credentials sections was specified, trying to add BasicAWSCredential to AWSOptions
+            // since by design GetAWSOptions skips Credential section while parsing config.
+            var credentials = section.GetSection("Credentials");
+            if (credentials.Exists())
             {
-                var chain = new CredentialProfileStoreChain();
-                if (chain.TryGetProfile(profileName, out var basicProfile))
+                var secretKey = credentials.GetValue(Constants.AwsCredentialParamNames.SecretKey, String.Empty);
+                var accessKey = credentials.GetValue(Constants.AwsCredentialParamNames.AccessKey, String.Empty);
+
+                if (!String.IsNullOrWhiteSpace(accessKey) ||
+                    !String.IsNullOrWhiteSpace(secretKey))
                 {
-                    var awsCredentials = basicProfile.GetAWSCredentials(chain)?.GetCredentials();
-                    if (awsCredentials != null)
-                    {
-                        options.Credentials = new AwsStorageCredentials
-                        {
-                            RegionEndpoint = basicProfile.Region.SystemName ?? RegionEndpoint.USEast1.SystemName,
-                            SecretKey = awsCredentials.SecretKey,
-                            AccessKeyId = awsCredentials.AccessKey
-                        };
-                    }
+                    var awsCredentials = new BasicAWSCredentials(accessKey, secretKey);
+                    options.AwsOptions.Credentials = awsCredentials;
                 }
             }
-        }
 
-        return options;
+            return options;
+        }
+        catch (ConfigurationException ex)
+        {
+            logger.LogCritical(ex, ex.Message);
+            throw;
+        }
     }
 }
diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Constants.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Constants.cs
@@ -6,13 +6,12 @@ internal static class ValidationMessages
     {
         public const string BucketNameIsEmpty = "BucketName is required attribute for S3 Media";
 
-        public const string SecretKeyIsEmpty =
-            "SecretKey is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";
-
-        public const string AccessKeyIdIsEmpty =
-            "AccessKeyId is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";
+        public const string RegionEndpointIsEmpty = "Region is required attribute for S3 Media";
+    }
 
-        public const string RegionEndpointIsEmpty =
-            "Region is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";
+    internal static class AwsCredentialParamNames
+    {
+        public const string SecretKey = "SecretKey";
+        public const string AccessKey = "AccessKey";
     }
 }
diff --git a/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs b/src/OrchardCore.Modules/OrchardCore.Media.AmazonS3/Startup.cs
@@ -2,8 +2,6 @@
 using System.IO;
 using System.Linq;
 using System.Text;
-using Amazon;
-using Amazon.Runtime;
 using Amazon.S3;
 using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
@@ -36,7 +34,7 @@ public override void ConfigureServices(IServiceCollection services)
     {
         services.AddTransient<IConfigureOptions<AwsStorageOptions>, AwsStorageOptionsConfiguration>();
 
-        var storeOptions = new AwsStorageOptions().BindConfiguration(_configuration);
+        var storeOptions = new AwsStorageOptions().BindConfiguration(_configuration, _logger);
         var validationErrors = storeOptions.Validate().ToList();
         var stringBuilder = new StringBuilder();
 
@@ -87,31 +85,8 @@ public override void ConfigureServices(IServiceCollection services)
             services.AddSingleton<IMediaFileStoreCache>(serviceProvider =>
                 serviceProvider.GetRequiredService<IMediaFileStoreCacheFileProvider>());
 
-            services.AddSingleton<IAmazonS3>(serviceProvider =>
-            {
-                var options = serviceProvider.GetRequiredService<IOptions<AwsStorageOptions>>().Value;
-                if (options.Credentials == null)
-                {
-                    return new AmazonS3Client();
-                }
-
-                var config = new AmazonS3Config
-                {
-                    RegionEndpoint = RegionEndpoint.GetBySystemName(options.Credentials.RegionEndpoint),
-                    UseHttp = true,
-                    ForcePathStyle = true,
-                    UseArnRegion = true
-                };
-
-                if (String.IsNullOrWhiteSpace(options.Credentials.AccessKeyId))
-                {
-                    return new AmazonS3Client(new ECSTaskCredentials(), config);
-                }
-
-                return new AmazonS3Client(options.Credentials.AccessKeyId,
-                    options.Credentials.SecretKey,
-                    config);
-            });
+            // Registering IAmazonS3 client using AWS registration factory.
+            services.AddAWSService<IAmazonS3>(storeOptions.AwsOptions);
 
             services.Replace(ServiceDescriptor.Singleton<IMediaFileStore>(serviceProvider =>
             {
diff --git a/src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsStorageOptions.cs b/src/OrchardCore/OrchardCore.FileStorage.AmazonS3/AwsStorageOptions.cs
@@ -1,4 +1,6 @@
-﻿namespace OrchardCore.FileStorage.AmazonS3;
+﻿using Amazon.Extensions.NETCore.Setup;
+
+namespace OrchardCore.FileStorage.AmazonS3;
 
 /// <summary>
 /// AWS storage options.
@@ -21,35 +23,8 @@ public class AwsStorageOptions
     public bool CreateBucket { get; set; }
 
     /// <summary>
-    /// Gets or sets the credentials.
-    /// <remarks>
-    /// Credentials section can be set directly via configuration or get loaded from the configured ProfileName.
-    /// For development purposes, it is recommended to specify just ProfileName.
-    /// For a production environment this section should be null, AWS SDK Services will get the default credentials
-    /// from environment variables.
-    /// </remarks>
+    /// Gets or sets the AWS Options.
     /// </summary>
-    public AwsStorageCredentials Credentials { get; set; }
-
+    public AWSOptions AwsOptions { get; set; }
 }
 
-/// <summary>
-/// The AWS storage credentials.
-/// </summary>
-public class AwsStorageCredentials
-{
-    /// <summary>
-    /// AWS region name
-    /// </summary>
-    public string RegionEndpoint { get; set; }
-
-    /// <summary>
-    /// AWS account secret key. Do not use root's user secret key!
-    /// </summary>
-    public string SecretKey { get; set; }
-
-    /// <summary>
-    /// AWS account access key Id.
-    /// </summary>
-    public string AccessKeyId { get; set; }
-}
diff --git a/src/OrchardCore/OrchardCore.FileStorage.AmazonS3/OrchardCore.FileStorage.AmazonS3.csproj b/src/OrchardCore/OrchardCore.FileStorage.AmazonS3/OrchardCore.FileStorage.AmazonS3.csproj
@@ -17,6 +17,7 @@
   <ItemGroup>
     <PackageReference Include="AWSSDK.S3" />
     <PackageReference Include="AWSSDK.SecurityToken" />
+    <PackageReference Include="AWSSDK.Extensions.NETCore.Setup" />
   </ItemGroup>
 
   <ItemGroup>
diff --git a/src/docs/reference/modules/Media.AmazonS3/README.md b/src/docs/reference/modules/Media.AmazonS3/README.md
@@ -18,20 +18,22 @@ The following configuration values are used by default and can be customized:
 {
    "OrchardCore": {
        "OrchardCore_Media_AmazonS3": {
-           // Your AWS S3 Bucket name
-           "BucketName": "",
+           // If you have AWS CLI installed and configured you may just specify a profile name.
+           "Profile": "",
+           // In case your AWS profiles are located not in the default place.
+           "ProfilesLocation": "",
+           "Region": "",
            // This section needed only if Orchard will be hosted not in the AWS Cloud
            // You can obtain all that information in the IAM Management Console
             "Credentials": {
               "SecretKey": "",
-              "AccessKeyId": "",
-              "RegionEndpoint": ""
+              "AccessKey": ""
            },
            // Optionally, set to a path to store media in a subdirectory inside your container.
            "BasePath": "/media",
-           // If you have aws cli installed and configured you may just specify profile name
-           "ProfileName": "",
-           "CreateBucket": false
+           "CreateBucket": false,
+           // Your AWS S3 Bucket name.
+           "BucketName": ""
        }
   }
 }
@@ -45,7 +47,29 @@ In case you are hosting Orchard Core inside AWS (EC2, EKS, etc.) you need to con
 
 In case you are hosting Orchard Core outside of AWS, you should fill the `Credentials` section or if you have AWS CLI installed and configured on your server you may specify only configured profile name (`default` if a profile name was not chosen during AWS CLI configuration).
 
-You can find region endpoints in the [Official AWS S3 Documentation](https://docs.aws.amazon.com/general/latest/gr/s3.html), see Region column. For example for the Frankfurt region you should use `eu-central-1` 
+You can find region endpoints in the [Official AWS S3 Documentation](https://docs.aws.amazon.com/general/latest/gr/s3.html), see Region column. For example for the Frankfurt region you should use `eu-central-1`
+
+## AWS Credentials and its loading order
+
+`OrchardCore_Media_AmazonS3` is a subset of `AWSOptions` configuration and should be configured the same as a generic [AWSOptions](https://docs.aws.amazon.com/sdk-for-net/v3/developer-guide/net-dg-config-netcore.html).
+
+### Credentials loading order
+
+1. Credentials property of `AWSOptions˙.
+2. Shared Credentials File (Custom Location). When both the profile and profile location are specified.
+3. SDK Store (Windows Only). When an instance of `AWSOptions˙ is provided and only the profile is set (profile location is null or empty).
+4. Shared Credentials File (Default Location). When an instance of `AWSOptions˙ is provided and only the profile is set (profile location is null or empty).
+5. AWS Web Identity Federation Credentials. When an OIDC token file exists and is set in the environment variables.
+6. `CredentialsProfileStoreChain`
+   1. SDK Store (Windows Only) encrypted using Windows Data Protection API.
+   2. Shared Credentials File in the default location.
+7. Environment variables. When the Access Key ID and Secret Access Key environment variables are set.
+8. ECS Task Credentials or EC2 Instance Credentials. When using IAM roles with ECS tasks and ECS instances.
+
+!!! note
+    The AWS team wants to encourage using profiles instead of embedding credentials directly into `appsettings.X.json` files where they would accidentally get checked into source control.
+    If you have an option to use profiles or environment variables - you should use it instead of direct credentials.
+
 
 ## AWS S3 Bucket Configuration
 
@@ -95,13 +119,14 @@ The `BucketName` property and the `BasePath` property are the only templatable p
     "OrchardCore": {
         "OrchardCore_Media_AmazonS3": {
             "BucketName": "{{ ShellSettings.Name }}-media",
+            "Region": "",
             "Credentials": {
                 "SecretKey": "",
-                "AccessKeyId": "",
-                "RegionEndpoint": ""
+                "AccessKey": ""
             },
             "BasePath": "/media",
-            "ProfileName": ""
+            "Profile": "",
+            "ProfilesLocation": ""
         }
     }
 }
@@ -114,13 +139,14 @@ The `BucketName` property and the `BasePath` property are the only templatable p
     "OrchardCore": {
         "OrchardCore_Media_AmazonS3": {
             "BucketName": "",
+            "Region": "",
             "Credentials": {
                 "SecretKey": "",
-                "AccessKeyId": "",
-                "RegionEndpoint": ""
+                "AccessKey": ""
             },
             "BasePath": "{{ ShellSettings.Name }}/Media",
-            "ProfileName": ""
+            "Profile": "",
+            "ProfilesLocation" : ""
         }
     }
 }
diff --git a/src/docs/releases/1.5.0.md b/src/docs/releases/1.5.0.md
@@ -0,0 +1,7 @@
+# Orchard Core 1.5.0
+
+## Breaking Changes
+
+* The `OrchardCore_Media_AmazonS3` config section was changed: `RegionEndpoint` was renamed to `Region` and extracted from `Credentials` section to the root section of `OrchardCore_Media_AmazonS3`, `AccessKeyId` was renamed to `AccessKey`, `ProfileName` was renamed to `Profile`. See [this pull request](https://github.com/OrchardCMS/OrchardCore/pull/11871) for details.
+
+

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ public AwsStorageOptionsConfiguration(`
`29`	`29`
`30`	`30`	`public void Configure(AwsStorageOptions options)`
`31`	`31`	`{`
`32`		`- options.BindConfiguration(_shellConfiguration);`
	`32`	`+ options.BindConfiguration(_shellConfiguration, _logger);`
`33`	`33`
`34`	`34`	`var templateOptions = new TemplateOptions();`
`35`	`35`	`var templateContext = new TemplateContext(templateOptions);`
Original file line number	Diff line number	Diff line change
`@@ -6,13 +6,12 @@ internal static class ValidationMessages`
`6`	`6`	`{`
`7`	`7`	`public const string BucketNameIsEmpty = "BucketName is required attribute for S3 Media";`
`8`	`8`
`9`		`- public const string SecretKeyIsEmpty =`
`10`		`- "SecretKey is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";`
`11`		`-`
`12`		`- public const string AccessKeyIdIsEmpty =`
`13`		`- "AccessKeyId is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";`
	`9`	`+ public const string RegionEndpointIsEmpty = "Region is required attribute for S3 Media";`
	`10`	`+ }`
`14`	`11`
`15`		`- public const string RegionEndpointIsEmpty =`
`16`		`- "Region is required attribute for S3 Media, make sure it exists in Credentials section or ProfileName you specified";`
	`12`	`+ internal static class AwsCredentialParamNames`
	`13`	`+ {`
	`14`	`+ public const string SecretKey = "SecretKey";`
	`15`	`+ public const string AccessKey = "AccessKey";`
`17`	`16`	`}`
`18`	`17`	`}`