remotestorage / remotestorage.js

⬡ JavaScript client library for integrating remoteStorage in apps
https://remotestoragejs.readthedocs.io
MIT License
2.31k stars 141 forks source link

redirectUri in createOAuthAddress #46

Closed ErikAndreas closed 12 years ago

ErikAndreas commented 12 years ago

Hi

why the addition of protocol check (http|https)? I use remoteStorage.js in an Android webview with local/bundled (html and js) files and a custom intent filter schema for catching the callback and pass control back to my app.

It would be nice to keep the unhosted idea and not have to have the callback on a http/https resource?

ErikAndreas commented 12 years ago

to clarify: this applies to the addition of the lines around https://github.com/unhosted/remoteStorage.js/commit/4ef0f76365171f74427c9ebb3958beb3d35cf8c4#L4R42

michielbdejong commented 12 years ago

sorry, for some reason i didn't receive a notification of this, i thought i usually get an email from github when an issue is opened... anyway, the reason we want to check that client_id is equal to the hostname of redirect_uri is so that nobody can claim client_id 'niceapp.com' and then use 'n1ceapp.com' as the malicious redirect_uri. what do your redirect_uri's look like?

ErikAndreas commented 12 years ago

np

to clarify; it's just the protocol check ending with throw exception if not http|https that troubles me.

In this context the redirectUrl is something like my.custom.schema://appname - a totally non standard w3c protocol/schema (some claim that's a bad thing to do, but not totally uncommon for android apps - see http://stackoverflow.com/questions/2958701/launch-custom-android-application-from-android-browser as an example). I think it's not uncommon in iOS apps as well.

michielbdejong commented 12 years ago

so maybe we should say the string my.custom.schema can contain [a-z] as well as . and - instead of only http or https?

ErikAndreas commented 12 years ago

Agreed, or just skip protocol/schema check totally? Den 25 jun 2012 18:09 skrev "Michiel@unhosted" < reply@reply.github.com

:

so maybe we should say the string my.custom.schema can contain [a-z] as well as . and - instead of only http or https?


Reply to this email directly or view it on GitHub: https://github.com/unhosted/remoteStorage.js/issues/46#issuecomment-6551630

michielbdejong commented 12 years ago

well, we have to check that the hostname is the right one. i could imagine a vulnerability where the redirect_uri is 'mailto:nice.com\/@evil.com' (not sure it that would work, but if we allow all protocols, then it could, i guess) or 'http://nice.com.evil.com/' or whatever, that is why that check is so strict there.

On Mon, Jun 25, 2012 at 6:20 PM, ErikAndreas reply@reply.github.com wrote:

Agreed, or just skip protocol/schema check totally? Den 25 jun 2012 18:09 skrev "Michiel@unhosted" < reply@reply.github.com

:

so maybe we should say the string my.custom.schema can contain [a-z] as well as . and - instead of only http or https?


Reply to this email directly or view it on GitHub: https://github.com/unhosted/remoteStorage.js/issues/46#issuecomment-6551630


Reply to this email directly or view it on GitHub: https://github.com/unhosted/remoteStorage.js/issues/46#issuecomment-6551957

ErikAndreas commented 12 years ago

Ok, just saying - there might be other protocols/schemas in scope than http|https such as chrome(-extension)://, my.custom.iosOrAndroidsschema:// etc

nilclass commented 12 years ago

This looks solved (or worked around) in https://github.com/unhosted/remoteStorage.js/blob/master/src/lib/widget.js#L90, so will be fixed with the release of v0.7.