Closed (fixed)
Project:
S3 File System
Version:
8.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
25 Feb 2021 at 07:36 UTC
Updated:
28 Mar 2021 at 07:44 UTC
Jump to comment: Most recent
There are a number of issues open that appear to need to be reconciled into a single ticket regarding Authentication providers.
Bringing centralized planning into this issue.
Comments
Comment #2
cmlaraSummary of linked issues
#3157066: Connecting with S3 without access_key/secret_key
Support request, no code.
#3158373: Configuring s3fs using AWS IAM role instead of keys
Feature request, ability to use IAM keys. No code.
#3024836: Support to additional credential provider
Proposes adding a new bool option to use ecsCredentials and adds a call to ecsCredentials() provider.
#3122091: Module should initialize default credentials provider instead of instance profile when use_instance_profile is TRUE
Proposes adding calls to assumeRoleWithWebIdentityCredentialProvider() chained with defaultProvider(). Asserts that this will cover the majority of environments. Reviewing latest AWS SDK it appears that defaultProvider calls assumeRoleWithWebIdentityCredentialProvider() and has for some time but was not documented (in code) until ~Dec 2020.
#3121830: Use CredentialProvider::defaultProvider rather than individual providers Patch proposes using defaultProvider only and removes the ability to set an instance path or the access/secret key outside of AWS locations.
#3088197: Need it to work on fargate
Patch 3: Relies on fact that if accesskey and secretkey are not set that we can set no credentials which causes AWS SDK to fallback to the the defaultProvider().
Patch 16: Adds a use_default_credential_provider option, when checked ignores access/secret key. Keeps instance profile.
#3120052: Allow using IAM roles while accessing s3 resources.
Proposes adding UI options for allow_iam_roles, role_arn, role_session_name, role_external_id and adds a call to assumeRole() for the credentials. Need to verify if this could possibly be handled by using config files by the defaultProvider() use of assumeRoleWithWebIdentityCredentialProvider()
defaultProvider
Takeaways
defaultProvider would add support for numerous deployment methods inside AWS ecosystem.
We can chain
Questions
Given work is underway to remove accesskey and secretkey from the database and UI, Is replacing the storage of secrets inside of settings.php also worth including? This would get configs completely out of Drupal #2976430: Integrate with the key module for access and secret keys
Is there any concern with using defaultProvider and getting the wrong credentials? I'm inclined to think not. Especially if we do keep the ability to chain an ini() with a custom path as currently exists as that can always be a first option before defaultProvider allowing a method to guarantee an override if needed.
Opinion
At the moment I'm inclined to do as follows (with appropriate migration code):
Remove Instance Profile selection option from settings page, instead rely on the credentials_file field having content or not.
If access_key and secret_key are empty()
Set access to use defaultProvider
If credentials_file is not empty chain before defaultProvider
else
Use access_key and secret_key
From what I can see this should allow for a minimal of interruption with maximum continuity for a next release while allowing all of the requests proposed to date.
Comment #3
cmlaraDoing the patch work in #3121830: Use CredentialProvider::defaultProvider rather than individual providers
Comment #4
cmlaraFixed in #3121830