fix missing users in rotations when RBAC is enabled (#3380)

# Which issue(s) this PR fixes
1. Enable RBAC
2. Create a schedule rotation layer which includes a user whom is Viewer
+ has role `Notifications Receiver` (this is the RBAC role we use to
filter which users show up in the user dropdown in the rotations modal
when creating a rotation)
3. The user _sorta_ shows up in the schedule but they are listed in
`missing_users`

<img width="1166" alt="Screenshot 2023-11-17 at 10 12 30"
src="https://github.com/grafana/oncall/assets/9406895/ae4d6449-3aff-4087-9b05-64645e84b40a">
<img width="1173" alt="Screenshot 2023-11-17 at 10 15 04"
src="https://github.com/grafana/oncall/assets/9406895/3ac4f0b9-49b3-4a7d-bfcf-39a8c51bbb74">


## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [ ] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not
required)
This commit is contained in:
Joey Orlando 2023-11-20 06:31:07 -05:00 committed by GitHub
parent 53ca5331c6
commit 6214ffbd66
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 7 deletions

View file

@ -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)

View file

@ -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]

View file

@ -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()