Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce limit on number of devices per user #35515

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 102 additions & 2 deletions corehq/apps/ota/tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import uuid
from datetime import datetime
from unittest.mock import patch

from django.test import TestCase
from django.test.utils import override_settings

from casexml.apps.phone.models import OTARestoreCommCareUser, OTARestoreWebUser
from freezegun import freeze_time

from casexml.apps.phone.models import OTARestoreCommCareUser, OTARestoreWebUser, SyncLogSQL
from corehq.apps.domain.models import Domain
from corehq.apps.locations.tests.util import LocationHierarchyTestCase
from corehq.apps.ota.utils import get_restore_user, is_permitted_to_restore
from corehq.apps.ota.utils import get_restore_user, is_permitted_to_restore, can_login_on_device
from corehq.apps.users.dbaccessors import delete_all_users
from corehq.apps.users.models import CommCareUser, WebUser
from corehq.apps.users.util import format_username
Expand Down Expand Up @@ -425,3 +430,98 @@ def test_get_restore_user_as_user_for_commcare_user(self):
)
self.assertEqual(user.user_id, self.other_commcare_user._id)
self.assertEqual(user.request_user_id, self.commcare_user.user_id)


@freeze_time("2024-12-10 12:00:00")
class CanLoginOnDeviceTest(TestCase):

@classmethod
def setUpClass(cls):
super().setUpClass()
cls.domain = 'devices-per-user-test'
cls.within_past_day = datetime(2024, 12, 10, 0, 0)
cls.over_one_day_ago = datetime(2024, 12, 9, 0, 0)

@override_settings(DEVICES_PER_USER=1)
def test_allowed_if_device_count_equals_limit_but_existing_device_id(self):
self._create_synclog(self.domain, 'abc123', 'device-id', date=self.within_past_day)

self.assertTrue(can_login_on_device('abc123', 'device-id'))

@override_settings(DEVICES_PER_USER=1)
def test_not_allowed_if_device_count_equals_limit_and_new_device_id(self):
self._create_synclog(self.domain, 'abc123', 'device-id', date=self.within_past_day)

self.assertFalse(can_login_on_device('abc123', 'new-device-id'))

@override_settings(DEVICES_PER_USER=2)
def test_allowed_if_device_count_is_under_limit(self):
self._create_synclog(self.domain, 'abc123', 'device-id', date=self.within_past_day)

self.assertTrue(can_login_on_device('abc123', 'new-device-id'))

@override_settings(DEVICES_PER_USER=1)
def test_web_apps_logins_are_always_allowed(self):
for i in range(3):
self._create_synclog(self.domain, 'abc123', f'WebAppsLogin*{i}', date=self.within_past_day)

self.assertTrue(can_login_on_device('abc123', 'WebAppsLogin*newlogin'))

@override_settings(DEVICES_PER_USER=2)
def test_web_apps_logins_do_not_count_towards_device_count(self):
for i in range(3):
self._create_synclog(self.domain, 'abc123', f'WebAppsLogin*{i}', date=self.within_past_day)

self._create_synclog(self.domain, 'abc123', 'device-id', date=self.within_past_day)

self.assertTrue(can_login_on_device('abc123', 'new-device-id'))

@override_settings(DEVICES_PER_USER=1)
def test_activity_older_than_a_day_is_ignored(self):
for i in range(2):
self._create_synclog(self.domain, 'abc123', f'device-{i}', date=self.over_one_day_ago)

self.assertTrue(can_login_on_device('abc123', 'new-device-id'))

@override_settings(DEVICES_PER_USER=2)
def test_either_date_or_last_submitted_counts_towards_device_count(self):
self._create_synclog(
self.domain,
'abc123',
'device-1',
date=self.over_one_day_ago,
last_submitted=self.within_past_day,
)

self._create_synclog(
self.domain,
'abc123',
'device-2',
date=self.within_past_day,
last_submitted=self.over_one_day_ago,
)

self.assertFalse(can_login_on_device('abc123', 'new-device-id'))

def test_allowed_if_empty_queryset(self):
self.assertTrue(can_login_on_device('abc123', 'device-0'))

@override_settings(DEVICES_PER_USER=1)
def test_allowed_for_different_user(self):
self._create_synclog(self.domain, 'abc123', 'device-id', date=self.within_past_day)

self.assertTrue(can_login_on_device('def456', 'device-id'))

@override_settings(DEVICES_PER_USER=1)
def test_device_id_is_none_is_allowed(self):
self.assertTrue(can_login_on_device('abc123', None))

def _create_synclog(self, domain, user_id, device_id, **kwargs):
SyncLogSQL.objects.create(
domain=domain,
doc={},
synclog_id=uuid.uuid4(),
user_id=user_id,
device_id=device_id,
**kwargs
)
28 changes: 28 additions & 0 deletions corehq/apps/ota/utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
from functools import wraps
from datetime import datetime, timedelta, timezone

from django.conf import settings
from django.contrib.postgres.aggregates import ArrayAgg
from django.db.models import Q
from django.utils.translation import gettext as _

from couchdbkit import ResourceConflict

from casexml.apps.case.xml import V2
from casexml.apps.phone.models import SyncLogSQL
from casexml.apps.phone.restore import RestoreConfig, RestoreParams
from dimagi.utils.logging import notify_exception
from dimagi.utils.web import json_response
Expand Down Expand Up @@ -219,3 +224,26 @@ def _inner(request, domain, *args, **kwargs):

return response
return _inner


def can_login_on_device(user_id, device_id):
if not device_id or device_id.startswith("WebAppsLogin"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can device ids be set by the submitting device? In other words, would it be possible for someone to make their mobile device(s) ids start with WebAppsLogin or be empty and exempt themselves from the limit? Do we care?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good question that I do not know the answer to off the top of my head. I can dig more into this one and get back to you.

Copy link
Contributor

@snopoke snopoke Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official CC Mobile app doesn't allow changing the device ID. Forms submitted by other means can set it to whatever they want. This makes me wonder if we need to account for API submitted forms (ignoring them)?

return True

end_time = datetime.now(timezone.utc)
start_time = end_time - timedelta(days=1)
date_query = Q(date__gte=start_time, date__lt=end_time)
last_submitted_query = Q(last_submitted__gte=start_time, last_submitted__lt=end_time)

result = (
SyncLogSQL.objects.filter(date_query | last_submitted_query, user_id=user_id)
.exclude(device_id__startswith="WebAppsLogin")
.values('user_id')
.annotate(device_ids=ArrayAgg("device_id", distinct=True))
)
device_ids = result[0]['device_ids'] if result else []

if len(device_ids) < settings.DEVICES_PER_USER or device_id in device_ids:
return True

return False
7 changes: 7 additions & 0 deletions corehq/apps/ota/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
from .models import DeviceLogRequest, MobileRecoveryMeasure, SerialIdBucket
from .rate_limiter import rate_limit_restore
from .utils import (
can_login_on_device,
demo_user_restore_response,
get_restore_user,
handle_401_response,
Expand Down Expand Up @@ -295,6 +296,12 @@ def get_restore_response(domain, couch_user, app_id=None, since=None, version='1
:return: Tuple of (http response, timing context or None)
"""

if not can_login_on_device(couch_user._id, device_id):
return HttpResponse(
_("Your user has exceeded the daily device limit of {limit}.").format(limit=settings.DEVICES_PER_USER),
status=403,
), None

if user_id and user_id != couch_user.user_id:
# sync with a user that has been deleted but a new
# user was created with the same username and password
Expand Down
2 changes: 2 additions & 0 deletions settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,8 @@ def _pkce_required(client_id):
MAX_MOBILE_UCR_LIMIT = 300 # used in corehq.apps.cloudcare.util.should_restrict_web_apps_usage
MAX_MOBILE_UCR_SIZE = 100000 # max number of rows allowed when syncing a mobile UCR

DEVICES_PER_USER = 100 # number of mobile devices a user can actively use within a specific time window

# used by periodic tasks that delete soft deleted data older than PERMANENT_DELETION_WINDOW days
PERMANENT_DELETION_WINDOW = 30 # days

Expand Down
Loading