add unique idx on user column in mobileapp authtoken table (#1482)
# Which issue(s) this PR fixes
Solves the (rare) issue where a user could potentially have > 1
mobileapp auth token, leading to 500 errors when trying to interact w/
the authtoken (ex. disconnect a mobile app from a user's profile):
```shell
2023-03-07 10:12:13 source=engine:app google_trace_id=e14bf933d634068a48caf093ce43c7f5/5550677047491218352 logger=django.request Internal Server Error: /api/internal/v1/users/U6WJ3BRLM1TR7/unlink_backend
Traceback (most recent call last):
File "/usr/local/lib/python3.9/site-packages/django/core/handlers/exception.py", line 47, in inner
response = get_response(request)
File "/usr/local/lib/python3.9/site-packages/django/core/handlers/base.py", line 181, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python3.9/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
return view_func(*args, **kwargs)
File "/usr/local/lib/python3.9/site-packages/rest_framework/viewsets.py", line 125, in view
return self.dispatch(request, *args, **kwargs)
File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 509, in dispatch
response = self.handle_exception(exc)
File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 469, in handle_exception
self.raise_uncaught_exception(exc)
File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
raise exc
File "/usr/local/lib/python3.9/site-packages/rest_framework/views.py", line 506, in dispatch
response = handler(request, *args, **kwargs)
File "/etc/app/apps/api/views/user.py", line 453, in unlink_backend
backend.unlink_user(user)
File "/etc/app/apps/mobile_app/backend.py", line 34, in unlink_user
token = MobileAppAuthToken.objects.get(user=user)
File "/usr/local/lib/python3.9/site-packages/django/db/models/manager.py", line 85, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/usr/local/lib/python3.9/site-packages/django/db/models/query.py", line 439, in get
raise self.model.MultipleObjectsReturned(
apps.mobile_app.models.MobileAppAuthToken.MultipleObjectsReturned: get() returned more than one MobileAppAuthToken -- it returned 2!
```
## Checklist
- [x] Tests updated
- [ ] Documentation added (N/A)
- [x] `CHANGELOG.md` updated
This commit is contained in:
parent
c4c5953f85
commit
0f23a449c7
5 changed files with 132 additions and 3 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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'),
|
||||
),
|
||||
]
|
||||
|
|
@ -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
|
||||
)
|
||||
|
|
|
|||
77
engine/apps/mobile_app/tests/test_mobile_app_auth_token.py
Normal file
77
engine/apps/mobile_app/tests/test_mobile_app_auth_token.py
Normal file
|
|
@ -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
|
||||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue