Page MenuHomeSoftware Heritage
Differential D4319

auth: Add user profile UI for managing account and bearer tokens
ClosedPublic
Actions

Authored by anlambert on Oct 21 2020, 2:47 PM.

Details

Reviewers
ardumont
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2718: Web UI: add a profile page
Commits
rDWAPPSe59d91ace16f: auth: Add user profile UI for managing account and bearer tokens
Summary

Add a new view enabling for a user to manage its account (email, password)
but also its bearer tokens for Web API authentication.

Account management is done by embedding Keycloak account UI in swh-web
through an iframe.

This requires an upgrade of the SWH Keycloak theme and content security policy
needs also to be updated in Keycloak in order to allow such embedding from the
*.softwareheritage.org domains (diff for puppet-swh-site incoming).

User information and permissions are displayed in the view and a link to the Software
Heritage Account Management is offered to edit personal information.

The view is reachable by clicking on the username on the top right part of
the Web UI once authenticated.

Below are some screenshots of the result:

Closes T2718

Depends on D4320

Diff Detail

Repository
rDWAPPS Web applications
Branch
oidc-profile-page
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16556
Build 25515: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 25514: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
0 msJenkins > should revoke a token when requested::Tests / Cypress tests / Run cypress tests / Test API tokens UI should revoke a token when requested
CypressError: Timed out retrying: `cy.click()` failed because this element is detached from the DOM. `<button class="btn btn-default" onclick="swh.auth.applyTokenAction('revoke', 1)">Revoke ...</button>`
0 msJenkins > should show a token when requested::Tests / Cypress tests / Run cypress tests / Test API tokens UI should show a token when requested
CypressError: Timed out retrying: `cy.click()` failed because this element is detached from the DOM. `<button class="btn btn-default" onclick="swh.auth.applyTokenAction('display', 1)">Display...</button>`
212 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_accept_pending_save_request
684 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_authorized_origin_url
112 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.web.tests.admin.test_origin_save::Tests / Python tests / test_add_unauthorized_origin_url
View Full Test Results (2 Failed · 607 Passed · 7 Skipped)

Event Timeline

anlambert created this revision.Oct 21 2020, 2:47 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 21 2020, 2:47 PM
swh-public-ci added a comment.Oct 21 2020, 2:55 PM

Build is green

Patch application report for D4319 (id=15279)

Rebasing onto d3b1bf3839...

Current branch diff-target is up to date.
Changes applied before test
commit 02c55cc09acc9227173027bf50402692cd7ee35a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Oct 21 11:47:32 2020 +0200

    auth: Add user profile UI for managing account and bearer tokens
    
    Add a new view enabling for a user to manage its account (email, password)
    but also its bearer tokens for Web API authentication.
    
    Account management is done by embedding Keycloak account UI in swh-web
    through an iframe (content security policy needs to be updated in Keycloak
    in order to allow such embedding from the *.softwareheritage.org domains).
    
    The view is reachable by clicking on the username on the top right part of
    the Web UI once authenticated.
    
    Closes T2718

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

Harbormaster completed remote builds in B16512: Diff 15279.Oct 21 2020, 2:55 PM
anlambert edited the summary of this revision. (Show Details)Oct 21 2020, 3:23 PM
anlambert added a parent revision: D4320: keycloak: Update puppet configuration.
ardumont accepted this revision.Oct 21 2020, 4:13 PM
This revision is now accepted and ready to land.Oct 21 2020, 4:13 PM
olasd requested changes to this revision.Oct 21 2020, 8:47 PM
olasd added a subscriber: olasd.

Is embedding the keycloak UI via an iframe something that people really do?

As a very security sensitive app (we want to gradually migrate staff access to most services to keycloak), I'd like to keep auth.softwareheritage.org as hardened as possible. Allowing to embed it in an iframe, even restricted to a given set of domains, opens it up to cross-frame scripting vulnerabilities (https://owasp.org/www-community/attacks/Cross_Frame_Scripting) if malicious (or buggy) code is introduced in these domains.

I'd much rather have the swh-webapp profile page:

  • show the profile info imported from keycloak
  • link to the built-in keycloak profile management page
  • allow the user to manage the swh-web specific tokens

even if that means a split user experience, rather than have the keycloak profile management embedded as an iframe.

This revision now requires changes to proceed.Oct 21 2020, 8:47 PM
olasd mentioned this in D4320: keycloak: Update puppet configuration.Oct 21 2020, 8:48 PM
anlambert added a comment.Oct 21 2020, 9:38 PM
In D4319#107533, @olasd wrote:

Is embedding the keycloak UI via an iframe something that people really do?

As a very security sensitive app (we want to gradually migrate staff access to most services to keycloak), I'd like to keep auth.softwareheritage.org as hardened as possible. Allowing to embed it in an iframe, even restricted to a given set of domains, opens it up to cross-frame scripting vulnerabilities (https://owasp.org/www-community/attacks/Cross_Frame_Scripting) if malicious (or buggy) code is introduced in these domains.

I'd much rather have the swh-webapp profile page:

  • show the profile info imported from keycloak
  • link to the built-in keycloak profile management page
  • allow the user to manage the swh-web specific tokens

even if that means a split user experience, rather than have the keycloak profile management embedded as an iframe.

Ack. I thought setting the content security policy was sufficient to protect against cross scripting attacks but did not think
about malicious code injection. As most of our frontend code is retrieved from npm, it seems indeed more secure to avoid
the iframe embedding here.

I will do the requested changes tomorrow.

anlambert updated this revision to Diff 15328.Oct 22 2020, 3:16 PM

Update:

  • Remove Keycloak Account UI embedding through an iframe
  • Display user information in Account tab
  • Add a link to Keycloak Account management to edit personal information
swh-public-ci added a comment.Oct 22 2020, 3:17 PM

Build has FAILED

Patch application report for D4319 (id=15328)

Rebasing onto d3b1bf3839...

Current branch diff-target is up to date.
Changes applied before test
commit f6ec8e8e0019246c49797ef1eb2ee9111734b86a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Oct 21 11:47:32 2020 +0200

    auth: Add user profile UI for managing account and bearer tokens
    
    Add a new view enabling for a user to manage its account (email, password)
    but also its bearer tokens for Web API authentication.
    
    Account details are displayed in the view but edition of personal information
    must be done through the dedicated Keycloak account management UI (link to it
    is available in the view).
    
    The view is reachable by clicking on the username on the top right part of
    the Web UI once authenticated.
    
    Closes T2718

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

Harbormaster failed remote builds in B16555: Diff 15328!Oct 22 2020, 3:17 PM
anlambert edited the summary of this revision. (Show Details)Oct 22 2020, 3:27 PM
anlambert updated this revision to Diff 15329.Oct 22 2020, 3:29 PM

Update: Remove useless : character in table headers

swh-public-ci added a comment.Oct 22 2020, 3:30 PM

Build has FAILED

Patch application report for D4319 (id=15329)

Rebasing onto d3b1bf3839...

Current branch diff-target is up to date.
Changes applied before test
commit 1609142000859ded1bee3d9cfc0d992c1e59f4e5
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Oct 21 11:47:32 2020 +0200

    auth: Add user profile UI for managing account and bearer tokens
    
    Add a new view enabling for a user to manage its account (email, password)
    but also its bearer tokens for Web API authentication.
    
    Account details are displayed in the view but edition of personal information
    must be done through the dedicated Keycloak account management UI (link to it
    is available in the view).
    
    The view is reachable by clicking on the username on the top right part of
    the Web UI once authenticated.
    
    Closes T2718

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

Harbormaster failed remote builds in B16556: Diff 15329!Oct 22 2020, 3:30 PM
swh-public-ci added a comment.Oct 22 2020, 7:41 PM

Build has FAILED

Patch application report for D4319 (id=15329)

Rebasing onto d3b1bf3839...

Current branch diff-target is up to date.
Changes applied before test
commit 1609142000859ded1bee3d9cfc0d992c1e59f4e5
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Oct 21 11:47:32 2020 +0200

    auth: Add user profile UI for managing account and bearer tokens
    
    Add a new view enabling for a user to manage its account (email, password)
    but also its bearer tokens for Web API authentication.
    
    Account details are displayed in the view but edition of personal information
    must be done through the dedicated Keycloak account management UI (link to it
    is available in the view).
    
    The view is reachable by clicking on the username on the top right part of
    the Web UI once authenticated.
    
    Closes T2718

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

anlambert updated this revision to Diff 15356.Oct 22 2020, 8:41 PM

Update: Fix cypress tests

swh-public-ci added a comment.Oct 22 2020, 8:49 PM

Build is green

Patch application report for D4319 (id=15356)

Rebasing onto d3b1bf3839...

Current branch diff-target is up to date.
Changes applied before test
commit e59d91ace16f38790cddb408b7fb29d96d3c24da
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Wed Oct 21 11:47:32 2020 +0200

    auth: Add user profile UI for managing account and bearer tokens
    
    Add a new view enabling for a user to manage its account (email, password)
    but also its bearer tokens for Web API authentication.
    
    Account details are displayed in the view but edition of personal information
    must be done through the dedicated Keycloak account management UI (link to it
    is available in the view).
    
    The view is reachable by clicking on the username on the top right part of
    the Web UI once authenticated.
    
    Closes T2718

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

Harbormaster completed remote builds in B16583: Diff 15356.Oct 22 2020, 8:49 PM
olasd accepted this revision.Oct 23 2020, 10:29 AM

Thanks !

This revision is now accepted and ready to land.Oct 23 2020, 10:29 AM
Closed by commit rDWAPPSe59d91ace16f: auth: Add user profile UI for managing account and bearer tokens (authored by anlambert). · Explain WhyOct 23 2020, 11:11 AM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPSe59d91ace16f: auth: Add user profile UI for managing account and bearer tokens.

Revision Contents
Changeset List

PathSize
cypress/
integration/
4 lines
swh/
web/
api/
11 lines
assets/
src/
bundles/
auth/
14 lines
auth/
8 lines
common/
1 line
templates/
api/
auth/
105 lines
8 lines
tests/
auth/
39 lines

Diff 15329

View Options

cypress/integration/api-tokens.spec.js

Loading...
View Options

swh/web/api/urls.py

Loading...
View Options

swh/web/assets/src/bundles/auth/index.js

Loading...
View Options

swh/web/auth/views.py

Loading...
View Options

swh/web/common/utils.py

Loading...
View Options

swh/web/templates/api/tokens.html

Loading...
View Options

swh/web/templates/auth/profile.html

Loading...
View Options

swh/web/templates/layout.html

Loading...
View Options

swh/web/tests/auth/test_views.py

Loading...
About · Developer information · Legal