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

Add location-based row level security to group and individual #121

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
80997e4
Add village to individual and group model
weilu Sep 5, 2024
e6db8b5
Unit test existing individual GQL permission control
weilu Sep 6, 2024
d4cd3ae
Enable row level security on group and individual list queries
weilu Sep 6, 2024
af44440
Row-level security for individual history query
weilu Sep 9, 2024
3269d6b
Row-level security for group history query
weilu Sep 9, 2024
48df524
Row-level security for group membership query
weilu Sep 9, 2024
93000ab
Row-level security for individual group membership history
weilu Sep 9, 2024
e01ebe2
Add row level security to individual creation query
weilu Sep 15, 2024
8cd6b76
Add row-level security to individual update query
weilu Sep 15, 2024
27a5d33
Add row-level security to individual delete query
weilu Sep 15, 2024
9b814cb
Add row-level security to individual undo delete query
weilu Sep 15, 2024
364ec1c
Add basic create group gql mutation test
weilu Sep 22, 2024
c90e48b
Add row level security to group create
weilu Sep 22, 2024
fc96164
Row level security for group update
weilu Sep 24, 2024
d9664a6
Add row level security to group delete
weilu Sep 29, 2024
80b9ba0
Add row level security to adding individual to group
weilu Sep 30, 2024
5cb938b
Verify that both individual_id and group_id are required by service
weilu Sep 30, 2024
2cbda50
Add row level security to editing individual in group
weilu Sep 30, 2024
c7556bb
Add row level security to removing individuals from group
weilu Sep 30, 2024
93b8d54
Make sure individual group move doesn’t create and delete unnecessarily
weilu Sep 30, 2024
629486f
Remove unnecessary location-individual relationship reversal in query
weilu Oct 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions individual/gql_mutations.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import graphene
from django.core.exceptions import ValidationError
from django.db import transaction
from django.db.models import Subquery, Q

from core.gql.gql_mutations.base_mutation import BaseHistoryModelDeleteMutationMixin, BaseMutation, \
BaseHistoryModelUpdateMutationMixin, BaseHistoryModelCreateMutationMixin
Expand All @@ -9,13 +10,15 @@
from individual.models import Individual, Group, GroupIndividual
from individual.services import IndividualService, GroupService, GroupIndividualService, \
CreateGroupAndMoveIndividualService
from location.models import Location, LocationManager


class CreateIndividualInputType(OpenIMISMutation.Input):
first_name = graphene.String(required=True, max_length=255)
last_name = graphene.String(required=True, max_length=255)
dob = graphene.Date(required=True)
json_ext = graphene.types.json.JSONString(required=False)
village_id = graphene.Int(required=False)


class UpdateIndividualInputType(CreateIndividualInputType):
Expand Down Expand Up @@ -55,6 +58,7 @@ def resolve_recipient_type(self, info):
class CreateGroupInputType(OpenIMISMutation.Input):
code = graphene.String(required=True)
individuals_data = graphene.List(CreateGroupIndividualInputTypeInputObjectType, required=False)
village_id = graphene.Int(required=False)
Copy link
Member

Choose a reason for hiding this comment

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

see comment on locations



class UpdateGroupInputType(CreateGroupInputType):
Expand Down Expand Up @@ -82,6 +86,10 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_individual_create_perms):
raise ValidationError("mutation.authentication_required")
if 'village_id' in data:
village = Location.objects.get(id=data['village_id'])
if village not in Location.get_queryset(None, user):
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down Expand Up @@ -110,6 +118,10 @@ def _validate_mutation(cls, user, **data):
IndividualConfig.gql_individual_update_perms):
raise ValidationError("mutation.authentication_required")

village = Individual.objects.get(id=data['id']).village
if village and village not in Location.get_queryset(None, user):
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
if "client_mutation_id" in data:
Expand Down Expand Up @@ -140,6 +152,15 @@ def _validate_mutation(cls, user, **data):
IndividualConfig.gql_individual_delete_perms):
raise ValidationError("mutation.authentication_required")

villages_qs = Location.objects.filter(individual__id__in=data['ids'], type='V')
Copy link
Member

Choose a reason for hiding this comment

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

not sure what you are doing here filtering Location with individual_id (for id you need only one _)

Copy link
Contributor Author

@weilu weilu Oct 4, 2024

Choose a reason for hiding this comment

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

It's for checking that the user has sufficient (geo) permission to delete the individual. See test case IndividualGQLMutationTest.test_delete_individual_row_security.

Good spotting on the double _. Fixed.

# must first check if villages_qs exists in case none of the individuals has location
if villages_qs.exists():
allowed_loc_ids = Location.get_queryset(None, user).values('id')
not_in_allowed = villages_qs.exclude(id__in=Subquery(allowed_loc_ids))
# all individuals' villages must be within permission for the given user
if not allowed_loc_ids.exists() or not_in_allowed.exists():
raise ValidationError("mutation.authentication_required")
Copy link
Member

Choose a reason for hiding this comment

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

to be tested but you get the idea,

Suggested change
villages_qs = Location.objects.filter(individual__id__in=data['ids'], type='V')
# must first check if villages_qs exists in case none of the individuals has location
if villages_qs.exists():
allowed_loc_ids = Location.get_queryset(None, user).values('id')
not_in_allowed = villages_qs.exclude(id__in=Subquery(allowed_loc_ids))
# all individuals' villages must be within permission for the given user
if not allowed_loc_ids.exists() or not_in_allowed.exists():
raise ValidationError("mutation.authentication_required")
individual_not_allowed = Individual.objects.filters(
id__in=data['ids'],
Q(~location__in=LocationManager().allowed(user=user))
)
if individual_not_allowed:
raise ValidationError("mutation.authentication_required")


@classmethod
def _mutate(cls, user, **data):
if "client_mutation_id" in data:
Expand Down Expand Up @@ -175,6 +196,15 @@ def _validate_mutation(cls, user, **data):
IndividualConfig.gql_individual_undo_delete_perms):
raise ValidationError("mutation.authentication_required")

villages_qs = Location.objects.filter(individual__id__in=data['ids'], type='V')
Copy link
Member

Choose a reason for hiding this comment

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

see above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed unnecessary __

# must first check if villages_qs exists in case none of the individuals has location
if villages_qs.exists():
allowed_loc_ids = Location.get_queryset(None, user).values('id')
not_in_allowed = villages_qs.exclude(id__in=Subquery(allowed_loc_ids))
# all individuals' villages must be within permission for the given user
if not allowed_loc_ids.exists() or not_in_allowed.exists():
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
if "client_mutation_id" in data:
Expand Down Expand Up @@ -205,6 +235,10 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_group_create_perms):
raise ValidationError("mutation.authentication_required")
if 'village_id' in data:
village = Location.objects.get(id=data['village_id'])
if village not in Location.get_queryset(None, user):
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down Expand Up @@ -232,6 +266,9 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_group_update_perms):
raise ValidationError("mutation.authentication_required")
village = Group.objects.get(id=data['id']).village
if village and village not in Location.get_queryset(None, user):
Copy link
Member

Choose a reason for hiding this comment

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

TBC id allowed won't be better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean?

raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down Expand Up @@ -260,6 +297,15 @@ def _validate_mutation(cls, user, **data):
IndividualConfig.gql_group_delete_perms):
raise ValidationError("mutation.authentication_required")

villages_qs = Location.objects.filter(group__id__in=data['ids'], type='V')
# must first check if villages_qs exists in case none of the groups has location
if villages_qs.exists():
allowed_loc_ids = Location.get_queryset(None, user).values('id')
not_in_allowed = villages_qs.exclude(id__in=Subquery(allowed_loc_ids))
# all groups' villages must be within permission for the given user
if not allowed_loc_ids.exists() or not_in_allowed.exists():
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
if "client_mutation_id" in data:
Expand Down Expand Up @@ -290,6 +336,13 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_group_create_perms):
raise ValidationError("mutation.authentication_required")
group_village = Group.objects.get(id=data['group_id']).village
individual_village = Individual.objects.get(id=data['individual_id']).village
if group_village and individual_village:
if group_village.id != individual_village.id:
raise ValidationError("mutation.individual_group_village_mismatch")
elif group_village not in Location.get_queryset(None, user):
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down Expand Up @@ -317,6 +370,13 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_group_update_perms):
raise ValidationError("mutation.authentication_required")
group_village = Group.objects.get(id=data['group_id']).village
individual_village = Individual.objects.get(id=data['individual_id']).village
if group_village and individual_village:
if group_village.id != individual_village.id:
raise ValidationError("mutation.individual_group_village_mismatch")
elif group_village not in Location.get_queryset(None, user):
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down Expand Up @@ -347,6 +407,19 @@ def _validate_mutation(cls, user, **data):
if not user.has_perms(
IndividualConfig.gql_group_delete_perms):
raise ValidationError("mutation.authentication_required")
villages_qs = Location.objects.filter(
type='V'
).filter(
Q(group__groupindividual__id__in=data['ids']) |
Q(individual__groupindividual__id__in=data['ids'])
)
# must first check if villages_qs exists in case none of the groups or individuals has location
if villages_qs.exists():
allowed_loc_ids = Location.get_queryset(None, user).values('id')
not_in_allowed = villages_qs.exclude(id__in=Subquery(allowed_loc_ids))
# all groups' & individuals' villages must be within permission for the given user
if not allowed_loc_ids.exists() or not_in_allowed.exists():
raise ValidationError("mutation.authentication_required")

@classmethod
def _mutate(cls, user, **data):
Expand Down
34 changes: 34 additions & 0 deletions individual/gql_queries.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import graphene
from django.contrib.auth.models import AnonymousUser
from graphene_django import DjangoObjectType
import graphene_django_optimizer as gql_optimizer

from core import prefix_filterset, ExtendedConnection
from core.gql_queries import UserGQLType
Expand Down Expand Up @@ -43,6 +44,10 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
return Individual.get_queryset(queryset, info.context.user)


class IndividualHistoryGQLType(DjangoObjectType):
uuid = graphene.String(source='uuid')
Expand All @@ -68,6 +73,13 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
accessible_individual_query = Individual.get_queryset(None, info.context.user)
accessible_individuals = gql_optimizer.query(accessible_individual_query, info)
accessible_uuids = set(accessible_individuals.values_list('uuid', flat=True))
return queryset.filter(id__in=accessible_uuids)


class IndividualDataSourceUploadGQLType(DjangoObjectType):
uuid = graphene.String(source='uuid')
Expand Down Expand Up @@ -132,6 +144,10 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
return Group.get_queryset(queryset, info.context.user)


class GroupHistoryGQLType(DjangoObjectType):
uuid = graphene.String(source='uuid')
Expand Down Expand Up @@ -161,6 +177,13 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
accessible_group_query = Group.get_queryset(None, info.context.user)
accessible_groups = gql_optimizer.query(accessible_group_query, info)
accessible_uuids = set(accessible_groups.values_list('uuid', flat=True))
return queryset.filter(id__in=accessible_uuids)


class GroupIndividualGQLType(DjangoObjectType):
uuid = graphene.String(source='uuid')
Expand All @@ -181,6 +204,10 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
return GroupIndividual.get_queryset(queryset, info.context.user)


class GroupIndividualHistoryGQLType(DjangoObjectType):
uuid = graphene.String(source='uuid')
Expand All @@ -205,6 +232,13 @@ class Meta:
}
connection_class = ExtendedConnection

@classmethod
def get_queryset(cls, queryset, info):
accessible_group_query = Group.get_queryset(None, info.context.user)
accessible_groups = gql_optimizer.query(accessible_group_query, info)
accessible_uuids = set(accessible_groups.values_list('uuid', flat=True))
return queryset.filter(group__id__in=accessible_uuids)


class IndividualDataUploadQGLType(DjangoObjectType, JsonExtMixin):
uuid = graphene.String(source='uuid')
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Generated by Django 4.2.13 on 2024-09-05 15:27

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('location', '0018_auto_20230925_2243'),
('individual', '0016_remove_recipient_from_role'),
]

operations = [
migrations.AddField(
model_name='group',
name='village',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='location.location'),
),
migrations.AddField(
model_name='historicalgroup',
name='village',
field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='location.location'),
),
migrations.AddField(
model_name='historicalindividual',
name='village',
field=models.ForeignKey(blank=True, db_constraint=False, null=True, on_delete=django.db.models.deletion.DO_NOTHING, related_name='+', to='location.location'),
),
migrations.AddField(
model_name='individual',
name='village',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.DO_NOTHING, to='location.location'),
),
]
Loading
Loading