horizon icon indicating copy to clipboard operation
horizon copied to clipboard

redirect_uri is hardcoded for https

Open Penagwin opened this issue 8 years ago • 7 comments

This affects github.js however it does/may affect other Oauth endpoints. Currently the code adds &redirect_uri=https%3A%2F%2F[Rest of url] to the url that the user is directed to at github. When this redirect_url isn't the same as the one specified in github's settings it will cause an error, and the github will redirect you back to your app with the error: horizon_error=The%20redirect_uri%20MUST%20match%20the%20registered%20callback%20URL%20for%20this%20application.

This causes an issue in development when working with local addresses where you need to use http (for example with a local address). I suggest that we change line 108 in utils.js to use the page's current protocol, although I'm not sure how you would want to go about doing that.

My other suggestion is to change line 28 in github.js to not include the redirect_url option, as it is optional to use with github, and it is configurable on github's website. Again this may or may not apply to other oath providers, I'm not super familiar with Oauth.

Now I made this change locally, however after logging in with github I get sent to http://localhost:8181/?horizon_error=session%20expired, I'm not sure if this is related.

Penagwin avatar May 25 '16 15:05 Penagwin

I may be wrong, but the documentation for most (if not all) auth providers say that you should only use HTTPS for the redirect URI. At least some of them will change http to https or reject your redirect URI if it is not secure. Out of security concerns, I'm not convinced we should allow http redirect URIs in the release branch. Omitting the redirect URI if it is not specified seems fine, as long as the provider handles that correctly.

Keep in mind that it is not too difficult to use HTTPS even when developing on localhost - using a self-signed key, you just need to tell your browser to ignore the warning.

Tryneus avatar May 25 '16 21:05 Tryneus

If you want to force https during production, great. But forcing it during development is an unnecessary burden. No other library I've encountered has forced me to use https for localhost.

Although you say it's "not too difficult", it is still an unnecessary, de-motivational blocker, especially when you're just trying to see if horizon works at all on your own computer.

gilbert avatar Jul 23 '16 15:07 gilbert

I agree that we should allow for http redirect URLs. We can print a warning maybe if we detect it, but I don't see a good reason for forbidding it completely.

As @mindeavor says, forcing https just adds another step to getting a test application up. And it's not a particularly intuitive step either (you'll have to look up documentation for how to enable encryption at the very least, plus figure out how to create self-signed keys, how to convince hz in dev-mode to enable encryption (https://github.com/rethinkdb/horizon/issues/643) etc.).

danielmewes avatar Jul 25 '16 18:07 danielmewes

Moving this to first post-2.0 release.

danielmewes avatar Jul 25 '16 18:07 danielmewes

This is actually kind of a roadblock for development thru proxy.

I created a Vue.js app, and I'm using Webpack with proxy configuration, where the main Horizon backend act as a normal backend and auth. I'm developing for a Facebook auth right now, and it's forced with a https://127.0.0.1:8181/... redirect_uri, whereas my main Webpack app is at http://localhost:8080. The Webpack is important because it provides nice features right-out-of-the-box, and the most important for me is the live reload with hot patching, I mean productivity and other nice features makes developer happy.

The current situation makes it hard to develop in this kind of scenario.

Obviously, I'm not the only one who is doing this kind of development, I'm sure there are others. But right now, few here have reported this.

Since Horizon is on the rise, roadblocks like this may turn-away people who should have actually adopted Horizon. I believe Horizon is great, but hardcoded things like these, and forcing HTTPS during development is just plain demotivating.

Convention is nice, but there should be a good reason should it be enforced. It's development on localhost and not production.

eliezedeck avatar Sep 28 '16 19:09 eliezedeck

Hey, friendly ping any news? :)

huv1k avatar Nov 01 '16 11:11 huv1k

@Huvik If you are looking for a quick fix right now try changing line 108 in utils.js to what you need

Penagwin avatar Nov 03 '16 14:11 Penagwin