Details

Reviewers

Group Reviewers

Commits

rDWAPPS05e9efac023a: auth: Add OIDC session silent refresh middleware

Summary

When a user has logged in to swh-web from a browser and the access token has expired,
try to perform a silent OIDC session refresh through the use of a Django middleware.

If the OIDC session has expired (idle time reached for instance), a redirection to the
logout page will be performed to inform the user and a link to login again will be
offered.

Related to T2267

Diff Detail

Repository

rDWAPPS Web applications

Branch

oidc-browser-session-refresh

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 11637
Build 17648: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 17647: arc lint + arc unit

Event Timeline

anlambert created this revision.Mar 25 2020, 2:04 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptMar 25 2020, 2:04 PM

Rebase

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/1031/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/1033/ for more details.

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/640/ for more details.

Harbormaster completed remote builds in B11328: Diff 10250.Mar 25 2020, 2:18 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/642/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/642/console

Harbormaster failed remote builds in B11330: Diff 10252!Mar 25 2020, 2:36 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/643/ for more details.

Harbormaster completed remote builds in B11330: Diff 10252.Mar 25 2020, 2:46 PM

Rebase

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/648/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/648/console

Harbormaster failed remote builds in B11386: Diff 10305!Mar 26 2020, 9:42 PM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/1038/ for more details.

Rebase

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/650/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/650/console

Harbormaster failed remote builds in B11404: Diff 10324!Mar 27 2020, 10:41 AM

Build is green
See https://jenkins.softwareheritage.org/job/DWAPPS/job/tox/1040/ for more details.

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/652/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/652/console

anlambert edited the summary of this revision. (Show Details)Mar 27 2020, 12:27 PM

anlambert removed a parent revision: D2876: auth/backends: Simplify and improve OIDC authentication.

Rebase

Build is green

Patch application report for D2877 (id=10424)

Rebasing onto 717159e459...

Current branch diff-target is up to date.

Changes applied before test

commit be5880b6778fa8db0f152685cfd4061acd7684c0
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Mar 24 22:11:05 2020 +0100

    auth: Add OIDC session silent refresh middleware
    
    When an access token has expired, try to perform a silent OIDC session
    refresh through the use of a Django middleware.
    
    Closes T2267

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/33/ for more details.

Harbormaster completed remote builds in B11505: Diff 10424.Mar 31 2020, 4:34 PM

Sorry I didn't review this sooner, not sure why I didn't see the diff.

What is login_data['prompt']?

swh/web/auth/middlewares.py
34–37	I don't understand: "If the latter failed [...], user will be redirected" but I don't see a conditional in the code
swh/web/tests/auth/test_middlewares.py
42	that's not a silent refresh

In D2877#70933, @vlorentz wrote:

Sorry I didn't review this sooner, not sure why I didn't see the diff.

What is login_data['prompt']?

prompt is a query parameter that can be sent to the OIDC server while attempting to login a user. You can find official OIDC reference below:

prompt
    OPTIONAL. Space delimited, case sensitive list of ASCII string values that specifies whether the Authorization Server prompts the End-User for reauthentication and consent. The defined values are:

        none
            The Authorization Server MUST NOT display any authentication or consent user interface pages. An error is returned if an End-User is not already authenticated or the Client does not have pre-configured consent for the requested Claims or does not fulfill other conditions for processing the request. The error code will typically be login_required, interaction_required, or another code defined in Section 3.1.2.6. This can be used as a method to check for existing authentication and/or consent. 
        login
            The Authorization Server SHOULD prompt the End-User for reauthentication. If it cannot reauthenticate the End-User, it MUST return an error, typically login_required. 
        consent
            The Authorization Server SHOULD prompt the End-User for consent before returning information to the Client. If it cannot obtain consent, it MUST return an error, typically consent_required. 
        select_account
            The Authorization Server SHOULD prompt the End-User to select a user account. This enables an End-User who has multiple accounts at the Authorization Server to select amongst the multiple accounts that they might have current sessions for. If it cannot obtain an account selection choice made by the End-User, it MUST return an error, typically account_selection_required.

I stored this parameter value in django user session in order to know what type of login was performed (initial one with login / password interface displayed or silent session refresh if its value is none).
If the prompt value was none and the OIDC server returns an error, it means corresponding OIDC session has expired and explicit relogin is needed.

For the record, this middleware is inspired from the one that can be found in the mozilla-django-oidc project.

swh/web/auth/middlewares.py
34–37	Indeed, the conditional processing and logout redirection is handled in the `oidc-login-complete` view as this is the place where we can check if a silent refresh (or relogin) failed or not based on the OIDC server response.
swh/web/tests/auth/test_middlewares.py
42	In practice, it is for a user as no login/password prompt will be displayed if the OIDC session has not expired (i.e. while the refresh token is still valid) . When a logged user requests a page, the access token will be renewed without user interaction prior redirecting to the requested page, so the silent refresh.

anlambert marked 2 inline comments as done.Apr 1 2020, 12:43 PM

anlambert added inline comments.

swh/web/auth/middlewares.py
34–37	I will update that comment to point to the `oidc-login-complete` view implementation for a better understanding of that process.

Update:

Rebase
Modify comment in middleware
Fix typo in test variable

Build is green

Patch application report for D2877 (id=10452)

Rebasing onto f9172f8a39...

Current branch diff-target is up to date.

Changes applied before test

commit 6d068f79f1223738569558cdb398b931789e0ac4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Mar 24 22:11:05 2020 +0100

    auth: Add OIDC session silent refresh middleware
    
    When an access token has expired, try to perform a silent OIDC session
    refresh through the use of a Django middleware.
    
    Closes T2267

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/36/ for more details.

Harbormaster completed remote builds in B11537: Diff 10452.Apr 1 2020, 2:25 PM

Rebase

vlorentz accepted this revision.Apr 7 2020, 11:48 AM

This revision is now accepted and ready to land.Apr 7 2020, 11:48 AM

Build is green

Patch application report for D2877 (id=10548)

Rebasing onto f77f32dc84...

Current branch diff-target is up to date.

Changes applied before test

commit 05e9efac023a01fd4a7f1971b149affc0d790e9d
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Mar 24 22:11:05 2020 +0100

    auth: Add OIDC session silent refresh middleware
    
    When an access token has expired, try to perform a silent OIDC session
    refresh through the use of a Django middleware.
    
    Closes T2267

See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/46/ for more details.

Harbormaster completed remote builds in B11637: Diff 10548.Apr 7 2020, 11:51 AM

Closed by commit rDWAPPS05e9efac023a: auth: Add OIDC session silent refresh middleware (authored by anlambert). · Explain WhyApr 7 2020, 4:12 PM

This revision was automatically updated to reflect the committed changes.

anlambert added a commit: rDWAPPS05e9efac023a: auth: Add OIDC session silent refresh middleware.

auth: Add OIDC session silent refresh middleware
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D2877 (id=10424)

Changes applied before test

Patch application report for D2877 (id=10452)

Changes applied before test

Patch application report for D2877 (id=10548)

Changes applied before test

Revision Contents
Changeset List

Diff 10548

swh/web/auth/middlewares.py

swh/web/auth/views.py

swh/web/settings/common.py

swh/web/templates/logout.html

swh/web/tests/api/test_throttling.py

swh/web/tests/auth/keycloak_mock.py

swh/web/tests/auth/test_middlewares.py

swh/web/tests/auth/test_views.py

auth: Add OIDC session silent refresh middlewareClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D2877 (id=10424)

Changes applied before test

Patch application report for D2877 (id=10452)

Changes applied before test

Patch application report for D2877 (id=10548)

Changes applied before test

Revision ContentsChangeset List

Diff 10548

swh/web/auth/middlewares.py

swh/web/auth/views.py

swh/web/settings/common.py

swh/web/templates/logout.html

swh/web/tests/api/test_throttling.py

swh/web/tests/auth/keycloak_mock.py

swh/web/tests/auth/test_middlewares.py

swh/web/tests/auth/test_views.py

auth: Add OIDC session silent refresh middleware
ClosedPublic
Actions

Revision Contents
Changeset List