741 refactoring. dataall.core.groups

[SofiaSazonova]

Feature or Bugfix

• Refactoring

Detail

• logic of group-listing was moved to services.group_service.py

Relates

• 741

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.

[dlpzx]

Overall it looks great. There is one additional thing that we should modify on the resolvers dataall/core/groups/api/resolvers.py. As you might have noticed, we are passing context as input to all API calls, sometimes it is needed and sometimes it is not, but we still have to pass it to all calls. In addition, we are opening a session directly in the resolver and pass it to the service which then passes it on the db layer. The api layer should not start a session, should only validate input/output, then service layer validates permissions and then we can start a session. I think it is easier to see with an example. In this function, we are opening the session, before validating the permissions in the service layer. (for groups there are no permissions, but in general services validates permissions.

def list_groups(context, source, filter: dict = None):
    with context.engine.scoped_session() as session:
        return GroupService.list_groups_without_invited(session, filter)

To avoid 1. passing the context as input to all APIs and 2. starting a session before validating permissions, we released a way of accessing the context of the API at any moment. With that in mind, we should change the resolver to:

def list_groups(context, source, filter: dict = None):
      if not filter:
        filter = {}
    return GroupService.list_groups_without_invited(filter)

Note that I have added some validation for filter, because if it is None it might fail in dataall.core.groups.services.group_service.GroupService.list_invited_groups

    def list_invited_groups(session, filter: dict = None):
        category, category_uri = filter.get('type'), filter.get('uri')

Then in dataall/core/groups/services/group_service.py we should import from dataall.base.context import get_context
and use it to open a sql alchemy session.

    @staticmethod
    def list_groups_without_invited(session, filter: dict = None):
        with get_context().db_engine.scoped_session() as session:
            cognito_groups = GroupService.list_cognito_groups()
            invited_groups = GroupService.list_invited_groups(session, filter)
    
            groups = []
            for group in cognito_groups:
                if group not in invited_groups:
                    groups.append({'groupName': group})
    
            return groups

[dlpzx]

Re-reviewed! It is ready to be merged :)