API authentication and security changes

.github/workflows/build-dev.yaml

@@ -85,12 +85,11 @@ jobs:
       with:
         context: .
         tags: ${{ steps.vars.outputs.BE_NAMESPACE }}/fragalysis-backend:${{ env.GITHUB_REF_SLUG }}
-    - name: Test
-      run: >
-        docker-compose -f docker-compose.test.yml up
-        --build
-        --exit-code-from tests
-        --abort-on-container-exit
+    - name: Test (docker compose)
+      uses: hoverkraft-tech/compose-action@v2.0.1
+      with:
+        compose-file: ./docker-compose.test.yml
+        up-flags: --build --exit-code-from tests --abort-on-container-exit
       env:
         BE_NAMESPACE: ${{ steps.vars.outputs.BE_NAMESPACE }}
         BE_IMAGE_TAG: ${{ env.GITHUB_REF_SLUG }}

.github/workflows/build-production.yaml

@@ -134,12 +134,11 @@ jobs:
         tags: |
           ${{ steps.vars.outputs.BE_NAMESPACE }}/fragalysis-backend:${{ steps.vars.outputs.tag }}
           ${{ steps.vars.outputs.BE_NAMESPACE }}/fragalysis-backend:stable
-    - name: Test
-      run: >
-        docker-compose -f docker-compose.test.yml up
-        --build
-        --exit-code-from tests
-        --abort-on-container-exit
+    - name: Test (docker compose)
+      uses: hoverkraft-tech/compose-action@v2.0.1
+      with:
+        compose-file: ./docker-compose.test.yml
+        up-flags: --build --exit-code-from tests --abort-on-container-exit
       env:
         BE_NAMESPACE: ${{ steps.vars.outputs.BE_NAMESPACE }}
         BE_IMAGE_TAG: ${{ steps.vars.outputs.tag }}

.github/workflows/build-staging.yaml

@@ -154,12 +154,11 @@ jobs:
       with:
         context: .
         tags: ${{ steps.vars.outputs.BE_NAMESPACE }}/fragalysis-backend:${{ steps.vars.outputs.tag }}
-    - name: Test
-      run: >
-        docker-compose -f docker-compose.test.yml up
-        --build
-        --exit-code-from tests
-        --abort-on-container-exit
+    - name: Test (docker compose)
+      uses: hoverkraft-tech/compose-action@v2.0.1
+      with:
+        compose-file: ./docker-compose.test.yml
+        up-flags: --build --exit-code-from tests --abort-on-container-exit
       env:
         BE_NAMESPACE: ${{ steps.vars.outputs.BE_NAMESPACE }}
         BE_IMAGE_TAG: ${{ steps.vars.outputs.tag }}

api/security.py

@@ -185,7 +185,7 @@ def ping_configured_connector() -> bool:
     return conn is not None
 
 
-class ISpyBSafeQuerySet(viewsets.ReadOnlyModelViewSet):
+class ISPyBSafeQuerySet(viewsets.ReadOnlyModelViewSet):
     """
     This ISpyBSafeQuerySet, which inherits from the DRF viewsets.ReadOnlyModelViewSet,
     is used for all views that need to yield (filter) view objects based on a
@@ -216,7 +216,7 @@ def get_queryset(self):
         q_filter = self._get_q_filter(proposal_list)
         return self.queryset.filter(q_filter).distinct()
 
-    def _get_open_proposals(self):
+    def get_open_proposals(self):
         """
         Returns the set of proposals anybody can access.
         These consist of any Projects that are marked "open_to_public"
@@ -226,6 +226,7 @@ def _get_open_proposals(self):
             Project.objects.filter(open_to_public=True).values_list("title", flat=True)
         )
         open_proposals.update(settings.PUBLIC_TAS_LIST)
+        # End Temporary Test Code (1247)
         return open_proposals
 
     def _get_proposals_for_user_from_django(self, user):
@@ -341,36 +342,67 @@ def _get_proposals_from_connector(self, user, conn):
         )
         CachedContent.set_content(user.username, prop_id_set)
 
-    def user_is_member_of_any_given_proposals(self, user, proposals):
+    def user_is_member_of_target(
+        self, user, target, restrict_public_to_membership=True
+    ):
+        """
+        Returns true if the user has access to any proposal the target belongs to.
+        """
+        target_proposals = [p.title for p in target.project_id.all()]
+        user_proposals = self.get_proposals_for_user(
+            user, restrict_public_to_membership=restrict_public_to_membership
+        )
+        is_member = any(proposal in user_proposals for proposal in target_proposals)
+        if not is_member:
+            logger.warning(
+                "Failed membership check user='%s' target='%s' target_proposals=%s",
+                user.username,
+                target.title,
+                target_proposals,
+            )
+        return is_member
+
+    def user_is_member_of_any_given_proposals(
+        self, user, proposals, restrict_public_to_membership=True
+    ):
         """
         Returns true if the user has access to any proposal in the given
-        proposals list.Only one needs to match for permission to be granted.
-        We 'restrict_to_membership' to only consider proposals the user
+        proposals list. Only one needs to match for permission to be granted.
+        We 'restrict_public_to_membership' to only consider proposals the user
         has explicit membership.
         """
-        user_proposals = self.get_proposals_for_user(user, restrict_to_membership=True)
-        return any(proposal in user_proposals for proposal in proposals)
+        user_proposals = self.get_proposals_for_user(
+            user, restrict_public_to_membership=restrict_public_to_membership
+        )
+        is_member = any(proposal in user_proposals for proposal in proposals)
+        if not is_member:
+            logger.warning(
+                "Failed membership check user='%s' proposals=%s",
+                user.username,
+                proposals,
+            )
+        return is_member
 
-    def get_proposals_for_user(self, user, restrict_to_membership=False):
+    def get_proposals_for_user(self, user, restrict_public_to_membership=False):
         """
         Returns a list of proposals that the user has access to.
 
-        If 'restrict_to_membership' is set only those proposals/visits where the user
+        If 'restrict_public_to_membership' is set only those proposals/visits where the user
         is a member of the visit will be returned. Otherwise the 'public'
-        proposals/visits will also be returned. Typically 'restrict_to_membership' is
+        proposals/visits will also be returned. Typically 'restrict_public_to_membership' is
         used for uploads/changes - this allows us to implement logic that (say)
         only permits explicit members of public proposals to add/load data for that
-        project (restrict_to_membership=True), but everyone can 'see' public data
-        (restrict_to_membership=False).
+        project (restrict_public_to_membership=True), but everyone can 'see' public data
+        (restrict_public_to_membership=False).
         """
         assert user
 
         proposals = set()
         ispyb_user = settings.ISPYB_USER
         logger.debug(
-            "ispyb_user=%s restrict_to_membership=%s (DISABLE_RESTRICT_PROPOSALS_TO_MEMBERSHIP=%s)",
+            "ispyb_user=%s restrict_public_to_membership=%s (DISABLE_RESTRICT_PROPOSALS_TO_MEMBERSHIP=%s)",
             ispyb_user,
-            restrict_to_membership,
+            restrict_public_to_membership,
             settings.DISABLE_RESTRICT_PROPOSALS_TO_MEMBERSHIP,
         )
         if ispyb_user:
@@ -384,10 +416,10 @@ def get_proposals_for_user(self, user, restrict_to_membership=False):
         # We have all the proposals where the user has authority.
         # Add open/public proposals?
         if (
-            not restrict_to_membership
+            not restrict_public_to_membership
             or settings.DISABLE_RESTRICT_PROPOSALS_TO_MEMBERSHIP
         ):
-            proposals.update(self._get_open_proposals())
+            proposals.update(self.get_open_proposals())
 
         # Return the set() as a list()
         return list(proposals)
@@ -411,9 +443,9 @@ def _get_q_filter(self, proposal_list):
             return Q(title__in=proposal_list) | Q(open_to_public=True)
 
 
-class ISpyBSafeStaticFiles:
+class ISPyBSafeStaticFiles:
     def get_queryset(self):
-        query = ISpyBSafeQuerySet()
+        query = ISPyBSafeQuerySet()
         query.request = self.request
         query.filter_permissions = self.permission_string
         query.queryset = self.model.objects.filter()
@@ -459,7 +491,7 @@ def get_response(self):
             raise Http404 from exc
 
 
-class ISpyBSafeStaticFiles2(ISpyBSafeStaticFiles):
+class ISPyBSafeStaticFiles2(ISPyBSafeStaticFiles):
     def get_response(self):
         logger.info("+ get_response called with: %s", self.input_string)
         # it wasn't working because found two objects with test file name

api/urls.py

@@ -3,20 +3,18 @@
 from rest_framework.authtoken import views as drf_views
 from rest_framework.routers import DefaultRouter
 
-# from xcdb import views as xchem_views
 from hotspots import views as hostpot_views
 from hypothesis import views as hypo_views
 from scoring import views as score_views
 from viewer import views as viewer_views
-from xcdb import views as xcdb_views
 
 router = DefaultRouter()
 # Register the basic data
 router.register("compounds", viewer_views.CompoundView, "compounds")
 router.register("targets", viewer_views.TargetView, "targets")
 router.register("projects", viewer_views.ProjectView)
-router.register("session-projects", viewer_views.SessionProjectsView)
-router.register("snapshots", viewer_views.SnapshotsView)
+router.register("session-projects", viewer_views.SessionProjectView)
+router.register("snapshots", viewer_views.SnapshotView)
 router.register("action-type", viewer_views.ActionTypeView)
 router.register("session-actions", viewer_views.SessionActionsView)
 router.register("snapshot-actions", viewer_views.SnapshotActionsView)
@@ -26,7 +24,7 @@
 # Compounds sets
 router.register("compound-sets", viewer_views.ComputedSetView)
 router.register("compound-molecules", viewer_views.ComputedMoleculesView)
-router.register("numerical-scores", viewer_views.NumericalScoresView)
+router.register("numerical-scores", viewer_views.NumericalScoreValuesView)
 router.register("text-scores", viewer_views.TextScoresView)
 router.register("compound-scores", viewer_views.CompoundScoresView, "compound-scores")
 router.register(
@@ -65,16 +63,13 @@
 # Get the information
 router.register("siteobservationannotation", score_views.SiteObservationAnnotationView)
 
-# fragspect
-router.register("fragspect", xcdb_views.FragspectCrystalView)
-
 # discourse posts
 router.register(
     "discourse_post", viewer_views.DiscoursePostView, basename='discourse_post'
 )
 
 # Take a dictionary and return a csv
-router.register("dicttocsv", viewer_views.DictToCsv, basename='dicttocsv')
+router.register("dicttocsv", viewer_views.DictToCSVView, basename='dicttocsv')
 
 # tags
 router.register("tag_category", viewer_views.TagCategoryView, basename='tag_category')
@@ -92,36 +87,38 @@
 # Download a zip file of the requested contents
 router.register(
     "download_structures",
-    viewer_views.DownloadStructures,
+    viewer_views.DownloadStructuresView,
     basename='download_structures',
 )
 
 # Experiments and Experiment (XChemAlign) upload support
 router.register(
     "upload_target_experiments",
-    viewer_views.UploadTargetExperiments,
+    viewer_views.UploadExperimentUploadView,
     basename='upload_target_experiments',
 )
 router.register(
     "download_target_experiments",
-    viewer_views.DownloadTargetExperiments,
+    viewer_views.DownloadExperimentUploadView,
     basename='download_target_experiments',
 )
 
 
 router.register(
     "target_experiment_uploads",
-    viewer_views.TargetExperimentUploads,
+    viewer_views.ExperimentUploadView,
     basename='target_experiment_uploads',
 )
 router.register(
-    "site_observations", viewer_views.SiteObservations, basename='site_observations'
+    "site_observations", viewer_views.SiteObservationView, basename='site_observations'
+)
+router.register("canon_sites", viewer_views.CanonSiteView, basename='canon_sites')
+router.register(
+    "canon_site_confs", viewer_views.CanonSiteConfView, basename='canon_site_confs'
 )
-router.register("canon_sites", viewer_views.CanonSites, basename='canon_sites')
 router.register(
-    "canon_site_confs", viewer_views.CanonSiteConfs, basename='canon_site_confs'
+    "xtalform_sites", viewer_views.XtalformSiteView, basename='xtalform_sites'
 )
-router.register("xtalform_sites", viewer_views.XtalformSites, basename='xtalform_sites')
 router.register("poses", viewer_views.PoseView, basename='poses')
 
 # Squonk Jobs

api/utils.py

@@ -305,20 +305,16 @@ def parse_xenons(input_smi):
     return bond_ids, bond_colours, e_mol.GetMol()
 
 
-def get_params(smiles, request):
+def get_img_from_smiles(smiles, request):
     # try:
     smiles = canon_input(smiles)
     # except:
     #     smiles = ""
-    height = None
     mol = None
     bond_id_list = []
     highlightBondColors = {}
-    if "height" in request.GET:
-        height = int(request.GET["height"])
-    width = None
-    if "width" in request.GET:
-        width = int(request.GET["width"])
+    height = int(request.GET.get("height", "128"))
+    width = int(request.GET.get("width", "128"))
     if "atom_indices" in request.GET:
         mol = Chem.MolFromSmiles(smiles)
         bond_id_list, highlightBondColors, mol = parse_atom_ids(
@@ -360,7 +356,7 @@ def get_highlighted_diffs(request):
 def mol_view(request):
     if "smiles" in request.GET:
         smiles = request.GET["smiles"].rstrip(".svg")
-        return get_params(smiles, request)
+        return get_img_from_smiles(smiles, request)
     else:
         return HttpResponse("Please insert SMILES")