aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.39k stars 2.11k forks source link

Sensitive information are being persisted in local storage #3436

Open jiachen247 opened 5 years ago

jiachen247 commented 5 years ago

Describe the bug All cognito session tokens id, access and refresh tokens are being persisted into localstorage. This goes against all industry security best practice of storing sensitive infomation in signed httponly cookies.

To see why its bad practice this article presents a summary.

Also known as Offline Storage, Web Storage. Underlying storage mechanism may vary from one user agent to the next. In other words, any authentication your application requires can be bypassed by a user with local privileges to the machine on which the data is stored. Therefore, it's recommended not to store any sensitive information in local storage.

sc0ttdav3y commented 2 months ago

I've also had a negative pen test report recently on Cognito's handling of secrets (specifically the refresh token), but this time I'm on a web application.

I'm experimenting with using a web worker to hide the data as per Auth0's blog post: https://auth0.com/blog/secure-browser-storage-the-facts/

Here's a gist of an experimental web worker implementation. It works, but still has a race condition to resolve due to the nature of web worker comms being asynchronous. https://gist.github.com/sc0ttdav3y/55acd1f0d7b9cac3955aaa074e394d7e

From my research, the web worker solution seems to be the only practical way forward in browser-land at present. If this code is useful to anyone then feel free to use it.

The other idea I've got is to simply not store the secret at all. This could be done for the refresh token, but obviously not the access token. To do it, I've been toying with the idea of implementing some form of API Gateway + Lambda solution, where the app would register its refresh token to the server when it first gets it, and then it would call the Lambda via API to rotate its access token, by simply passing its access token and having it all happen server-side and return the new access token.

I hope AWS is reading this. It's an almost 5 year old issue so I hold out no hope, but it's certainly not best practice as it is now.

sc0ttdav3y commented 2 months ago

One more point to make on this topic. I see there's CookieStorage, and I initially went to use it but then realised that client-generated cookies offers no additional security over Local Storage.

Cookies are only secure if set on the server with the secure and httpsOnly flags.

gary-archer commented 2 months ago

If it helps anyone, I have an example SPA you can run that integrates with AWS Cognito.

Originally it used Cognito tokens in the browser and I updated it to use the token handler pattern, to store all tokens in HTTP-only SameSite=strict cookies that are issued by utility APIs. This deals with malicious JavaScript threats.

It doesn't use Amplify though, and requires a more complex flow with more moving parts.

JacobDel commented 2 months ago

One more point to make on this topic. I see there's CookieStorage, and I initially went to use it but then realised that client-generated cookies offers no additional security over Local Storage.

Cookies are only secure if set on the server with the secure and httpsOnly flags.

@sc0ttdav3y, I don't understand the difference between storing the refresh token in a cookie (so storing it at your server) vs the implementation above

I'm referring to :

I've been toying with the idea of implementing some form of API Gateway + Lambda solution, where the app would register its refresh token to the server when it first gets it, and then it would call the Lambda via API to rotate its access token

@sc0ttdav3y I'm also confused what the difference is between your web worker and using SessionStorage? Both methods store the data in memory and in both cases the data is lost when the browser is closed, if I understand correctly.

sc0ttdav3y commented 2 months ago

@JacobDel There's two types of cookie. The one currently implemented with Amplify (CookieStorage) is a cookie set by JS and it's vulnerable to malicious JS, as any cookie set in JS can also be trivially read by any other JS running on the same page via document.cookie. The other type is a server-issued cookie (not implemented by Amplify). This is actually still stored in the browser, but when set with httpOnly its contents cannot be read by JS code.

You also mentioned the SessionStorage implementation — that stores your tokens in plain sight just like local storage, so any JS on the page can trivially hit up your secrets with window.sessionStorage. It has the same risks as client-issued cookies and local storage.

It's different to the web worker solution I linked to because the web worker storage is not accessible by injected JS, while session storage is accessible. However, as it stands now the web worker example I offered is about the same as MemoryStorage, as both offer about the same security but no persistence. Auth0's article explains why web workers are interesting beyond in-memory solutions.

I'm really interested in what @gary-archer has done. His solution sounds like it implements the httpOnly cookie by using a utility server to issue it. This is similar to my own server-side thinking, however, I'm trying to remain within the Amplify SDK if I can so I'm looking to do everything through a custom storage adapter. My thinking is to keep the access token stored as-is in local storage but offload the refresh token to a server endpoint, which would be protected via the access token (i.e. Cognito + API Gateway + Lambda). With some polling, I can keep the access token refreshed without storing the refresh token itself to the JS. And by doing it in a custom storage adapter, Amplify's SDK will continue to work.

It's certainly an interesting conversation. Perhaps others subscribed to this issue have had the past 5 years to think of better ways than this?

sam-6174 commented 1 month ago

@sc0ttdav3y A "custom storage adapter" as you've suggested is interesting, but I'm wondering if there's another way to do this that completely avoids local storage. It could work something like this:

1) Keep all tokens on the heap, e.g. store everything in a local JavaScript object, similar to how is shown in https://docs.amplify.aws/gen2/build-a-backend/auth/manage-user-session#custom-storage 1) As is, the above implementation avoids security issues noted in this thread but would require user to login on every refresh or new tab/window. 1) Modify the above by implementing: 1) A /refresh endpoint on your domain that issues an http-only secure cookie, similar to what is described by @ravenscar here 1) The logic within the custom storage: 1) Upon initial page load, attempt to hydrate token storage contents via the /refresh endpoint. (This will work if we have a still-valid cookie from a prior page load, and we can reinstantiate storage contents without user interaction.) 1) If above is unsuccessful (because we lack still-valid cookie) then render Amplify's Authentication component, let normal login flow happen (user clicks buttons to authenticate), and then use token storage contents to get our cookie from /refresh so that future page loads will "just work"