Improve user permissions query (#3291)
The query for checking a user permission (used to get users from a Slack usergroup, for example) is timing out (and generating retries, besides affecting some use cases: [logs](https://ops.grafana-ops.net/explore?panes=%7B%22FCQ%22:%7B%22datasource%22:%22c-R8UWvVk%22,%22queries%22:%5B%7B%22refId%22:%22A%22,%22expr%22:%22%7Bnamespace%3D%5C%22amixr-prod%5C%22,%20cluster%3D%5C%22prod-us-central-0%5C%22%7D%20%7C%3D%20%5C%22Timeout%20exceeded%20in%20regular%20expression%20match.%5C%22%22,%22queryType%22:%22range%22,%22datasource%22:%7B%22type%22:%22loki%22,%22uid%22:%22c-R8UWvVk%22%7D,%22editorMode%22:%22code%22%7D%5D,%22range%22:%7B%22from%22:%22now-6h%22,%22to%22:%22now%22%7D%7D%7D&schemaVersion=1&orgId=1)): `django.db.utils.OperationalError: (3699, 'Timeout exceeded in regular expression match.')` Change to a `contains` query except for SQLite (not supported), where a simplified version of the original regex query is used.
This commit is contained in:
parent
2e4713c803
commit
ac01dd173d
4 changed files with 35 additions and 2 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue