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.

Sign up for GitHub

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

Jump to bottom

Uncaught exception for SCP-related AccessDeniedException in QuickSight DescribeGroup operation in us-east-1 #851

Closed
lorchda opened this issue Nov 1, 2023 · 4 comments
Closed

Uncaught exception for SCP-related AccessDeniedException in QuickSight DescribeGroup operation in us-east-1 #851

lorchda opened this issue Nov 1, 2023 · 4 comments
Labels
effort: low effort: medium priority: medium status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up type: bug Something isn't working
Milestone
v2.2.0

Comments

@lorchda
Copy link
Contributor

lorchda commented Nov 1, 2023 •
edited
Loading

Describe the bug

When trying to start a QuickSight session, the following error is displayed:

An error occurred (AccessDeniedException) when calling the DescribeGroup operation:
User:arn:aws:sts:REDACTED:assumed-role/REDACTED is not authorized to perform: quicksight:DescribeGroup
on resource: am:aws:quicksight.us-east-1:REDACTED:group/default/dataall with an explicit deny in a service
control policy

How to Reproduce

The call trace is as follows:

  1. User clicks on "Start a QuickSight Session"
  2. data.all frontend issues GraphQL operation GetAuthorSession
  3. data.all backend handles get_identity_region, which uses hardcoded parameter us-east-1 in backend/dataall/base/aws/quicksight.py#L40
  4. The exception block catches AccessDeniedException, but does not foresee exceptions due to service control policies

We were able to temporarily fix the issue by changing the value from us-east-1 to eu-central-1 (but thereby changing the logic of the function).

A more correct fix would be to review the block in except client.exceptions.AccessDeniedException as e:, ensure the explicit deny in a service control policy is caught with a fall back to the user's local region for identity_region.

Expected behavior

We are able to Start a QuickSight Session without errors.

Your project

Screenshots

data.all

1-quicksight-error

Developer Tools: Request

2-devtools-request

Developer Tools: Response

3-devtools-response

CloudWatch Logs for GraphQL Lambda

4-cwlogs

OS

Windows

Python version

python3.8

AWS data.all version

v2.0.0

Additional context

No response
@lorchda lorchda changed the title Hardcoded region us-east-1 in QuickSight DescribeGroup operation Uncaught exception for SCP in QuickSight DescribeGroup operation for us-east-1 Nov 1, 2023
@lorchda lorchda changed the title Uncaught exception for SCP in QuickSight DescribeGroup operation for us-east-1 Uncaught exception for SCP-related AccessDeniedException in QuickSight DescribeGroup operation for us-east-1 Nov 1, 2023
@lorchda lorchda changed the title Uncaught exception for SCP-related AccessDeniedException in QuickSight DescribeGroup operation for us-east-1 Uncaught exception for SCP-related AccessDeniedException in QuickSight DescribeGroup operation in us-east-1 Nov 1, 2023
@dlpzx
Copy link
Contributor

dlpzx commented Nov 7, 2023

Hi @lorchda, we are facing similar issues when registering users. We defaulted the identity region to the default when subscribing to Quicksight, but as more regions are added as identity regions it seems like there is a need to resolve the identity region in a better way. Let us discuss this feature internally but we will look into it.

I'll leave here some resources related to this issue (1, 2)

@dlpzx dlpzx added type: bug Something isn't working priority: medium effort: low effort: medium status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up labels Nov 7, 2023
@anmolsgandhi anmolsgandhi added this to the v2.2.0 milestone Nov 7, 2023
@dlpzx
Copy link
Contributor

dlpzx commented Nov 30, 2023

We will iterate only through the Quicksight identity regions available
image

@dlpzx dlpzx mentioned this issue Nov 30, 2023
Merged
dlpzx added a commit that referenced this issue Dec 6, 2023
@dlpzx
Add SCP error handling in Quicksight identity region checks (#896)
2e0fd39 
### 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.
@dlpzx
Copy link
Contributor

dlpzx commented Dec 6, 2023

Merged to main @lorchda :)

@dlpzx dlpzx closed this as completed Dec 6, 2023
@lorchda
Copy link
Contributor Author

lorchda commented Dec 18, 2023

I see it was a larger investigation behind, thank you for the deep dive and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
effort: low effort: medium priority: medium status: not-picked-yet At the moment we have not picked this item. Anyone can pick it up type: bug Something isn't working
Projects
None yet
Milestone
v2.2.0
Development

No branches or pull requests
3 participants
@dlpzx @anmolsgandhi @lorchda