diff --git a/CHANGELOG.md b/CHANGELOG.md index 205455c6..e974fd9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Prohibit creating & updating past overrides ([1474](https://github.com/grafana/oncall/pull/1474)) +- Add unique index on `user_id` column in `mobile_app_mobileappauthtoken` table to avoid allowing a user + to have more than one mobile app auth token at a time ([1482](https://github.com/grafana/oncall/pull/1482)) ## v1.1.33 (2023-03-07) diff --git a/engine/apps/mobile_app/migrations/0002_alter_mobileappauthtoken_user.py b/engine/apps/mobile_app/migrations/0002_alter_mobileappauthtoken_user.py new file mode 100644 index 00000000..4efa17eb --- /dev/null +++ b/engine/apps/mobile_app/migrations/0002_alter_mobileappauthtoken_user.py @@ -0,0 +1,29 @@ +from django.db import migrations, models +import django.db.models.deletion + + +def delete_user_duplicate_mobileappauthtokens(apps, _): + MobileAppAuthToken = apps.get_model('mobile_app', 'MobileAppAuthToken') + + # start w/ the oldest mobile app auth tokens (ORDER BY id ASC) + # and if we find any newer tokens, delete the earlier ones (ie. `row` variable) + for row in MobileAppAuthToken.objects.all().order_by('id'): + if MobileAppAuthToken.objects.filter(user_id=row.user_id).count() > 1: + row.delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('user_management', '0008_organization_is_grafana_incident_enabled'), + ('mobile_app', '0001_initial'), + ] + + operations = [ + migrations.RunPython(delete_user_duplicate_mobileappauthtokens, migrations.RunPython.noop), + migrations.AlterField( + model_name='mobileappauthtoken', + name='user', + field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='user_management.user'), + ), + ] diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index 7ad107dd..08d2c1f2 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -51,9 +51,7 @@ class MobileAppVerificationToken(BaseAuthToken): class MobileAppAuthToken(BaseAuthToken): - user = models.ForeignKey( - to=User, null=False, blank=False, related_name="mobile_app_auth_tokens", on_delete=models.CASCADE - ) + user = models.OneToOneField(to=User, null=False, blank=False, on_delete=models.CASCADE) organization = models.ForeignKey( to=Organization, null=False, blank=False, related_name="mobile_app_auth_tokens", on_delete=models.CASCADE ) diff --git a/engine/apps/mobile_app/tests/test_mobile_app_auth_token.py b/engine/apps/mobile_app/tests/test_mobile_app_auth_token.py new file mode 100644 index 00000000..af92a8bd --- /dev/null +++ b/engine/apps/mobile_app/tests/test_mobile_app_auth_token.py @@ -0,0 +1,77 @@ +import pytest +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from apps.mobile_app.models import MobileAppAuthToken + + +@pytest.mark.django_db +def test_mobile_app_auth_token( + make_organization_and_user_with_mobile_app_verification_token, +): + organization, user, verification_token = make_organization_and_user_with_mobile_app_verification_token() + + client = APIClient() + url = reverse("mobile_app:auth_token") + + response = client.post(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + response = client.post(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_201_CREATED + + original_auth_token_id = response.data["id"] + original_auth_token = response.data["token"] + original_auth_token_created_at = response.data["created_at"] + + assert original_auth_token_id is not None + assert original_auth_token is not None + assert original_auth_token_created_at is not None + + # we can fetch the token + response = client.get(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + response = client.get(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_200_OK + + assert response.data["token_id"] == original_auth_token_id + assert response.data["user_id"] == user.id + assert response.data["organization_id"] == organization.id + assert response.data["created_at"] == original_auth_token_created_at + assert response.data["revoked_at"] is None + + # can only ever have one mobile app auth token.. old one gets deleted if we try + # creating a new one + response = client.post(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_201_CREATED + + new_auth_token_id = response.data["id"] + new_auth_token = response.data["token"] + new_auth_token_created_at = response.data["created_at"] + + assert new_auth_token_id is not None + assert new_auth_token is not None + assert new_auth_token_created_at is not None + + assert new_auth_token_id != original_auth_token_id + assert new_auth_token != original_auth_token + assert new_auth_token_created_at != original_auth_token_created_at + + assert MobileAppAuthToken.objects.filter(user=user).count() == 1 + + # we can delete the token + response = client.delete(url) + assert response.status_code == status.HTTP_403_FORBIDDEN + + response = client.delete(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_204_NO_CONTENT + + assert MobileAppAuthToken.objects.filter(user=user).count() == 0 + + response = client.delete(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_404_NOT_FOUND + + response = client.get(url, HTTP_AUTHORIZATION=verification_token) + assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/engine/conftest.py b/engine/conftest.py index 7c58208a..5d15c8c2 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -55,6 +55,7 @@ from apps.base.tests.factories import ( ) from apps.email.tests.factories import EmailMessageFactory from apps.heartbeat.tests.factories import IntegrationHeartBeatFactory +from apps.mobile_app.models import MobileAppVerificationToken from apps.schedules.tests.factories import ( CustomOnCallShiftFactory, OnCallScheduleCalendarFactory, @@ -175,6 +176,14 @@ def make_token_for_organization(): return _make_token_for_organization +@pytest.fixture +def make_mobile_app_verification_token_for_user(): + def _make_mobile_app_verification_token_for_user(user, organization): + return MobileAppVerificationToken.create_auth_token(user, organization) + + return _make_mobile_app_verification_token_for_user + + @pytest.fixture def make_public_api_token(): def _make_public_api_token(user, organization, name="test_api_token"): @@ -643,6 +652,20 @@ def make_organization_and_user_with_plugin_token(make_organization_and_user, mak return _make_organization_and_user_with_plugin_token +@pytest.fixture() +def make_organization_and_user_with_mobile_app_verification_token( + make_organization_and_user, make_mobile_app_verification_token_for_user +): + def _make_organization_and_user_with_mobile_app_verification_token( + role: typing.Optional[LegacyAccessControlRole] = None, + ): + organization, user = make_organization_and_user(role) + _, token = make_mobile_app_verification_token_for_user(user, organization) + return organization, user, token + + return _make_organization_and_user_with_mobile_app_verification_token + + @pytest.fixture() def mock_send_user_notification_signal(monkeypatch): def mocked_send_signal(*args, **kwargs):