Skip to content

Commit

Permalink
DA v2: fix default permissions and update migrations scripts (#728)
Browse files Browse the repository at this point in the history
### Feature or Bugfix
- Bugfix

### Detail
There are some issues with the permissions that appear in the invitation
request

Original:


![image](https://github.com/awslabs/aws-dataall/assets/71252798/3b4f409c-e9f4-4bc7-9f4b-123e4c4d0f0b)

Fresh deployment (with mlStudio module disabled):

<img width="500" alt="image"
src="https://github.com/awslabs/aws-dataall/assets/71252798/996aae76-3069-4562-9baf-d7255d009650">

With a pre-existing deployment:


![image](https://github.com/awslabs/aws-dataall/assets/71252798/e3b82bb8-7431-4dda-90d4-7f632c4fcbbb)


old deployment:
```
	Invite other teams
	Add consumption roles
	create networks
	create pipelines
	create notebooks
	Request datasets access
	create datasets
	create redshift ---> removed! already done
	create ML Studio ---> renamed! already done
```

The following are new or wrong permissions in fresh deployments /
backwards
```
	List datasets on this environment / LIST_ENVIRONMENT_DATASETS
	Run athena queries / RUN_ATHENA_QUERY
	List datasets shared with this environments (TYPO!) / LIST_ENVIRONMENT_SHARED_WITH_OBJECTS??
	nothing (mlstudio disabled) / LIST_ENVIRONMENT_SGMSTUDIO_USERS
```

This PR includes 3 fixes:
1) `RUN_ATHENA_QUERY`: Good to add, but was not there before so we need
to update the description in the migration scrips

2) `LIST_ENVIRONMENT_DATASETS`, `LIST_ENVIRONMENT_SHARED_WITH_OBJECTS`:
they are needed by default, so we can add a new list
`ENVIRONMENT_INVITED_DEFAULT` and add them there instead of adding them
in the list that is used for the toogle menu.

3) `LIST_ENVIRONMENT_SGMSTUDIO_USERS`: this permission is not used, we
just need to remove it

In addition some permissions have been renamed. I used the
`migrations/versions/4a0618805341_rename_sgm_studio_permissions.py`
script as it already handles renames

Testing:
[X] - Local testing of renaming and descriptions
[X] - AWS testing of the permissions that appear on screen
[ ] - AWS testing with an invited group - check that they can list
datasets and shares in environment

This is the end result for a deployment with the dashboards modules
disabled:

![image](https://github.com/awslabs/aws-dataall/assets/71252798/b2222c1f-6d37-447d-bee5-e5d8521ff145)


### Relates
- V2

### 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 Sep 5, 2023
1 parent 12db96a commit 0c22580
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 45 deletions.
18 changes: 5 additions & 13 deletions backend/dataall/core/environment/services/environment_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,22 +275,14 @@ def invite_group(session, uri, data=None) -> (Environment, EnvironmentGroup):

@staticmethod
def validate_permissions(session, uri, g_permissions, group):
"""
g_permissions: coming from frontend = ENVIRONMENT_INVITATION_REQUEST
"""
if permissions.INVITE_ENVIRONMENT_GROUP in g_permissions:
g_permissions.append(permissions.LIST_ENVIRONMENT_GROUPS)
g_permissions.append(permissions.REMOVE_ENVIRONMENT_GROUP)

if permissions.ADD_ENVIRONMENT_CONSUMPTION_ROLES in g_permissions:
g_permissions.append(permissions.LIST_ENVIRONMENT_CONSUMPTION_ROLES)

if permissions.CREATE_NETWORK in g_permissions:
g_permissions.append(permissions.LIST_ENVIRONMENT_NETWORKS)

g_permissions.append(permissions.GET_ENVIRONMENT)
g_permissions.append(permissions.LIST_ENVIRONMENT_GROUPS)
g_permissions.append(permissions.LIST_ENVIRONMENT_GROUP_PERMISSIONS)
g_permissions.append(permissions.LIST_ENVIRONMENT_NETWORKS)
g_permissions.append(permissions.CREDENTIALS_ENVIRONMENT)

g_permissions.extend(permissions.ENVIRONMENT_INVITED_DEFAULT)
g_permissions = list(set(g_permissions))

if g_permissions not in permissions.ENVIRONMENT_INVITED:
Expand Down
25 changes: 12 additions & 13 deletions backend/dataall/core/permissions/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
MANAGE_GLOSSARIES = 'MANAGE_GLOSSARIES'
MANAGE_ENVIRONMENTS = 'MANAGE_ENVIRONMENTS'
MANAGE_ORGANIZATIONS = 'MANAGE_ORGANIZATIONS'
MANAGE_SGMSTUDIO_NOTEBOOKS = 'MANAGE_SGMSTUDIO_NOTEBOOKS'

"""
ENVIRONMENT
Expand All @@ -49,21 +48,23 @@
LIST_ENVIRONMENT_NETWORKS = 'LIST_ENVIRONMENT_NETWORKS'


ENVIRONMENT_INVITED = [
LIST_ENVIRONMENT_GROUP_PERMISSIONS,
GET_ENVIRONMENT,
LIST_ENVIRONMENT_GROUPS,
LIST_ENVIRONMENT_CONSUMPTION_ROLES,
INVITE_ENVIRONMENT_GROUP,
ADD_ENVIRONMENT_CONSUMPTION_ROLES,
CREATE_NETWORK,
LIST_ENVIRONMENT_NETWORKS,
]
ENVIRONMENT_INVITATION_REQUEST = [
INVITE_ENVIRONMENT_GROUP,
ADD_ENVIRONMENT_CONSUMPTION_ROLES,
CREATE_NETWORK,
]

ENVIRONMENT_INVITED_DEFAULT = [
GET_ENVIRONMENT,
LIST_ENVIRONMENT_GROUPS,
LIST_ENVIRONMENT_CONSUMPTION_ROLES,
LIST_ENVIRONMENT_GROUP_PERMISSIONS,
LIST_ENVIRONMENT_NETWORKS,
CREDENTIALS_ENVIRONMENT
]

ENVIRONMENT_INVITED = (ENVIRONMENT_INVITED_DEFAULT + ENVIRONMENT_INVITATION_REQUEST)

ENVIRONMENT_ALL = [
UPDATE_ENVIRONMENT,
GET_ENVIRONMENT,
Expand Down Expand Up @@ -117,15 +118,13 @@
MANAGE_GROUPS,
MANAGE_ENVIRONMENTS,
MANAGE_ORGANIZATIONS,
MANAGE_SGMSTUDIO_NOTEBOOKS,
]

TENANT_ALL_WITH_DESC = {k: k for k in TENANT_ALL}
TENANT_ALL_WITH_DESC[MANAGE_GLOSSARIES] = 'Manage glossaries'
TENANT_ALL_WITH_DESC[MANAGE_ENVIRONMENTS] = 'Manage environments'
TENANT_ALL_WITH_DESC[MANAGE_GROUPS] = 'Manage teams'
TENANT_ALL_WITH_DESC[MANAGE_ORGANIZATIONS] = 'Manage organizations'
TENANT_ALL_WITH_DESC[MANAGE_SGMSTUDIO_NOTEBOOKS] = 'Manage ML studio notebooks'

"""
NETWORKS
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""
SHARE OBJECT
"""
from dataall.core.permissions.permissions import ENVIRONMENT_INVITED, ENVIRONMENT_INVITATION_REQUEST, ENVIRONMENT_ALL, RESOURCES_ALL, \
RESOURCES_ALL_WITH_DESC
from dataall.core.permissions.permissions import ENVIRONMENT_INVITED, ENVIRONMENT_INVITATION_REQUEST, \
ENVIRONMENT_INVITED_DEFAULT, ENVIRONMENT_ALL, RESOURCES_ALL, RESOURCES_ALL_WITH_DESC

ADD_ITEM = 'ADD_ITEM'
REMOVE_ITEM = 'REMOVE_ITEM'
Expand Down Expand Up @@ -46,13 +46,13 @@
ENVIRONMENT_INVITED.append(CREATE_SHARE_OBJECT)
ENVIRONMENT_INVITED.append(LIST_ENVIRONMENT_SHARED_WITH_OBJECTS)
ENVIRONMENT_INVITATION_REQUEST.append(CREATE_SHARE_OBJECT)
ENVIRONMENT_INVITATION_REQUEST.append(LIST_ENVIRONMENT_SHARED_WITH_OBJECTS)
ENVIRONMENT_INVITED_DEFAULT.append(LIST_ENVIRONMENT_SHARED_WITH_OBJECTS)
ENVIRONMENT_ALL.append(CREATE_SHARE_OBJECT)
ENVIRONMENT_ALL.append(LIST_ENVIRONMENT_SHARED_WITH_OBJECTS)

RESOURCES_ALL.extend(SHARE_OBJECT_ALL)
for perm in SHARE_OBJECT_ALL:
RESOURCES_ALL_WITH_DESC[perm] = perm

RESOURCES_ALL_WITH_DESC[CREATE_SHARE_OBJECT] = 'Request datasets access for this environment'
RESOURCES_ALL_WITH_DESC[LIST_ENVIRONMENT_SHARED_WITH_OBJECTS] = "List datasets shared with this environments"
RESOURCES_ALL_WITH_DESC[CREATE_SHARE_OBJECT] = 'Create dataset Share requests for this environment'
RESOURCES_ALL_WITH_DESC[LIST_ENVIRONMENT_SHARED_WITH_OBJECTS] = 'LIST_ENVIRONMENT_SHARED_WITH_OBJECTS'
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from itertools import chain

from dataall.core.permissions.permissions import TENANT_ALL, TENANT_ALL_WITH_DESC, RESOURCES_ALL, RESOURCES_ALL_WITH_DESC, \
ENVIRONMENT_INVITED, ENVIRONMENT_INVITATION_REQUEST, ENVIRONMENT_ALL
ENVIRONMENT_INVITED, ENVIRONMENT_INVITATION_REQUEST, ENVIRONMENT_INVITED_DEFAULT, ENVIRONMENT_ALL
from dataall.modules.datasets_base.services.permissions import DATASET_TABLE_READ

MANAGE_DATASETS = 'MANAGE_DATASETS'
Expand Down Expand Up @@ -69,7 +69,7 @@
ENVIRONMENT_INVITED.append(LIST_ENVIRONMENT_DATASETS)

ENVIRONMENT_INVITATION_REQUEST.append(CREATE_DATASET)
ENVIRONMENT_INVITATION_REQUEST.append(LIST_ENVIRONMENT_DATASETS)
ENVIRONMENT_INVITED_DEFAULT.append(LIST_ENVIRONMENT_DATASETS)

ENVIRONMENT_ALL.append(CREATE_DATASET)
ENVIRONMENT_ALL.append(LIST_ENVIRONMENT_DATASETS)
Expand All @@ -82,4 +82,4 @@
RESOURCES_ALL_WITH_DESC[perm] = perm

RESOURCES_ALL_WITH_DESC[CREATE_DATASET] = 'Create datasets on this environment'
RESOURCES_ALL_WITH_DESC[LIST_ENVIRONMENT_DATASETS] = "List datasets on this environment"
RESOURCES_ALL_WITH_DESC[LIST_ENVIRONMENT_DATASETS] = 'LIST_ENVIRONMENT_DATASETS'
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,11 @@

# Definition of ENVIRONMENT_PERMISSIONS for SageMaker ML Studio
CREATE_SGMSTUDIO_USER = 'CREATE_SGMSTUDIO_USER'
# TODO: cleanup permissions = LIST_ENVIRONMENT_SGMSTUDIO_USERS and other LIST_ENVIRONMENT permissions
LIST_ENVIRONMENT_SGMSTUDIO_USERS = 'LIST_ENVIRONMENT_SGMSTUDIO_USERS'


ENVIRONMENT_ALL.append(CREATE_SGMSTUDIO_USER)
ENVIRONMENT_ALL.append(LIST_ENVIRONMENT_SGMSTUDIO_USERS)
ENVIRONMENT_INVITED.append(CREATE_SGMSTUDIO_USER)
ENVIRONMENT_INVITED.append(LIST_ENVIRONMENT_SGMSTUDIO_USERS)
ENVIRONMENT_INVITATION_REQUEST.append(CREATE_SGMSTUDIO_USER)
ENVIRONMENT_INVITATION_REQUEST.append(LIST_ENVIRONMENT_SGMSTUDIO_USERS)

# Definition of RESOURCE_PERMISSIONS for SageMaker ML Studio
GET_SGMSTUDIO_USER = 'GET_SGMSTUDIO_USER'
Expand All @@ -61,12 +57,10 @@

RESOURCES_ALL.extend(SGMSTUDIO_USER_ALL)
RESOURCES_ALL.append(CREATE_SGMSTUDIO_USER)
RESOURCES_ALL.append(LIST_ENVIRONMENT_SGMSTUDIO_USERS)


RESOURCES_ALL_WITH_DESC[GET_SGMSTUDIO_USER] = "General permission to get a SageMaker Studio user"
RESOURCES_ALL_WITH_DESC[UPDATE_SGMSTUDIO_USER] = "Permission to get a SageMaker Studio user"
RESOURCES_ALL_WITH_DESC[DELETE_SGMSTUDIO_USER] = "Permission to delete a SageMaker Studio user"
RESOURCES_ALL_WITH_DESC[SGMSTUDIO_USER_URL] = "Permission to generate the URL for a SageMaker Studio user"
RESOURCES_ALL_WITH_DESC[CREATE_SGMSTUDIO_USER] = "Create SageMaker Studio users on this environment"
RESOURCES_ALL_WITH_DESC[LIST_ENVIRONMENT_SGMSTUDIO_USERS] = "List SageMaker Studio users on this environment"
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,4 @@
ENVIRONMENT_ALL.append(RUN_ATHENA_QUERY)

RESOURCES_ALL.append(RUN_ATHENA_QUERY)
RESOURCES_ALL_WITH_DESC[RUN_ATHENA_QUERY] = "Run Athena queries on this environment"
RESOURCES_ALL_WITH_DESC[RUN_ATHENA_QUERY] = "Run Worksheet Athena queries on this environment"
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,23 @@
UPDATE_SGMSTUDIO_NOTEBOOK = 'UPDATE_SGMSTUDIO_NOTEBOOK'
DELETE_SGMSTUDIO_NOTEBOOK = 'DELETE_SGMSTUDIO_NOTEBOOK'
SGMSTUDIO_NOTEBOOK_URL = 'SGMSTUDIO_NOTEBOOK_URL'
RUN_ATHENA_QUERY = 'RUN_ATHENA_QUERY'
CREATE_SHARE_OBJECT = 'CREATE_SHARE_OBJECT'

OLD_PERMISSIONS = [
CREATE_SGMSTUDIO_NOTEBOOK,
LIST_ENVIRONMENT_SGMSTUDIO_NOTEBOOKS,
GET_SGMSTUDIO_NOTEBOOK,
UPDATE_SGMSTUDIO_NOTEBOOK,
DELETE_SGMSTUDIO_NOTEBOOK,
SGMSTUDIO_NOTEBOOK_URL
SGMSTUDIO_NOTEBOOK_URL,
RUN_ATHENA_QUERY,
CREATE_SHARE_OBJECT

]
old_permissions = {k: k for k in OLD_PERMISSIONS}
old_permissions[CREATE_SGMSTUDIO_NOTEBOOK] = 'Create ML Studio profiles on this environment'

old_permissions[CREATE_SHARE_OBJECT] = 'Request datasets access for this environment'

CREATE_SGMSTUDIO_USER = 'CREATE_SGMSTUDIO_USER'
LIST_ENVIRONMENT_SGMSTUDIO_USERS = 'LIST_ENVIRONMENT_SGMSTUDIO_USERS'
Expand All @@ -50,24 +55,30 @@
UPDATE_SGMSTUDIO_USER = 'UPDATE_SGMSTUDIO_USER'
DELETE_SGMSTUDIO_USER = 'DELETE_SGMSTUDIO_USER'
SGMSTUDIO_USER_URL = 'SGMSTUDIO_USER_URL'
RUN_ATHENA_QUERY = 'RUN_ATHENA_QUERY'

NEW_PERMISSIONS = [
CREATE_SGMSTUDIO_USER,
LIST_ENVIRONMENT_SGMSTUDIO_USERS,
GET_SGMSTUDIO_USER,
UPDATE_SGMSTUDIO_USER,
DELETE_SGMSTUDIO_USER,
SGMSTUDIO_USER_URL
SGMSTUDIO_USER_URL,
RUN_ATHENA_QUERY,
CREATE_SHARE_OBJECT
]
new_permissions = {k: k for k in NEW_PERMISSIONS}
new_permissions[CREATE_SGMSTUDIO_USER] = 'Create SageMaker Studio users on this environment'
new_permissions[RUN_ATHENA_QUERY] = 'Run Worksheet Athena queries on this environment'
new_permissions[CREATE_SHARE_OBJECT] = 'Create dataset Share requests for this environment'


def upgrade():
"""
The script does the following migration:
1) create missing permissions MANAGE_SGMSTUDIO_USERS from MANAGE_NOTEBOOKS tenant permission
2) Rename SageMaker Studio permissions from SGMSTUDIO_NOTEBOOK to SGMSTUDIO_USER
and add description to RUN_ATHENA_QUERY and create share object
3) Rename sagemaker_studio_user_profile column names
"""
try:
Expand Down

0 comments on commit 0c22580

Please sign in to comment.