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

Grant IAM permissions to read data to environment team IAM roles independently from CREATE_DATASET permissions #1137

Merged

Conversation

SofiaSazonova
Copy link
Contributor

@SofiaSazonova SofiaSazonova commented Apr 2, 2024

Feature or Bugfix

  • Bugfix

Detail

⚠️ ⚠️ ⚠️ IMPORTANT ⚠️ ⚠️ ⚠️
Environments stacks should be updated for the changes to be effective.

Glue-permissions were added to the team's env role only if the role had CREATE_DATASET permissions. But `glue-permissions are about the get-operations, so i removed that if, so the required permissions are added to the team's env role regardless of CREATE_DATASET

Relates

Security

Please answer the questions below briefly where applicable, or write N/A. Based on
OWASP 10.

  • 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)? N/A
  • Does this PR introduce any functionality or component that requires authorization? N/A
  • Are you using or adding any cryptographic features? N/A
  • Are you introducing any new policies/roles/users? N/A

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@SofiaSazonova SofiaSazonova marked this pull request as ready for review April 2, 2024 15:30
@@ -1,4 +1,5 @@
"""Indexes DatasetStorageLocation in OpenSearch"""

Copy link
Contributor

Choose a reason for hiding this comment

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

This is because of the linting right? It was also flagged for me while executing ruff locally

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's linting.

@dlpzx
Copy link
Contributor

dlpzx commented Apr 3, 2024

Hi @SofiaSazonova how have you tested this PR?

When this is merged the users will need to update the environment cloudformation stack for the changes to be effective. We should include some more instructions and clarification on what this PR does

@noah-paige noah-paige linked an issue Apr 3, 2024 that may be closed by this pull request
@SofiaSazonova
Copy link
Contributor Author

@dlpzx Yes, It's tested.

@SofiaSazonova SofiaSazonova merged commit 00134c4 into data-dot-all:main Apr 4, 2024
8 checks passed
@dlpzx dlpzx changed the title Restricting Team/User Access to Worksheets Grant IAM permissions to read data to environment team IAM roles independently from CREATE_DATASET permissions Apr 17, 2024
@SofiaSazonova SofiaSazonova deleted the 945-access-to-Worksheets branch October 3, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restricting Team/User Access to Worksheets
2 participants