From d6baa6f27fdf43d629af0e29cb9abf65922f45f0 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Mon, 7 Mar 2022 12:08:42 +0100 Subject: [PATCH 01/39] Fix the dark mode switch behaviour --- dds_web/static/js/dds.js | 65 ++++++++++++++++++++++++------- dds_web/static/scss/dds_auto.scss | 2 +- dds_web/templates/base.html | 10 +++-- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/dds_web/static/js/dds.js b/dds_web/static/js/dds.js index c6b8577b9..a9643ca49 100644 --- a/dds_web/static/js/dds.js +++ b/dds_web/static/js/dds.js @@ -23,22 +23,57 @@ }); })(); -$(function () { - // Initialise Datatables - $(".datatable").DataTable(); +// +// Dark mode switch +// - // Function to switch CSS theme file - $(".theme-switcher").click(function (e) { - var theme = $("#theme-switcher-check").prop("checked") ? "dark" : "light"; - - // Switch the stylesheet - var newlink = "/static/css/dds_" + theme + ".css"; - $("#dds-stylesheet").attr("href", newlink); +// OS set to dark mode +if (window.matchMedia && window.matchMedia("(prefers-color-scheme: dark)").matches) { + // Only continue if we have no cookie set + if (document.cookie.indexOf("ddstheme") == -1) { + // Set the dark mode switch to checked + document.getElementById("theme-switcher-check").checked = true; + document.getElementById("theme-switcher-sun").classList.add("d-none"); + document.getElementById("theme-switcher-moon").classList.remove("d-none"); + } +} +// OS theme changes (unlikely, but nice to support!) +window.matchMedia("(prefers-color-scheme: dark)").addEventListener("change", (e) => { + const newTheme = e.matches ? "dark" : "light"; + // Only continue if we have no cookie set + if (document.cookie.indexOf("ddstheme") == -1) { + // Set the dark mode switch + if (newTheme == "dark") { + document.getElementById("theme-switcher-check").checked = true; + document.getElementById("theme-switcher-sun").classList.add("d-none"); + document.getElementById("theme-switcher-moon").classList.remove("d-none"); + } else { + document.getElementById("theme-switcher-check").checked = false; + document.getElementById("theme-switcher-sun").classList.remove("d-none"); + document.getElementById("theme-switcher-moon").classList.add("d-none"); + } + } +}); - // Toggle the button - $(".theme-switcher label i, .theme-switcher label svg").toggleClass("d-none"); +// Manually set dark / light mode +document.querySelector(".theme-switcher").addEventListener("click", (e) => { + const theme = document.getElementById("theme-switcher-check").checked ? "dark" : "light"; + // Change the CSS for the page + var newlink = "/static/css/dds_" + theme + ".css"; + document.getElementById("dds-stylesheet").setAttribute("href", newlink); + // Toggle the button + document.getElementById("theme-switcher-check").checked = theme == "dark" ? true : false; + document.getElementById("theme-switcher-sun").classList.toggle("d-none"); + document.getElementById("theme-switcher-moon").classList.toggle("d-none"); + // Set a cookie + document.cookie = "ddstheme=" + theme + "; expires=Fri, 31 Dec 9999 23:59:59 GMT; path=/"; + console.log(document.cookie); +}); - // Set a cookie to remember - document.cookie = "ddstheme=" + theme + "; expires=Thu, 2 Dec 2032 12:00:00 UTC; path=/"; - }); +// +// Legacy jQuery code +// +$(function () { + // Initialise Datatables + $(".datatable").DataTable(); }); diff --git a/dds_web/static/scss/dds_auto.scss b/dds_web/static/scss/dds_auto.scss index 944c136c5..a703ac233 100644 --- a/dds_web/static/scss/dds_auto.scss +++ b/dds_web/static/scss/dds_auto.scss @@ -1,2 +1,2 @@ -@import url("dds_light.css"); +@import url("dds_light.css") screen and (prefers-color-scheme: light); @import url("dds_dark.css") screen and (prefers-color-scheme: dark); diff --git a/dds_web/templates/base.html b/dds_web/templates/base.html index 8f0eea065..0ff98b303 100644 --- a/dds_web/templates/base.html +++ b/dds_web/templates/base.html @@ -47,10 +47,12 @@ From 881928ab6e47a90648153a074bbb05df9b2b3d5f Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Mon, 7 Mar 2022 12:15:04 +0100 Subject: [PATCH 02/39] Revert light CSS file loading in auto mode --- dds_web/static/scss/dds_auto.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/static/scss/dds_auto.scss b/dds_web/static/scss/dds_auto.scss index a703ac233..944c136c5 100644 --- a/dds_web/static/scss/dds_auto.scss +++ b/dds_web/static/scss/dds_auto.scss @@ -1,2 +1,2 @@ -@import url("dds_light.css") screen and (prefers-color-scheme: light); +@import url("dds_light.css"); @import url("dds_dark.css") screen and (prefers-color-scheme: dark); From 5e1748cf0ef32af2ec6820b78b95f3a178c2244b Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Mon, 7 Mar 2022 12:27:07 +0100 Subject: [PATCH 03/39] Fix styling incompatability between auto-dark and dark --- dds_web/static/scss/dds_dark.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dds_web/static/scss/dds_dark.scss b/dds_web/static/scss/dds_dark.scss index 5552f0a7e..840a52df5 100644 --- a/dds_web/static/scss/dds_dark.scss +++ b/dds_web/static/scss/dds_dark.scss @@ -70,3 +70,11 @@ // @import "custom"; @import "homepage"; + +// Dark-mode specific overwrites + +// Custom opacity, not set in cookies-dark-mode, not sure why +.bg-light { + --bs-bg-opacity: 0.2; + background-color: rgba(var(--bs-light-rgb), var(--bs-bg-opacity)) !important; +} From cc2a9b0435d90ee68a107ad355cff0c2c0da7e97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Mon, 7 Mar 2022 12:48:55 +0100 Subject: [PATCH 04/39] Use http for status codes --- dds_web/api/project.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 09ec6d2ae..d3e042272 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -5,6 +5,7 @@ #################################################################################################### # Standard Library +import http # Installed import flask_restful @@ -470,6 +471,7 @@ 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"]: @@ -481,7 +483,7 @@ def post(self): 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 +507,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, From 838947925cd7e0fe9e644e5d24ffa36222bdc27e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Mon, 7 Mar 2022 12:49:30 +0100 Subject: [PATCH 05/39] Add missing part --- dds_web/api/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index d3e042272..1e0649216 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -460,7 +460,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: From f26389040e9525f84110cd39bc103e0987ca37fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Mon, 7 Mar 2022 13:00:38 +0100 Subject: [PATCH 06/39] Catch email error so we can get a correct status message for the request --- dds_web/api/project.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 1e0649216..531893ee9 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -475,7 +475,13 @@ def post(self): 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( @@ -483,6 +489,7 @@ def post(self): new_user_role=user.get("role"), project=new_project, ) + if invite_user_result["status"] == http.HTTPStatus.OK: invite_msg = ( f"Invitation sent to {user['email']}. " From 0c0a15fc3f612896cbd57d8c4f5b99561ef898d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Mon, 7 Mar 2022 13:05:31 +0100 Subject: [PATCH 07/39] Changelog note --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d93b13d23..0e639b663 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,3 +37,4 @@ 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)) From 06b140fe8d34eef77633dba9d36d4f09f19ff8d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Mon, 7 Mar 2022 17:07:30 +0100 Subject: [PATCH 08/39] forbid superadmin from creating project --- dds_web/api/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 531893ee9..3e5b2cab4 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -427,7 +427,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 From c07fdcd682a83141e4a44ce9f24345538cbefa6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Mon, 7 Mar 2022 21:07:08 +0100 Subject: [PATCH 09/39] remove bucket --- dds_web/api/api_s3_connector.py | 8 +++++++- dds_web/api/project.py | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index c4579135c..a2f6d3c8c 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -84,11 +84,17 @@ 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_must_exists def remove_multiple(self, items, *args, **kwargs): """Removes all with prefix.""" diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 3e5b2cab4..591b5c4cc 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -398,7 +398,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) From 602e087736640e4d1a926bac280aca39fd510515 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Mon, 7 Mar 2022 21:08:08 +0100 Subject: [PATCH 10/39] clean up bucket --- dds_web/api/api_s3_connector.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index a2f6d3c8c..0f411b05f 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -9,6 +9,7 @@ import traceback import pathlib import json +import gc # Installed import botocore @@ -95,6 +96,10 @@ def remove_bucket(self, *args, **kwargs): # Delete bucket bucket.delete() + # Clean up + del bucket + gc.collect() + @bucket_must_exists def remove_multiple(self, items, *args, **kwargs): """Removes all with prefix.""" From ce9ff1c6b417a9907d73bbbee7c2f6c925537a79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 09:54:49 +0100 Subject: [PATCH 11/39] remove garbage collection --- dds_web/api/api_s3_connector.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dds_web/api/api_s3_connector.py b/dds_web/api/api_s3_connector.py index 0f411b05f..324aec81e 100644 --- a/dds_web/api/api_s3_connector.py +++ b/dds_web/api/api_s3_connector.py @@ -95,10 +95,7 @@ def remove_bucket(self, *args, **kwargs): # Delete bucket bucket.delete() - - # Clean up - del bucket - gc.collect() + bucket = None @bucket_must_exists def remove_multiple(self, items, *args, **kwargs): From d6f93e5b6822d4f71197087d102583a763d2287e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 11:20:14 +0100 Subject: [PATCH 12/39] change delete_folder regex --- dds_web/api/files.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dds_web/api/files.py b/dds_web/api/files.py index 963d500d8..d18884f04 100644 --- a/dds_web/api/files.py +++ b/dds_web/api/files.py @@ -527,11 +527,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"^{folder}(\/[^\/]+)*$"), ) ) .all() ) + except sqlalchemy.exc.SQLAlchemyError as err: raise DatabaseError(message=str(err)) From f6d4664846beb8495637ff7f49dac79e9b5ba642 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 8 Mar 2022 11:57:23 +0100 Subject: [PATCH 13/39] Rewite login page to use component, nicer HTML styling. --- dds_web/forms.py | 2 +- dds_web/templates/components/login_form.html | 35 ++++++------ dds_web/templates/home.html | 5 ++ dds_web/templates/user/login.html | 56 ++++---------------- dds_web/web/root.py | 4 +- 5 files changed, 38 insertions(+), 64 deletions(-) diff --git a/dds_web/forms.py b/dds_web/forms.py index 5b4438f0a..41f81e6b6 100644 --- a/dds_web/forms.py +++ b/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): diff --git a/dds_web/templates/components/login_form.html b/dds_web/templates/components/login_form.html index 7f5ade9e7..7fbe53798 100644 --- a/dds_web/templates/components/login_form.html +++ b/dds_web/templates/components/login_form.html @@ -1,20 +1,16 @@ -{% if login_error_message %} - -{% endif %} -
-

or

-
+ + {{ form.csrf_token }}
- - + {{ form.username(class="form-control" + (" is-invalid" if form.username.errors else "") + " rounded-0 shadow-none", **{"placeholder": form.username.label.text, "autocomplete": "off"}) }} + + {% if form.username.errors %} + {% for error in form.username.errors %} +
{{ error }}
+ {% endfor %} + {% endif %}
@@ -22,8 +18,13 @@
- - + {{ form.password(class="form-control" + (" is-invalid" if form.password.errors else "") + " rounded-0 shadow-none", **{"placeholder": form.password.label.text, "autocomplete": "off"}) }} + + {% if form.password.errors %} + {% for error in form.password.errors %} +
{{ error }}
+ {% endfor %} + {% endif %}
@@ -32,7 +33,7 @@ {% endif %} -

Forgot your password?

+

Forgot your password?

diff --git a/dds_web/templates/home.html b/dds_web/templates/home.html index 4c99477b3..400adae23 100644 --- a/dds_web/templates/home.html +++ b/dds_web/templates/home.html @@ -1,6 +1,11 @@ {% extends 'base.html' %} {% block body %} +
+ For information on how to use the Data Delivery System, as well as to see a testing protocol, see + here. +
+

Welcome to the SciLifeLab Data Delivery System

diff --git a/dds_web/templates/user/login.html b/dds_web/templates/user/login.html index fcf0c1be5..d7f99cb16 100644 --- a/dds_web/templates/user/login.html +++ b/dds_web/templates/user/login.html @@ -6,50 +6,16 @@ {% block body %} - -

- For information on how to use the Data Delivery System, as well as - see a test protocol, go here. +

Please use the form below to log in to the SciLifeLab Data Delivery System.

+
+
+ {% include "components/login_form.html" %} +
+
+

Typically, you will arrive here because you have recieved an email from a unit that uses + the SciLifeLab DDS to deliver data. + These invites will register an account for you if you have not already used the DDS system.

- -
- {{ form.csrf_token }} - - - {{ form.username.label }} - {{ form.username }} -
    - {% for error in form.username.errors %} -
  • - {{ error }} -
    • - {% endfor %} -
- - - {{ form.password.label }} - {{ form.password }} -
    - {% for error in form.password.errors %} -
  • - {{ error }} -
    • - {% endfor %} -
- - - - {{ form.submit }} - - - - Forgot Password? - - - -
- +
+
{% endblock %} diff --git a/dds_web/web/root.py b/dds_web/web/root.py index 25b9ea4cc..b3a221042 100644 --- a/dds_web/web/root.py +++ b/dds_web/web/root.py @@ -5,6 +5,7 @@ """ from flask import Blueprint, render_template, jsonify from flask import current_app as app +from dds_web import forms pages = Blueprint("pages", __name__) @@ -13,7 +14,8 @@ @pages.route("/", methods=["GET"]) def home(): """Home page.""" - return render_template("home.html") + form = forms.LoginForm() + return render_template("home.html", form=form) @pages.route("/status") From 09c98b1a8e6ea1617ad3608b72d5d6395241e009 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Mon, 7 Mar 2022 20:07:27 +0100 Subject: [PATCH 14/39] Added an validator to prevent Smilies and other strange characters from project descriptions --- dds_web/api/schemas/project_schemas.py | 13 ++++++++++--- dds_web/utils.py | 9 +++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/dds_web/api/schemas/project_schemas.py b/dds_web/api/schemas/project_schemas.py index f2b927eb4..ecc38020d 100644 --- a/dds_web/api/schemas/project_schemas.py +++ b/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."}, diff --git a/dds_web/utils.py b/dds_web/utils.py index fc33f9dd3..5582368b1 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -60,6 +60,15 @@ def contains_digit_or_specialchar(input): ) +def contains_disallowed_characters(input): + """Inputs like <9f><98><80> cause issues in Project names etc.""" + disallowed = re.findall(r"[^\w\s]+", input) + if disallowed: + raise marshmallow.ValidationError( + f" {'These characters are not allowed:'.join(set(disallowed))}" + ) + + def email_not_taken(input): """Validator - verify that email is not taken. From 3e0a174b0a4edf707fab718b5b2536595c248402 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 8 Mar 2022 11:57:48 +0100 Subject: [PATCH 15/39] Add validator regex to prevent invalid entries into the project's title, description or principal investigator fields. --- dds_web/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dds_web/utils.py b/dds_web/utils.py index 5582368b1..9783aa453 100644 --- a/dds_web/utils.py +++ b/dds_web/utils.py @@ -64,8 +64,10 @@ def contains_disallowed_characters(input): """Inputs like <9f><98><80> cause issues in Project names etc.""" disallowed = re.findall(r"[^\w\s]+", input) if disallowed: + disallowed = set(disallowed) # unique values + chars = "characters" raise marshmallow.ValidationError( - f" {'These characters are not allowed:'.join(set(disallowed))}" + f"The {chars if len(disallowed) > 1 else chars[:-1]} '{' '.join(disallowed)}' within '[italic]{input}[/italic]' {'are' if len(disallowed) > 1 else 'is'} not allowed." ) From 9b631dbad728491d60123c608f3cccf2c1d644af Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Tue, 8 Mar 2022 12:13:20 +0100 Subject: [PATCH 16/39] Changelog.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e639b663..fd190973f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,3 +38,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - 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)). From daee5c5d76f8fb311a3f3defa22b7ac71b33d2cf Mon Sep 17 00:00:00 2001 From: Zishan Mirza Date: Tue, 8 Mar 2022 12:13:36 +0000 Subject: [PATCH 17/39] Size column This patch hides the "Size" and "total_size" variables according to the role and project status. Signed-off-by: Zishan Mirza --- dds_web/api/project.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 591b5c4cc..8e905a5f6 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -310,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) @@ -334,9 +336,13 @@ def get(self): "usage": total_bhours_db, "cost": total_cost_db, }, - "total_size": total_size, } + if ( + current_user.role == "Researcher" and p.current_status == "Available" + ) or current_user.role != "Researcher": + return_info["total_size"] = total_size + return return_info @staticmethod From dc0fd2061610ae2c47d7dcd8a68a8e1d502a8867 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 8 Mar 2022 13:59:24 +0100 Subject: [PATCH 18/39] Update text on login page, suggested by @inaod568 --- dds_web/templates/user/login.html | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dds_web/templates/user/login.html b/dds_web/templates/user/login.html index d7f99cb16..a7ae9c0dd 100644 --- a/dds_web/templates/user/login.html +++ b/dds_web/templates/user/login.html @@ -12,10 +12,8 @@ {% include "components/login_form.html" %}
-

Typically, you will arrive here because you have recieved an email from a unit that uses - the SciLifeLab DDS to deliver data. - These invites will register an account for you if you have not already used the DDS system. -

+

In order to get a DDS account you need an invitation from one of the SciLifeLab units that use the DDS for data delivery. + Once you have received the invitation, you can register an account and log in to the system.

{% endblock %} From 7356db9d6744869d4ef7c2cea35de56785d7a573 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 8 Mar 2022 14:04:04 +0100 Subject: [PATCH 19/39] Add a big jinja2 comment to remove temporary alert block --- dds_web/templates/home.html | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dds_web/templates/home.html b/dds_web/templates/home.html index 400adae23..82264d938 100644 --- a/dds_web/templates/home.html +++ b/dds_web/templates/home.html @@ -1,6 +1,11 @@ {% extends 'base.html' %} {% block body %} +{# + ######################################################### + ##### TODO: REMOVE THIS ONCE TESTING PERIOD IS OVER ##### + ######################################################### +#}
For information on how to use the Data Delivery System, as well as to see a testing protocol, see here. From 2a9b86129d1f86d6b140c671f7f64cfb5da6d8e6 Mon Sep 17 00:00:00 2001 From: Phil Ewels Date: Tue, 8 Mar 2022 14:11:32 +0100 Subject: [PATCH 20/39] Remove double label tag in login form --- dds_web/templates/components/login_form.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dds_web/templates/components/login_form.html b/dds_web/templates/components/login_form.html index 7fbe53798..c2ba0c732 100644 --- a/dds_web/templates/components/login_form.html +++ b/dds_web/templates/components/login_form.html @@ -5,7 +5,7 @@
{{ form.username(class="form-control" + (" is-invalid" if form.username.errors else "") + " rounded-0 shadow-none", **{"placeholder": form.username.label.text, "autocomplete": "off"}) }} - + {{ form.username.label }} {% if form.username.errors %} {% for error in form.username.errors %}
{{ error }}
@@ -19,7 +19,7 @@
{{ form.password(class="form-control" + (" is-invalid" if form.password.errors else "") + " rounded-0 shadow-none", **{"placeholder": form.password.label.text, "autocomplete": "off"}) }} - + {{ form.password.label }} {% if form.password.errors %} {% for error in form.password.errors %}
{{ error }}
From 523d9093481d46d162316c373b7486907758774a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 17:22:36 +0100 Subject: [PATCH 21/39] clarify in user activation --- dds_web/api/user.py | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 1bbf2e48b..a185a4203 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -539,7 +539,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 +559,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 +568,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=( + "You can only activate/deactivate users with " + "the role Unit Admin or Unit Personnel." + ) + ) + + if current_user.unit != user.unit: 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." + "As a Unit Admin, you can only activate/deactivate other Unit Admins or " + "Unit Personnel within your specific unit." ) ) @@ -581,7 +591,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": From 3dd8017a45ca51a0398bb2668429b1401f157862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 17:31:09 +0100 Subject: [PATCH 22/39] change the message returned from user deletion --- dds_web/api/user.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index a185a4203..ab680e4b3 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -641,7 +641,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. Super admins can delete any user.""" @auth.login_required(role=["Super Admin", "Unit Admin"]) @logging_bind_request @@ -661,11 +661,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 only allowed to delete Unit Admins " + "and Unit Personnel within your specific unit." ) ) From 9e58313d7791fdadca596b8d04b9ab9cc8176c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 17:31:48 +0100 Subject: [PATCH 23/39] slight change --- dds_web/api/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index ab680e4b3..b6981cf88 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -668,7 +668,7 @@ def delete(self): if current_user.unit != user.unit: raise ddserr.UserDeletionError( message=( - "As a unit admin, you're only allowed to delete Unit Admins " + "As a unit admin, you're can only delete Unit Admins " "and Unit Personnel within your specific unit." ) ) From 4efbc75a811349c8d567349f8fd86987f836bf30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 17:35:58 +0100 Subject: [PATCH 24/39] fix tests --- tests/test_user_activation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_user_activation.py b/tests/test_user_activation.py index 3823f1b7c..71f35c7b7 100644 --- a/tests/test_user_activation.py +++ b/tests/test_user_activation.py @@ -71,7 +71,7 @@ def test_deactivate_deactivated_user_as_superadmin(module_client): json={**user, "action": "deactivate"}, ) assert response.status_code == http.HTTPStatus.BAD_REQUEST - assert "User is already in desired state!" in response.json["message"] + assert "User is already in deactivated!" in response.json["message"] def test_reactivate_user_as_superadmin(module_client): @@ -120,7 +120,7 @@ def test_deactivate_user_as_unitadmin(module_client): ) assert response.status_code == http.HTTPStatus.FORBIDDEN assert ( - "You are not allowed to deactivate this user. As a unit admin, you're only allowed to deactivate users in your unit." + "You can only activate/deactivate users with the role Unit Admin or Unit Personnel" in response.json["message"] ) From b6270117c695b75628c7e65dcd1085d823a8188b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Tue, 8 Mar 2022 17:44:42 +0100 Subject: [PATCH 25/39] typo --- tests/test_user_activation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_user_activation.py b/tests/test_user_activation.py index 71f35c7b7..0d8e641d3 100644 --- a/tests/test_user_activation.py +++ b/tests/test_user_activation.py @@ -71,7 +71,7 @@ def test_deactivate_deactivated_user_as_superadmin(module_client): json={**user, "action": "deactivate"}, ) assert response.status_code == http.HTTPStatus.BAD_REQUEST - assert "User is already in deactivated!" in response.json["message"] + assert "User is already deactivated!" in response.json["message"] def test_reactivate_user_as_superadmin(module_client): From 23869d11fe7182331690171daf74b2a3478c5f6b Mon Sep 17 00:00:00 2001 From: Zishan Mirza Date: Wed, 9 Mar 2022 08:01:58 +0000 Subject: [PATCH 26/39] Size column This patch updates "CHANGELOG.md" and switches the condition in the "if" statement to "total_size". Signed-off-by: Zishan Mirza --- CHANGELOG.md | 1 + dds_web/api/project.py | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd190973f..febd49ea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,3 +39,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - 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)). +- Hides the "Size" and "total_size" variables according to the role and project status ([#1032](https://github.com/ScilifelabDataCentre/dds_web/pull/1032)). diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 8e905a5f6..4f20db612 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -338,9 +338,7 @@ def get(self): }, } - if ( - current_user.role == "Researcher" and p.current_status == "Available" - ) or current_user.role != "Researcher": + if total_size: return_info["total_size"] = total_size return return_info From d34b378bfffde5591b1a15e395f35d919e377f77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 9 Mar 2022 09:28:25 +0100 Subject: [PATCH 27/39] Fix regex for file rm/ls --- dds_web/api/files.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dds_web/api/files.py b/dds_web/api/files.py index d18884f04..70c3f429f 100644 --- a/dds_web/api/files.py +++ b/dds_web/api/files.py @@ -6,6 +6,7 @@ # Standard library import os +import re # Installed import flask_restful @@ -340,8 +341,9 @@ def items_in_subpath(project, folder="."): else: # Get distinct sub folders in specific folder with regex # Match / 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 +520,7 @@ def delete_folder(self, project, folder): """Delete all items in folder""" exists = False names_in_bucket = [] + folder = re.escape(folder) try: # File names in root files = ( @@ -527,7 +530,7 @@ 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"^{folder}(/[^/]+)*$"), ) ) .all() From bd2fc37086aa06168cad1a79c41717e6032aebbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 9 Mar 2022 09:38:06 +0100 Subject: [PATCH 28/39] Don't overwrite the original variable --- CHANGELOG.md | 1 + dds_web/api/files.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd190973f..706851d8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,3 +39,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - 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) diff --git a/dds_web/api/files.py b/dds_web/api/files.py index 70c3f429f..4bed9c18e 100644 --- a/dds_web/api/files.py +++ b/dds_web/api/files.py @@ -520,7 +520,7 @@ def delete_folder(self, project, folder): """Delete all items in folder""" exists = False names_in_bucket = [] - folder = re.escape(folder) + re_folder = re.escape(folder) try: # File names in root files = ( @@ -530,7 +530,7 @@ 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() From 9b2dd5564e0f04c7f606269dec9f468f52ccc19e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 9 Mar 2022 11:21:52 +0100 Subject: [PATCH 29/39] Remove trailing / in infolder --- dds_web/api/files.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dds_web/api/files.py b/dds_web/api/files.py index 4bed9c18e..459067bfd 100644 --- a/dds_web/api/files.py +++ b/dds_web/api/files.py @@ -311,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( @@ -520,6 +522,8 @@ 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 From 418ad7367ae5b3cd3a2622fd4e44ec49cddc40ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20=C3=96stberg?= Date: Wed, 9 Mar 2022 11:25:00 +0100 Subject: [PATCH 30/39] Fix ' --- dds_web/api/files.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dds_web/api/files.py b/dds_web/api/files.py index 459067bfd..6b0d4d7c4 100644 --- a/dds_web/api/files.py +++ b/dds_web/api/files.py @@ -311,7 +311,7 @@ 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] == '/': + if folder[-1] == "/": folder = folder[:-1] try: # All files in project @@ -522,7 +522,7 @@ def delete_folder(self, project, folder): """Delete all items in folder""" exists = False names_in_bucket = [] - if folder[-1] == '/': + if folder[-1] == "/": folder = folder[:-1] re_folder = re.escape(folder) try: From b25e97ef2090db3dbcd7ca61f09fa3d77f152660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:03:44 +0100 Subject: [PATCH 31/39] fix the Unit Admin vs Unit admin --- dds_web/api/user.py | 2 +- tests/test_project_access.py | 6 +++--- tests/test_project_user_keys.py | 2 +- tests/test_user_delete.py | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index b6981cf88..13b7ba57c 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -568,7 +568,7 @@ def post(self): current_user = auth.current_user() if current_user.role == "Unit Admin": - # Unit admin can only activate/deactivate Unit admins and personnel + # Unit Admin can only activate/deactivate Unit admins and personnel if user.role not in ["Unit Admin", "Unit Personnel"]: raise ddserr.AccessDeniedError( message=( diff --git a/tests/test_project_access.py b/tests/test_project_access.py index 92a46860d..666aabae1 100644 --- a/tests/test_project_access.py +++ b/tests/test_project_access.py @@ -309,7 +309,7 @@ def test_fix_access_unitpersonnel_valid_email_unitpersonnel(client): def test_fix_access_unitadmin_valid_email_researcher(client): - """Unit admin giving access to researcher - ok.""" + """Unit Admin giving access to researcher - ok.""" project = models.Project.query.filter_by(public_id="public_project_id").one_or_none() assert project @@ -345,7 +345,7 @@ def test_fix_access_unitadmin_valid_email_researcher(client): def test_fix_access_unitadmin_valid_email_projectowner(client): - """Unit admin giving access to project owner - ok.""" + """Unit Admin giving access to project owner - ok.""" project = models.Project.query.filter_by(public_id="public_project_id").one_or_none() assert project @@ -381,7 +381,7 @@ def test_fix_access_unitadmin_valid_email_projectowner(client): def test_fix_access_unitadmin_valid_email_unituser(client): - """Unit admin giving access to unituser - ok.""" + """Unit Admin giving access to unituser - ok.""" project = models.Project.query.filter_by(public_id="public_project_id").one_or_none() assert project diff --git a/tests/test_project_user_keys.py b/tests/test_project_user_keys.py index 45438dfcc..81d2e748d 100644 --- a/tests/test_project_user_keys.py +++ b/tests/test_project_user_keys.py @@ -254,7 +254,7 @@ def test_project_key_sharing(client): def test_delete_user_deletes_project_user_keys(client): - """Unit admin deletes unit user""" + """Unit Admin deletes unit user""" email_to_delete = "unituser2@mailtrap.io" diff --git a/tests/test_user_delete.py b/tests/test_user_delete.py index b04dbdb02..65396660f 100644 --- a/tests/test_user_delete.py +++ b/tests/test_user_delete.py @@ -193,7 +193,7 @@ def test_del_request_others_unprivileged(client): def test_del_request_others_researcher(client): - """Unit admin tries to delete research user""" + """Unit Admin tries to delete research user""" email_to_delete = "researchuser@mailtrap.io" @@ -213,7 +213,7 @@ def test_del_request_others_researcher(client): def test_del_request_others_researcher(client): - """Unit admin tries to delete unit user from different unit""" + """Unit Admin tries to delete unit user from different unit""" email_to_delete = "unituser1@mailtrap.io" @@ -233,7 +233,7 @@ def test_del_request_others_researcher(client): def test_del_request_others_self(client): - """Unit admin tries to instantly self-delete via this endpoint""" + """Unit Admin tries to instantly self-delete via this endpoint""" email_to_delete = "delete_me_unitadmin@mailtrap.io" @@ -253,7 +253,7 @@ def test_del_request_others_self(client): def test_del_request_others_success(client): - """Unit admin deletes unit user""" + """Unit Admin deletes unit user""" email_to_delete = "delete_me_unituser@mailtrap.io" From c193a9f0260fdf76a256e9106bcd0e321d87e236 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:33:20 +0100 Subject: [PATCH 32/39] missed the admins in plural --- .env | 2 +- dds_web/api/user.py | 6 +++--- dds_web/config.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.env b/.env index b61d2f9bd..ba12b883b 100644 --- a/.env +++ b/.env @@ -14,4 +14,4 @@ DDS_LOG_DIR="./run_dir/log" DDS_UPLOAD_TEST="./run_dir/upload" DDS_DOWNLOAD_TEST="./run_dir/download" DDS_TEMP_CACHE_TEST="./run_dir/cache_test" -DDS_DB_TYPE="dev-small" # "dev-big production" +DDS_DB_TYPE="production"# "dev-small" # "dev-big production" diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 13b7ba57c..970371ed3 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -539,7 +539,7 @@ 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"]) @@ -568,7 +568,7 @@ def post(self): current_user = auth.current_user() if current_user.role == "Unit Admin": - # Unit Admin can only activate/deactivate Unit admins and personnel + # Unit Admin can only activate/deactivate Unit Admins and personnel if user.role not in ["Unit Admin", "Unit Personnel"]: raise ddserr.AccessDeniedError( message=( @@ -641,7 +641,7 @@ def post(self): class DeleteUser(flask_restful.Resource): """Endpoint to remove users from the system - Unit admins can delete Unit Admins. Super admins can delete any user.""" + Unit Admins can delete Unit Admins. Super admins can delete any user.""" @auth.login_required(role=["Super Admin", "Unit Admin"]) @logging_bind_request diff --git a/dds_web/config.py b/dds_web/config.py index e0924e200..49d1017a1 100644 --- a/dds_web/config.py +++ b/dds_web/config.py @@ -57,7 +57,7 @@ class Config(object): MAIL_USE_SSL = False MAIL_DEFAULT_SENDER = "dds@noreply.se" - TOKEN_ENDPOINT_ACCESS_LIMIT = "10/hour" + TOKEN_ENDPOINT_ACCESS_LIMIT = "1/hour" RATELIMIT_STORAGE_URI = os.environ.get( "RATELIMIT_STORAGE_URI", "memory://" ) # Use in devel only! Use Redis or memcached in prod From 1df294a8540b7e807958d468fe2340eee0fdcf5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:34:24 +0100 Subject: [PATCH 33/39] unit admins as well --- dds_web/api/user.py | 4 ++-- tests/test_flow_invite_to_access.py | 2 +- tests/test_project_access.py | 4 ++-- tests/test_user_activation.py | 6 +++--- tests/test_user_delete.py | 2 +- tests/test_user_info.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 970371ed3..207f78541 100644 --- a/dds_web/api/user.py +++ b/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() @@ -668,7 +668,7 @@ def delete(self): if current_user.unit != user.unit: raise ddserr.UserDeletionError( message=( - "As a unit admin, you're can only delete Unit Admins " + "As a Unit Admin, you're can only delete Unit Admins " "and Unit Personnel within your specific unit." ) ) diff --git a/tests/test_flow_invite_to_access.py b/tests/test_flow_invite_to_access.py index f3d3d08ab..3153d445d 100644 --- a/tests/test_flow_invite_to_access.py +++ b/tests/test_flow_invite_to_access.py @@ -335,7 +335,7 @@ def test_invite_to_register_researcher_with_project_as_owner_by_project_owner(cl assert user.project_user_keys[0].project_id == project.id -# Unituser invited by unit admin ################################# Unituser invited by unit admin # +# Unituser invited by Unit Admin ################################# Unituser invited by Unit Admin # def test_invite_to_register_unituser_by_unitadmin(client): diff --git a/tests/test_project_access.py b/tests/test_project_access.py index 666aabae1..f66b2354e 100644 --- a/tests/test_project_access.py +++ b/tests/test_project_access.py @@ -119,7 +119,7 @@ def test_fix_access_projectowner_valid_email_invalid_otheruser(client): def test_fix_access_unituser_valid_email_invalid_otheruser(client): - """Unit user giving access to unit admin - no permissions.""" + """Unit user giving access to Unit Admin - no permissions.""" token = tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(client) response = client.post( tests.DDSEndpoint.PROJECT_ACCESS, @@ -132,7 +132,7 @@ def test_fix_access_unituser_valid_email_invalid_otheruser(client): def test_fix_access_user_trying_themselves(client): - """Unit user giving access to unit admin - no permissions.""" + """Unit user giving access to Unit Admin - no permissions.""" token = tests.UserAuth(tests.USER_CREDENTIALS["projectowner"]).token(client) response = client.post( tests.DDSEndpoint.PROJECT_ACCESS, diff --git a/tests/test_user_activation.py b/tests/test_user_activation.py index 0d8e641d3..ee80f1d1e 100644 --- a/tests/test_user_activation.py +++ b/tests/test_user_activation.py @@ -104,7 +104,7 @@ def test_reactivate_user_as_superadmin(module_client): def test_deactivate_user_as_unitadmin(module_client): - """Deactivate researchuser as unit admin""" + """Deactivate researchuser as Unit Admin""" # Try to get token as user that is to be deactivated response = module_client.get( tests.DDSEndpoint.ENCRYPTED_TOKEN, @@ -126,7 +126,7 @@ def test_deactivate_user_as_unitadmin(module_client): def test_deactivate_unituser_as_unitadmin(module_client): - """Deactivate unit user as unit admin""" + """Deactivate unit user as Unit Admin""" # Try to get token as user that is to be deactivated response = module_client.get( tests.DDSEndpoint.ENCRYPTED_TOKEN, @@ -167,7 +167,7 @@ def test_deactivate_unituser_as_unitadmin(module_client): def test_reactivate_unituser_as_unitadmin(module_client): - """Reactivate unituser as unit admin""" + """Reactivate unituser as Unit Admin""" # Try to get token as user that is to be deactivated response = module_client.get( tests.DDSEndpoint.ENCRYPTED_TOKEN, diff --git a/tests/test_user_delete.py b/tests/test_user_delete.py index 65396660f..fa7e35ba2 100644 --- a/tests/test_user_delete.py +++ b/tests/test_user_delete.py @@ -271,7 +271,7 @@ def test_del_request_others_success(client): def test_del_request_others_superaction(client): - """Super admin deletes unit admin""" + """Super admin deletes Unit Admin""" email_to_delete = "delete_me_unitadmin@mailtrap.io" diff --git a/tests/test_user_info.py b/tests/test_user_info.py index 15e4fa39b..d5e8b8bbf 100644 --- a/tests/test_user_info.py +++ b/tests/test_user_info.py @@ -8,7 +8,7 @@ def test_get_info_unit_user(client): - """Get info for unit user/unit admin""" + """Get info for unit user/Unit Admin""" response = client.get( tests.DDSEndpoint.USER_INFO, From 1a2dd44226f40a63b38f57e161ce82c495e008a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:44:11 +0100 Subject: [PATCH 34/39] remove production --- .env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env b/.env index ba12b883b..b61d2f9bd 100644 --- a/.env +++ b/.env @@ -14,4 +14,4 @@ DDS_LOG_DIR="./run_dir/log" DDS_UPLOAD_TEST="./run_dir/upload" DDS_DOWNLOAD_TEST="./run_dir/download" DDS_TEMP_CACHE_TEST="./run_dir/cache_test" -DDS_DB_TYPE="production"# "dev-small" # "dev-big production" +DDS_DB_TYPE="dev-small" # "dev-big production" From 0af11a7112bc11f0963c1bda392237e64ca86d7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:45:37 +0100 Subject: [PATCH 35/39] forgot personnel --- dds_web/api/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 207f78541..3183ab277 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -641,7 +641,7 @@ def post(self): class DeleteUser(flask_restful.Resource): """Endpoint to remove users from the system - Unit Admins can delete Unit Admins. 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 From fe8cab2bb7214a502205e388b3af8c180644fefc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= Date: Wed, 9 Mar 2022 13:46:30 +0100 Subject: [PATCH 36/39] restore to 10 --- dds_web/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/config.py b/dds_web/config.py index 49d1017a1..e0924e200 100644 --- a/dds_web/config.py +++ b/dds_web/config.py @@ -57,7 +57,7 @@ class Config(object): MAIL_USE_SSL = False MAIL_DEFAULT_SENDER = "dds@noreply.se" - TOKEN_ENDPOINT_ACCESS_LIMIT = "1/hour" + TOKEN_ENDPOINT_ACCESS_LIMIT = "10/hour" RATELIMIT_STORAGE_URI = os.environ.get( "RATELIMIT_STORAGE_URI", "memory://" ) # Use in devel only! Use Redis or memcached in prod From 6f1f0bbdd7bdc7e1735fef00f862159035932a15 Mon Sep 17 00:00:00 2001 From: Zishan Mirza Date: Wed, 9 Mar 2022 12:52:29 +0000 Subject: [PATCH 37/39] Size column This patch adds roles to the condition of the "if" statement. Signed-off-by: Zishan Mirza --- dds_web/api/project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 4f20db612..d0d3a41d1 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -338,7 +338,7 @@ def get(self): }, } - if total_size: + if total_size or current_user.role in ["Unit Admin", "Unit Personnel"]: return_info["total_size"] = total_size return return_info From 84c9d4c7f0fa1db369e71ac96e6e60f9d886efa8 Mon Sep 17 00:00:00 2001 From: Matthias Zepper Date: Wed, 9 Mar 2022 15:45:50 +0100 Subject: [PATCH 38/39] UX Improvement: Distinct error message if the user to be added to a project is a unit user. --- CHANGELOG.md | 4 ++++ dds_web/api/user.py | 10 +++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 759ed53b5..191004673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,3 +41,7 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe - 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. diff --git a/dds_web/api/user.py b/dds_web/api/user.py index 1bbf2e48b..c0d1f73a2 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -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 From fcc56e50c875af4f2913d08f169c812bb4bbd988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ina=20Od=C3=A9n=20=C3=96sterbo?= <35953392+inaod568@users.noreply.github.com> Date: Wed, 9 Mar 2022 17:47:20 +0100 Subject: [PATCH 39/39] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 191004673..bedcb179c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,4 +44,4 @@ Please add a _short_ line describing the PR you make, if the PR implements a spe ## Sprint (2022-03-09 - 2022-03-23) -- Introduce a separate error message if someone tried to add an unit user to projects individually. +- 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))