Rework how supersearch fields permissions are handled (#6764)

Previously, the code in the webapp that managed the permissions for
supersearch fields was kind of clunky. This fixes it so it's easier to
continue with the Elasticsearch migration and also improves code clarity
around how permissions are converted and checked.

socorro/external/es/crashstorage.py

@@ -255,6 +255,8 @@ def build_document(src, crash_document, fields, all_keys):
 class ESCrashStorage(CrashStorageBase):
     """Indexes documents based on the processed crash to Elasticsearch."""
 
+    SUPERSEARCH_FIELDS = FIELDS
+
     # These regex will catch field names from Elasticsearch exceptions. They
     # have been tested with Elasticsearch 1.4.
     field_name_string_error_re = re.compile(r"field=\"([\w\-.]+)\"")
@@ -420,7 +422,7 @@ def delete_expired_indices(self):
         return was_deleted
 
     def get_keys_for_indexable_fields(self):
-        """Return keys for FIELDS in "namespace.key" format
+        """Return keys for SUPERSEARCH_FIELDS in "namespace.key" format
 
         NOTE(willkg): Results are cached on this ESCrashStorage instance. If you change
         FIELDS (like in tests), create a new ESCrashStorage instance.
@@ -431,7 +433,7 @@ def get_keys_for_indexable_fields(self):
         keys = self._keys_for_indexable_fields_cache
         if keys is None:
             keys = set()
-            for field in FIELDS.values():
+            for field in self.SUPERSEARCH_FIELDS.values():
                 if not is_indexable(field):
                     continue
 
@@ -492,7 +494,9 @@ def save_processed_crash(self, raw_crash, processed_crash):
             "crash_id": crash_id,
             "processed_crash": {},
         }
-        build_document(src, crash_document, fields=FIELDS, all_keys=all_valid_keys)
+        build_document(
+            src, crash_document, fields=self.SUPERSEARCH_FIELDS, all_keys=all_valid_keys
+        )
 
         # Capture crash data size metrics
         self.capture_crash_metrics(crash_document)

socorro/external/legacy_es/crashstorage.py

@@ -262,6 +262,8 @@ def build_document(src, crash_document, fields, all_keys):
 class LegacyESCrashStorage(CrashStorageBase):
     """Indexes documents based on the processed crash to Elasticsearch."""
 
+    SUPERSEARCH_FIELDS = FIELDS
+
     # These regex will catch field names from Elasticsearch exceptions. They
     # have been tested with Elasticsearch 1.4.
     field_name_string_error_re = re.compile(r"field=\"([\w\-.]+)\"")
@@ -438,7 +440,7 @@ def get_keys_for_indexable_fields(self):
         keys = self._keys_for_indexable_fields_cache
         if keys is None:
             keys = set()
-            for field in FIELDS.values():
+            for field in self.SUPERSEARCH_FIELDS.values():
                 if not is_indexable(field):
                     continue
 
@@ -499,7 +501,9 @@ def save_processed_crash(self, raw_crash, processed_crash):
             "crash_id": crash_id,
             "processed_crash": {},
         }
-        build_document(src, crash_document, fields=FIELDS, all_keys=all_valid_keys)
+        build_document(
+            src, crash_document, fields=self.SUPERSEARCH_FIELDS, all_keys=all_valid_keys
+        )
 
         # Capture crash data size metrics
         self.capture_crash_metrics(crash_document)

webapp/crashstats/crashstats/management/commands/updatesignatures.py

@@ -14,7 +14,7 @@
 
 from crashstats.crashstats.models import Signature
 from crashstats.supersearch.models import SuperSearch
-from crashstats.supersearch.libsupersearch import SUPERSEARCH_FIELDS
+from crashstats.supersearch.libsupersearch import get_supersearch_fields
 from socorro.lib.libdatetime import string_to_datetime
 
 
@@ -87,7 +87,7 @@ def handle(self, **options):
 
         # Do a super search and get the signature, buildid, and date processed for
         # every crash in the range
-        all_fields = SUPERSEARCH_FIELDS
+        all_fields = get_supersearch_fields()
         api = SuperSearch()
         self.stdout.write("Looking at %s to %s" % (start_datetime, end_datetime))
 

webapp/crashstats/crashstats/tests/conftest.py

@@ -14,7 +14,7 @@
 from crashstats import libproduct
 from crashstats.crashstats.signals import PERMISSIONS
 from crashstats.crashstats.tests.testbase import DjangoTestCase
-from crashstats.supersearch.libsupersearch import SUPERSEARCH_FIELDS
+from crashstats.supersearch.libsupersearch import get_supersearch_fields
 
 
 class Response:
@@ -116,7 +116,7 @@ def setUp(self):
         super().setUp()
 
         def mocked_supersearchfields(**params):
-            results = copy.deepcopy(SUPERSEARCH_FIELDS)
+            results = copy.deepcopy(get_supersearch_fields())
             # to be realistic we want to introduce some dupes that have a
             # different key but its `in_database_name` is one that is already
             # in the hardcoded list (the baseline)

webapp/crashstats/supersearch/forms.py

@@ -60,9 +60,9 @@ def __init__(
                 del self.all_fields[field_name]
                 continue
 
-            if field_data["permissions_needed"]:
+            if field_data["webapp_permissions_needed"]:
                 user_has_permissions = True
-                for permission in field_data["permissions_needed"]:
+                for permission in field_data["webapp_permissions_needed"]:
                     if not user.has_perm(permission):
                         user_has_permissions = False
                         break

webapp/crashstats/supersearch/libsupersearch.py

@@ -9,7 +9,6 @@
 from elasticsearch_dsl_0_0_11 import Search
 
 from socorro import settings as socorro_settings
-from socorro.external.legacy_es.super_search_fields import FIELDS
 from socorro.libclass import build_instance_from_settings
 
 
@@ -25,7 +24,7 @@ def convert_permissions(fields):
 
     :arg fields: super search fields structure to convert permissions of
 
-    :returns: fields with permissions converted
+    :returns: fields with new "webapp_permissions_needed" value with webapp permissions
 
     """
 
@@ -39,12 +38,15 @@ def _convert(permissions):
         return [perm for perm in new_permissions if perm]
 
     for val in fields.values():
-        val["permissions_needed"] = _convert(val["permissions_needed"])
+        if "webapp_permissions_needed" not in val:
+            val["webapp_permissions_needed"] = _convert(val["permissions_needed"])
 
     return fields
 
 
-SUPERSEARCH_FIELDS = convert_permissions(FIELDS)
+def get_supersearch_fields():
+    es_crash_dest = build_instance_from_settings(socorro_settings.ES_STORAGE)
+    return convert_permissions(es_crash_dest.SUPERSEARCH_FIELDS)
 
 
 @dataclass

webapp/crashstats/supersearch/models.py

@@ -7,7 +7,7 @@
 from crashstats import libproduct
 from crashstats.crashstats import models
 from crashstats.supersearch.libsupersearch import (
-    SUPERSEARCH_FIELDS,
+    get_supersearch_fields,
     SuperSearchStatusModel,
 )
 from socorro import settings as socorro_settings
@@ -44,13 +44,13 @@
 def get_api_allowlist(include_all_fields=False):
     """Returns an API_ALLOWLIST value based on SUPERSEARCH_FIELDS"""
 
-    all_fields = SUPERSEARCH_FIELDS
+    all_fields = get_supersearch_fields()
     fields = []
     for meta in all_fields.values():
         if (
             meta["name"] not in fields
             and meta["is_returned"]
-            and (include_all_fields or not meta["permissions_needed"])
+            and (include_all_fields or not meta["webapp_permissions_needed"])
         ):
             fields.append(meta["name"])
 
@@ -95,7 +95,7 @@ class SuperSearch(ESSocorroMiddleware):
     API_ALLOWLIST = get_api_allowlist()
 
     def __init__(self):
-        self.all_fields = SUPERSEARCH_FIELDS
+        self.all_fields = get_supersearch_fields()
 
         # These fields contain lists of other fields. Later on, we want to
         # make sure that none of those listed fields are restricted.
@@ -110,7 +110,7 @@ def __init__(self):
             tuple(
                 (x["name"], list)
                 for x in self.all_fields.values()
-                if x["is_exposed"] and not x["permissions_needed"]
+                if x["is_exposed"] and not x["webapp_permissions_needed"]
             )
             + SUPERSEARCH_META_PARAMS
             + tuple(self.extended_fields)
@@ -124,7 +124,7 @@ def _get_extended_params(self):
         # Add histogram fields for all 'date','integer', or 'float' fields.
         extended_fields = []
         for field in self.all_fields.values():
-            if not field["is_exposed"] or field["permissions_needed"]:
+            if not field["is_exposed"] or field["webapp_permissions_needed"]:
                 continue
 
             extended_fields.append(("_aggs.%s" % field["name"], list))
@@ -158,7 +158,7 @@ def get(self, **kwargs):
             x
             for x in self.all_fields
             if self.all_fields[x]["is_returned"]
-            and not self.all_fields[x]["permissions_needed"]
+            and not self.all_fields[x]["webapp_permissions_needed"]
         }
 
         # Extend that list with the special fields, like `_histogram.*`.
@@ -172,7 +172,7 @@ def get(self, **kwargs):
             if (
                 field_name in self.all_fields
                 and self.all_fields[field_name]["is_returned"]
-                and not self.all_fields[field_name]["permissions_needed"]
+                and not self.all_fields[field_name]["webapp_permissions_needed"]
             ):
                 allowed_fields.add(histogram)
 
@@ -206,7 +206,7 @@ class SuperSearchUnredacted(SuperSearch):
     API_ALLOWLIST = get_api_allowlist(include_all_fields=True)
 
     def __init__(self):
-        self.all_fields = SUPERSEARCH_FIELDS
+        self.all_fields = get_supersearch_fields()
 
         histogram_fields = self._get_extended_params()
 
@@ -220,7 +220,7 @@ def __init__(self):
 
         permissions = {}
         for field_data in self.all_fields.values():
-            for perm in field_data["permissions_needed"]:
+            for perm in field_data["webapp_permissions_needed"]:
                 permissions[perm] = True
 
         self.API_REQUIRED_PERMISSIONS = tuple(permissions.keys())
@@ -243,7 +243,7 @@ def get(self, **kwargs):
 
 
 class SuperSearchFields(ESSocorroMiddleware):
-    _fields = SUPERSEARCH_FIELDS
+    _fields = get_supersearch_fields()
 
     IS_PUBLIC = True
 

webapp/crashstats/supersearch/tests/test_forms.py

@@ -3,15 +3,15 @@
 # file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 from crashstats.supersearch import forms
-from crashstats.supersearch.libsupersearch import SUPERSEARCH_FIELDS
+from crashstats.supersearch.libsupersearch import get_supersearch_fields
 
 
 class TestForms:
     def setup_method(self):
         self.products = ["WaterWolf", "NightTrain", "SeaMonkey", "Tinkerbell"]
         self.product_versions = ["20.0", "21.0a1", "20.0", "9.5"]
         self.platforms = ["Windows", "Mac OS X", "Linux"]
-        self.all_fields = SUPERSEARCH_FIELDS
+        self.all_fields = get_supersearch_fields()
 
     def test_search_form(self):
         def get_new_form(data):

webapp/crashstats/supersearch/tests/test_libsupersearch.py

@@ -15,22 +15,25 @@ def test_convert_permissions():
             "permissions_needed": ["public"],
         },
         "version": {
-            "permissions_needed": ["public", "protected"],
+            "permissions_needed": ["protected"],
         },
     }
 
     expected = {
         "build": {
             # No permission -> no required permissions
             "permissions_needed": [],
+            "webapp_permissions_needed": [],
         },
         "product": {
             # "public" -> no required permissions
-            "permissions_needed": [],
+            "permissions_needed": ["public"],
+            "webapp_permissions_needed": [],
         },
         "version": {
             # "protected" -> "crashstats.view_pii"
-            "permissions_needed": ["crashstats.view_pii"],
+            "permissions_needed": ["protected"],
+            "webapp_permissions_needed": ["crashstats.view_pii"],
         },
     }
 

webapp/crashstats/supersearch/views.py

@@ -54,7 +54,7 @@ def get_allowed_fields(user):
     return tuple(
         x["name"]
         for x in SuperSearchFields().get().values()
-        if x["is_exposed"] and user.has_perms(x["permissions_needed"])
+        if x["is_exposed"] and user.has_perms(x["webapp_permissions_needed"])
     )