remotestorage / spec

remoteStorage Protocol Specification
https://tools.ietf.org/html/draft-dejong-remotestorage
87 stars 5 forks source link

OAuth: Specify allowed schemes for redirect_uri parameter #175

Open palant opened 5 years ago

palant commented 5 years ago

The spec says:

if no client registration is required, the server MUST ignore the value of the client_id parameter in favor of relying on the origin of the redirect_uri parameter for unique client identification.

For this to be a good measure, the scheme of the redirect_uri parameter needs to be something where the origin is something meaningful. However, so far all servers I've seen which implement this part of the spec correctly (such as reStore) do not enforce particular schemes for redirect_uri. This makes redirect_uri values such as data://google.com/,text possible where google.com is considered the origin. Luckily, browsers usually will refuse to redirect to this URL.

There is another aspect here: php-remote-storage, while failing to implement this part of the spec correctly, has the following line in its validation routine:

// XXX we also should enforce HTTPS

Should this ever be implemented, it would break most non-web clients.

I think that the spec should prevent such issues by explicitly stating: allowed schemes for redirect_uri are HTTP and HTTPS.

raucao commented 5 years ago

There are actually implementations of RS clients in Web Extensions (Chrome/FF add-ons e.g.), which use a browser extension origin that is not HTTP. So this change would also break browser extension clients.

palant commented 5 years ago

Web servers can redirect to browser extension origins? That's new for me, I wonder whether that can be relied upon. It definitely requires using web_accessible_resources however which has considerable privacy and possibly security drawbacks. And the URL origin displayed to users is quite meaningless...

I solved this by redirecting to http://localhost:nnnnn/ instead. It doesn't matter that there is no web server running on this port, a browser extension can monitor the authorization tab and detect when it redirects to this URL.

raucao commented 5 years ago

Yes, an HTTP response can redirect anywhere, and a user agent can decide to take you there. A browser extension origin also falls under the single-origin policy, same as other origins, and thus has its own sandboxed local storage, permissions, etc.

raucao commented 5 years ago

FYI: At 5apps, we're currently working on a new UI for our remoteStorage accounts, and one new feature is that users can name the client when they authorize it first. The default name suggested will just be HTML <title> content, if present, but always showing the origin next to that.

palant commented 5 years ago

Actually, it seems that there is even a Chrome API to do this: https://developer.chrome.com/extensions/identity#method-launchWebAuthFlow. That's what the WebVault extension is using. The "LessPass remoteStorage" extension will redirect to an author-owned URL instead. That's all the extensions with remoteStorage support I could find.

raucao commented 5 years ago

I remember https://github.com/lesion/memm by @lesion. Forgot about the others.

Interesting that they have an API for Chrome extensions for that now! We should probably document that in https://remotestoragejs.readthedocs.io/en/latest/ somewhere (and how to use it in extensions in general).

palant commented 5 years ago

Firefox supports it as well: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/identity. The documentation is confusing, but in the source code I can see that it will redirect to https://<hash>.extensions.allizom.org/. Here as well - redirecting to a site that doesn't exist, the browser will detect the redirect itself, same thing that I implemented manually.

palant commented 5 years ago

I remember https://github.com/lesion/memm by @lesion. Forgot about the others.

It also uses identity.launchWebAuthFlow(). I didn't find it because it isn't on Chrome Web Store any more.

raucao commented 5 years ago

Cool. Personally, I don't see anything standing in the way of a pull request then.