diff --git a/CHANGELOG.md b/CHANGELOG.md index db38ae7b..f58ccc8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixes forwarding of Amazon SNS headers @mderynck ([#3371](https://github.com/grafana/oncall/pull/3371)) +- Fix issue when RBAC is enabled where Viewers with "Notifications Receiver" role do not properly show up in schedule + rotations by @joeyorlando ([#3378](https://github.com/grafana/oncall/pull/3378)) ## v1.3.59 (2023-11-16) diff --git a/engine/apps/schedules/ical_utils.py b/engine/apps/schedules/ical_utils.py index 8e9de956..fab25fff 100644 --- a/engine/apps/schedules/ical_utils.py +++ b/engine/apps/schedules/ical_utils.py @@ -79,7 +79,7 @@ def users_in_ical( organization : apps.user_management.models.organization.Organization The organization in question """ - required_permission = RBACPermission.Permissions.SCHEDULES_WRITE + required_permission = RBACPermission.Permissions.NOTIFICATIONS_READ emails_from_ical = [username.lower() for username in usernames_from_ical] diff --git a/engine/apps/schedules/tests/test_ical_utils.py b/engine/apps/schedules/tests/test_ical_utils.py index 430544a5..97e98b40 100644 --- a/engine/apps/schedules/tests/test_ical_utils.py +++ b/engine/apps/schedules/tests/test_ical_utils.py @@ -9,7 +9,7 @@ import pytz from django.core.cache import cache from django.utils import timezone -from apps.api.permissions import LegacyAccessControlRole +from apps.api.permissions import LegacyAccessControlRole, RBACPermission from apps.schedules.ical_utils import ( get_cached_oncall_users_for_multiple_schedules, get_icalendar_tz_or_utc, @@ -93,17 +93,59 @@ def test_users_in_ical_email_case_insensitive(make_organization_and_user, make_u @pytest.mark.django_db -def test_users_in_ical_viewers_inclusion(make_organization_and_user, make_user_for_organization): +@pytest.mark.parametrize( + "role,included", + [ + (LegacyAccessControlRole.ADMIN, True), + (LegacyAccessControlRole.EDITOR, True), + (LegacyAccessControlRole.VIEWER, False), + (LegacyAccessControlRole.NONE, False), + ], +) +def test_users_in_ical_basic_role(make_organization_and_user, make_user_for_organization, role, included): organization, user = make_organization_and_user() - viewer = make_user_for_organization(organization, role=LegacyAccessControlRole.VIEWER) + organization.is_rbac_permissions_enabled = False + organization.save() + + other_user = make_user_for_organization(organization, role=role) + + usernames = [user.username, other_user.username] + expected_result = {user} + if included: + expected_result.add(other_user) - usernames = [user.username, viewer.username] result = users_in_ical(usernames, organization) - assert set(result) == {user} + assert set(result) == expected_result @pytest.mark.django_db -def test_list_users_to_notify_from_ical_viewers_inclusion( +@pytest.mark.parametrize( + "permission,included", + [ + (RBACPermission.Permissions.NOTIFICATIONS_READ, True), + (RBACPermission.Permissions.SCHEDULES_READ, False), + (None, False), + ], +) +def test_users_in_ical_rbac(make_organization_and_user, make_user_for_organization, permission, included): + organization, _ = make_organization_and_user() + organization.is_rbac_permissions_enabled = True + organization.save() + + viewer = make_user_for_organization(organization, role=LegacyAccessControlRole.VIEWER) + usernames = [viewer.username] + + # viewer doesn't yet have the required permission, they shouldn't be included + assert len(users_in_ical(usernames, organization)) == 0 + + viewer.permissions = [{"action": permission.value}] if permission else [] + viewer.save() + + assert users_in_ical(usernames, organization) == ([viewer] if included else []) + + +@pytest.mark.django_db +def test_list_users_to_notify_from_ical_viewers_exclusion( make_organization_and_user, make_user_for_organization, make_schedule, make_on_call_shift ): organization, user = make_organization_and_user()