search

OriginProtocol / origin-js

We've moved to a monorepo: https://github.com/OriginProtocol/origin
MIT License
81 stars 33 forks source link

fix for issue #484 on ipfs hosted key leaking #565

Closed crazybuster closed 5 years ago

crazybuster commented 5 years ago

First pull request? Read our guide to contributing

Checklist:

Description:

Please explain the changes in this PR:

This fixes the issue where messaging keys might be leaked over to other dapps in a ipfs hosted domain. Current implementation will lock the messaging keys to our dapp, or the dapp have to enable messaging in a root sub directory.

DanielVF commented 5 years ago

This breaks commit breaks Origin running under node, because node does not have a location variable. https://github.com/OriginProtocol/origin-js/blob/b58a97a836496a9acd3542f34eed3109b2e1301d/src/resources/messaging.js#L145

The tests still pass because the tests do not create a Messaging object, nor do they create an Origin object that creates a Messaging object.

franckc commented 5 years ago

Good catch @DanielVF - Do we anticipate we would we'll need in the future to run Messaging on node vs in-browser ? I'm not clear for what use case ? Are you thinking for writing like a customer service bot for example ?

micahalcorn commented 5 years ago

@franckc @DanielVF I should have caught this because origin-js will need to be able to run in the mobile app and we generally want developers to be able to import it anywhere.

franckc commented 5 years ago

But doesn't cookieStorage require to run in browser ? It refers to "document" - will that be available in the mobile app ?

DanielVF commented 5 years ago

@micahalcorn We just need to have even a simple test case that we can successfully call the messaging constructor from the tests. With no tests, I'd not expect anyone to catch it.

@franckc cookieStorage is acting like a localstorage. We just need something with that same interface that runs in node, and use it as required.