Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Error pages when published studies dispatcher fails, access rights fixes and plugin refactoring #3962

Merged
merged 66 commits into from
Mar 20, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Mar 10, 2023

What do these changes do?

To publish a study it needs to:

  1. Be a template
  2. Share with everybody
  3. published fiend in the database must be checked

Currently any published study can be accessed via a link with path /study/{project_id}. That link will copy and open the study even if the user is not logged it. If this request fails it will now display an error page with details.

This PR provides:

  • ✨ Improves error handling and reporting. Redirects to an error page created in the front-end (previously was just raw text)
  • ✨ Guest users (i.e. the ones that are not registered) use now temp accounts with expiration date
  • 🐛 Fixes permission errors while copying data from published template

@odeimaiz some improvements in the FE are necessary for the two first items -> #3987

In addition, this PR

Related issue/s

How to test

  1. Publish a study (follow the definition above to publish a study). browse https://{host}:{port}/studies/{study_uuid} as guest and will open a copy of the study
  2. log in. browse https://{host}:{port}/studies/{study_uuid} and will open a copy of the study
  3. disable publish flag. browse https://{host}:{port}/studies/{study_uuid} should fail with an error page
  4. Enable publish flag and remove everyone access rights. browse https://{host}:{port}/studies/{study_uuid} should fail with an error page
  5. Enable publish flag and remove everyone access rights, then browse https://{host}:{port}/studies/{study_uuid} should fail with an error page

Pasted image 20230316142241

cd services/web/server
make install-dev
pytest -vv tests/unit/with_dbs/01/test_studies_dispatcher_studies_access.py

Checklist

  • make version-*
  • Unit tests for the changes exist
  • Runs in the swarm

@pcrespov pcrespov self-assigned this Mar 10, 2023
@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #3962 (77210b0) into master (77c36e0) will decrease coverage by 2.3%.
The diff coverage is 92.9%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3962      +/-   ##
=========================================
- Coverage    85.6%   83.4%    -2.3%     
=========================================
  Files         793     429     -364     
  Lines       36675   21960   -14715     
  Branches      842     137     -705     
=========================================
- Hits        31418   18316   -13102     
+ Misses       5034    3595    -1439     
+ Partials      223      49     -174     
Flag Coverage Δ
integrationtests 66.7% <57.1%> (-0.1%) ⬇️
unittests 84.3% <92.9%> (+2.1%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
..._service_webserver/studies_dispatcher/_projects.py 90.0% <ø> (ø)
...ce_webserver/studies_dispatcher/_studies_access.py 90.9% <87.2%> (+4.5%) ⬆️
..._service_webserver/projects/projects_exceptions.py 93.6% <91.6%> (-1.0%) ⬇️
...re_service_webserver/projects/projects_db_utils.py 97.0% <97.3%> (-0.4%) ⬇️
.../storage/src/simcore_service_storage/exceptions.py 100.0% <100.0%> (ø)
...e_service_webserver/login/handlers_registration.py 90.8% <100.0%> (ø)
.../simcore_service_webserver/projects/projects_db.py 96.4% <100.0%> (+<0.1%) ⬆️
...service_webserver/studies_dispatcher/_constants.py 100.0% <100.0%> (ø)
...ore_service_webserver/studies_dispatcher/_users.py 84.1% <100.0%> (+1.0%) ⬆️
...e_service_webserver/studies_dispatcher/settings.py 100.0% <100.0%> (ø)

... and 453 files with indirect coverage changes

@pcrespov pcrespov force-pushed the is829/study-dispatcher-errors branch 2 times, most recently from e7c3668 to c156a62 Compare March 15, 2023 15:45
@pcrespov pcrespov added the a:webserver issue related to the webserver service label Mar 15, 2023
@pcrespov pcrespov changed the title ✨ Better error handling for published studies dispatcher ♻️ Better error handling for published studies dispatcher Mar 15, 2023
@pcrespov pcrespov changed the title ♻️ Better error handling for published studies dispatcher ✨ Error pages when published studies dispatcher fails and ♻️ plugin refactoring Mar 15, 2023
@pcrespov pcrespov changed the title ✨ Error pages when published studies dispatcher fails and ♻️ plugin refactoring ✨ Error pages when published studies dispatcher fails, access rights fixes and plugin refactoring Mar 16, 2023
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor things 👍

@odeimaiz odeimaiz mentioned this pull request Mar 17, 2023
11 tasks
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good! some small comments. and also be careful with usage of utcnow

Copy link
Collaborator

@elisabettai elisabettai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pcrespov for this!
If I understand it well, we are NOT yet blocking "guests" from accessing certain templates, right?
Is it something that will be done soon? EN just asked when we can put online links to templates with Jupyterlabs. They are needed for the SC meeting (=ideally today or tomorrow, when the first documents will be sent to the SC).

Maybe we can workaround by using the temp accounts that you have introduced with this PR? We can make the duration quite short, do avoid any malicious intervention...

@codeclimate
Copy link

codeclimate bot commented Mar 20, 2023

Code Climate has analyzed commit 77210b0 and detected 0 issues on this pull request.

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Mar 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pcrespov pcrespov merged commit 35fa716 into ITISFoundation:master Mar 20, 2023
@pcrespov pcrespov deleted the is829/study-dispatcher-errors branch March 20, 2023 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't instantiate some ISAN tutorials jLab: you need to be logged in
6 participants