From ce942d6cc9d7c3c25bd3131b91933347c8521c49 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Stefan=20K=C3=B6gl?= <stefan@skoegl.net>
Date: Mon, 5 Sep 2016 14:24:03 +0200
Subject: [PATCH] Catch invalid device UID in get_device()

---
 mygpo/api/backend.py | 27 +++++++++++++++++++--------
 mygpo/api/tests.py   | 33 ++++++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/mygpo/api/backend.py b/mygpo/api/backend.py
index d683b294..c1d8f15a 100644
--- a/mygpo/api/backend.py
+++ b/mygpo/api/backend.py
@@ -17,9 +17,14 @@
 
 import uuid
 
+from django.db import transaction, IntegrityError
+
 from mygpo.users.settings import STORE_UA
 from mygpo.users.models import Client
 
+import logging
+logger = logging.getLogger(__name__)
+
 
 def get_device(user, uid, user_agent, undelete=True):
     """
@@ -30,21 +35,27 @@ def get_device(user, uid, user_agent, undelete=True):
 
     store_ua = user.profile.settings.get_wksetting(STORE_UA)
 
-    save = False
+    # list of fields to update -- empty list = no update
+    update_fields = []
+
+    with transaction.atomic():
+        try:
+            client = Client(id=uuid.uuid1(), user=user, uid=uid)
+            client.full_clean()
+            client.save()
 
-    client, created = Client.objects.get_or_create(user=user, uid=uid,
-                        defaults = {
-                            'id': uuid.uuid1()
-                        })
+        except IntegrityError:
+            client = Client.objects.get(user=user, uid=uid)
 
     if client.deleted and undelete:
         client.deleted = False
-        save = True
+        update_fields.append('deleted')
 
     if store_ua and user_agent and client.user_agent != user_agent:
         client.user_agent = user_agent
+        update_fields.append('user_agent')
 
-    if save:
-        client.save()
+    if update_fields:
+        client.save(update_fields=update_fields)
 
     return client
diff --git a/mygpo/api/tests.py b/mygpo/api/tests.py
index 98fdf512..ee17a29f 100644
--- a/mygpo/api/tests.py
+++ b/mygpo/api/tests.py
@@ -17,10 +17,9 @@
 
 
 import json
+import copy
 import unittest
-import doctest
 from urllib.parse import urlencode
-from copy import deepcopy
 
 from django.test.client import Client
 from django.test import TestCase
@@ -71,23 +70,39 @@ class AdvancedAPITests(unittest.TestCase):
         self.user.delete()
 
     def test_episode_actions(self):
+        response = self._upload_episode_actions(self.user, self.action_data,
+                                                self.extra)
+        self.assertEqual(response.status_code, 200, response.content)
+
         url = reverse(episodes, kwargs={
             'version': '2',
             'username': self.user.username,
         })
-
-        # upload actions
-        response = self.client.post(url, json.dumps(self.action_data),
-                                    content_type="application/json",
-                                    **self.extra)
-        self.assertEqual(response.status_code, 200, response.content)
-
         response = self.client.get(url, {'since': '0'}, **self.extra)
         self.assertEqual(response.status_code, 200, response.content)
         response_obj = json.loads(response.content.decode('utf-8'))
         actions = response_obj['actions']
         self.assertTrue(self.compare_action_list(self.action_data, actions))
 
+    def test_invalid_client_id(self):
+        """ Invalid Client ID should return 400 """
+        action_data = copy.deepcopy(self.action_data)
+        action_data[0]['device'] = "gpodder@abcdef123"
+
+        response = self._upload_episode_actions(self.user, action_data,
+                                                self.extra)
+
+        self.assertEqual(response.status_code, 400, response.content)
+
+    def _upload_episode_actions(self, user, action_data, extra):
+        url = reverse(episodes, kwargs={
+            'version': '2',
+            'username': self.user.username,
+        })
+        return self.client.post(url, json.dumps(action_data),
+                                content_type="application/json",
+                                **extra)
+
     def compare_action_list(self, as1, as2):
         for a1 in as1:
             found = False
-- 
2.11.4.GIT