diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d9b86cf5f..30c19b0dc 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,15 @@ Changelog ========== +.. _2.5.2: + +2.5.2 - 2023-10-25 +~~~~~~~~~~~~~~~~~~~~~ + +- Users can revoke project access given to unaccepted invites (e.g. after a mistake). +- Email layout changed. When project is released, important information is now highlighted, and the Project Title is displayed along with the DDS project ID. +- New endpoint `ProjectStatus.patch`: Unit Admins / Personnel can extend the project deadline. + .. _2.5.1: 2.5.1 - 2023-09-27 diff --git a/SPRINTLOG.md b/SPRINTLOG.md index 0844d56b3..e56b41d8d 100644 --- a/SPRINTLOG.md +++ b/SPRINTLOG.md @@ -303,6 +303,19 @@ _Nothing merged in CLI during this sprint_ # 2023-09-18 - 2023-09-29 -- Column `sto4_start_time` is automatically set when the create-unit command is run ([#1668])(https://scilifelab.atlassian.net/jira/software/projects/DDS/boards/13?selectedIssue=DDS-1668) +- Column `sto4_start_time` is automatically set when the create-unit command is run ([#1469](https://github.com/ScilifelabDataCentre/dds_web/pull/1469)) - Replace expired invites when there's a new invitation attempt ([#1466](https://github.com/ScilifelabDataCentre/dds_web/pull/1466)) - New version: 2.5.1 ([#1471](https://github.com/ScilifelabDataCentre/dds_web/pull/1471)) +- Revoke project access for unaccepted invites ([#1468](https://github.com/ScilifelabDataCentre/dds_web/pull/1468)) + +# 2023-10-02 - 2023-10-13 + +- Project title displayed along with the internal project ID email sent when a project is released ([#1475](https://github.com/ScilifelabDataCentre/dds_web/pull/1475)) +- Use full DDS name in MOTD email subject ([#1477](https://github.com/ScilifelabDataCentre/dds_web/pull/1477)) +- Add flag --verify-checksum to the comand in email template ([#1478])(https://github.com/ScilifelabDataCentre/dds_web/pull/1478) +- Improved email layout; Highlighted information and commands when project is released ([#1479])(https://github.com/ScilifelabDataCentre/dds_web/pull/1479) + +# 2023-10-16 - 2023-10-27 + +- Added new API endpoint ProjectStatus.patch to extend the deadline ([#1480])(https://github.com/ScilifelabDataCentre/dds_web/pull/1480) +- New version: 2.5.2 ([#1482](https://github.com/ScilifelabDataCentre/dds_web/pull/1482)) diff --git a/dds_web/api/project.py b/dds_web/api/project.py index 4b00c8042..ffe2c95ef 100644 --- a/dds_web/api/project.py +++ b/dds_web/api/project.py @@ -188,6 +188,135 @@ def post(self): return {"message": return_message} + @auth.login_required(role=["Unit Admin", "Unit Personnel"]) + @logging_bind_request + @json_required + @handle_validation_errors + @handle_db_error + def patch(self): + """Partially update a the project status""" + # Get project ID, project and verify access + project_id = dds_web.utils.get_required_item(obj=flask.request.args, req="project") + project = dds_web.utils.collect_project(project_id=project_id) + dds_web.utils.verify_project_access(project=project) + + # Get json input from request + json_input = flask.request.get_json(silent=True) # Already checked by json_required + + # the status has changed at least two times, + # next time the project expires it wont change again -> error + if project.times_expired >= 2: + raise DDSArgumentError( + "Project availability limit: The maximum number of changes in data availability has been reached." + ) + + # Operation must be confirmed by the user - False by default + confirmed_operation = json_input.get("confirmed", False) + if not isinstance(confirmed_operation, bool): + raise DDSArgumentError(message="`confirmed` is a boolean value: True or False.") + if not confirmed_operation: + warning_message = "Operation must be confirmed before proceding." + # When not confirmed, return information about the project + project_info = ProjectInfo().get() + project_status = self.get() + json_returned = { + **project_info, + "project_status": project_status, + "warning": warning_message, + "default_unit_days": project.responsible_unit.days_in_available, + } + return json_returned + + # Cannot change project status if project is busy + if project.busy: + raise ProjectBusyError( + message=( + f"The deadline for the project '{project_id}' is already in the process of being changed. " + "Please try again later. \n\nIf you know that the project is not busy, contact support." + ) + ) + + self.set_busy(project=project, busy=True) + + # Extend deadline + try: + new_deadline_in = json_input.get( + "new_deadline_in", None + ) # if not provided --> is None -> deadline is not updated + + # some variable definition + send_email = False + default_unit_days = project.responsible_unit.days_in_available + + # Update the deadline functionality + if new_deadline_in: + # deadline can only be extended from Available + if not project.current_status == "Available": + raise DDSArgumentError( + "You can only extend the deadline for a project that has the status 'Available'." + ) + + if type(new_deadline_in) is not int: + raise DDSArgumentError( + message="The deadline attribute passed should be of type Int (i.e a number)." + ) + + # New deadline shouldnt surpass the default unit days + if new_deadline_in > default_unit_days: + raise DDSArgumentError( + message=f"You requested the deadline to be extended {new_deadline_in} days. The number of days has to be lower than the default deadline extension number of {default_unit_days} days" + ) + + # the new deadline + days left shouldnt surpass 90 days + curr_date = dds_web.utils.current_time() + current_deadline = (project.current_deadline - curr_date).days + if new_deadline_in + current_deadline > 90: + raise DDSArgumentError( + message=f"You requested the deadline to be extended with {new_deadline_in} days (from {current_deadline}), giving a new total deadline of {new_deadline_in + current_deadline} days. The new deadline needs to be less than (or equal to) 90 days." + ) + try: + # add a fake expire status to mimick a re-release in order to have an udpated deadline + curr_date = ( + dds_web.utils.current_time() + ) # call current_time before each call so it is stored with different timestamps + new_status_row = self.expire_project( + project=project, + current_time=curr_date, + deadline_in=1, # some dummy deadline bc it will re-release now again + ) + project.project_statuses.append(new_status_row) + + curr_date = ( + dds_web.utils.current_time() + ) # call current_time before each call so it is stored with different timestamps + new_status_row = self.release_project( + project=project, + current_time=curr_date, + deadline_in=new_deadline_in + current_deadline, + ) + project.project_statuses.append(new_status_row) + + project.busy = False # return to not busy + db.session.commit() + + except (sqlalchemy.exc.OperationalError, sqlalchemy.exc.SQLAlchemyError) as err: + flask.current_app.logger.exception("Failed to extend deadline") + db.session.rollback() + raise + + return_message = ( + f"The project '{project.public_id}' has been given a new deadline. " + f"An e-mail notification has{' not ' if not send_email else ' '}been sent." + ) + else: + # leave it for future new functionality of updating the status + return_message = "Nothing to update." + except: + self.set_busy(project=project, busy=False) + raise + + return {"message": return_message} + @staticmethod @dbsession def set_busy(project: models.Project, busy: bool) -> None: diff --git a/dds_web/api/superadmin_only.py b/dds_web/api/superadmin_only.py index 44fae0fa0..3c73305f1 100644 --- a/dds_web/api/superadmin_only.py +++ b/dds_web/api/superadmin_only.py @@ -164,7 +164,7 @@ def post(self): # Create email content # put motd_obj.message etc in there etc - subject: str = "DDS Important Information" + subject: str = "Important Information: Data Delivery System" body: str = flask.render_template(f"mail/motd.txt", motd=motd_obj.message) html = flask.render_template(f"mail/motd.html", motd=motd_obj.message) diff --git a/dds_web/api/user.py b/dds_web/api/user.py index b0800c20b..35ec90405 100644 --- a/dds_web/api/user.py +++ b/dds_web/api/user.py @@ -423,6 +423,7 @@ def compose_and_send_email_to_user(userobj, mail_type, link=None, project=None): unit_email = None project_id = None + project_title = None deadline = None # Don't display unit admins or personnels name @@ -440,6 +441,7 @@ def compose_and_send_email_to_user(userobj, mail_type, link=None, project=None): elif mail_type == "project_release": subject = f"Project made available by {displayed_sender} in the SciLifeLab Data Delivery System" project_id = project.public_id + project_title = project.title deadline = project.current_deadline.astimezone(datetime.timezone.utc).strftime( "%Y-%m-%d %H:%M:%S %Z" ) @@ -470,6 +472,7 @@ def compose_and_send_email_to_user(userobj, mail_type, link=None, project=None): displayed_sender=displayed_sender, unit_email=unit_email, project_id=project_id, + project_title=project_title, deadline=deadline, ) msg.html = flask.render_template( @@ -478,6 +481,7 @@ def compose_and_send_email_to_user(userobj, mail_type, link=None, project=None): displayed_sender=displayed_sender, unit_email=unit_email, project_id=project_id, + project_title=project_title, deadline=deadline, ) @@ -864,7 +868,7 @@ def delete_invite(email): class RemoveUserAssociation(flask_restful.Resource): - @auth.login_required(role=["Unit Admin", "Unit Personnel", "Project Owner", "Researcher"]) + @auth.login_required(role=["Unit Admin", "Unit Personnel", "Project Owner"]) @logging_bind_request @json_required @handle_validation_errors @@ -876,34 +880,81 @@ def post(self): if not (user_email := json_input.get("email")): raise ddserr.DDSArgumentError(message="User email missing.") - # Check if email is registered to a user + # Check if the user exists or has a pending invite try: existing_user = user_schemas.UserSchema().load({"email": user_email}) + unanswered_invite = user_schemas.UnansweredInvite().load({"email": user_email}) except sqlalchemy.exc.OperationalError as err: raise ddserr.DatabaseError(message=str(err), alt_message="Unexpected database error.") - if not existing_user: + # If the user doesn't exist and doesn't have a pending invite + if not existing_user and not unanswered_invite: raise ddserr.NoSuchUserError( - f"The user with email '{user_email}' does not have access to the specified project." - " Cannot remove non-existent project access." + f"The user / invite with email '{user_email}' does not have access to the specified project. " + "Cannot remove non-existent project access." ) - user_in_project = False - for user_association in project.researchusers: - if user_association.user_id == existing_user.username: - user_in_project = True - db.session.delete(user_association) - project_user_key = models.ProjectUserKeys.query.filter_by( - project_id=project.id, user_id=existing_user.username - ).first() - if project_user_key: - db.session.delete(project_user_key) - - if not user_in_project: - raise ddserr.NoSuchUserError( - f"The user with email '{user_email}' does not have access to the specified project." - " Cannot remove non-existent project access." - ) + if unanswered_invite: + # If there is a unit_id value, it means the invite was associated to a unit + # i.e The invite is for a Unit Personel which shouldn't be removed from individual projects + if unanswered_invite.unit_id: + raise ddserr.UserDeletionError( + "Cannot remove a Unit Admin / Unit Personnel from individual projects." + ) + + invite_id = unanswered_invite.id + + # Check if the unanswered invite is associated with the project + project_invite_key = models.ProjectInviteKeys.query.filter_by( + invite_id=invite_id, project_id=project.id + ).one_or_none() + + if project_invite_key: + msg = ( + f"Invited user is no longer associated with the project '{project.public_id}'." + ) + # Remove the association if it exists + db.session.delete(project_invite_key) + + # Check if the invite is associated with only one project, if it is -> delete the invite + project_invite_key = models.ProjectInviteKeys.query.filter_by( + invite_id=invite_id + ).one_or_none() + + if not project_invite_key: + db.session.delete(unanswered_invite) + else: + # The unanswered invite is not associated with the project + raise ddserr.NoSuchUserError( + f"The invite with email '{user_email}' does not have access to the specified project. " + "Cannot remove non-existent project access." + ) + + else: + if auth.current_user().username == existing_user.username: + raise ddserr.AccessDeniedError(message="You cannot revoke your own access.") + + # Search the user in the project, when found delete from the database all references to them + user_in_project = False + for ( + user_association + ) in project.researchusers: # TODO Possible optimization -> comprehesion list + if user_association.user_id == existing_user.username: + user_in_project = True + db.session.delete(user_association) + project_user_key = models.ProjectUserKeys.query.filter_by( + project_id=project.id, user_id=existing_user.username + ).first() + if project_user_key: + db.session.delete(project_user_key) + break + + if not user_in_project: + raise ddserr.NoSuchUserError( + f"The user with email '{user_email}' does not have access to the specified project. " + "Cannot remove non-existent project access." + ) + msg = f"User with email {user_email} no longer associated with {project.public_id}." try: db.session.commit() @@ -924,13 +975,7 @@ def post(self): ), ) from err - flask.current_app.logger.debug( - f"User {existing_user.username} no longer associated with project {project.public_id}." - ) - - return { - "message": f"User with email {user_email} no longer associated with {project.public_id}." - } + return {"message": msg} class EncryptedToken(flask_restful.Resource): diff --git a/dds_web/database/models.py b/dds_web/database/models.py index 104fa932d..8d27ea779 100644 --- a/dds_web/database/models.py +++ b/dds_web/database/models.py @@ -195,7 +195,7 @@ class Unit(db.Model): sto2_access = db.Column(db.String(255), unique=False, nullable=True) # unique=True later sto2_secret = db.Column(db.String(255), unique=False, nullable=True) # unique=True later - # New safespring storage + # New safespring storage - NOTE: MAKE SURE IPS ARE WHITELISTED ON UPPMAX AND OTHER SERVERS sto4_start_time = db.Column(db.DateTime(), nullable=True) sto4_endpoint = db.Column(db.String(255), unique=False, nullable=True) # unique=True later sto4_name = db.Column(db.String(255), unique=False, nullable=True) # unique=True later diff --git a/dds_web/templates/mail/mail_base.html b/dds_web/templates/mail/mail_base.html index d0dd52e7f..a9531bcb7 100644 --- a/dds_web/templates/mail/mail_base.html +++ b/dds_web/templates/mail/mail_base.html @@ -1,6 +1,23 @@ - + + @@ -122,6 +139,10 @@ border-color: #a00202 !important; } } + code { + background-color: pink; + color: black; +} diff --git a/dds_web/templates/mail/project_release.html b/dds_web/templates/mail/project_release.html index 24262a413..06d536664 100644 --- a/dds_web/templates/mail/project_release.html +++ b/dds_web/templates/mail/project_release.html @@ -10,24 +10,37 @@ -

The - project {{project_id}} is now available for your access in the SciLifeLab Data Delivery System (DDS).

-

- The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple - way.

+

+ The following project is now available for your access in the SciLifeLab Data Delivery System (DDS) and you can download your data. +

+

+ You were added to this project {% if unit_email %} on behalf of {% else %} by {% endif %} {{displayed_sender}}. +

+

+ To list the files in this project, run:
+ dds ls -p {{project_id}}

+
+

+ To download all the files in this project to your current directory, run:
+ dds data get -p {{project_id}} -a --verify-checksum

+

+ For more information (including an installation guide), see the DDS CLI documentation here: scilifelabdatacentre.github.io/dds_cli.

+

+ {% if unit_email %} - You were added to this project on behalf of {{displayed_sender}} ({{unit_email}}). + If you experience issues, please contact the SciLifeLab unit {{displayed_sender}} at {{unit_email}}. {% else %} - You were added to this project by {{displayed_sender}}. + If you experience issues, please contact the SciLifeLab unit {{displayed_sender}}. {% endif %}

-

- The DDS CLI command dds ls -p {{project_id}} can be used to list the files in this project.

-

- The DDS CLI command dds data get -p {{project_id}} -a can be used to download all the files in this project to your current directory.

-

- Your access to this project will expire on {{deadline}}

+

+ Your access to this project will expire on:
{{deadline}}

+

+ What is the DDS? The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple way.

diff --git a/dds_web/templates/mail/project_release.txt b/dds_web/templates/mail/project_release.txt index 121d5ea6f..9b6dd9220 100644 --- a/dds_web/templates/mail/project_release.txt +++ b/dds_web/templates/mail/project_release.txt @@ -1,14 +1,24 @@ -The project {{project_id}} is now available for your access in the SciLifeLab Data Delivery System (DDS). -The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple way. +The following project is now available for your access in the SciLifeLab Data Delivery System (DDS) and you can now download your data. + - Project Title: {{project_title}} + - DDS project ID: {{project_id}} -{% if unit_email %} -You were added to this project on behalf of {{displayed_sender}} ({{unit_email}}). -{% else %} -You were added to this project by {{displayed_sender}}. -{% endif %} +You were added to this project {% if unit_email %} on behalf of {% else %} by {% endif %} {{displayed_sender}}. -The DDS CLI command 'dds ls -p {{project_id}}' can be used to list the files in this project. +To list the files in this project, run: + dds ls -p {{project_id}} -The DDS CLI command 'dds data get -p {{project_id}} -a' can be used to download all the files in this project to your current directory. +To download all the files in this project to your current directory, run: + dds data get -p {{project_id}} -a --verify-checksum. + +For more information (including an installation guide), see the DDS CLI documentation here: https://scilifelabdatacentre.github.io/dds_cli/ + +{% if unit_email %} +If you experience issues, please contact the SciLifeLab unit {{displayed_sender}} at {{unit_email}}. +{% else %} +If you experience issues, please contact the SciLifeLab unit {{displayed_sender}}. +{% endif %} Your access to this project will expire on {{deadline}} + +What is the DDS? The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple way. + diff --git a/dds_web/version.py b/dds_web/version.py index 7a2056f56..667b52f95 100644 --- a/dds_web/version.py +++ b/dds_web/version.py @@ -1 +1 @@ -__version__ = "2.5.1" +__version__ = "2.5.2" diff --git a/doc/procedures/new_release.md b/doc/procedures/new_release.md index cdb83dc91..82bd4fbe4 100644 --- a/doc/procedures/new_release.md +++ b/doc/procedures/new_release.md @@ -1,14 +1,14 @@ # How to create a new release 1. Create a PR from `dev` to `master`: "New release" -2. Confirm that the development instance works and that the newest changes have been deployed +2. Confirm that the development instance works and that the newest changes have been deployed. If not, make a new redeployment of dds-dev (via argocd). 1. _In general_, e.g. that it's up and running 2. _Specific feature has been added or changed:_ Confirm that it also works in the development instance 3. _The change is in the API:_ Confirm that the development instance works together with the CLI -3. Fork a new branch from `dev` -4. Update the version [changelog](../../CHANGELOG.rst) +3. Fork a new branch from `dev` (locally) +4. Update the version [changelog](../../CHANGELOG.rst), located at `dds_web/CHANGELOG.rst` **Tip:** Use the PR to `master` to see all changes since last release. @@ -20,12 +20,12 @@ - _Minor changes, e.g. bug fix_: Minor version upgrade, e.g. `1.0.1 --> 1.0.2` - _Small changes, e.g. new feature_: Mid version upgrade, e.g. `1.1.0 --> 1.2.0` - - _Breaking changes or large new feature(s)_: Major version upgrade, e.g. `1.0.0 --> 2.0.0` + - _Breaking changes or large new feature(s)_: Major version upgrade, e.g. `1.0.0 --> 2.0.0` _AVOID THIS -- NEED TO INFORM USERS WELL IN ADVANCE IN THAT CASE SINCE IT WILL BLOCK THE USERS FROM USING ANY OLDER VERSIONS_ > Will break if CLI version not bumped as well 6. Push version change to branch -7. Create a new PR from `` to `dev` +7. Create a new PR from `` to `dev`: "New version & changelog" Wait for approval and merge by Product Owner or admin. diff --git a/doc/status_codes_api.md b/doc/status_codes_api.md index 2bd2bc1a1..84fb085c0 100644 --- a/doc/status_codes_api.md +++ b/doc/status_codes_api.md @@ -237,6 +237,31 @@ - `archive_project` - Database or S3 issues +#### `patch` + +- [Authentication errors](#authentication) +- `400 Bad Request` + - Decorators + - Json required but not provided + - Validation error + - Schemas + - Project does not exist + - Project is busy + - `extend_deadline` + - Invalid deadline + - No deadline provided + - Project is not in Available state + - Max number of times available reached +- `403 Forbidden` + - Schemas + - User does not have access to project +- `500 Internal Server Error` + - Database errors + - `delete_project` + - Database or S3 issues + - `archive_project` + - Database or S3 issues + ### GetPublic - [Authentication errors](#authentication) diff --git a/tests/api/test_project.py b/tests/api/test_project.py index cd1d7f740..18fcfcb10 100644 --- a/tests/api/test_project.py +++ b/tests/api/test_project.py @@ -4,6 +4,8 @@ import http from sqlite3 import OperationalError import pytest +from _pytest.logging import LogCaptureFixture +import logging import datetime import time import unittest.mock @@ -16,7 +18,7 @@ # Own import dds_web -from dds_web import db +from dds_web import mail, db from dds_web.errors import BucketNotFoundError, DatabaseError, DeletionError import tests from tests.test_files_new import project_row, file_in_db, FIRST_NEW_FILE @@ -44,6 +46,49 @@ # "date_updated", ] +release_project = {"new_status": "Available"} +release_project_small_deadline = {**release_project, "deadline": 5} +release_project_big_deadline = {**release_project, "deadline": 80} + +extend_deadline_data_no_confirmed = { + "new_deadline_in": 20, +} + +extend_deadline_data = {**extend_deadline_data_no_confirmed, "confirmed": True} + + +# HELPER FUNCTIONS ################################################################################## CONFIG # + + +def create_and_release_project(client, proj_data, release_data): + """Helper function that creates a project and set it ups as available""" + + current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() + if current_unit_admins < 3: + create_unit_admins(num_admins=2) + current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() + assert current_unit_admins >= 3 + + response = client.post( + tests.DDSEndpoint.PROJECT_CREATE, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(client), + json=proj_data, + ) + assert response.status_code == http.HTTPStatus.OK + project_id = response.json.get("project_id") + project = project_row(project_id=project_id) + + # Release project + response = client.post( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project_id}, + json=release_data, + ) + assert response.status_code == http.HTTPStatus.OK + + return project_id, project + @pytest.fixture(scope="module") def test_project(module_client): @@ -1079,6 +1124,335 @@ def test_projectstatus_post_invalid_deadline_expire(module_client, boto3_session assert "The deadline needs to be less than (or equal to) 30 days." in response.json["message"] +def test_extend_deadline_bad_confirmed(module_client, boto3_session): + """Try to extend a deadline and send a not boolean for confirmation""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # try to extend deadline with a string as confirmed - should fail + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json={**extend_deadline_data, "confirmed": "true"}, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert "`confirmed` is a boolean value: True or False." in response.json["message"] + + +def test_extend_deadline_no_confirmed(module_client, boto3_session): + """Try to extend a deadline before confirmation - should sent a warning and no operation is perfrom""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # try to extend deadline + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data_no_confirmed, + ) + # status code is ok but no operation perform + assert response.status_code == http.HTTPStatus.OK + assert project.times_expired == 0 + + assert "Operation must be confirmed before proceding." in response.json["warning"] + assert all( + item in response.json + for item in ["project_info", "project_status", "warning", "default_unit_days"] + ) + + +def test_extend_deadline_when_busy(module_client, boto3_session): + """Request should not be possible when project is busy.""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # set to busy + project.busy = True + db.session.commit() + assert project.busy + + # attempt to extend deadline + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + + assert ( + f"The deadline for the project '{project_id}' is already in the process of being changed. " + in response.json["message"] + ) + assert ( + "Please try again later. \n\nIf you know that the project is not busy, contact support." + in response.json["message"] + ) + + +def test_extend_deadline_no_deadline(module_client, boto3_session): + """If no deadline has been provided it should not be executed anything""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # try to extend deadline - no new deadline provided + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json={"confirmed": True}, + ) + assert response.status_code == http.HTTPStatus.OK + assert project.times_expired == 0 + assert "Nothing to update." in response.json["message"] + + +def test_extend_deadline_project_not_available(module_client, boto3_session): + """Is not possible to extend deadline to a project in another status than available.""" + + # create a new project and never release it + current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() + if current_unit_admins < 3: + create_unit_admins(num_admins=2) + current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() + assert current_unit_admins >= 3 + + response = module_client.post( + tests.DDSEndpoint.PROJECT_CREATE, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(module_client), + json=proj_data, + ) + assert response.status_code == http.HTTPStatus.OK + project_id = response.json.get("project_id") + + # attempt to extend deadline - project is in progress + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + + assert ( + "You can only extend the deadline for a project that has the status 'Available'." + in response.json["message"] + ) + + +def test_extend_deadline_too_much_days(module_client, boto3_session): + """If the new deadline together with the time left already is more than 90 days it should not work""" + + # create project and release it with big dealdine + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project_big_deadline + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # try to extend deadline -> 80 + 11 > 90 + extend_deadline_data_big_deadline = {**extend_deadline_data, "new_deadline_in": 11} + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data_big_deadline, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert ( + "The new deadline needs to be less than (or equal to) 90 days." in response.json["message"] + ) + + +def test_extend_deadline_bad_new_deadline(module_client, boto3_session): + """If the new deadlien provided is not an integer it should fail""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + # try to extend deadline with a bad new deadline + extend_deadline_data_bad_deadline = {**extend_deadline_data, "new_deadline_in": "20"} + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data_bad_deadline, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert ( + "The deadline attribute passed should be of type Int (i.e a number)." + in response.json["message"] + ) + + +def test_extend_deadline_more_than_default(module_client, boto3_session): + """If the new deadline provided is more than the default unit days to release a project it should fail""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project_small_deadline + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + default_unit_days = project.responsible_unit.days_in_available + + # try to extend deadline with a bigger deadline that it is suppose to have + extend_deadline_data_bad_deadline = { + **extend_deadline_data, + "new_deadline_in": default_unit_days + 1, + } + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data_bad_deadline, + ) + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert ( + "The number of days has to be lower than the default deadline extension number" + in response.json["message"] + ) + + +def test_extend_deadline_maxium_number_available_exceded(module_client, boto3_session): + """If the deadline has been extended more than 2 times it should not work""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project_small_deadline + ) + assert project.times_expired == 0 + deadline = project.current_deadline # current deadline + new_deadline_in = 1 # small new deadline + + for i in range(1, 4): + time.sleep(1) # tests are too fast + + # extend deadline by a small new deadline so we can do it several times + extend_deadline_data_small_deadline = { + **extend_deadline_data, + "new_deadline_in": new_deadline_in, + } + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data_small_deadline, + ) + if i < 3: + assert response.status_code == http.HTTPStatus.OK + assert project.times_expired == i + assert project.current_deadline == deadline + datetime.timedelta(days=new_deadline_in) + deadline = project.current_deadline # update current deadline + assert project.current_status == "Available" + assert ( + f"The project '{project_id}' has been given a new deadline" + in response.json["message"] + ) + assert "An e-mail notification has not been sent." in response.json["message"] + else: + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert ( + "Project availability limit: The maximum number of changes in data availability has been reached." + in response.json["message"] + ) + + +def test_extend_deadline_ok(module_client, boto3_session): + """Extend a project deadline of a project - it should work ok""" + + # create project and release it + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project_small_deadline + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + deadline = project.current_deadline # save current deadline + + # extend deadline + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client), + query_string={"project": project_id}, + json=extend_deadline_data, + ) + assert response.status_code == http.HTTPStatus.OK + assert project.times_expired == 1 + assert project.current_deadline == deadline + datetime.timedelta( + days=extend_deadline_data.get("new_deadline_in") + ) + assert project.current_status == "Available" + + assert f"The project '{project_id}' has been given a new deadline" in response.json["message"] + assert "An e-mail notification has not been sent." in response.json["message"] + + +def test_extend_deadline_mock_database_error( + module_client, boto3_session, capfd: LogCaptureFixture +): + """Operation fails when trying to save in the Database""" + + project_id, project = create_and_release_project( + client=module_client, proj_data=proj_data, release_data=release_project_small_deadline + ) + assert project.times_expired == 0 + time.sleep(1) # tests are too fast + + token = tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(module_client) + + with unittest.mock.patch.object(db.session, "rollback") as rollback: + with unittest.mock.patch("dds_web.db.session.commit") as mock_commit: + # we need this because the first time the commit function is called is when set_busy() + def side_effect_generator(): + yield None # First call, no exception + while True: + yield sqlalchemy.exc.SQLAlchemyError() # Subsequent calls, exception + + mock_commit.side_effect = side_effect_generator() + + # extend deadline + response = module_client.patch( + tests.DDSEndpoint.PROJECT_STATUS, + headers=token, + query_string={"project": project_id}, + json=extend_deadline_data, + ) + assert response.status_code == http.HTTPStatus.INTERNAL_SERVER_ERROR + assert "Saving database changes failed." in response.json["message"] + + assert rollback.called + _, err = capfd.readouterr() + assert "Failed to extend deadline" in err + + def test_projectstatus_post_deletion_and_archivation_errors(module_client, boto3_session): """Mock the different expections that can occur when deleting project.""" current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() @@ -1388,3 +1762,59 @@ def test_project_usage(module_client): # Call project_usage() for the project and check if cost is calculated correctly proj_bhours, proj_cost = UserProjects.project_usage(project=project_0) assert (proj_bhours / 1e9) * cost_gbhour == proj_cost + + +def test_email_project_release(module_client, boto3_session): + """Test that check that the email sent to the researchers when project is released is correct""" + public_project_id = "public_project_id" + + create_unit_admins(num_admins=2) + current_unit_admins = models.UnitUser.query.filter_by(unit_id=1, is_admin=True).count() + assert current_unit_admins >= 3 + + # user to perfrom the operation + token = tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(module_client) + # project to be released + project = models.Project.query.filter_by(public_id=public_project_id).first() + # num of researchers that will receive email + num_users = models.ProjectUsers.query.filter_by(project_id=project.id).count() + + # Release project and check email + with mail.record_messages() as outbox: + response = module_client.post( + tests.DDSEndpoint.PROJECT_STATUS, + headers=token, + query_string={"project": public_project_id}, + json={"new_status": "Available", "deadline": 10, "send_email": True}, + ) + assert len(outbox) == num_users # nÂș of Emails informing researchers + assert "Project made available by" in outbox[-1].subject + + body = outbox[-1].body # plain text + html = outbox[-1].html + + project_title = project.title + + ## check plain text message + assert f"- Project Title: {project_title}" in body + assert f"- DDS project ID: {public_project_id}" in body + assert f"dds ls -p {public_project_id}" in body + assert f"dds data get -p {public_project_id} -a --verify-checksum" in body + assert "If you experience issues, please contact the SciLifeLab unit" in body + assert ( + "What is the DDS? The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple way" + in body + ) + + ## check html + assert f"
  • Project Title: {project_title}
  • " in html + assert f"
  • DDS project ID: {public_project_id}
  • " in html + assert f"dds ls -p {public_project_id}" in html + assert f"dds data get -p {public_project_id} -a --verify-checksum" in html + assert "If you experience issues, please contact the SciLifeLab unit" in html + assert ( + "What is the DDS? The DDS is a system for SciLifeLab infrastructures to deliver data to researchers in a fast, secure and simple way." + in html + ) + + assert response.status_code == http.HTTPStatus.OK diff --git a/tests/api/test_superadmin_only.py b/tests/api/test_superadmin_only.py index 022c537a7..ca1d5d428 100644 --- a/tests/api/test_superadmin_only.py +++ b/tests/api/test_superadmin_only.py @@ -20,7 +20,7 @@ import click # Own -from dds_web import db +from dds_web import db, mail from dds_web.database import models import tests from dds_web.commands import collect_stats @@ -577,13 +577,15 @@ def test_send_motd_no_primary_email(client: flask.testing.FlaskClient) -> None: assert not models.Email.query.filter_by(email=email).one_or_none() assert not models.User.query.filter_by(username=username).one().primary_email - # Attempt request - with unittest.mock.patch.object(flask_mail.Connection, "send") as mock_mail_send: + # Attempt request and catch email + with mail.record_messages() as outbox: response: werkzeug.test.WrapperTestResponse = client.post( tests.DDSEndpoint.MOTD_SEND, headers=token, json={"motd_id": created_motd.id} ) assert response.status_code == http.HTTPStatus.OK - assert mock_mail_send.call_count == num_users - 1 + assert len(outbox) == num_users - 1 + assert "Important Information: Data Delivery System" in outbox[-1].subject + assert "incorrect subject" not in outbox[-1].subject def test_send_motd_ok(client: flask.testing.FlaskClient) -> None: @@ -604,13 +606,14 @@ def test_send_motd_ok(client: flask.testing.FlaskClient) -> None: # Get number of users num_users = models.User.query.count() - # Attempt request - with unittest.mock.patch.object(flask_mail.Connection, "send") as mock_mail_send: + # Attempt request and catch email + with mail.record_messages() as outbox: response: werkzeug.test.WrapperTestResponse = client.post( tests.DDSEndpoint.MOTD_SEND, headers=token, json={"motd_id": created_motd.id} ) assert response.status_code == http.HTTPStatus.OK - assert mock_mail_send.call_count == num_users + assert len(outbox) == num_users + assert "Important Information: Data Delivery System" in outbox[-1].subject # Maintenance ###################################################################################### diff --git a/tests/api/test_user.py b/tests/api/test_user.py index 3473a96eb..615590e5a 100644 --- a/tests/api/test_user.py +++ b/tests/api/test_user.py @@ -16,11 +16,15 @@ import werkzeug import time +# CONFIG ################################################################################## CONFIG # + existing_project = "public_project_id" existing_project_2 = "second_public_project_id" first_new_email = {"email": "first_test_email@mailtrap.io"} first_new_user = {**first_new_email, "role": "Researcher"} first_new_owner = {**first_new_email, "role": "Project Owner"} +first_new_user_unit_admin = {**first_new_email, "role": "Unit Admin"} +first_new_user_unit_personel = {**first_new_email, "role": "Unit Personnel"} first_new_user_existing_project = {**first_new_user, "project": "public_project_id"} first_new_user_extra_args = {**first_new_user, "extra": "test"} first_new_user_invalid_role = {**first_new_email, "role": "Invalid Role"} @@ -48,6 +52,43 @@ **existing_research_user_owner, "project": "second_public_project_id", } +remove_user_project_owner = {"email": "projectowner@mailtrap.io"} +remove_user_unit_user = {"email": "unituser2@mailtrap.io"} +remove_user_project_owner = {"email": "projectowner@mailtrap.io"} + +# UTILITY FUNCTIONS ############################################################ UTILITY FUNCTIONS # + + +def get_existing_projects(): + """Return existing projects for the tests""" + existing_project_1 = models.Project.query.filter_by(public_id="public_project_id").one_or_none() + existing_project_2 = models.Project.query.filter_by( + public_id="second_public_project_id" + ).one_or_none() + + return existing_project_1, existing_project_2 + + +def invite_to_project(project, client, json_query): + """Create a invitation of a user for a project""" + response = client.post( + tests.DDSEndpoint.USER_ADD, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project.public_id}, + json=json_query, + ) + assert response.status_code == http.HTTPStatus.OK + + invited_user = models.Invite.query.filter_by(email=json_query["email"]).one_or_none() + assert invited_user + + project_invite_keys = models.ProjectInviteKeys.query.filter_by( + invite_id=invited_user.id, project_id=project.id + ).one_or_none() + assert project_invite_keys + + return invited_user + # AddUser ################################################################# AddUser # @@ -1258,3 +1299,215 @@ def get_list(as_user) -> dict: assert response.status_code == http.HTTPStatus.FORBIDDEN assert not response.json.get("invites") assert not response.json.get("keys") + + +##### Test for RemoveUserAssociation + + +def test_remove_access_invite_associated_several_projects(client): + """If an invite is associated with several projects then a single revoke access should not delete the invite""" + + project_1, project_2 = get_existing_projects() + + # invite a new user to both projects + invited_user = invite_to_project(project=project_1, client=client, json_query=first_new_user) + _ = invite_to_project(project=project_2, client=client, json_query=first_new_user) + + # Now revoke access for the first project + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project_1.public_id}, + json=first_new_user, + ) + assert response.status_code == http.HTTPStatus.OK + + assert ( + f"Invited user is no longer associated with the project '{project_1.public_id}'." + in response.json["message"] + ) + + # The project invite row should only be deleted for project 1 and the invite should still exist + invited_user = models.Invite.query.filter_by(email=first_new_user["email"]).one_or_none() + assert invited_user + + project_invite_keys = models.ProjectInviteKeys.query.filter_by( + invite_id=invited_user.id, project_id=project_2.id + ).one_or_none() + assert project_invite_keys + + project_invite_keys = models.ProjectInviteKeys.query.filter_by( + invite_id=invited_user.id, project_id=project_1.id + ).one_or_none() + assert not project_invite_keys + + +def test_revoking_access_to_unacepted_invite(client): + """Revoking access to an unacepted invite for an existing project should delete the invite from the db""" + + project, _ = get_existing_projects() + + # Invite a new user to the project + invited_user = invite_to_project(project=project, client=client, json_query=first_new_user) + invited_user_id = invited_user.id + + # Now, revoke access to said user. The invite should be deleted + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project.public_id}, + json=first_new_user, + ) + assert response.status_code == http.HTTPStatus.OK + + assert ( + f"Invited user is no longer associated with the project '{project.public_id}'." + in response.json["message"] + ) + + # Check that the invite is deleted + invited_user = models.Invite.query.filter_by(email=first_new_user["email"]).one_or_none() + assert not invited_user + + project_invite_keys = models.ProjectInviteKeys.query.filter_by( + invite_id=invited_user_id, project_id=project.id + ).one_or_none() + assert not project_invite_keys + + +def test_remove_nonacepted_user_from_other_project(client, boto3_session): + """Try to remove an User with an unacepted invite from another project should result in an error""" + + project_1, project_2 = get_existing_projects() + + # invite a new user to a project + invite_to_project(project=project_1, client=client, json_query=first_new_user) + + # try to remove the same user from a different one + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unituser"]).token(client), + query_string={"project": project_2.public_id}, + json=first_new_user, + ) + + assert response.status_code == http.HTTPStatus.BAD_REQUEST + assert "Cannot remove non-existent project access" in response.json["message"] + + +def test_researcher_removes_project_owner(client): + """ + A Researcher who is not a PO should not be able to delete a PO + """ + + project, _ = get_existing_projects() + + # Research user trying to delete PO + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["researchuser"]).token(client), + query_string={"project": project.public_id}, + json=remove_user_project_owner, + ) + + assert response.status_code == http.HTTPStatus.FORBIDDEN + assert "Insufficient credentials" in response.json["message"] + + +def test_unit_personnel_removed(client): + """ + Unit Personnel cannot be deleted from individual projects (they should be removed from the unit instead) + """ + project, _ = get_existing_projects() + + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project.public_id}, + json=remove_user_unit_user, + ) + + assert response.status_code == http.HTTPStatus.BAD_REQUEST + # Should give error because a unit personal cannot be granted access to individual projects + assert "Cannot remove non-existent project access." in response.json["message"] + + +def test_removed_myself(client): + """ + An User cannot remove themselves from a project + """ + project, _ = get_existing_projects() + + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["projectowner"]).token(client), + query_string={"project": project.public_id}, + json=remove_user_project_owner, + ) + + assert response.status_code == http.HTTPStatus.FORBIDDEN + assert "You cannot revoke your own access" in response.json["message"] + + +def test_remove_invite_unit_admin(client): + """ + A project removal request for an unanswered invite of unit admin should not work + """ + project, _ = get_existing_projects() + + # invite a new unit admin to the system + response = client.post( + tests.DDSEndpoint.USER_ADD, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + json=first_new_user_unit_admin, + ) + assert response.status_code == http.HTTPStatus.OK + + # try to remove the unitadmin for a specific project within their unit -> should not work + email = first_new_user_unit_admin["email"] + rem_user = {"email": email} + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project.public_id}, + json=rem_user, + ) + + assert response.status_code == http.HTTPStatus.BAD_REQUEST + # Should give error because a unit personal cannot be granted access to individual projects + assert ( + "Cannot remove a Unit Admin / Unit Personnel from individual projects" + in response.json["message"] + ) + + +def test_invite_unit_user(client): + """ + A project removal request for an unanswered invite of unit personel should not work + """ + project, _ = get_existing_projects() + + # invite a new unit user to the system + response = client.post( + tests.DDSEndpoint.USER_ADD, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + json=first_new_user_unit_personel, + ) + assert response.status_code == http.HTTPStatus.OK + + # try to remove the Unit Personnel for a specific project within their unit -> should not work + email = first_new_user_unit_personel["email"] + rem_user = {"email": email} + response = client.post( + tests.DDSEndpoint.REMOVE_USER_FROM_PROJ, + headers=tests.UserAuth(tests.USER_CREDENTIALS["unitadmin"]).token(client), + query_string={"project": project.public_id}, + json=rem_user, + ) + + assert response.status_code == http.HTTPStatus.BAD_REQUEST + # Should give error because a Unit Personnel cannot be granted access to individual projects + assert ( + "Cannot remove a Unit Admin / Unit Personnel from individual projects" + in response.json["message"] + ) diff --git a/tests/test_project_access.py b/tests/test_project_access.py index 3b423af38..9466f7c73 100644 --- a/tests/test_project_access.py +++ b/tests/test_project_access.py @@ -15,7 +15,6 @@ proj_query = {"project": "public_project_id"} # proj_query_restricted = {"project": "restricted_project_id"} - # UTILITY FUNCTIONS ############################################################ UTILITY FUNCTIONS # diff --git a/tests/test_user_remove_association.py b/tests/test_user_remove_association.py index 39c0ecbf5..34b726886 100644 --- a/tests/test_user_remove_association.py +++ b/tests/test_user_remove_association.py @@ -7,6 +7,8 @@ from tests.test_project_creation import proj_data_with_existing_users, create_unit_admins from dds_web.database import models +# TESTS ################################################################################## TEST # + def test_remove_user_from_project(client, boto3_session): """Remove an associated user from a project""" diff --git a/tests/test_version.py b/tests/test_version.py index 02a5bc16a..534eab711 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -2,4 +2,4 @@ def test_version(): - assert version.__version__ == "2.5.1" + assert version.__version__ == "2.5.2"