search

HumanCellAtlas / ingest-central

Ingest Central is the hub repository for the ingest service
Apache License 2.0
0 stars 1 forks source link

High vulnerability: User token valid/usable after logout #568

Closed stahiri closed 4 years ago

stahiri commented 5 years ago

A new High severity vulnerability has been identified by manual business logic testing. Please remediate this within 30 days. Details follow below.

Service Found: https://ui.ingest.integration.data.humancellatlas.org Date found: 09/13/2019 Reporter: zbedo@broadinstitute.org Date reported: 09/19/2019

Category

Broken Authentication

Description

After the user logs out, his/her token is still VALID, that token can be used to perform different tasks.

Risk/CVSSv3

Risk: High Difficulty to Exploit: Easy

Steps to Reproduce

Step 1: User A logs out, but his token is still valid and can be used to send requests to the server.

This information + supporting screenshots can be found here.

stahiri commented 5 years ago

FYI @adrazhi @Lilalamar

aaclan-ebi commented 5 years ago

We're using tokens generated from Fusillade.

Hi @Bento007 , could you confirm if this is already supported ? I saw this ticket which is still open https://github.com/HumanCellAtlas/fusillade/issues/26 Thanks!

aaclan-ebi commented 5 years ago

Hi @stahiri , This ticket is related to both #567 and #566 in terms of fix? Is it alright if I close those tickets as duplicates? Or I can mark those blocked by this ticket?

stahiri commented 5 years ago

Please mark those as blocked by this ticket. That will allow us to track the remediation of these separate, but related, findings.

Bento007 commented 5 years ago

@aaclan-ebi, HumanCellAtlas/fusillade#26 is not currently supported.

Bento007 commented 5 years ago

I suggested a few options we can consider to provide a logout option. https://github.com/HumanCellAtlas/fusillade/issues/26#issuecomment-533282463

Bento007 commented 5 years ago

The tokens expire after a fixed amount of time. We can reduce the lifetime of a token to reduce the impact.

aaclan-ebi commented 5 years ago

Thanks @Bento007 !

Changes in Ingest UI:

  1. Change params to redirect endpoint to also request for refresh token
  2. Add logic to refresh tokens before JWT expires
  3. When a user logs out, send request to fusillade to revoke refresh tokens
aaclan-ebi commented 5 years ago

@Bento007 could you note here the actual endpoints for refreshing jwts and revoking refresh tokens?

@justincc should we include this in our current sprint? next sprint? I think this needs to be fixed by 19th Oct.

justincc commented 5 years ago

@aaclan-ebi We will include this in our next sprint.

Bento007 commented 5 years ago

@aaclan-ebi use https://auth.data.humancellatlas.org/.well-known/openid-configuration to retrieve the endpoint you need to refresh and revoke.

aaclan-ebi commented 5 years ago

Thanks @Bento007 ! May I know what will be the new expiry time of the tokens?

aaclan-ebi commented 5 years ago

Hi @Bento007,

It looks like in order to get a refresh token, we need to set the scope param in /authorize endpoint to also include offline_access (https://auth0.com/docs/tokens/refresh-token/current#get-a-refresh-token). However, we’re currently using the “Simple Webapp flow” (https://allspark.dev.data.humancellatlas.org/dcp-ops/docs/wikis/Security/Authentication%20and%20Authorization/Setup%20DCP%20Auth#example-simple-webapp-flow), and we’re only passing redirect_uri .

Would it be possible to set that in fusillade so that when we get the response in the callback uri, we'd already get the refresh token?

Many thanks!

aaclan-ebi commented 5 years ago

It looks like that even if the change requested above is applied, it would still require the UI to be using Auth Code flow to actually implement the refreshing of tokens.

The Ingest UI is a Single Page application (SPA) and is currently using the "Simple Webapp flow" which I believe is the same as the Implicit grant flow which most SPA's use (access token instead of code is received by the client after redirect).

I think to proceed with this, the Ingest UI must use the OIDC webapp flow (Auth Code flow) described here https://allspark.dev.data.humancellatlas.org/dcp-ops/docs/wikis/Security/Authentication%20and%20Authorization/Setup%20DCP%20Auth#example-oidc-webapp-flow

Bento007 commented 5 years ago

Let's try using the implicit flow described by auth0 with silent authentication.

aaclan-ebi commented 5 years ago

We're still going with the option 1 approach (described here https://github.com/HumanCellAtlas/fusillade/issues/26#issuecomment-533282463) :

  1. User logs in and gets access token + refresh token. The (JWT) access tokens will have very short lifetime.
  2. Use refresh token to get new access tokens while user is logged in.
  3. Upon user logout, revoke the refresh token.

As discussed with Trent, Ingest UI is going to adapt the Authentication code + PKCE flow implemented in the new JavaScript library for implementing authentication & authorization in single page apps (SPA) with Auth0.https://auth0.com/docs/libraries/auth0-spa-js https://auth0.com/docs/api-auth/tutorials/authorization-code-grant-pkce

We can't readily use that Auth0 library as it's too specific for Auth0 and authentication endpoints are hardcoded in the library (not retrieved from the discovery document (/.well-known/openid-configuration)

I'm looking into using this OIDC Certified library to implement the Authentication code + PKCE flow for Ingest UI. https://www.npmjs.com/package/angular-oauth2-oidc

Currently, I'm encountering an issue making a request to revoke the refresh token from the application. I posted a topic in Auth0 Community regarding the issue. https://community.auth0.com/t/post-oauth-revoke-blocked-by-cors-policy/32303

justincc commented 5 years ago

Hi @Bento007. Having reviewed this ticket and discussed it with @aaclan-ebi, I don't see much clarity on how Fusillade wants us to tackle this problem. We've spent a long time on it now without getting anywhere much. Please could we have a meeting to discuss? I'm available Tuesday but then away Wed-Fri, so after that the next opportunity will be to talk at the F2F. I and @aaclan-ebi will slack you as well about this.

In the meantime, I am asking @aaclan-ebi to stop working on this until we have more clarity.

Bento007 commented 5 years ago

@aaclan-ebi I agree with your strategy up to the apart where we pass the refresh token to the SPA. Refresh token should be used only on clients we can trust, also the revoke endpoint will revoke all refresh token for that user. This include any refresh tokens provided for HCA cli login or any where else in the HCA they have requested a refresh token.

Rather than providing a refresh token we can use the implicit grant flow . This is a standards in OIDC and will allow you to retrieve new tokens while the user is logged into auth0. You can then logout from auth0 to prevent new access token from being generated.

I'll work on a prototype to demonstrate how it works.

Bento007 commented 5 years ago

https://github.com/HumanCellAtlas/fusillade/pull/289 adds silent authentication and logout for simple application flow. This can be used by a Single page application to verify the state of a users sessions in fusillade.

aaclan-ebi commented 5 years ago

During DCP F2F meeting, Trent (@Bento007 ) and I discussed the use of cookies to store the JWT tokens instead of browser's local storage. This would require changes to Ingest API service to also check user cookies. I filed a separate ticket for that.

https://app.zenhub.com/workspaces/dcp-5ac7bcf9465cb172b77760d9/issues/humancellatlas/ingest-central/602

The changes to do implicit flow authentication with silent auth is in PR now. I noticed that the ff issues are also happening so often recently and somehow blocking the testing of this change.

https://github.com/HumanCellAtlas/fusillade/issues/282 https://github.com/HumanCellAtlas/fusillade/issues/283

I will deploy it to dev to monitor. It's connecting to the DCP Auth Service dev env.

aaclan-ebi commented 5 years ago

Hi @Bento007 ! So I deployed the changes (https://github.com/HumanCellAtlas/ingest-ui/pull/37) in dev this week and I noticed the redirect error issue happening frequently when sending requests to authorize and userInfo endpoints. (Redirect Error Issue https://github.com/HumanCellAtlas/fusillade/issues/282)

I'm worried that if this is deployed and promoted to all envs, users' access token will expire and not get renewed and not be able to make submissions. Is it possible to fix the redirect issue first?

Many thanks!

aaclan-ebi commented 4 years ago

The Redirect Error issue is now fixed. Thanks for fixing, @Bento007. I deployed the UI changes to DEV and INTEGRATION, should be released to staging next week, then prod the ff week. Currently it's still pointing to Auth Service in dev. @Bento007 Please let us know when we can point to integration.