diff --git a/greg/migrations/0019_add_ou_parent_relatedname.py b/greg/migrations/0019_add_ou_parent_relatedname.py new file mode 100644 index 0000000000000000000000000000000000000000..87eae55db0dd2b6ccdb98a68ca2b3a164b296072 --- /dev/null +++ b/greg/migrations/0019_add_ou_parent_relatedname.py @@ -0,0 +1,25 @@ +# Generated by Django 3.2.9 on 2021-12-03 08:01 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("greg", "0018_alter_identity_type"), + ] + + operations = [ + migrations.AlterField( + model_name="organizationalunit", + name="parent", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="children", + to="greg.organizationalunit", + ), + ), + ] diff --git a/greg/models.py b/greg/models.py index 6d04f570ad5bdef058f1373b291b67491a4ccf3b..bc6403ecb0ee7b880338ddcf438da5457416e7ae 100644 --- a/greg/models.py +++ b/greg/models.py @@ -449,7 +449,13 @@ class OrganizationalUnit(BaseModel): name_nb = models.CharField(max_length=256) name_en = models.CharField(max_length=256) - parent = models.ForeignKey("self", on_delete=models.PROTECT, null=True, blank=True) + parent = models.ForeignKey( + "self", + on_delete=models.PROTECT, + null=True, + blank=True, + related_name="children", + ) active = models.BooleanField(default=True) deleted = models.BooleanField(default=False) @@ -461,6 +467,29 @@ class OrganizationalUnit(BaseModel): def __str__(self): return "{}".format(str(self.name_en or self.name_nb)) + def fetch_tree(self): + """ + Convenience method for fetching a set of the entire tree below + this unit including itself. + """ + units = set() + self.get_children(units) + return units + + def get_children(self, units: set["OrganizationalUnit"]): + """ + Fetch the entire tree of units with this unit at the top. + + Expects an empty set for bookkeeping to prevent an infinite loop. + """ + # shortcut to prevent infinite loop in case of a loop in the tree + if self.id in [i.id for i in units]: + return + # Recursion loop for fetching children of children of ... + units.add(self) + for child in self.children.all(): + child.get_children(units) + class Sponsor(BaseModel): """ @@ -495,6 +524,24 @@ class Sponsor(BaseModel): models.UniqueConstraint(name="unique_feide_id", fields=["feide_id"]) ] + def get_allowed_units(self) -> set[OrganizationalUnit]: + """ + Fetch every unit the sponsor has access to. + + This includes both through direct access and hierarchical. + """ + connections = self.link_sponsor.all() + ha_units = [i.organizational_unit for i in connections if i.hierarchical_access] + + units: set[OrganizationalUnit] = set() # Collector set + # Add units accessible through hierarchical access + for unit in ha_units: + units.update(unit.fetch_tree()) + + # Add units accessible through direct access + units = units.union({i.organizational_unit for i in connections}) + return units + class SponsorOrganizationalUnit(BaseModel): """ diff --git a/greg/tests/conftest.py b/greg/tests/conftest.py index 8315054552d2a98a70dd77e34af6991e5b80d5e8..7b6acedc3680b78217467923cae30bed61fa769a 100644 --- a/greg/tests/conftest.py +++ b/greg/tests/conftest.py @@ -238,3 +238,32 @@ def notification(): meta="foometa", ) return Notification.objects.get(id=1) + + +@pytest.fixture +def looped_units(): + # the loop + a = OrganizationalUnit.objects.create() + b = OrganizationalUnit.objects.create(parent=a) + c = OrganizationalUnit.objects.create(parent=b) + d = OrganizationalUnit.objects.create(parent=c) + e = OrganizationalUnit.objects.create(parent=d) + a.parent = e + a.save() + return a, b, c, d, e + + +@pytest.fixture +def loop_sponsor(looped_units, unit_foo) -> Sponsor: + """A sponsor that is connected to a loop, and a single unit.""" + loop_unit = looped_units[2] + sp = Sponsor.objects.create() + # Connection to the loop + SponsorOrganizationalUnit.objects.create( + sponsor=sp, organizational_unit=loop_unit, hierarchical_access=True + ) + # Stand alone connection + SponsorOrganizationalUnit.objects.create( + sponsor=sp, organizational_unit=unit_foo, hierarchical_access=False + ) + return sp diff --git a/greg/tests/models/test_organizational_unit.py b/greg/tests/models/test_organizational_unit.py index ce2043065ff5b367bbb1975976d7173130b55385..dfbcfa3ebe9621ca45113bd1c6eec4e41a486e55 100644 --- a/greg/tests/models/test_organizational_unit.py +++ b/greg/tests/models/test_organizational_unit.py @@ -18,3 +18,10 @@ def test_org_repr(unit_foo): @pytest.mark.django_db def test_org_str(unit_foo): assert str(unit_foo) == "foo_unit" + + +@pytest.mark.django_db +def test_fetch_tree_loop(looped_units): + a = looped_units[0] + units = a.fetch_tree() + assert [x.id for x in units] == [i.id for i in looped_units] diff --git a/greg/tests/models/test_sponsor.py b/greg/tests/models/test_sponsor.py index 581111194aca42b87195e0cab985ff17aec618ef..322ada7d7f559a7a319d53de5194317989360980 100644 --- a/greg/tests/models/test_sponsor.py +++ b/greg/tests/models/test_sponsor.py @@ -63,3 +63,10 @@ def test_sponsor_repr(sponsor_guy): @pytest.mark.django_db def test_sponsor_str(sponsor_guy): assert str(sponsor_guy) == "guy@example.org (Sponsor Guy)" + + +@pytest.mark.django_db +def test_get_allowed_loop(loop_sponsor, looped_units, unit_foo): + units = loop_sponsor.get_allowed_units() + expected = [i.id for i in looped_units] + [unit_foo.id] + assert [x.id for x in units] == expected diff --git a/gregui/api/serializers/role.py b/gregui/api/serializers/role.py index 0b867f092a1ca0f6eea1d9a1f71cbd9aa703a175..66fa2a867368414fc53938ab60466cce2c43e3fa 100644 --- a/gregui/api/serializers/role.py +++ b/gregui/api/serializers/role.py @@ -53,13 +53,11 @@ class RoleSerializerUi(serializers.ModelSerializer): def validate_orgunit(self, unit): """Enforce rules related to orgunit""" - sponsor = self.context["sponsor"] - units = sponsor.units.all() - # Restrict to a sponsor's own units - if not units or unit not in units: - raise ValidationError( - "A sponsor can only make changes to roles at units they are sponsors for." - ) + # A sponsor can only add roles to units they are sponsors at, or children of + # that unit if they have hierarchical access. + allowed_units = self.context["sponsor"].get_allowed_units() + if unit not in allowed_units: + raise ValidationError("You can only edit roles connected to your units.") return unit def validate(self, attrs): @@ -87,10 +85,10 @@ class RoleSerializerUi(serializers.ModelSerializer): raise serializers.ValidationError( "End date cannot be before start date." ) - # If we are updating an existing roles, we must be the sponsor of the role - sponsor = self.context["sponsor"] - if self.instance and self.instance.sponsor != sponsor: - raise ValidationError("You can only edit your own roles.") + # A sponsor can not modify roles connected to units they do not have access to + allowed_units = self.context["sponsor"].get_allowed_units() + if self.instance and self.instance.orgunit not in allowed_units: + raise ValidationError("You can only edit roles connected to your units.") return attrs diff --git a/gregui/api/views/person.py b/gregui/api/views/person.py index fec45bb3b1467dccb7dd6dcae4b5e01705a9c2da..b7173c76af9e087d5b31047b6f7df285148dfedc 100644 --- a/gregui/api/views/person.py +++ b/gregui/api/views/person.py @@ -57,7 +57,10 @@ class PersonSearchViewSet(mixins.ListModelMixin, GenericViewSet): class GuestInfoViewSet(mixins.ListModelMixin, GenericViewSet): """ - Fetch all the sponsor's guests. + Fetch all guests at the sponsor's units. + + If the Sponsor's connection to the Unit is marked with hierarchical access, guests + at child units of the Unit are included. Lists all persons connected to the roles the logged in sponsor is connected to. """ @@ -67,5 +70,17 @@ class GuestInfoViewSet(mixins.ListModelMixin, GenericViewSet): serializer_class = SpecialPersonSerializer def get_queryset(self): + """ + Fetch Persons connected to the sponsor some way. + + Any person with a role connected to the same unit as the sponsor, or a unit + that the sponsor is connected to through hierarchical access. + """ + user = GregUserProfile.objects.get(user=self.request.user) - return Person.objects.filter(roles__sponsor=user.sponsor).distinct() + units = user.sponsor.get_allowed_units() + return ( + Person.objects.filter(roles__orgunit__in=list(units)) + .distinct() + .order_by("id") + ) diff --git a/gregui/tests/api/serializers/test_role.py b/gregui/tests/api/serializers/test_role.py index b36c2b7c4e152b7a82851a2612cd2806f0e276ba..a34ef3ef49f32d73875bbbac4981c14fce019d45 100644 --- a/gregui/tests/api/serializers/test_role.py +++ b/gregui/tests/api/serializers/test_role.py @@ -98,8 +98,8 @@ def test_end_date_expired_role_fail(role, sponsor_foo): @pytest.mark.django_db def test_wrong_sponsor(role, sponsor_foo, sponsor_bar): - """Touching another sponsor's roles does not work""" - # Try to touch sponsor_foo's guest role as sponsor_bar + """Touching roles connected to another unit does not work""" + # Try to touch a unit at a unit we are not sponsor for ser = RoleSerializerUi( instance=role, data={ @@ -115,7 +115,7 @@ def test_wrong_sponsor(role, sponsor_foo, sponsor_bar): with pytest.raises( ValidationError, match=re.escape( - "{'non_field_errors': [ErrorDetail(string='You can only edit your own roles.', code='invalid')]}" + "{'orgunit': [ErrorDetail(string='You can only edit roles connected to your units.', code='invalid')]}" ), ): ser.is_valid(raise_exception=True) diff --git a/gregui/tests/api/views/test_role.py b/gregui/tests/api/views/test_role.py index 3b740f9ec366e315b1dbc661a0d73e80a3119758..26916673c704f1bf259848428f84954c48241d01 100644 --- a/gregui/tests/api/views/test_role.py +++ b/gregui/tests/api/views/test_role.py @@ -70,7 +70,7 @@ def test_role_sponsor_post_fail(client, log_in, user_sponsor, role): ) assert ( resp.content - == b'{"orgunit":["A sponsor can only make changes to roles at units they are sponsors for."]}' + == b'{"orgunit":["You can only edit roles connected to your units."]}' ) assert resp.status_code == status.HTTP_400_BAD_REQUEST @@ -99,24 +99,25 @@ def test_role_sponsor_patch_ok( @pytest.mark.django_db -def test_role_sponsor_patch_fail(client, log_in, user_sponsor, role): +def test_role_sponsor_patch_fail(client, log_in, user_sponsor_bar, role): """Should fail since we are not a sponsor at this unit.""" # Unit the sponsor is not connected to so that we fail - ou = OrganizationalUnit.objects.create(name_nb="foo", name_en="foo_en") + OrganizationalUnit.objects.create(name_nb="foo", name_en="foo_en") - log_in(user_sponsor) + log_in(user_sponsor_bar) + new_date = (timezone.now() + datetime.timedelta(days=10)).date() resp = client.patch( reverse("gregui-v1:role-detail", kwargs={"pk": role.id}), data={ - "orgunit": ou.id, + "end_date": new_date.strftime("%Y-%m-%d"), }, ) + assert resp.status_code == status.HTTP_400_BAD_REQUEST assert ( resp.content - == b'{"orgunit":["A sponsor can only make changes to roles at units they are sponsors for."]}' + == b'{"non_field_errors":["You can only edit roles connected to your units."]}' ) - assert resp.status_code == status.HTTP_400_BAD_REQUEST @pytest.mark.django_db diff --git a/gregui/tests/conftest.py b/gregui/tests/conftest.py index 8386752d53ad86292066851b8324c80d8b2e4a30..5fd153b0242d8c213d06b5c91e1a01596ab81c73 100644 --- a/gregui/tests/conftest.py +++ b/gregui/tests/conftest.py @@ -74,6 +74,12 @@ def unit_foo() -> OrganizationalUnit: return OrganizationalUnit.objects.get(id=ou.id) +@pytest.fixture +def unit_bar() -> OrganizationalUnit: + ou = OrganizationalUnit.objects.create(name_en="Bar EN", name_nb="Bar NB") + return OrganizationalUnit.objects.get(id=ou.id) + + @pytest.fixture def role_type_foo() -> RoleType: rt = RoleType.objects.create( @@ -118,9 +124,9 @@ def sponsor_foo( @pytest.fixture -def sponsor_bar(unit_foo: OrganizationalUnit, create_sponsor) -> Sponsor: +def sponsor_bar(unit_bar: OrganizationalUnit, create_sponsor) -> Sponsor: return create_sponsor( - feide_id="bar@example.com", first_name="Bar", last_name="Baz", unit=unit_foo + feide_id="bar@example.com", first_name="Bar", last_name="Baz", unit=unit_bar ) @@ -161,7 +167,7 @@ def user_sponsor(sponsor_foo: Sponsor, create_user) -> User: # Create a user and link him to a sponsor user = create_user( - username="test_sponsor", + username="test_sponsor_foo", email="test@example.org", first_name="Test", last_name="Sponsor", @@ -172,6 +178,23 @@ def user_sponsor(sponsor_foo: Sponsor, create_user) -> User: return user_model.objects.get(id=user.id) +@pytest.fixture +def user_sponsor_bar(sponsor_bar: Sponsor, create_user) -> User: + user_model = get_user_model() + + # Create a user and link him to a sponsor + user = create_user( + username="test_sponsor_bar", + email="test2@example.org", + first_name="Sponsor", + last_name="Bar", + ) + GregUserProfile.objects.create(user=user, sponsor=sponsor_bar) + + # This user is a sponsor for unit_foo + return user_model.objects.get(id=user.id) + + @pytest.fixture def user_person(person_foo: Sponsor, create_user) -> User: user_model = get_user_model()