Skip to content

Commit

Permalink
Core/vpc refactoring - Refactor vpc module to follow api/services/db …
Browse files Browse the repository at this point in the history
…layers (#1044)

### Feature or Bugfix
- Refactoring

### Detail
Some modules under `core` do not completely follow the standard
structure agreed for modules. In this PR I just focus on the module
`vpc`:
- Add input validation in the `api` layer in the resolvers
- Create a `service` layer for `vpc`, where we check permissions and
execute business logic
- The `db` subpackage should contain all operations on RDS. It should
not contain business logic, permissions checks or input validation.

### Relates
- #741 
- #989 same but for the `vpc` module

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

- 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 Feb 8, 2024
1 parent 18e5b56 commit e1e9e08
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 130 deletions.
10 changes: 3 additions & 7 deletions backend/dataall/core/environment/api/resolvers.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@
from dataall.core.stacks.api import stack_helper
from dataall.core.stacks.aws.cloudformation import CloudFormation
from dataall.core.stacks.db.stack_repositories import Stack
from dataall.core.vpc.db.vpc_repositories import Vpc
from dataall.core.vpc.services.vpc_service import VpcService
from dataall.base.aws.ec2_client import EC2
from dataall.base.db import exceptions
from dataall.core.permissions import permissions
from dataall.base.feature_toggle_checker import is_feature_enabled
from dataall.base.utils.naming_convention import (
Expand Down Expand Up @@ -361,11 +360,8 @@ def get_parent_organization(context: Context, source, **kwargs):
return org


def resolve_vpc_list(context: Context, source, **kwargs):
with context.engine.scoped_session() as session:
return Vpc.get_environment_vpc_list(
session=session, environment_uri=source.environmentUri
)
def resolve_environment_networks(context: Context, source, **kwargs):
return VpcService.get_environment_networks(environment_uri=source.environmentUri)


def get_environment(context: Context, source, environmentUri: str = None):
Expand Down
4 changes: 2 additions & 2 deletions backend/dataall/core/environment/api/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
gql.Field(
name='networks',
type=gql.ArrayType(gql.Ref('Vpc')),
resolver=resolve_vpc_list,
resolver=resolve_environment_networks,
),
gql.Field(
name='parameters',
Expand Down Expand Up @@ -139,7 +139,7 @@
gql.Field(
name='networks',
type=gql.ArrayType(gql.Ref('Vpc')),
resolver=resolve_vpc_list,
resolver=resolve_environment_networks,
)
],
)
Expand Down
30 changes: 19 additions & 11 deletions backend/dataall/core/vpc/api/resolvers.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,30 @@
import logging

from dataall.base.api.context import Context
from dataall.core.vpc.db.vpc_repositories import Vpc
from dataall.base.db import exceptions
from dataall.core.vpc.services.vpc_service import VpcService


log = logging.getLogger(__name__)


def _validate_input(data):
if not data:
raise exceptions.RequiredParameter(data)
if not data.get('environmentUri'):
raise exceptions.RequiredParameter('environmentUri')
if not data.get('SamlGroupName'):
raise exceptions.RequiredParameter('group')
if not data.get('label'):
raise exceptions.RequiredParameter('label')


def create_network(context: Context, source, input):
with context.engine.scoped_session() as session:
vpc = Vpc.create_network(
session=session,
uri=input['environmentUri'],
admin_group=input['SamlGroupName'],
data=input,
)
return vpc
_validate_input(input)
return VpcService.create_network(uri=input.get('environmentUri'), admin_group=input.get('SamlGroupName'), data=input)


def delete_network(context: Context, source, vpcUri=None):
with context.engine.scoped_session() as session:
return Vpc.delete(session=session, uri=vpcUri)
if not vpcUri:
raise exceptions.RequiredParameter("vpcUri")
return VpcService.delete_network(uri=vpcUri)
131 changes: 21 additions & 110 deletions backend/dataall/core/vpc/db/vpc_repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,140 +2,51 @@

from sqlalchemy import and_

from dataall.core.environment.db.environment_repositories import EnvironmentRepository
from dataall.core.environment.env_permission_checker import has_group_permission
from dataall.base.db import exceptions
from dataall.core.permissions import permissions
from dataall.core.vpc.db import vpc_models as models
from dataall.core.permissions.permission_checker import has_resource_permission, has_tenant_permission
from dataall.base.context import get_context
from dataall.core.activity.db.activity_models import Activity
from dataall.core.permissions.db.resource_policy_repositories import ResourcePolicy
from dataall.core.vpc.db.vpc_models import Vpc

log = logging.getLogger(__name__)


class Vpc:
def __init__(self):
pass
class VpcRepository:

@staticmethod
@has_tenant_permission(permissions.MANAGE_ENVIRONMENTS)
@has_resource_permission(permissions.CREATE_NETWORK)
@has_group_permission(permissions.CREATE_NETWORK)
def create_network(session, uri: str, admin_group: str, data: dict = None) -> models.Vpc:
Vpc._validate_input(data)
username = get_context().username

vpc = (
session.query(models.Vpc)
.filter(
and_(
models.Vpc.VpcId == data['vpcId'], models.Vpc.environmentUri == uri
)
)
.first()
)

if vpc:
raise exceptions.ResourceAlreadyExists(
action=permissions.CREATE_NETWORK,
message=f'Vpc {data["vpcId"]} is already associated to environment {uri}',
)

environment = EnvironmentRepository.get_environment_by_uri(session, uri)
vpc = models.Vpc(
environmentUri=environment.environmentUri,
region=environment.region,
AwsAccountId=environment.AwsAccountId,
VpcId=data['vpcId'],
privateSubnetIds=data.get('privateSubnetIds', []),
publicSubnetIds=data.get('publicSubnetIds', []),
SamlGroupName=data['SamlGroupName'],
owner=username,
label=data['label'],
name=data['label'],
default=data.get('default', False),
)
def save_network(session, vpc):
session.add(vpc)
session.commit()

activity = Activity(
action='NETWORK:CREATE',
label='NETWORK:CREATE',
owner=username,
summary=f'{username} created network {vpc.label} in {environment.label}',
targetUri=vpc.vpcUri,
targetType='Vpc',
)
session.add(activity)

ResourcePolicy.attach_resource_policy(
session=session,
group=vpc.SamlGroupName,
permissions=permissions.NETWORK_ALL,
resource_uri=vpc.vpcUri,
resource_type=models.Vpc.__name__,
)

if environment.SamlGroupName != vpc.SamlGroupName:
ResourcePolicy.attach_resource_policy(
session=session,
group=environment.SamlGroupName,
permissions=permissions.NETWORK_ALL,
resource_uri=vpc.vpcUri,
resource_type=models.Vpc.__name__,
)

return vpc

@staticmethod
def _validate_input(data):
if not data:
raise exceptions.RequiredParameter(data)
if not data.get('environmentUri'):
raise exceptions.RequiredParameter('environmentUri')
if not data.get('SamlGroupName'):
raise exceptions.RequiredParameter('group')
if not data.get('label'):
raise exceptions.RequiredParameter('label')

@staticmethod
@has_tenant_permission(permissions.MANAGE_ENVIRONMENTS)
@has_resource_permission(permissions.DELETE_NETWORK)
def delete(session, uri) -> bool:
vpc = Vpc.get_vpc_by_uri(session, uri)
def delete_network(session, uri) -> bool:
vpc = VpcRepository.get_vpc_by_uri(session, uri)
session.delete(vpc)
ResourcePolicy.delete_resource_policy(
session=session, resource_uri=uri, group=vpc.SamlGroupName
)
session.commit()
return True

@staticmethod
def get_vpc_by_uri(session, vpc_uri) -> models.Vpc:
vpc = session.query(models.Vpc).get(vpc_uri)
def get_vpc_by_uri(session, vpc_uri) -> Vpc:
vpc = session.query(Vpc).get(vpc_uri)
if not vpc:
raise exceptions.ObjectNotFound('VPC', vpc_uri)
return vpc

@staticmethod
def get_environment_vpc_list(session, environment_uri):
return (
session.query(models.Vpc)
.filter(models.Vpc.environmentUri == environment_uri)
.all()
)

@staticmethod
def get_environment_default_vpc(session, environment_uri):
return (
session.query(models.Vpc)
def find_vpc_by_id_environment(session, vpc_id, environment_uri) -> Vpc:
vpc = (
session.query(Vpc)
.filter(
and_(
models.Vpc.environmentUri == environment_uri,
models.Vpc.default == True,
Vpc.VpcId == vpc_id,
Vpc.environmentUri == environment_uri
)
)
.first()
)
return vpc

@staticmethod
def get_environment_networks(session, environment_uri):
return (
session.query(Vpc)
.filter(Vpc.environmentUri == environment_uri)
.all()
)
Empty file.
97 changes: 97 additions & 0 deletions backend/dataall/core/vpc/services/vpc_service.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
from dataall.base.context import get_context
from dataall.base.db import exceptions
from dataall.core.permissions import permissions
from dataall.core.permissions.permission_checker import has_resource_permission, has_tenant_permission
from dataall.core.permissions.db.resource_policy_repositories import ResourcePolicy
from dataall.core.environment.env_permission_checker import has_group_permission
from dataall.core.environment.db.environment_repositories import EnvironmentRepository
from dataall.core.activity.db.activity_models import Activity
from dataall.core.vpc.db.vpc_repositories import VpcRepository
from dataall.core.vpc.db.vpc_models import Vpc


def _session():
return get_context().db_engine.scoped_session()


class VpcService:
@staticmethod
@has_tenant_permission(permissions.MANAGE_ENVIRONMENTS)
@has_resource_permission(permissions.CREATE_NETWORK)
@has_group_permission(permissions.CREATE_NETWORK)
def create_network(uri: str, admin_group: str, data: dict):
with _session() as session:
username = get_context().username
vpc = VpcRepository.find_vpc_by_id_environment(session=session, vpc_id=data['vpcId'], environment_uri=uri)

if vpc:
raise exceptions.ResourceAlreadyExists(
action=permissions.CREATE_NETWORK,
message=f'Vpc {data["vpcId"]} is already associated to environment {uri}',
)

environment = EnvironmentRepository.get_environment_by_uri(session, uri)
vpc = Vpc(
environmentUri=environment.environmentUri,
region=environment.region,
AwsAccountId=environment.AwsAccountId,
VpcId=data['vpcId'],
privateSubnetIds=data.get('privateSubnetIds', []),
publicSubnetIds=data.get('publicSubnetIds', []),
SamlGroupName=data['SamlGroupName'],
owner=username,
label=data['label'],
name=data['label'],
default=data.get('default', False),
)
VpcRepository.save_network(session, vpc)

activity = Activity(
action='NETWORK:CREATE',
label='NETWORK:CREATE',
owner=username,
summary=f'{username} created network {vpc.label} in {environment.label}',
targetUri=vpc.vpcUri,
targetType='Vpc',
)
session.add(activity)

ResourcePolicy.attach_resource_policy(
session=session,
group=vpc.SamlGroupName,
permissions=permissions.NETWORK_ALL,
resource_uri=vpc.vpcUri,
resource_type=Vpc.__name__,
)

if environment.SamlGroupName != vpc.SamlGroupName:
ResourcePolicy.attach_resource_policy(
session=session,
group=environment.SamlGroupName,
permissions=permissions.NETWORK_ALL,
resource_uri=vpc.vpcUri,
resource_type=Vpc.__name__,
)

return vpc

@staticmethod
@has_tenant_permission(permissions.MANAGE_ENVIRONMENTS)
@has_resource_permission(permissions.DELETE_NETWORK)
def delete_network(uri):
with _session() as session:
vpc = VpcRepository.get_vpc_by_uri(session=session, vpc_uri=uri)
ResourcePolicy.delete_resource_policy(
session=session, resource_uri=uri, group=vpc.SamlGroupName
)
return VpcRepository.delete_network(
session=session,
uri=uri
)

@staticmethod
def get_environment_networks(environment_uri):
with _session() as session:
return VpcRepository.get_environment_networks(
session=session, environment_uri=environment_uri
)

0 comments on commit e1e9e08

Please sign in to comment.