Don't query ObjectPermission table when user has access_all_objects c…

…apability (#783)

Co-authored-by: Paweł Srokosz <[email protected]>

docker-compose-e2e.yml

@@ -93,7 +93,7 @@ services:
     ports:
       - "127.0.0.1:8025:8025"
   karton-system:
-    image: certpl/karton-system:v5.0.0
+    image: certpl/karton-system:v5.1.0
     depends_on:
       - redis
       - minio
@@ -102,14 +102,14 @@ services:
     entrypoint: karton-system
     command: --setup-bucket
   karton-classifier:
-    image: certpl/karton-classifier:v1.4.0
+    image: certpl/karton-classifier:v2.0.0
     depends_on:
       - redis
       - minio
     volumes:
       - "./dev/karton.ini:/etc/karton/karton.ini"
   karton-dashboard:
-    image: certpl/karton-dashboard:v1.4.0
+    image: certpl/karton-dashboard:v1.5.0
     depends_on:
       - redis
       - minio
@@ -118,7 +118,7 @@ services:
     ports:
       - "127.0.0.1:8030:5000"
   karton-mwdb-reporter:
-    image: certpl/karton-mwdb-reporter:v1.2.0
+    image: certpl/karton-mwdb-reporter:v1.3.0
     depends_on:
       - redis
       - minio

mwdb/cli/database.py

@@ -24,13 +24,6 @@ def _initialize(admin_password):
     )
     db.session.add(public_group)
 
-    everything_group = Group(
-        name=Group.DEFAULT_EVERYTHING_GROUP_NAME,
-        capabilities=[Capabilities.access_all_objects],
-        workspace=False,
-    )
-    db.session.add(everything_group)
-
     registered_group = Group(
         name=Group.DEFAULT_REGISTERED_GROUP_NAME,
         capabilities=[
@@ -52,7 +45,7 @@ def _initialize(admin_password):
         login=app_config.mwdb.admin_login,
         email="[email protected]",
         additional_info="MWDB built-in administrator account",
-        groups=[admin_group, everything_group, public_group, registered_group],
+        groups=[admin_group, public_group, registered_group],
     )
     admin_user.reset_sessions()
     admin_user.set_password(admin_password)

mwdb/model/group.py

@@ -47,7 +47,6 @@ class Group(db.Model):
 
     PUBLIC_GROUP_NAME = "public"
     # These groups are just pre-created for convenience by 'mwdb-core configure'
-    DEFAULT_EVERYTHING_GROUP_NAME = "everything"
     DEFAULT_REGISTERED_GROUP_NAME = "registered"
 
     @property

mwdb/model/migrations/versions/6a7aefae72d3_rolling_back_objectpermissions_from_.py

@@ -0,0 +1,65 @@
+"""Rolling back ObjectPermissions from access_all_objects
+
+Revision ID: 6a7aefae72d3
+Revises: f02c42a17695
+Create Date: 2023-06-22 14:19:34.730831
+
+"""
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy.dialects.postgresql.array import ARRAY
+
+# revision identifiers, used by Alembic.
+revision = "6a7aefae72d3"
+down_revision = "f02c42a17695"
+branch_labels = None
+depends_on = None
+
+group_helper = sa.Table(
+    "group",
+    sa.MetaData(),
+    sa.Column("id", sa.Integer()),
+    sa.Column("name", sa.String(32)),
+    sa.Column("capabilities", ARRAY(sa.Text())),
+    sa.Column("private", sa.Boolean()),
+    sa.Column("default", sa.Boolean()),
+    sa.Column("workspace", sa.Boolean()),
+)
+
+object_perm_helper = sa.Table(
+    "permission",
+    sa.MetaData(),
+    sa.Column("object_id", sa.Integer()),
+    sa.Column("group_id", sa.Integer()),
+    sa.Column("reason_type", sa.String(32)),
+)
+
+logger = logging.getLogger("alembic")
+
+
+def upgrade():
+    connection = op.get_bind()
+    access_all_objects_groups = connection.execute(
+        group_helper.select().where(
+            group_helper.c.capabilities.any("access_all_objects")
+        )
+    )
+    for group in access_all_objects_groups:
+        logger.info(f"Removing unnecessary rows access_all_objects group: {group.name}")
+        rowcount = connection.execute(
+            object_perm_helper.delete(
+                sa.and_(
+                    object_perm_helper.c.group_id == group.id,
+                    object_perm_helper.c.reason_type == "shared",
+                )
+            )
+        ).rowcount
+        logger.info(f"{rowcount} rows removed")
+    logger.info("Running analyze...")
+    connection.execute("ANALYZE")
+
+
+def downgrade():
+    raise NotImplementedError("This migration is not downgradable")

mwdb/model/object.py

@@ -251,7 +251,7 @@ def get_shares_filter(include_inherited_uploads=True):
         """
         if include_inherited_uploads:
             type_filter = or_(
-                ObjectPermission.reason_type != "added",
+                ObjectPermission.reason_type != AccessType.ADDED,
                 ObjectPermission.related_object_id != ObjectPermission.object_id,
             )
         else:
@@ -528,6 +528,9 @@ def has_explicit_access(self, user):
         Check whether user has access via explicit ObjectPermissions
         Used by Object.access
         """
+        if user.has_rights(Capabilities.access_all_objects):
+            return True
+
         return db.session.query(
             exists().where(
                 and_(
@@ -542,6 +545,9 @@ def check_group_explicit_access(self, group):
         Check whether group has access via explicit ObjectPermissions
         Used by Object.access
         """
+        if Capabilities.access_all_objects in group.capabilities:
+            return True
+
         return db.session.query(
             exists().where(
                 and_(
@@ -582,7 +588,6 @@ def _get_or_create(
 
         We don't perform permission checks, all data needs to be validated by Resource.
         """
-        from .group import Group
 
         share_with = share_with or []
         attributes = attributes or []
@@ -643,16 +648,6 @@ def _get_or_create(
                     commit=False,
                 )
 
-        # Share with all groups that access all objects
-        for all_access_group in Group.all_access_groups():
-            created_object.give_access(
-                all_access_group.id,
-                AccessType.SHARED,
-                created_object,
-                g.auth_user,
-                commit=False,
-            )
-
         # Add parent to object if specified
         # Inherited share entries must be added AFTER we add share entries
         # related with upload itself

mwdb/model/user.py

@@ -9,6 +9,7 @@
 from sqlalchemy.orm.exc import NoResultFound
 
 from mwdb.core.auth import AuthScope, generate_token, verify_legacy_token, verify_token
+from mwdb.core.capabilities import Capabilities
 
 from . import db
 from .group import Group, Member
@@ -257,6 +258,9 @@ def has_access_to_object(self, object_id):
         """
         Query filter for objects visible by this user
         """
+        if self.has_rights(Capabilities.access_all_objects):
+            return True
+
         return object_id.in_(
             db.session.query(ObjectPermission.object_id).filter(
                 self.is_member(ObjectPermission.group_id)

mwdb/resources/tag.py

@@ -1,12 +1,11 @@
 from flask import g, request
 from flask_restful import Resource
-from sqlalchemy.sql import and_
 from werkzeug.exceptions import NotFound
 
 from mwdb.core.capabilities import Capabilities
 from mwdb.core.plugins import hooks
 from mwdb.core.rate_limit import rate_limited_resource
-from mwdb.model import ObjectPermission, Tag, db
+from mwdb.model import Tag, db
 from mwdb.schema.tag import (
     TagItemResponseSchema,
     TagListRequestSchema,
@@ -64,13 +63,7 @@ def get(self):
         tags = (
             db.session.query(Tag.tag)
             .distinct(Tag.tag)
-            .join(
-                ObjectPermission,
-                and_(
-                    ObjectPermission.object_id == Tag.object_id,
-                    g.auth_user.is_member(ObjectPermission.group_id),
-                ),
-            )
+            .filter(g.auth_user.has_access_to_object(Tag.object_id))
         )
 
         tag_prefix = obj["query"]

tests/backend/test_blobs.py

@@ -139,6 +139,9 @@ def test_blob_search_upload_count(admin_session):
         blob = users_session.add_blob(None, blobname=blob_name, blobtype="inject", content=blob_content)
         users_session.add_tag(blob["id"], tag)
 
+    found_configs = admin_session.search(f'tag:{tag}')
+    assert len(found_configs) == 1
+
     found_configs = admin_session.search(f'tag:{tag} AND blob.upload_count:{len(test_users)}')
     assert len(found_configs) == 1