check that obj_id is a string (#554)

* check that obj_id is a string * add check for filter
fritz-marshal · Jun 29, 2024 · 65f42e6 · 65f42e6
1 parent 1fb70ea
commit 65f42e6
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 62 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,3 +1,5 @@
+default_language_version:
+  python: python3
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v3.3.0

diff --git a/extensions/skyportal/skyportal/handlers/api/archive.py b/extensions/skyportal/skyportal/handlers/api/archive.py
@@ -919,6 +919,10 @@ def post(self):
 
         if obj_id is None and (group_ids is None or len(group_ids) == 0):
             return self.error("Parameter group_ids is required if obj_id is not set")
+
+        if obj_id is not None and (type(obj_id) is not str):
+            return self.error("obj_id must be string if set")
+
         if catalog is None:
             return self.error("Missing required parameter: catalog")
         if light_curve_ids is None or len(light_curve_ids) == 0:

diff --git a/extensions/skyportal/skyportal/handlers/api/kowalski_filter.py b/extensions/skyportal/skyportal/handlers/api/kowalski_filter.py
@@ -70,23 +70,27 @@ def get(self, filter_id):
         if kowalski is None:
             return self.error("Couldn't connect to Kowalski")
 
-        # permission check
-        _ = Filter.get_if_accessible_by(
-            filter_id, self.current_user, raise_if_none=True
-        )
-        response = kowalski.api(
-            method="get",
-            endpoint=f"api/filters/{filter_id}",
-        )
-        data = response.get("data")
-        # drop monogdb's _id's which are not (default) JSON-serializable
-        if data is not None:
-            data.pop("_id", None)
-        status = response.get("status")
-        if status == "error":
-            message = response.get("message")
-            return self.error(message=message)
-        return self.success(data=data)
+        with self.Session() as session:
+            stmt = Filter.select(session.user_or_token).where(
+                Filter.id == int(filter_id)
+            )
+            f = session.scalars(stmt).first()
+            if f is None:
+                return self.error(f"No filter with ID: {filter_id}")
+
+            response = kowalski.api(
+                method="get",
+                endpoint=f"api/filters/{filter_id}",
+            )
+            data = response.get("data")
+            # drop monogdb's _id's which are not (default) JSON-serializable
+            if data is not None:
+                data.pop("_id", None)
+            status = response.get("status")
+            if status == "error":
+                message = response.get("message")
+                return self.error(message=message)
+            return self.success(data=data)
 
     @auth_or_token
     def post(self, filter_id):
@@ -130,37 +134,43 @@ def post(self, filter_id):
         if pipeline is None:
             return self.error("Missing pipeline parameter")
 
-        f = Filter.get_if_accessible_by(
-            filter_id, self.current_user, raise_if_none=True
-        )
+        with self.Session() as session:
+            stmt = Filter.select(session.user_or_token).where(
+                Filter.id == int(filter_id)
+            )
+            f = session.scalars(stmt).first()
+            if f is None:
+                return self.error(f"No filter with ID: {filter_id}")
 
-        group_id = f.group_id
+            group_id = f.group_id
 
-        # get stream:
-        stream = Stream.get_if_accessible_by(
-            f.stream_id, self.current_user, raise_if_none=True
-        )
+            stmt = Stream.select(session.user_or_token).where(
+                Stream.id == int(f.stream_id)
+            )
+            stream = session.scalars(stmt).first()
+            if f is None:
+                return self.error(f"No stream with ID: {filter_id}")
 
-        post_data = {
-            "group_id": group_id,
-            "filter_id": int(filter_id),
-            "catalog": stream.altdata["collection"],
-            "permissions": stream.altdata["selector"],
-            "pipeline": pipeline,
-        }
-        response = kowalski.api(
-            method="post",
-            endpoint="api/filters",
-            data=post_data,
-        )
-        data = response.get("data")
-        if data is not None:
-            data.pop("_id", None)
-        status = response.get("status")
-        if status == "error":
-            message = response.get("message")
-            return self.error(message=message)
-        return self.success(data=data)
+            post_data = {
+                "group_id": group_id,
+                "filter_id": int(filter_id),
+                "catalog": stream.altdata["collection"],
+                "permissions": stream.altdata["selector"],
+                "pipeline": pipeline,
+            }
+            response = kowalski.api(
+                method="post",
+                endpoint="api/filters",
+                data=post_data,
+            )
+            data = response.get("data")
+            if data is not None:
+                data.pop("_id", None)
+            status = response.get("status")
+            if status == "error":
+                message = response.get("message")
+                return self.error(message=message)
+            return self.success(data=data)
 
     @auth_or_token
     def patch(self, filter_id):
@@ -232,10 +242,13 @@ def patch(self, filter_id):
                 "At least one of (active, active_fid, autosave, update_annotations, auto_followup) must be set"
             )
 
-        # permission check
-        _ = Filter.get_if_accessible_by(
-            filter_id, self.current_user, raise_if_none=True
-        )
+        with self.Session() as session:
+            stmt = Filter.select(session.user_or_token).where(
+                Filter.id == int(filter_id)
+            )
+            f = session.scalars(stmt).first()
+            if f is None:
+                return self.error(f"No filter with ID: {filter_id}")
 
         # get the existing filter
         response = kowalski.api(
@@ -408,17 +421,20 @@ def delete(self, filter_id):
         if kowalski is None:
             return self.error("Couldn't connect to Kowalski")
 
-        # permission check
-        _ = Filter.get_if_accessible_by(
-            filter_id, self.current_user, raise_if_none=True
-        )
+        with self.Session() as session:
+            stmt = Filter.select(session.user_or_token).where(
+                Filter.id == int(filter_id)
+            )
+            f = session.scalars(stmt).first()
+            if f is None:
+                return self.error(f"No filter with ID: {filter_id}")
 
-        response = kowalski.api(
-            method="patch",
-            endpoint=f"api/filters/{filter_id}",
-        )
-        status = response.get("status")
-        if status == "error":
-            message = response.get("message")
-            return self.error(message=message)
-        return self.success()
+            response = kowalski.api(
+                method="patch",
+                endpoint=f"api/filters/{filter_id}",
+            )
+            status = response.get("status")
+            if status == "error":
+                message = response.get("message")
+                return self.error(message=message)
+            return self.success()