Skip to content

Commit

Permalink
Add SCP error handling in Quicksight identity region checks (#896)
Browse files Browse the repository at this point in the history
### Feature or Bugfix
- Feature
- Bugfix

### Detail
There is no API to obtain the Quicksight identity region used for an
account, we obtain it form the error logs of the response of
describe_groups. However, it does not take into account AccessDenied
errors based on SCPs.
A more detailed description of the issue can be found in #851 
 
This PR:
- handles AccessDenied errors based on SCPs and retries other Quicksight
identity regions
- fixes some methods for registering users that should be using the
Quicksight client in the identity region.

### Relates
- #851 

### Security
Please answer the questions below briefly where applicable, or write
`N/A`. Based on
[OWASP 10](https://owasp.org/Top10/en/). --> `N/A`

- Does this PR introduce or modify any input fields or queries - this
includes
fetching data from storage outside the application (e.g. a database, an
S3 bucket)?
  - Is the input sanitized?
- What precautions are you taking before deserializing the data you
consume?
  - Is injection prevented by parametrizing queries?
  - Have you ensured no `eval` or similar functions are used?
- Does this PR introduce any functionality or component that requires
authorization?
- How have you ensured it respects the existing AuthN/AuthZ mechanisms?
  - Are you logging failed auth attempts?
- Are you using or adding any cryptographic features?
  - Do you use a standard proven implementations?
  - Are the used keys controlled by the customer? Where are they stored?
- Are you introducing any new policies/roles/users?
  - Have you used the least-privilege principle? How?


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.
  • Loading branch information
dlpzx authored Dec 6, 2023
1 parent e7c87df commit 2e0fd39
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 28 deletions.
60 changes: 43 additions & 17 deletions backend/dataall/base/aws/quicksight.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,23 @@
class QuicksightClient:

DEFAULT_GROUP_NAME = 'dataall'
QUICKSIGHT_IDENTITY_REGIONS = [
{"name": 'US East (N. Virginia)', "code": 'us-east-1'},
{"name": 'US East (Ohio)', "code": 'us-east-2'},
{"name": 'US West (Oregon)', "code": 'us-west-2'},
{"name": 'Europe (Frankfurt)', "code": 'eu-central-1'},
{"name": 'Europe (Stockholm)', "code": 'eu-north-1'},
{"name": 'Europe (Ireland)', "code": 'eu-west-1'},
{"name": 'Europe (London)', "code": 'eu-west-2'},
{"name": 'Europe (Paris)', "code": 'eu-west-3'},
{"name": 'Asia Pacific (Singapore)', "code": 'ap-southeast-1'},
{"name": 'Asia Pacific (Sydney)', "code": 'ap-southeast-2'},
{"name": 'Asia Pacific (Tokyo)', "code": 'ap-northeast-1'},
{"name": 'Asia Pacific (Seoul)', "code": 'ap-northeast-2'},
{"name": 'South America (São Paulo)', "code": 'sa-east-1'},
{"name": 'Canada (Central)', "code": 'ca-central-1'},
{"name": 'Asia Pacific (Mumbai)', "code": 'ap-south-1'},
]

def __init__(self):
pass
Expand Down Expand Up @@ -37,21 +54,29 @@ def get_identity_region(AwsAccountId):
the region quicksight uses as identity region
"""
identity_region_rex = re.compile('Please use the (?P<region>.*) endpoint.')
identity_region = 'us-east-1'
client = QuicksightClient.get_quicksight_client(AwsAccountId=AwsAccountId, region=identity_region)
try:
response = client.describe_group(
AwsAccountId=AwsAccountId, GroupName=QuicksightClient.DEFAULT_GROUP_NAME, Namespace='default'
)
except client.exceptions.AccessDeniedException as e:
match = identity_region_rex.findall(str(e))
if match:
identity_region = match[0]
else:
raise e
except client.exceptions.ResourceNotFoundException:
pass
return identity_region
scp = 'with an explicit deny in a service control policy'
index = 0
while index < len(QuicksightClient.QUICKSIGHT_IDENTITY_REGIONS):
try:
identity_region = QuicksightClient.QUICKSIGHT_IDENTITY_REGIONS[index].get("code")
index += 1
client = QuicksightClient.get_quicksight_client(AwsAccountId=AwsAccountId, region=identity_region)
response = client.describe_account_settings(AwsAccountId=AwsAccountId)
logger.info(f'Returning identity region = {identity_region} for account {AwsAccountId}')
return identity_region
except client.exceptions.AccessDeniedException as e:
if scp in str(e):
logger.info(f'Quicksight SCP found in {identity_region} for account {AwsAccountId}. Trying next region...')
else:
logger.info(f'Quicksight identity region is not {identity_region}, selecting correct region endpoint...')
match = identity_region_rex.findall(str(e))
if match:
identity_region = match[0]
logger.info(f'Returning identity region = {identity_region} for account {AwsAccountId}')
return identity_region
else:
raise e
raise Exception(f'Quicksight subscription is inactive or the identity region has SCPs preventing access from data.all to account {AwsAccountId}')

@staticmethod
def get_quicksight_client_in_identity_region(AwsAccountId):
Expand Down Expand Up @@ -99,10 +124,11 @@ def check_quicksight_enterprise_subscription(AwsAccountId, region=None):
return False

@staticmethod
def create_quicksight_group(AwsAccountId, GroupName=DEFAULT_GROUP_NAME):
def create_quicksight_group(AwsAccountId, region, GroupName=DEFAULT_GROUP_NAME):
"""Creates a Quicksight group called GroupName
Args:
AwsAccountId(str): aws account
region: aws region
GroupName(str): name of the QS group
Returns:dict
Expand All @@ -113,7 +139,7 @@ def create_quicksight_group(AwsAccountId, GroupName=DEFAULT_GROUP_NAME):
if not group:
if GroupName == QuicksightClient.DEFAULT_GROUP_NAME:
logger.info(f'Initializing data.all default group = {GroupName}')
QuicksightClient.check_quicksight_enterprise_subscription(AwsAccountId)
QuicksightClient.check_quicksight_enterprise_subscription(AwsAccountId, region)

logger.info(f'Attempting to create Quicksight group `{GroupName}...')
response = client.create_group(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,26 @@ class DashboardQuicksightClient:
_DEFAULT_GROUP_NAME = QuicksightClient.DEFAULT_GROUP_NAME

def __init__(self, username, aws_account_id, region='eu-west-1'):
session = SessionHelper.remote_session(accountid=aws_account_id)
self._client = session.client('quicksight', region_name=region)
self._account_id = aws_account_id
self._region = region
self._username = username
self._client = QuicksightClient.get_quicksight_client(aws_account_id, region)

def register_user_in_group(self, group_name, user_role='READER'):
QuicksightClient.create_quicksight_group(self._account_id, group_name)
identity_region_client = QuicksightClient.get_quicksight_client_in_identity_region(self._account_id)
QuicksightClient.create_quicksight_group(AwsAccountId=self._account_id, region=self._region, GroupName=group_name)
user = self._describe_user()

if user is not None:
self._client.update_user(
identity_region_client.update_user(
UserName=self._username,
AwsAccountId=self._account_id,
Namespace='default',
Email=self._username,
Role=user_role,
)
else:
self._client.register_user(
identity_region_client.register_user(
UserName=self._username,
Email=self._username,
AwsAccountId=self._account_id,
Expand All @@ -45,13 +45,13 @@ def register_user_in_group(self, group_name, user_role='READER'):
UserRole=user_role,
)

response = self._client.list_user_groups(
response = identity_region_client.list_user_groups(
UserName=self._username, AwsAccountId=self._account_id, Namespace='default'
)
log.info(f'list_user_groups for {self._username}: {response})')
if group_name not in [g['GroupName'] for g in response['GroupList']]:
log.warning(f'Adding {self._username} to Quicksight group {group_name} on {self._account_id}')
self._client.create_group_membership(
identity_region_client.create_group_membership(
MemberName=self._username,
GroupName=group_name,
AwsAccountId=self._account_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def get_statements(self):
'quicksight:GetAuthCode',
'quicksight:CreateGroupMembership',
'quicksight:DescribeAccountSubscription',
'quicksight:DescribeAccountSettings',
],
resources=[
f'arn:aws:quicksight:*:{self.account}:group/default/*',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ def get_share_principals(self) -> [str]:
dashboard_enabled = EnvironmentService.get_boolean_env_param(self.session, self.target_environment, "dashboardsEnabled")

if dashboard_enabled:
group = QuicksightClient.create_quicksight_group(AwsAccountId=self.target_environment.AwsAccountId)
group = QuicksightClient.create_quicksight_group(
AwsAccountId=self.target_environment.AwsAccountId, region=self.target_environment.region
)
if group and group.get('Group'):
group_arn = group.get('Group').get('Arn')
if group_arn:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def submit_share_object(cls, uri: str):
if dashboard_enabled:
share_table_items = ShareObjectRepository.find_all_share_items(session, uri, ShareableType.Table.value)
if share_table_items:
QuicksightClient.check_quicksight_enterprise_subscription(AwsAccountId=env.AwsAccountId)
QuicksightClient.check_quicksight_enterprise_subscription(AwsAccountId=env.AwsAccountId, region=env.region)

cls._run_transitions(session, share, states, ShareObjectActions.Submit)

Expand Down
6 changes: 4 additions & 2 deletions backend/dataall/modules/datasets/services/dataset_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ def check_dataset_account(session, environment):
dashboards_enabled = EnvironmentService.get_boolean_env_param(session, environment, "dashboardsEnabled")
if dashboards_enabled:
quicksight_subscription = QuicksightClient.check_quicksight_enterprise_subscription(
AwsAccountId=environment.AwsAccountId)
AwsAccountId=environment.AwsAccountId, region=environment.region)
if quicksight_subscription:
group = QuicksightClient.create_quicksight_group(AwsAccountId=environment.AwsAccountId)
group = QuicksightClient.create_quicksight_group(
AwsAccountId=environment.AwsAccountId, region=environment.region
)
return True if group else False
return True

Expand Down
1 change: 1 addition & 0 deletions deploy/pivot_role/pivotRole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ Resources:
- "quicksight:GetAuthCode"
- "quicksight:CreateGroupMembership"
- "quicksight:DescribeAccountSubscription"
- "quicksight:DescribeAccountSettings"
Resource:
- !Sub "arn:aws:quicksight:*:${AWS::AccountId}:group/default/*"
- !Sub "arn:aws:quicksight:*:${AWS::AccountId}:user/default/*"
Expand Down

0 comments on commit 2e0fd39

Please sign in to comment.