diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b1fe8a6..2aedaafe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix escalation policy importance going back to default by @vadimkerr ([#3282](https://github.com/grafana/oncall/pull/3282)) +- Improve user permissions query ([#3291](https://github.com/grafana/oncall/pull/3291)) ## v1.3.54 (2023-11-06) diff --git a/engine/apps/slack/tests/test_user_group.py b/engine/apps/slack/tests/test_user_group.py index 2f4d354d..0af070f2 100644 --- a/engine/apps/slack/tests/test_user_group.py +++ b/engine/apps/slack/tests/test_user_group.py @@ -154,3 +154,25 @@ def test_populate_slack_usergroups_for_team( assert usergroup.handle == "test_handle" assert usergroup.members == ["test_user_1", "test_user_2"] assert usergroup.is_active + + +@pytest.mark.django_db +def test_get_users_from_members_for_organization( + make_organization_with_slack_team_identity, + make_slack_user_group, + make_user_with_slack_user_identity, +): + organization, slack_team_identity = make_organization_with_slack_team_identity() + + user_1, slack_user_identity_1 = make_user_with_slack_user_identity( + slack_team_identity, organization, slack_id="slack_id_1" + ) + user_2, slack_user_identity_2 = make_user_with_slack_user_identity( + slack_team_identity, organization, slack_id="slack_id_2" + ) + user_group = make_slack_user_group(slack_team_identity) + user_group.members = ["slack_id_1", "slack_id_2"] + user_group.save(update_fields=["members"]) + + users = user_group.get_users_from_members_for_organization(organization) + assert set(users) == {user_1, user_2} diff --git a/engine/apps/user_management/models/user.py b/engine/apps/user_management/models/user.py index 92829216..d6dbbd71 100644 --- a/engine/apps/user_management/models/user.py +++ b/engine/apps/user_management/models/user.py @@ -1,6 +1,7 @@ import datetime import json import logging +import re import typing from urllib.parse import urljoin @@ -34,6 +35,10 @@ if typing.TYPE_CHECKING: logger = logging.getLogger(__name__) +class PermissionsQuery(typing.TypedDict): + permissions__contains: typing.Dict + + class PermissionsRegexQuery(typing.TypedDict): permissions__regex: str @@ -387,7 +392,7 @@ class User(models.Model): @staticmethod def build_permissions_query( permission: LegacyAccessControlCompatiblePermission, organization - ) -> typing.Union[PermissionsRegexQuery, RoleInQuery]: + ) -> typing.Union[PermissionsQuery, PermissionsRegexQuery, RoleInQuery]: """ This method returns a django query filter that is compatible with RBAC as well as legacy "basic" role based authorization. If a permission is provided we simply do @@ -399,7 +404,11 @@ class User(models.Model): """ if organization.is_rbac_permissions_enabled: # https://stackoverflow.com/a/50251879 - return PermissionsRegexQuery(permissions__regex=r".*{0}.*".format(permission.value)) + if settings.DATABASE_TYPE == settings.DATABASE_TYPES.SQLITE3: + # https://docs.djangoproject.com/en/4.2/topics/db/queries/#contains + return PermissionsRegexQuery(permissions__regex=re.escape(permission.value)) + required_permission = {"action": permission.value} + return PermissionsQuery(permissions__contains=[required_permission]) return RoleInQuery(role__lte=permission.fallback_role.value) def get_or_create_notification_policies(self, important=False): diff --git a/engine/settings/base.py b/engine/settings/base.py index cd58cb26..bbc14bb6 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -120,6 +120,7 @@ DATABASE_DEFAULTS = { }, } +DATABASE_TYPES = DatabaseTypes DATABASE_NAME = os.getenv("DATABASE_NAME") or os.getenv("MYSQL_DB_NAME") DATABASE_USER = os.getenv("DATABASE_USER") or os.getenv("MYSQL_USER") DATABASE_PASSWORD = os.getenv("DATABASE_PASSWORD") or os.getenv("MYSQL_PASSWORD")