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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 Login: Fixes Unauthorized during resend-code #3775

Merged
merged 16 commits into from
Jan 19, 2023

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jan 18, 2023

What do these changes do?

This PR redesigns the session access to overcome some limitations detected during manual exploratory testing.

Some of the new authentication workflows include multiple requests to the auth entrypoints. In order to secure these entrypoints, there is a session access control mechanism in place. Once the starting auth entrypoint succeds, an access token is created for the next entrypoint/s and so on. For example, a request to auth/login-continue is unauthorized until a request to auth/login-start is successful. This is implemented as

@router.post("/auth/login-start)
@on_success_grant_session_access_to( name="login_continue" ) # <--
def login_start(request):
    ...

@router.post("/auth/login-continue)
@session_access_required( name="login_continue" ) # <--
def login_continue(request):
    ...
 
@router.get("/me)
@login_required # <--
def user_profile(request):
   ...

@router.get("/unprotected)
def unprotected_entrypoint(request):
   ...

Note that login_continue and login_start do not have login_required.

Related issue/s

  • Login

How to test

  • services/web/server/tests/unit/with_dbs/03/test_session_access.py tests workflows followed by the front-end
  • test

Checklist

  • manual test+record workflows with @odeimaiz
  • Unit tests for the changes exist
  • Runs in the swarm
  • Documentation reflects the changes

@pcrespov pcrespov self-assigned this Jan 18, 2023
@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #3775 (b8c7883) into master (4105a93) will increase coverage by 1.3%.
The diff coverage is 96.9%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3775     +/-   ##
========================================
+ Coverage    83.5%   84.8%   +1.3%     
========================================
  Files         915     915             
  Lines       38837   38866     +29     
  Branches      791     791             
========================================
+ Hits        32450   32991    +541     
+ Misses       6177    5665    -512     
  Partials      210     210             
Flag Coverage 螖
integrationtests 66.9% <78.4%> (+2.2%) 猬嗭笍
unittests 82.0% <96.9%> (+1.0%) 猬嗭笍

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

Impacted Files Coverage 螖
...er/src/simcore_service_webserver/session_access.py 96.8% <95.3%> (+1.5%) 猬嗭笍
.../src/simcore_service_webserver/login/_constants.py 100.0% <100.0%> (酶)
...rc/simcore_service_webserver/login/handlers_2fa.py 84.7% <100.0%> (酶)
...c/simcore_service_webserver/login/handlers_auth.py 94.0% <100.0%> (+0.1%) 猬嗭笍
...e_service_webserver/login/handlers_confirmation.py 90.6% <100.0%> (+0.1%) 猬嗭笍
...e_service_webserver/login/handlers_registration.py 89.1% <100.0%> (-0.2%) 猬囷笍
.../src/simcore_service_webserver/session_settings.py 100.0% <100.0%> (酶)
...eb/server/src/simcore_service_webserver/session.py 86.9% <0.0%> (-13.1%) 猬囷笍
...mcore_service_webserver/garbage_collector_utils.py 83.3% <0.0%> (-1.3%) 猬囷笍
.../director/src/simcore_service_director/producer.py 67.7% <0.0%> (+0.4%) 猬嗭笍
... and 26 more

@pcrespov pcrespov changed the title WIP: Fix/unauthorized resend 馃悰 Fixes unauthorized resend code during authentication Jan 18, 2023
@pcrespov pcrespov added a:webserver issue related to the webserver service changelog:馃悰bugfix labels Jan 18, 2023
@pcrespov pcrespov added this to the Resistance Is Futile milestone Jan 18, 2023
@pcrespov pcrespov marked this pull request as ready for review January 18, 2023 21:48
@pcrespov pcrespov changed the title 馃悰 Fixes unauthorized resend code during authentication 馃悰 Login: Fixes unauthorized resend code during authentication Jan 18, 2023
@pcrespov pcrespov changed the title 馃悰 Login: Fixes unauthorized resend code during authentication 馃悰 Login: Fixes Unauthorized resend code during authentication Jan 18, 2023
@pcrespov pcrespov changed the title 馃悰 Login: Fixes Unauthorized resend code during authentication 馃悰 Login: Fixes Unauthorized resend code during authentication Jan 18, 2023
@pcrespov pcrespov changed the title 馃悰 Login: Fixes Unauthorized resend code during authentication 馃悰 Login: Fixes Unauthorized during resend-code Jan 18, 2023
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.

cool! maybe finish the description of the PR...

@pcrespov
Copy link
Member Author

cool! maybe finish the description of the PR...

@sanderegg what do you mean?

@pcrespov pcrespov enabled auto-merge (squash) January 19, 2023 18:25
@codeclimate
Copy link

codeclimate bot commented Jan 19, 2023

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

View more on Code Climate.

@sonarcloud
Copy link

sonarcloud bot commented Jan 19, 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

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.

None yet

3 participants