Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credential Scope #3048

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions botocore/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,10 +622,10 @@ def _ensure_boolean(self, val):

def _construct_builtin_resolver(self, credentials, client_config):
credential_builtin_resolver = CredentialBuiltinResolver(
credentials, client_config.account_id_endpoint_mode
credentials,
client_config.account_id_endpoint_mode,
)
resolver_map = {'credentials': credential_builtin_resolver}
return EndpointBuiltinResolver(resolver_map)
return EndpointBuiltinResolver([credential_builtin_resolver])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe this is a pythonic thing that I don't really understand; it seems odd to allow a method that we pass in to be able to set arbitrary attributes.

Copy link
Contributor Author

@dlm6693 dlm6693 Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't call it Pythonic, maybe Botonic™️? We have precedence for iterating over a list of resolvers that arbitrarily resolve credentials. However, in that case the first one that is successful terminates the loop whereas this one will always complete the loop.

Only endpoint builtins that are defined in the EndpointResolverBuiltins enum can be used as an input parameter to the the endpoint resolver. So if somebody decided to inject a custom resolver that set some arbitrary key value pair {'foo': 'bar'} in the builtins dict at runtime, an error will be raised.


def _build_endpoint_resolver(
self,
Expand Down Expand Up @@ -778,6 +778,9 @@ def compute_endpoint_resolver_builtin_defaults(
# account ID is calculated later if account based routing is
# enabled and configured for the service
EPRBuiltins.AWS_ACCOUNT_ID: None,
# credential scope is calculated later if configured on the
# credentials
EPRBuiltins.AWS_CREDENTIAL_SCOPE: None,
}

def _compute_user_agent_appid_config(self, config_kwargs):
Expand Down
10 changes: 10 additions & 0 deletions botocore/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
# ANY KIND, either express or implied. See the License for the specific
# language governing permissions and limitations under the License.
import logging
import warnings

from botocore import waiter, xform_name
from botocore.args import ClientArgsCreator
Expand Down Expand Up @@ -788,8 +789,17 @@ def _pick_region_values(self, resolved, region_name, endpoint_url):
and 'region' in resolved['credentialScope']
):
signing_region = resolved['credentialScope']['region']
self._warn_custom_signing_region(signing_region)
return region_name, signing_region

def _warn_custom_signing_region(self, signing_region):
if not self.resolver_uses_builtin_data():
warnings.warn(
"Detected a signing region resolved from a custom "
f"endpoints.json file: {signing_region}. This value "
"may not be honored during request signing."
)

def _resolve_signature_version(self, service_name, resolved):
configured_version = _get_configured_signature_version(
service_name, self.client_config, self.scoped_config
Expand Down
77 changes: 70 additions & 7 deletions botocore/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@
logger = logging.getLogger(__name__)
ReadOnlyCredentials = namedtuple(
'ReadOnlyCredentials',
['access_key', 'secret_key', 'token', 'account_id'],
defaults=(None,),
['access_key', 'secret_key', 'token', 'account_id', 'scope'],
defaults=(None, None),
dlm6693 marked this conversation as resolved.
Show resolved Hide resolved
)

_DEFAULT_MANDATORY_REFRESH_TIMEOUT = 10 * 60 # 10 min
Expand Down Expand Up @@ -312,6 +312,7 @@ class Credentials:
:param str method: A string which identifies where the credentials
were found.
:param str account_id: The account ID associated with the credentials.
:param str scope: The scope associated with the credentials.
"""

def __init__(
Expand All @@ -321,6 +322,7 @@ def __init__(
token=None,
method=None,
account_id=None,
scope=None,
):
self.access_key = access_key
self.secret_key = secret_key
Expand All @@ -330,6 +332,7 @@ def __init__(
method = 'explicit'
self.method = method
self.account_id = account_id
self.scope = scope

self._normalize()

Expand All @@ -345,7 +348,11 @@ def _normalize(self):

def get_frozen_credentials(self):
return ReadOnlyCredentials(
self.access_key, self.secret_key, self.token, self.account_id
self.access_key,
self.secret_key,
self.token,
self.account_id,
self.scope,
)


Expand All @@ -363,6 +370,7 @@ class RefreshableCredentials(Credentials):
were found.
:param function time_fetcher: Callback function to retrieve current time.
:param str account_id: The account ID associated with the credentials.
:param str scope: The scope associated with the credentials.
"""

# The time at which we'll attempt to refresh, but not
Expand All @@ -382,6 +390,7 @@ def __init__(
method,
time_fetcher=_local_now,
account_id=None,
scope=None,
):
self._refresh_using = refresh_using
self._access_key = access_key
Expand All @@ -392,10 +401,11 @@ def __init__(
self._refresh_lock = threading.Lock()
self.method = method
self._frozen_credentials = ReadOnlyCredentials(
access_key, secret_key, token, account_id
access_key, secret_key, token, account_id, scope
)
self._normalize()
self._account_id = account_id
self._scope = scope

def _normalize(self):
self._access_key = botocore.compat.ensure_unicode(self._access_key)
Expand All @@ -411,6 +421,7 @@ def create_from_metadata(cls, metadata, refresh_using, method):
method=method,
refresh_using=refresh_using,
account_id=metadata.get('account_id'),
scope=metadata.get('scope'),
)
return instance

Expand Down Expand Up @@ -466,6 +477,19 @@ def account_id(self):
def account_id(self, value):
self._account_id = value

@property
def scope(self):
"""Warning: Using this property can lead to race conditions if you
access another property subsequently along the refresh boundary.
Please use get_frozen_credentials instead.
"""
self._refresh()
return self._scope

@scope.setter
def scope(self, value):
self._scope = value

def _seconds_remaining(self):
delta = self._expiry_time - self._time_fetcher()
return total_seconds(delta)
Expand Down Expand Up @@ -562,7 +586,11 @@ def _protected_refresh(self, is_mandatory):
return
self._set_from_data(metadata)
self._frozen_credentials = ReadOnlyCredentials(
self._access_key, self._secret_key, self._token, self._account_id
self._access_key,
self._secret_key,
self._token,
self._account_id,
self._scope,
)
if self._is_expired():
# We successfully refreshed credentials but for whatever
Expand Down Expand Up @@ -603,6 +631,7 @@ def _set_from_data(self, data):
"Retrieved credentials will expire at: %s", self._expiry_time
)
self.account_id = data.get('account_id')
self.scope = data.get('scope')
self._normalize()

def get_frozen_credentials(self):
Expand Down Expand Up @@ -660,6 +689,7 @@ def __init__(self, refresh_using, method, time_fetcher=_local_now):
self.method = method
self._frozen_credentials = None
self._account_id = None
self._scope = None

def refresh_needed(self, refresh_in=None):
if self._frozen_credentials is None:
Expand Down Expand Up @@ -717,6 +747,9 @@ def _get_cached_credentials(self):
account_id = creds.get('AccountId')
if account_id is not None:
creds_dict['account_id'] = account_id
scope = creds.get('CredentialScope')
if scope is not None:
creds_dict['scope'] = scope
return creds_dict

def _load_from_cache(self):
Expand Down Expand Up @@ -1040,6 +1073,7 @@ def load(self):
token=creds_dict.get('token'),
method=self.METHOD,
account_id=creds_dict.get('account_id'),
scope=creds_dict.get('scope'),
)

def _retrieve_credentials_using(self, credential_process):
Expand Down Expand Up @@ -1080,12 +1114,20 @@ def _retrieve_credentials_using(self, credential_process):
account_id = self._resolve_account_id(parsed)
if account_id is not None:
creds_dict['account_id'] = account_id
scope = self._resolve_scope(parsed)
if scope is not None:
creds_dict['scope'] = scope

return creds_dict

def _resolve_account_id(self, parsed_response):
account_id = parsed_response.get('AccountId')
return account_id or self.profile_config.get('aws_account_id')

def _resolve_scope(self, parsed_response):
scope = parsed_response.get('CredentialScope')
return scope or self.profile_config.get('aws_credential_scope')

@property
def _credential_process(self):
return self.profile_config.get('credential_process')
Expand Down Expand Up @@ -1137,6 +1179,7 @@ class EnvProvider(CredentialProvider):
TOKENS = ['AWS_SECURITY_TOKEN', 'AWS_SESSION_TOKEN']
EXPIRY_TIME = 'AWS_CREDENTIAL_EXPIRATION'
ACCOUNT_ID = 'AWS_ACCOUNT_ID'
SCOPE = 'AWS_CREDENTIAL_SCOPE'

def __init__(self, environ=None, mapping=None):
"""
Expand All @@ -1146,8 +1189,8 @@ def __init__(self, environ=None, mapping=None):
:param mapping: An optional mapping of variable names to
environment variable names. Use this if you want to
change the mapping of access_key->AWS_ACCESS_KEY_ID, etc.
The dict can have up to 5 keys: ``access_key``, ``secret_key``,
``token``, ``expiry_time``, and ``account_id``.
The dict can have up to 6 keys: ``access_key``, ``secret_key``,
``token``, ``expiry_time``, ``account_id`` and ``scope``.
"""
if environ is None:
environ = os.environ
Expand All @@ -1164,6 +1207,7 @@ def _build_mapping(self, mapping):
var_mapping['token'] = self.TOKENS
var_mapping['expiry_time'] = self.EXPIRY_TIME
var_mapping['account_id'] = self.ACCOUNT_ID
var_mapping['scope'] = self.SCOPE
else:
var_mapping['access_key'] = mapping.get(
'access_key', self.ACCESS_KEY
Expand All @@ -1180,6 +1224,7 @@ def _build_mapping(self, mapping):
var_mapping['account_id'] = mapping.get(
'account_id', self.ACCOUNT_ID
)
var_mapping['scope'] = mapping.get('scope', self.SCOPE)
return var_mapping

def load(self):
Expand All @@ -1205,6 +1250,7 @@ def load(self):
refresh_using=fetcher,
method=self.METHOD,
account_id=credentials.get('account_id'),
scope=credentials.get('scope'),
)

return Credentials(
Expand All @@ -1213,6 +1259,7 @@ def load(self):
credentials['token'],
method=self.METHOD,
account_id=credentials.get('account_id'),
scope=credentials.get('scope'),
)
else:
return None
Expand Down Expand Up @@ -1259,6 +1306,10 @@ def fetch_credentials(require_expiry=True):
if account_id is not None:
credentials['account_id'] = account_id

scope = environ.get(mapping['scope'])
if scope is not None:
credentials['scope'] = scope

return credentials

return fetch_credentials
Expand Down Expand Up @@ -1310,6 +1361,7 @@ class SharedCredentialProvider(CredentialProvider):
# so we support both.
TOKENS = ['aws_security_token', 'aws_session_token']
ACCOUNT_ID = 'aws_account_id'
SCOPE = 'aws_credential_scope'

def __init__(self, creds_filename, profile_name=None, ini_parser=None):
self._creds_filename = creds_filename
Expand Down Expand Up @@ -1337,12 +1389,14 @@ def load(self):
)
token = self._get_session_token(config)
account_id = self._get_account_id(config)
scope = self._get_scope(config)
return Credentials(
access_key,
secret_key,
token,
method=self.METHOD,
account_id=account_id,
scope=scope,
)

def _get_session_token(self, config):
Expand All @@ -1353,6 +1407,9 @@ def _get_session_token(self, config):
def _get_account_id(self, config):
return config.get(self.ACCOUNT_ID)

def _get_scope(self, config):
return config.get(self.SCOPE)


class ConfigProvider(CredentialProvider):
"""INI based config provider with profile sections."""
Expand All @@ -1367,6 +1424,7 @@ class ConfigProvider(CredentialProvider):
# so we support both.
TOKENS = ['aws_security_token', 'aws_session_token']
ACCOUNT_ID = 'aws_account_id'
SCOPE = 'aws_credential_scope'

def __init__(self, config_filename, profile_name, config_parser=None):
"""
Expand Down Expand Up @@ -1404,12 +1462,14 @@ def load(self):
)
token = self._get_session_token(profile_config)
account_id = self._get_account_id(profile_config)
scope = self._get_scope(profile_config)
return Credentials(
access_key,
secret_key,
token,
method=self.METHOD,
account_id=account_id,
scope=scope,
)
else:
return None
Expand All @@ -1422,6 +1482,9 @@ def _get_session_token(self, profile_config):
def _get_account_id(self, profile_config):
return profile_config.get(self.ACCOUNT_ID)

def _get_scope(self, profile_config):
return profile_config.get(self.SCOPE)


class BotoProvider(CredentialProvider):
METHOD = 'boto-config'
Expand Down
Loading