From c96fb54b04b4c70ebf382626776ec5e9200334db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Tue, 9 Aug 2022 11:23:22 +0200
Subject: [PATCH 2/5] wcs: use only_for_user & without_user flags (#68037)

---
 combo/apps/wcs/forms.py  |  1 +
 combo/apps/wcs/models.py | 92 ++++++++++++++++++++++++++--------------
 tests/test_wcs.py        | 92 +++++++++++++++++++++++++++++++++++-----
 3 files changed, 144 insertions(+), 41 deletions(-)

diff --git a/combo/apps/wcs/forms.py b/combo/apps/wcs/forms.py
index 60e21de6..408d1904 100644
--- a/combo/apps/wcs/forms.py
+++ b/combo/apps/wcs/forms.py
@@ -86,6 +86,7 @@ class WcsCardInfoCellForm(forms.ModelForm):
             'carddef_reference',
             'related_card_path',
             'card_ids',
+            'only_for_user',
             'custom_schema',
         )
         widgets = {
diff --git a/combo/apps/wcs/models.py b/combo/apps/wcs/models.py
index 68448470..43f054df 100644
--- a/combo/apps/wcs/models.py
+++ b/combo/apps/wcs/models.py
@@ -1012,6 +1012,12 @@ class WcsCardInfosCell(CardMixin, CellBase):
 
         populate_cache()
 
+    def is_visible(self, request, **kwargs):
+        user = getattr(request, 'user', None)
+        if self.only_for_user and (not user or user.is_anonymous):
+            return False
+        return super().is_visible(request, **kwargs)
+
     def check_validity(self):
         if self.related_card_path:
             relations = [r[0] for r in self.get_related_card_paths()]
@@ -1033,24 +1039,21 @@ class WcsCardInfosCell(CardMixin, CellBase):
     def get_repeat_template(self, context):
         return len(context.get(self.global_context_key) or [])
 
-    def get_card_data_from_ids(self, card_id, context):
-        card_ids = context.get(self.global_context_key)
-        if not card_ids:
-            return None
-
+    def get_cards_from_ids(self, card_ids, context, synchronous=False):
+        # if check_user, get all cards from ids in context, with correct filter-user-uuid and without_user value
+        # use custom view if requested
         api_url = '/api/cards/%s/list?full=on' % (self.card_slug)
         if self.card_custom_view:
             api_url = '/api/cards/%s/list/%s?full=on' % (
                 self.card_slug,
                 self.card_custom_view,
             )
-            user = self.get_concerned_user(context)
-            if user and not user.is_anonymous:
-                user_name_id = user.get_name_id()
-                if user_name_id:
-                    api_url += '&filter-user-uuid=%s' % user_name_id
+        user = self.get_concerned_user(context)
+        if self.only_for_user and user and not user.is_anonymous and user.get_name_id():
+            api_url += '&filter-user-uuid=%s' % user.get_name_id()
         api_url += '&%s' % '&'.join(['filter-internal-id=%s' % cid for cid in card_ids])
-        synchronous = bool(context.get('synchronous'))
+        if not synchronous:
+            synchronous = bool(context.get('synchronous'))
         wcs_site = get_wcs_services().get(self.wcs_site)
         try:
             response = requests.get(
@@ -1068,16 +1071,31 @@ class WcsCardInfosCell(CardMixin, CellBase):
                 # if there's a custom view consider the error is a 404 because
                 # the card was not found in that view, and mark it so.
                 context['card_not_found'] = True
-            return None
+            return []
 
         if response.status_code == 200:
-            data = response.json()
-            for card_data in data.get('data') or []:
-                if str(card_data.get('id')) == str(card_id):
-                    return card_data
+            return response.json().get('data') or []
+        return []
+
+    def filter_card_ids(self, card_ids, context):
+        # check that all ids in context are available for user
+        cards = self.get_cards_from_ids(card_ids, context, synchronous=True)
+        return [c['id'] for c in cards if str(c['id']) in [str(i) for i in card_ids]]
+
+    def get_card_data_from_ids(self, card_id, context):
+        # get the correct card from all known cards for ids in context
+        card_ids = context.get(self.global_context_key)
+        if not card_ids:
+            return None
+        cards = self.get_cards_from_ids(card_ids, context)
+        for card_data in cards:
+            if str(card_data.get('id')) == str(card_id):
+                return card_data
         return None
 
-    def get_card_data(self, card_slug, card_custom_view, card_id, context, synchronous=False):
+    def get_card_data(
+        self, card_slug, card_custom_view, card_id, only_for_user, without_user, context, synchronous=False
+    ):
         api_url = '/api/cards/%s/%s/?include-files-content=off' % (card_slug, card_id)
         if card_custom_view:
             api_url = '/api/cards/%s/%s/%s/?include-files-content=off' % (
@@ -1085,11 +1103,9 @@ class WcsCardInfosCell(CardMixin, CellBase):
                 card_custom_view,
                 card_id,
             )
-            user = self.get_concerned_user(context)
-            if user and not user.is_anonymous:
-                user_name_id = user.get_name_id()
-                if user_name_id:
-                    api_url += '&filter-user-uuid=%s' % user_name_id
+        user = self.get_concerned_user(context)
+        if only_for_user and user and not user.is_anonymous and user.get_name_id():
+            api_url += '&filter-user-uuid=%s' % user.get_name_id()
         if not synchronous:
             synchronous = bool(context.get('synchronous'))
         wcs_site = get_wcs_services().get(self.wcs_site)
@@ -1097,18 +1113,14 @@ class WcsCardInfosCell(CardMixin, CellBase):
             response = requests.get(
                 api_url,
                 remote_service=wcs_site,
-                user=None if self.without_user else getattr(context.get('request'), 'user', None),
-                without_user=self.without_user,
+                user=None if without_user else getattr(context.get('request'), 'user', None),
+                without_user=without_user,
                 cache_duration=5,
                 raise_if_not_cached=not synchronous,
                 log_errors=False,
             )
             response.raise_for_status()
         except RequestException:
-            if self.card_custom_view:
-                # if there's a custom view consider the error is a 404 because
-                # the card was not found in that view, and mark it so.
-                context['card_not_found'] = True
             return {}
 
         if response.status_code == 200:
@@ -1264,7 +1276,13 @@ class WcsCardInfosCell(CardMixin, CellBase):
                 return []
             # and data
             next_card_data = self.get_card_data(
-                card_slug, None, card_data['fields']['%s_raw' % varname], context, synchronous=True
+                card_slug=card_slug,
+                card_custom_view=None,
+                card_id=card_data['fields']['%s_raw' % varname],
+                only_for_user=False,
+                without_user=False,
+                context=context,
+                synchronous=True,
             )
             if not next_card_data:
                 # card data not found
@@ -1294,7 +1312,13 @@ class WcsCardInfosCell(CardMixin, CellBase):
             # no card id found
             return []
         card_data = self.get_card_data(
-            first_cell.card_slug, first_cell.card_custom_view, card_id, context, synchronous=True
+            card_slug=first_cell.card_slug,
+            card_custom_view=first_cell.card_custom_view,
+            card_id=card_id,
+            only_for_user=first_cell.only_for_user,
+            without_user=first_cell.without_user,
+            context=context,
+            synchronous=True,
         )
         if not card_data:
             # card data not found
@@ -1323,7 +1347,13 @@ class WcsCardInfosCell(CardMixin, CellBase):
 
         if self.related_card_path:
             # look at other cells to get related ids
-            return self.get_card_ids_from_related(original_context, request)
+            card_ids = self.get_card_ids_from_related(original_context, request)
+            if card_ids == []:
+                return []
+            elif card_ids:
+                # check that ids from related are available for user
+                # (use only_for_user and without_user flags)
+                return self.filter_card_ids(card_ids, original_context)
 
         if self.card_ids:
             # card ids template is defined
diff --git a/tests/test_wcs.py b/tests/test_wcs.py
index dc1a1f3d..465e869b 100644
--- a/tests/test_wcs.py
+++ b/tests/test_wcs.py
@@ -3474,7 +3474,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get first cell data
             '/api/cards/card_a/1/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=1&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=1&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=1&'),  # get card
         ]
     )
     cell3.delete()  # reset
@@ -3490,7 +3491,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # follow cardc relation
             '/api/cards/card_c/6/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=7&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=7&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=7&'),  # get card
         ]
     )
 
@@ -3510,7 +3512,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
         # follow cardc relation
         '/api/cards/card_c/6/',
         # and follow cardb relation
-        ('/api/cards/card_b/list', '&filter-internal-id=7&'),
+        ('/api/cards/card_b/list', '&filter-internal-id=7&'),  # check user access
+        ('/api/cards/card_b/list', '&filter-internal-id=7&'),  # get card
     ]
     resp = app.get(page.get_online_url())
     assert len(resp.context['cells']) == 2
@@ -3546,7 +3549,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # follow cardc relation
             '/api/cards/card_c/6/',
             # and follow cardb relation
-            ('/api/cards/card_b/list/a-custom-view', '&filter-internal-id=7&'),
+            ('/api/cards/card_b/list/a-custom-view', '&filter-internal-id=7&'),  # check user access
+            ('/api/cards/card_b/list/a-custom-view', '&filter-internal-id=7&'),  # get card
         ]
     )
 
@@ -3559,7 +3563,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get first cell data
             '/api/cards/card_a/1/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=2&filter-internal-id=3&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=2&filter-internal-id=3&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=2&filter-internal-id=3&'),  # get card
         ]
     )
 
@@ -3574,7 +3579,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # follow cardc relation
             '/api/cards/card_c/6/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=8&filter-internal-id=9&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=8&filter-internal-id=9&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=8&filter-internal-id=9&'),  # get card
         ]
     )
 
@@ -3586,7 +3592,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get first cell data
             '/api/cards/card_a/1/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=4&filter-internal-id=5&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=4&filter-internal-id=5&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=4&filter-internal-id=5&'),  # get card
         ]
     )
 
@@ -3601,7 +3608,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # follow cardc relation
             '/api/cards/card_c/6/',
             # and follow cardb relation
-            ('/api/cards/card_b/list', '&filter-internal-id=10&filter-internal-id=11&'),
+            ('/api/cards/card_b/list', '&filter-internal-id=10&filter-internal-id=11&'),  # check user access
+            ('/api/cards/card_b/list', '&filter-internal-id=10&filter-internal-id=11&'),  # get card
         ]
     )
 
@@ -3763,6 +3771,12 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get list of card_a with cardb=1
             '/api/cards/card_a/list?orig=combo&filter-cardb=1',
             # and follow carda reverse relation
+            # check user access
+            (
+                '/api/cards/card_a/list',
+                '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
+            ),
+            # get card
             (
                 '/api/cards/card_a/list',
                 '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
@@ -3780,6 +3794,12 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get list of card_a with cardsb=1
             '/api/cards/card_a/list?orig=combo&filter-cardsb=1',
             # and follow carda reverse relation
+            # check user access
+            (
+                '/api/cards/card_a/list',
+                '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
+            ),
+            # get card
             (
                 '/api/cards/card_a/list',
                 '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
@@ -3797,6 +3817,12 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get list of card_a with cardsb=1
             '/api/cards/card_a/list?orig=combo&filter-blockb_cardb=1',
             # and follow carda reverse relation
+            # check user access
+            (
+                '/api/cards/card_a/list',
+                '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
+            ),
+            # get card
             (
                 '/api/cards/card_a/list',
                 '&filter-internal-id=1&filter-internal-id=2&filter-internal-id=3&filter-internal-id=4&',
@@ -3842,7 +3868,8 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get list of card_f with cardf=42
             '/api/cards/card_f/list?orig=combo&filter-cardh=42',
             # and follow cardf reverse relation
-            ('/api/cards/card_f/list', '&filter-internal-id=41&'),
+            ('/api/cards/card_f/list', '&filter-internal-id=41&'),  # check user access
+            ('/api/cards/card_f/list', '&filter-internal-id=41&'),  # get card
         ]
     )
 
@@ -3861,11 +3888,56 @@ def test_card_cell_render_identifier_from_related(mock_send, nocache, app):
             # get list of card_g with cardf=44
             '/api/cards/card_g/list?orig=combo&filter-cardh=44',
             # and follow cardf reverse relation
-            ('/api/cards/card_g/list', '&filter-internal-id=43&'),
+            ('/api/cards/card_g/list', '&filter-internal-id=43&'),  # check user access
+            ('/api/cards/card_g/list', '&filter-internal-id=43&'),  # get card
         ]
     )
 
 
+@pytest.mark.parametrize('carddef_reference', ['default:card_model_1', 'default:card_model_1:foo'])
+@mock.patch('requests.Session.send', side_effect=mocked_requests_send)
+def test_card_cell_only_for_user(mock_send, context, carddef_reference):
+    page = Page.objects.create(title='xxx', template_name='standard')
+    cell = WcsCardInfosCell(page=page, placeholder='content', order=0)
+    cell.carddef_reference = carddef_reference
+    cell.only_for_user = False
+    cell.save()
+
+    context['card_model_1_id'] = 11
+    request = RequestFactory().get('/')
+    cell.modify_global_context(context, request)
+    cell.repeat_index = 0
+
+    assert cell.is_visible(request=context['request']) is True
+    context['request'].user = MockUserWithNameId()
+    assert cell.is_visible(request=context['request']) is True
+
+    cell.only_for_user = True
+    cell.save()
+    context['request'].user = None
+    assert cell.is_visible(request=context['request']) is False
+    context['request'].user = MockUserWithNameId()
+    assert cell.is_visible(request=context['request']) is True
+
+    cache.clear()
+    context['synchronous'] = True  # to get fresh content
+    context['request'].user = None
+
+    mock_send.reset_mock()
+    cell.render(context)
+    assert 'filter-user-uuid' not in mock_send.call_args_list[0][0][0].url
+
+    context['request'].user = MockUser()
+    mock_send.reset_mock()
+    cell.render(context)
+    assert 'filter-user-uuid' not in mock_send.call_args_list[0][0][0].url
+
+    context['request'].user = MockUserWithNameId()
+    mock_send.reset_mock()
+    cell.render(context)
+    assert 'filter-user-uuid=xyz' in mock_send.call_args_list[0][0][0].url
+
+
 @pytest.mark.parametrize('carddef_reference', ['default:card_model_1', 'default:card_model_1:foo'])
 @mock.patch('requests.Session.send', side_effect=mocked_requests_send)
 def test_card_cell_render_user(mock_send, context, nocache, carddef_reference):
-- 
2.35.1

