From 1d9c619f0fa57e6f141577651eac106dd79404e0 Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Thu, 30 Jul 2020 17:31:00 +0200
Subject: [PATCH] api: handle multiple objects returned in get-or-create mixin
 (#44301)

---
 src/authentic2/api_exceptions.py | 26 ++++++++++++++++++++++++
 src/authentic2/api_mixins.py     |  7 ++++++-
 tests/test_api.py                | 35 ++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 1 deletion(-)
 create mode 100644 src/authentic2/api_exceptions.py

diff --git a/src/authentic2/api_exceptions.py b/src/authentic2/api_exceptions.py
new file mode 100644
index 000000000..4e0cfc0e1
--- /dev/null
+++ b/src/authentic2/api_exceptions.py
@@ -0,0 +1,26 @@
+# authentic2 - versatile identity manager
+# Copyright (C) 2010-2020 Entr'ouvert
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License as published
+# by the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from django.utils.translation import ugettext_lazy as _
+
+from rest_framework.exceptions import APIException
+from rest_framework import status
+
+
+class Conflict(APIException):
+    status_code = status.HTTP_409_CONFLICT
+    default_detail = _('Cannot process request because of conflicting resources.')
+    default_code = 'conflict'
diff --git a/src/authentic2/api_mixins.py b/src/authentic2/api_mixins.py
index 886896ad9..d0f25b5df 100644
--- a/src/authentic2/api_mixins.py
+++ b/src/authentic2/api_mixins.py
@@ -1,5 +1,5 @@
 # authentic2 - versatile identity manager
-# Copyright (C) 2010-2018 Entr'ouvert
+# Copyright (C) 2010-2020 Entr'ouvert
 #
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU Affero General Public License as published
@@ -20,6 +20,8 @@ from rest_framework.settings import api_settings
 from rest_framework.exceptions import ValidationError
 from rest_framework.utils import model_meta
 
+from .api_exceptions import Conflict
+
 
 class GetOrCreateMixinView(object):
     _lookup_object = None
@@ -49,6 +51,9 @@ class GetOrCreateMixinView(object):
             return ModelClass.objects.get(**kwargs)
         except ModelClass.DoesNotExist:
             return None
+        except ModelClass.MultipleObjectsReturned:
+            raise Conflict('retrieved several instances of model %s for key attributes %s' % (
+                    ModelClass.__name__, kwargs))
 
     def _validate_get_keys(self, keys):
         ModelClass = self._get_model_class()
diff --git a/tests/test_api.py b/tests/test_api.py
index db7736f04..a0fc36cc3 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1420,6 +1420,41 @@ def test_api_users_get_or_create_email_is_unique(settings, app, admin):
     assert User.objects.get(id=id).last_name == 'Doe'
 
 
+def test_api_users_get_or_create_email_not_unique(settings, app, admin):
+    settings.A2_EMAIL_IS_UNIQUE = False
+    User = get_user_model()
+    OU = get_ou_model()
+    ou1 = OU.objects.create(name='OU1', slug='ou1', email_is_unique=True)
+    ou2 = OU.objects.create(name='OU2', slug='ou2', email_is_unique=False)
+
+    app.authorization = ('Basic', (admin.username, admin.username))
+    payload = {
+        'email': 'john.doe@example.net',
+        'first_name': 'John',
+        'last_name': 'Doe',
+        'ou': 'ou1'
+    }
+    # 1. create
+    resp = app.post_json('/api/users/?get_or_create=email', params=payload, status=201)
+    id_user = resp.json['id']
+    assert User.objects.get(id=id_user).first_name == 'John'
+    assert User.objects.get(id=id_user).last_name == 'Doe'
+    assert User.objects.get(id=id_user).ou == ou1
+    # 2. get
+    resp = app.post_json('/api/users/?get_or_create=email', params=payload, status=200)
+    # 3. explicitly create in a different OU
+    payload['ou'] = 'ou2'
+    resp = app.post_json('/api/users/', params=payload, status=201)
+    id_user2 = resp.json['id']
+    assert id_user2 != id_user
+    assert User.objects.get(id=id_user2).first_name == 'John'
+    assert User.objects.get(id=id_user2).last_name == 'Doe'
+    assert User.objects.get(id=id_user2).ou == ou2
+    # 4. fail to retrieve a single instance for an ambiguous get-or-create key
+    resp = app.post_json('/api/users/?get_or_create=email', params=payload, status=409)
+    assert resp.json['errors'] == "retrieved several instances of model User for key attributes {'email': 'john.doe@example.net'}"
+
+
 def test_api_users_get_or_create_multi_key(settings, app, admin):
     app.authorization = ('Basic', (admin.username, admin.username))
     # test missing first_name
-- 
2.28.0

