Proposed structure for user roles. (#832)

* Add roles

* Remove use of unix user group. Add note for retroactive role assign

* Add docs on roles and external tables. Reduce key length

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Change Log
 
+## Unreleased
+
+### Infrastructure
+
+- Add user roles to `database_settings.py`. #832
+
 ## [0.5.0] (February 9, 2024)
 
 ### Infrastructure

config/add_dj_collaborator.py

@@ -9,4 +9,4 @@
         "This script is deprecated. "
         + "Use spyglass.utils.database_settings.DatabaseSettings instead."
     )
-    DatabaseSettings(user_name=sys.argv[1]).add_collab_user()
+    DatabaseSettings(user_name=sys.argv[1]).add_collab()

config/add_dj_guest.py

@@ -9,4 +9,4 @@
         "This script is deprecated. "
         + "Use spyglass.utils.database_settings.DatabaseSettings instead."
     )
-    DatabaseSettings(user_name=sys.argv[1]).add_dj_guest()
+    DatabaseSettings(user_name=sys.argv[1]).add_guest()

docs/src/misc/database_management.md

@@ -36,19 +36,28 @@ schema/database prefix.
 - `ALL` privileges allow users to create, alter, or drop tables and schemas in
     addition to operations above.
 
-In practice, DataJoint only permits alerations of secondary keys on existing
+In practice, DataJoint only permits alterations of secondary keys on existing
 tables, and more derstructive operations would require using DataJoint to
 execeute MySQL commands.
 
 Shared schema prefixes are those defined in the Spyglass package (e.g.,
 `common`, `lfp`, etc.). A 'user schema' is any schema with the username as
 prefix. User types differ in the privileges they are granted on these prifixes.
+Declaring a table with the SpyglassMixin on a schema other than a shared module
+or the user's own prefix will raise a warning.
 
-### Users types
+### Users roles
+
+When a database is first initialized, the team should run `add_roles` to create
+the following roles:
 
-- `collab_user`: `ALL` on user schema, `SELECT` on all other schemas.
 - `dj_guest`: `SELECT` on all schemas.
+- `dj_collab`: `ALL` on user schema, `SELECT` on all other schemas.
 - `dj_user`: `ALL` on shared and user schema, `SELECT` on all other schemas.
+- `dj_admin`: `ALL` on all schemas.
+
+If new shared modules are introduced, the `add_module` method should be used to
+expand the privileges of the `dj_user` role.
 
 ### Setting Passwords
 
@@ -224,9 +233,19 @@ To remove orphaned files, we run the following commands in our cron jobs:
 ```python
 from spyglass.common import AnalysisNwbfile
 from spyglass.spikesorting import SpikeSorting
+from spyglass.common.common_nwbfile import schema as nwbfile_schema
+from spyglass.decoding.v1.sorted_spikes import schema as spikes_schema
+from spyglass.decoding.v1.clusterless import schema as clusterless_schema
 
 
 def main():
     AnalysisNwbfile().nightly_cleanup()
     SpikeSorting().nightly_cleanup()
+    nwbfile_schema.external['analysis'].delete(delete_external_files=True))
+    nwbfile_schema.external['raw'].delete(delete_external_files=True))
+    spikes_schema.external['analysis'].delete(delete_external_files=True))
+    clusterless_schema.external['analysis'].delete(delete_external_files=True))
 ```
+
+The `delete` calls above use DataJoint's `ExternalTable.delete` method, which
+will remove files from disk that are no longer referenced in the database.

src/spyglass/decoding/v1/sorted_spikes.py

@@ -47,6 +47,7 @@ class SortedSpikesDecodingSelection(SpyglassMixin, dj.Manual):
     -> IntervalList.proj(decoding_interval='interval_list_name')
     estimate_decoding_params = 1 : bool # whether to estimate the decoding parameters
     """
+    # NOTE: Excessive key length fixed by reducing UnitSelectionParams.unit_filter_params_name
 
 
 @schema

src/spyglass/spikesorting/analysis/v1/group.py

@@ -14,11 +14,12 @@
 @schema
 class UnitSelectionParams(SpyglassMixin, dj.Manual):
     definition = """
-    unit_filter_params_name: varchar(128)
+    unit_filter_params_name: varchar(32)
     ---
     include_labels = Null: longblob
     exclude_labels = Null: longblob
     """
+    # NOTE: pk reduced from 128 to 32 to avoid long primary key error
     contents = [
         [
             "all_units",

src/spyglass/utils/database_settings.py

@@ -1,5 +1,4 @@
 #!/usr/bin/env python
-import grp
 import os
 import sys
 import tempfile
@@ -9,11 +8,6 @@
 
 from spyglass.utils.logging import logger
 
-GRANT_ALL = "GRANT ALL PRIVILEGES ON "
-GRANT_SEL = "GRANT SELECT ON "
-CREATE_USR = "CREATE USER IF NOT EXISTS "
-TEMP_PASS = " IDENTIFIED BY 'temppass';"
-ESC = r"\_%"
 SHARED_MODULES = [
     "common",
     "spikesorting",
@@ -25,27 +19,42 @@
     "waveform",
     "mua",
 ]
+GRANT_ALL = "GRANT ALL PRIVILEGES ON "
+GRANT_SEL = "GRANT SELECT ON "
+CREATE_USR = "CREATE USER IF NOT EXISTS "
+CREATE_ROLE = "CREATE ROLE IF NOT EXISTS "
+TEMP_PASS = " IDENTIFIED BY 'temppass';"
+ESC = r"\_%"
 
 
 class DatabaseSettings:
     def __init__(
         self,
         user_name=None,
         host_name=None,
-        target_group=None,
         debug=False,
         target_database=None,
     ):
         """Class to manage common database settings
 
+        Roles:
+        - dj_guest:  select for all prefix
+        - dj_collab: select for all prefix, all for user prefix
+        - dj_user:   select for all prefix, all for user prefix, all for shared
+        - dj_admin:     all for all prefix
+
+        Note: To add dj_user role to all those with common access, run:
+            query = "SELECT user, host FROM mysql.db WHERE Db LIKE 'common%';"
+            users = dj.conn().query(query).fetchall()
+            for user in users:
+                dj.conn().query(f"GRANT dj_user TO '{user[0][0]}'@'%';")
+
         Parameters
         ----------
         user_name : str, optional
             The name of the user to add to the database. Default from dj.config
         host_name : str, optional
             The name of the host to add to the database. Default from dj.config
-        target_group : str, optional
-            Group to which user belongs. Default is kachery-users
         debug : bool, optional
             Default False. If True, pprint sql instead of running
         target_database : str, optional
@@ -56,91 +65,78 @@ def __init__(
         self.host = (
             host_name or dj.config["database.host"] or "lmf-db.cin.ucsf.edu"
         )
-        self.target_group = target_group or "kachery-users"
         self.debug = debug
         self.target_database = target_database or "mysql"
 
     @property
-    def _add_collab_usr_sql(self):
-        return [
-            # Create the user (if not already created) and set the password
-            f"{CREATE_USR}'{self.user}'@'%'{TEMP_PASS}\n",
-            # Grant privileges to databases matching the user_name pattern
-            f"{GRANT_ALL}`{self.user}{ESC}`.* TO '{self.user}'@'%';\n",
-            # Grant SELECT privileges on all databases
-            f"{GRANT_SEL}`%`.* TO '{self.user}'@'%';\n",
+    def _create_roles_sql(self):
+        guest_role = [
+            f"{CREATE_ROLE}'dj_guest';\n",
+            f"{GRANT_SEL}`%`.* TO 'dj_guest';\n",
+        ]
+        collab_role = [
+            f"{CREATE_ROLE}'dj_collab';\n",
+            f"{GRANT_SEL}`%`.* TO 'dj_collab';\n",
+        ]  # also gets own prefix below
+        user_role = [
+            f"{CREATE_ROLE}'dj_user';\n",
+            f"{GRANT_SEL}`%`.* TO 'dj_user';\n",
+        ] + [
+            f"{GRANT_ALL}`{module}`.* TO 'dj_user';\n"
+            for module in self.shared_modules
+        ]  # also gets own prefix below
+        admin_role = [
+            f"{CREATE_ROLE}'dj_admin';\n",
+            f"{GRANT_ALL}`%`.* TO 'dj_admin';\n",
         ]
 
-    def add_collab_user(self):
-        """Add collaborator user with full permissions to shared modules"""
-        file = self.write_temp_file(self._add_collab_usr_sql)
-        self.exec(file)
+        return guest_role + collab_role + user_role + admin_role
+
+    def _create_user_sql(self, role):
+        """Create user and grant role"""
+        return [
+            f"{CREATE_USR}'{self.user}'@'%'{TEMP_PASS}\n",  # create user
+            f"GRANT {role} TO '{self.user}'@'%';\n",  # grant role
+        ]
 
     @property
-    def _add_dj_guest_sql(self):
-        # Note: changing to temppass for uniformity
+    def _user_prefix_sql(self):
+        """Grant user all permissions for user prefix"""
         return [
-            # Create the user (if not already created) and set the password
-            f"{CREATE_USR}'{self.user}'@'%'{TEMP_PASS}\n",
-            # Grant privileges
-            f"{GRANT_SEL}`%`.* TO '{self.user}'@'%';\n",
+            f"{GRANT_ALL}`{self.user}{ESC}`.* TO '{self.user}'@'%';\n",
         ]
 
-    def add_dj_guest(self, method="file"):
-        """Add guest user with select permissions to shared modules"""
-        file = self.write_temp_file(self._add_dj_guest_sql)
-        self.exec(file)
+    @property
+    def _add_guest_sql(self):
+        return self._create_user_sql("dj_guest")
 
-    def _find_group(self):
-        # find the kachery-users group
-        groups = grp.getgrall()
-        group_found = False  # initialize the flag as False
-        for group in groups:
-            if group.gr_name == self.target_group:
-                group_found = (
-                    True  # set the flag to True when the group is found
-                )
-                break
+    @property
+    def _add_collab_sql(self):
+        return self._create_user_sql("dj_collab") + self._user_prefix_sql
 
-        # Check if the group was found
-        if not group_found:
-            if self.debug:
-                logger.info(f"All groups: {[g.gr_name for g in groups]}")
-            sys.exit(
-                f"Error: The target group {self.target_group} was not found."
-            )
+    @property
+    def _add_user_sql(self):
+        return self._create_user_sql("dj_user") + self._user_prefix_sql
 
-        return group
+    def _add_module_sql(self, module_name):
+        return [f"{GRANT_ALL}`{module_name}{ESC}`.* TO dj_user;\n"]
 
-    def _add_module_sql(self, module_name, group):
-        return [
-            f"{GRANT_ALL}`{module_name}{ESC}`.* TO `{user}`@'%';\n"
-            # get a list of usernames
-            for user in group.gr_mem
-        ]
+    def add_collab(self):
+        """Add collaborator user with full permissions to shared modules"""
+        file = self.write_temp_file(self._add_collab_sql)
+        self.exec(file)
+
+    def add_guest(self):
+        """Add guest user with select permissions to shared modules"""
+        file = self.write_temp_file(self._add_guest_sql)
+        self.exec(file)
 
     def add_module(self, module_name):
         """Add module to database. Grant permissions to all users in group"""
         logger.info(f"Granting everyone permissions to module {module_name}")
-        group = self._find_group()
-        file = self.write_temp_file(self._add_module_sql(module_name, group))
+        file = self.write_temp_file(self._add_module_sql(module_name))
         self.exec(file)
 
-    @property
-    def _add_dj_user_sql(self):
-        return (
-            [
-                f"{CREATE_USR}'{self.user}'@'%' "
-                + "IDENTIFIED BY 'temppass';\n",
-                f"{GRANT_ALL}`{self.user}{ESC}`.* TO '{self.user}'@'%';" + "\n",
-            ]
-            + [
-                f"{GRANT_ALL}`{module}`.* TO '{self.user}'@'%';\n"
-                for module in self.shared_modules
-            ]
-            + [f"{GRANT_SEL}`%`.* TO '{self.user}'@'%';\n"]
-        )
-
     def add_dj_user(self, check_exists=True):
         """Add user to database with permissions to shared modules"""
         if check_exists:
@@ -149,10 +145,15 @@ def add_dj_user(self, check_exists=True):
                 logger.info("Creating database user ", self.user)
             else:
                 sys.exit(
-                    f"Error: could not find {self.user} in home dir: {user_home}"
+                    f"Error: couldn't find {self.user} in home dir: {user_home}"
                 )
 
-        file = self.write_temp_file(self._add_dj_user_sql)
+        file = self.write_temp_file(self._add_user_sql)
+        self.exec(file)
+
+    def add_roles(self):
+        """Add roles to database"""
+        file = self.write_temp_file(self._create_roles_sql)
         self.exec(file)
 
     def write_temp_file(self, content: list) -> tempfile.NamedTemporaryFile:
@@ -176,11 +177,10 @@ def exec(self, file):
         if self.debug:
             return
 
-        if self.target_database == "mysql":
-            cmd = f"mysql -p -h {self.host} < {file.name}"
-        else:
-            cmd = (
-                f"docker exec -i {self.target_database} mysql -u {self.user} "
-                + f"--password=tutorial < {file.name}"
-            )
+        cmd = (
+            f"mysql -p -h {self.host} < {file.name}"
+            if self.target_database == "mysql"
+            else f"docker exec -i {self.target_database} mysql -u {self.user} "
+            + f"--password=tutorial < {file.name}"
+        )
         os.system(cmd)

src/spyglass/utils/dj_mixin.py

@@ -10,6 +10,7 @@
 from datajoint.utils import get_master, user_choice
 from pymysql.err import DataError
 
+from spyglass.utils.database_settings import SHARED_MODULES
 from spyglass.utils.dj_chains import TableChain, TableChains
 from spyglass.utils.dj_helper_fn import fetch_nwb
 from spyglass.utils.dj_merge_tables import RESERVED_PRIMARY_KEY as MERGE_PK
@@ -55,6 +56,26 @@ class SpyglassMixin:
     _session_pk = None  # Session primary key. Mixin is ambivalent to Session pk
     _member_pk = None  # LabMember primary key. Mixin ambivalent table structure
 
+    def __init__(self, *args, **kwargs):
+        """Initialize SpyglassMixin.
+
+        Checks that schema prefix is in SHARED_MODULES.
+        """
+        if (
+            self.database  # Connected to a database
+            and not self.is_declared  # New table
+            and self.database.split("_")[0]  # Prefix
+            not in [
+                *SHARED_MODULES,  # Shared modules
+                dj.config["database.user"],  # User schema
+                "temp",
+                "test",
+            ]
+        ):
+            logger.error(
+                f"Schema prefix not in SHARED_MODULES: {self.database}"
+            )
+
     # ------------------------------- fetch_nwb -------------------------------
 
     @cached_property