Merge pull request #1041 from ScilifelabDataCentre/dev

Improve messages, hide size from Researchers, disallow characters and fix regex

CHANGELOG.md

@@ -37,3 +37,11 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe
 - Fix format of self deletion email ([#984](https://github.com/ScilifelabDataCentre/dds_web/pull/984))
 - Add a full zero-conf development environment ([#993](https://github.com/ScilifelabDataCentre/dds_web/pull/993))
 - Include frontend build in the backend production target ([#1011](https://github.com/ScilifelabDataCentre/dds_web/pull/1011))
+- Correct response about project being created when email validation fails for users ([#1014](https://github.com/ScilifelabDataCentre/dds_web/pull/1014))
+- Introduced an additional validator `dds_web.utils.contains_disallowed_characters` to fix issue [#1007](https://github.com/scilifelabdatacentre/dds_web/issues/1007) ([#1021](https://github.com/ScilifelabDataCentre/dds_web/pull/1021)).
+- Fix regex for listing and deleting files [#1029](https://github.com/scilifelabdatacentre/dds_web/issues/1029)
+- Hides the "Size" and "total_size" variables according to the role and project status ([#1032](https://github.com/ScilifelabDataCentre/dds_web/pull/1032)).
+
+## Sprint (2022-03-09 - 2022-03-23)
+
+- Introduce a separate error message if someone tried to add an unit user to projects individually. ([#1039](https://github.com/ScilifelabDataCentre/dds_web/pull/1039))

dds_web/api/api_s3_connector.py

@@ -9,6 +9,7 @@
 import traceback
 import pathlib
 import json
+import gc
 
 # Installed
 import botocore
@@ -84,11 +85,18 @@ def get_s3_info(self):
         )
 
     @bucket_must_exists
-    def remove_all(self, *args, **kwargs):
+    def remove_bucket(self, *args, **kwargs):
         """Removes all contents from the project specific s3 bucket."""
+        # Get bucket object
         bucket = self.resource.Bucket(self.project.bucket)
+
+        # Delete objects first
         bucket.objects.all().delete()
 
+        # Delete bucket
+        bucket.delete()
+        bucket = None
+
     @bucket_must_exists
     def remove_multiple(self, items, *args, **kwargs):
         """Removes all with prefix."""

dds_web/api/files.py

@@ -6,6 +6,7 @@
 
 # Standard library
 import os
+import re
 
 # Installed
 import flask_restful
@@ -310,6 +311,8 @@ def items_in_subpath(project, folder="."):
         # Files have subpath "." and folders do not have child folders
         # Get everything in folder:
         # Files have subpath == folder and folders have child folders (regexp)
+        if folder[-1] == "/":
+            folder = folder[:-1]
         try:
             # All files in project
             files = models.File.query.filter(
@@ -340,8 +343,9 @@ def items_in_subpath(project, folder="."):
             else:
                 # Get distinct sub folders in specific folder with regex
                 # Match /<something that is not /> x number of times
+                re_folder = re.escape(folder)
                 distinct_folders = (
-                    files.filter(models.File.subpath.regexp_match(rf"^{folder}(/[^/]+)+$"))
+                    files.filter(models.File.subpath.regexp_match(rf"^{re_folder}(/[^/]+)+$"))
                     .with_entities(models.File.subpath)
                     .distinct()
                     .all()
@@ -518,6 +522,9 @@ def delete_folder(self, project, folder):
         """Delete all items in folder"""
         exists = False
         names_in_bucket = []
+        if folder[-1] == "/":
+            folder = folder[:-1]
+        re_folder = re.escape(folder)
         try:
             # File names in root
             files = (
@@ -527,11 +534,12 @@ def delete_folder(self, project, folder):
                 .filter(
                     sqlalchemy.or_(
                         models.File.subpath == sqlalchemy.func.binary(folder),
-                        models.File.subpath.regexp_match(rf"^{folder}(/[^/]+)*$"),
+                        models.File.subpath.regexp_match(rf"^{re_folder}(/[^/]+)*$"),
                     )
                 )
                 .all()
             )
+
         except sqlalchemy.exc.SQLAlchemyError as err:
             raise DatabaseError(message=str(err))
 

dds_web/api/project.py

@@ -5,6 +5,7 @@
 ####################################################################################################
 
 # Standard Library
+import http
 
 # Installed
 import flask_restful
@@ -309,13 +310,15 @@ def get(self):
                 "PI": p.pi,
                 "Status": p.current_status,
                 "Last updated": p.date_updated if p.date_updated else p.date_created,
-                "Size": p.size,
             }
 
-            # Get proj size and update total size
-            proj_size = p.size
-            total_size += proj_size
-            project_info["Size"] = proj_size
+            if (
+                current_user.role == "Researcher" and p.current_status == "Available"
+            ) or current_user.role != "Researcher":
+                # Get proj size and update total size
+                proj_size = p.size
+                total_size += proj_size
+                project_info["Size"] = proj_size
 
             if usage:
                 proj_bhours, proj_cost = self.project_usage(project=p)
@@ -333,9 +336,11 @@ def get(self):
                 "usage": total_bhours_db,
                 "cost": total_cost_db,
             },
-            "total_size": total_size,
         }
 
+        if total_size or current_user.role in ["Unit Admin", "Unit Personnel"]:
+            return_info["total_size"] = total_size
+
         return return_info
 
     @staticmethod
@@ -397,7 +402,7 @@ def delete_project_contents(project):
         # Delete from cloud
         with ApiS3Connector(project=project) as s3conn:
             try:
-                s3conn.remove_all()
+                s3conn.remove_bucket()
             except botocore.client.ClientError as err:
                 raise DeletionError(message=str(err), project=project.public_id)
 
@@ -426,7 +431,7 @@ def delete_project_contents(project):
 
 
 class CreateProject(flask_restful.Resource):
-    @auth.login_required(role=["Super Admin", "Unit Admin", "Unit Personnel"])
+    @auth.login_required(role=["Unit Admin", "Unit Personnel"])
     @logging_bind_request
     @json_required
     @handle_validation_errors
@@ -459,7 +464,7 @@ def post(self):
             db.session.rollback()
             raise DatabaseError(message="Server Error: Project was not created") from err
         except (
-            marshmallow.ValidationError,
+            marshmallow.exceptions.ValidationError,
             DDSArgumentError,
             AccessDeniedError,
         ) as err:
@@ -470,18 +475,26 @@ def post(self):
         flask.current_app.logger.debug(
             f"Project {new_project.public_id} created by user {auth.current_user().username}."
         )
+
         user_addition_statuses = []
         if "users_to_add" in p_info:
             for user in p_info["users_to_add"]:
-                existing_user = user_schemas.UserSchema().load(user)
+                try:
+                    existing_user = user_schemas.UserSchema().load(user)
+                except marshmallow.exceptions.ValidationError as err:
+                    addition_status = f"Error for {user.get('email')}: {err}"
+                    user_addition_statuses.append(addition_status)
+                    continue
+
                 if not existing_user:
                     # Send invite if the user doesn't exist
                     invite_user_result = AddUser.invite_user(
                         email=user.get("email"),
                         new_user_role=user.get("role"),
                         project=new_project,
                     )
-                    if invite_user_result["status"] == 200:
+
+                    if invite_user_result["status"] == http.HTTPStatus.OK:
                         invite_msg = (
                             f"Invitation sent to {user['email']}. "
                             "The user should have a valid account to be added to a project"
@@ -505,7 +518,7 @@ def post(self):
                     user_addition_statuses.append(addition_status)
 
         return {
-            "status": 200,
+            "status": http.HTTPStatus.OK,
             "message": f"Added new project '{new_project.title}'",
             "project_id": new_project.public_id,
             "user_addition_statuses": user_addition_statuses,

dds_web/api/schemas/project_schemas.py

@@ -66,7 +66,9 @@ class Meta:
     title = marshmallow.fields.String(
         required=True,
         allow_none=False,
-        validate=marshmallow.validate.Length(min=1),
+        validate=marshmallow.validate.And(
+            marshmallow.validate.Length(min=1), dds_web.utils.contains_disallowed_characters
+        ),
         error_messages={
             "required": {"message": "Title is required."},
             "null": {"message": "Title is required."},
@@ -75,7 +77,9 @@ class Meta:
     description = marshmallow.fields.String(
         required=True,
         allow_none=False,
-        validate=marshmallow.validate.Length(min=1),
+        validate=marshmallow.validate.And(
+            marshmallow.validate.Length(min=1), dds_web.utils.contains_disallowed_characters
+        ),
         error_messages={
             "required": {"message": "A project description is required."},
             "null": {"message": "A project description is required."},
@@ -84,7 +88,10 @@ class Meta:
     pi = marshmallow.fields.String(
         required=True,
         allow_none=False,
-        validate=marshmallow.validate.Length(min=1, max=255),
+        validate=marshmallow.validate.And(
+            marshmallow.validate.Length(min=1, max=255),
+            dds_web.utils.contains_disallowed_characters,
+        ),
         error_messages={
             "required": {"message": "A principal investigator is required."},
             "null": {"message": "A principal investigator is required."},

dds_web/api/user.py

@@ -198,7 +198,7 @@ def invite_user(email, new_user_role, project=None, unit=None):
 
         # Append invite to unit if applicable
         if new_invite.role in ["Unit Admin", "Unit Personnel"]:
-            # TODO Change / move this later. This is just so that we can add an initial unit admin.
+            # TODO Change / move this later. This is just so that we can add an initial Unit Admin.
             if auth.current_user().role == "Super Admin":
                 if unit:
                     unit_row = models.Unit.query.filter_by(public_id=unit).one_or_none()
@@ -267,7 +267,7 @@ def add_to_project(whom, project, role, send_email=True):
 
         allowed_roles = ["Project Owner", "Researcher"]
 
-        if role not in allowed_roles or whom.role not in allowed_roles:
+        if role not in allowed_roles:
             return {
                 "status": ddserr.AccessDeniedError.code.value,
                 "message": (
@@ -276,6 +276,14 @@ def add_to_project(whom, project, role, send_email=True):
                 ),
             }
 
+        if whom.role not in allowed_roles:
+            return {
+                "status": ddserr.AccessDeniedError.code.value,
+                "message": (
+                    "Users affiliated with units can not be added to projects individually."
+                ),
+            }
+
         is_owner = role == "Project Owner"
         ownership_change = False
 
@@ -539,7 +547,8 @@ def delete(self):
 class UserActivation(flask_restful.Resource):
     """Endpoint to reactivate/deactivate users in the system
 
-    Unit admins can reactivate/deactivate unitusers. Super admins can reactivate/deactivate any user."""
+    Unit Admins can reactivate/deactivate unitusers. Super admins can reactivate/deactivate any user.
+    """
 
     @auth.login_required(role=["Super Admin", "Unit Admin"])
     @logging_bind_request
@@ -558,7 +567,7 @@ def post(self):
 
         # Verify that the action is specified -- reactivate or deactivate
         action = json_input.get("action")
-        if action is None or action == "":
+        if not action:
             raise ddserr.DDSArgumentError(
                 message="Please provide an action 'deactivate' or 'reactivate' for this request."
             )
@@ -567,11 +576,20 @@ def post(self):
         current_user = auth.current_user()
 
         if current_user.role == "Unit Admin":
-            if user.role not in ["Unit Admin", "Unit Personnel"] or current_user.unit != user.unit:
+            # Unit Admin can only activate/deactivate Unit Admins and personnel
+            if user.role not in ["Unit Admin", "Unit Personnel"]:
                 raise ddserr.AccessDeniedError(
                     message=(
-                        f"You are not allowed to {action} this user. As a unit admin, "
-                        f"you're only allowed to {action} users in your unit."
+                        "You can only activate/deactivate users with "
+                        "the role Unit Admin or Unit Personnel."
+                    )
+                )
+
+            if current_user.unit != user.unit:
+                raise ddserr.AccessDeniedError(
+                    message=(
+                        "As a Unit Admin, you can only activate/deactivate other Unit Admins or "
+                        "Unit Personnel within your specific unit."
                     )
                 )
 
@@ -581,7 +599,7 @@ def post(self):
         if (action == "reactivate" and user.is_active) or (
             action == "deactivate" and not user.is_active
         ):
-            raise ddserr.DDSArgumentError(message="User is already in desired state!")
+            raise ddserr.DDSArgumentError(message=f"User is already {action}d!")
 
         # TODO: Check if user has lost access to any projects and if so, grant access again.
         if action == "reactivate":
@@ -631,7 +649,7 @@ def post(self):
 class DeleteUser(flask_restful.Resource):
     """Endpoint to remove users from the system
 
-    Unit admins can delete unitusers. Super admins can delete any user."""
+    Unit Admins can delete Unit Admins and Unit Personnel. Super admins can delete any user."""
 
     @auth.login_required(role=["Super Admin", "Unit Admin"])
     @logging_bind_request
@@ -651,11 +669,15 @@ def delete(self):
         current_user = auth.current_user()
 
         if current_user.role == "Unit Admin":
-            if user.role not in ["Unit Admin", "Unit Personnel"] or current_user.unit != user.unit:
+            if user.role not in ["Unit Admin", "Unit Personnel"]:
+                raise ddserr.UserDeletionError(
+                    message="You can only delete users with the role Unit Admin or Unit Personnel."
+                )
+            if current_user.unit != user.unit:
                 raise ddserr.UserDeletionError(
                     message=(
-                        "You are not allowed to delete this user. As a unit admin, "
-                        "you're only allowed to delete users in your unit."
+                        "As a Unit Admin, you're can only delete Unit Admins "
+                        "and Unit Personnel within your specific unit."
                     )
                 )
 

dds_web/forms.py

@@ -75,7 +75,7 @@ class LoginForm(flask_wtf.FlaskForm):
         validators=[wtforms.validators.InputRequired(), wtforms.validators.Length(1, 64)],
     )
     password = wtforms.PasswordField("Password", validators=[wtforms.validators.InputRequired()])
-    submit = wtforms.SubmitField("Login")
+    submit = wtforms.SubmitField("Log in")
 
 
 class LogoutForm(flask_wtf.FlaskForm):