Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for insecure redirect URI for development #1021

Closed
apognu opened this issue Sep 1, 2018 · 9 comments
Closed

Allow for insecure redirect URI for development #1021

apognu opened this issue Sep 1, 2018 · 9 comments
Assignees
Labels
Milestone

Comments

@apognu
Copy link

apognu commented Sep 1, 2018

Hi,

This is a feature request.

Currently, even when dangerous-force-http is enabled, there is a hard requirement that all redirect URIs be HTTPS, except for .localhost suffixes (and possibly raw IP adresses). I have to admit this is a pain for development, when using a domain that maps to a private network (Docker, VM, etc.).

The hardcoded .localhost suffix is also problematic, as on some Linux distros (with nss-myhostname), it automatically resolves to 127.0.0.1, bypassing DNS totally (and therefore not hitting my Hydra box), unless the NSS module is totally disabled.

My company also has a policy to use a public TLD for development domains even if the IP is non-routable, and as far as I know, Hydra is not usable like that without a proper DNS certificate.

I totally understand that forcing HTTPS is a feature for production, but I think it would be interesting to either:

  • have a flag that marks the instance as non-production (that would remove all HTTPS failsafes);
  • or allow for setting which domain are to be whitelisted by the redirect URI failsafe.

Thanks for your input.

@apognu
Copy link
Author

apognu commented Sep 2, 2018

I just saw that this is actually handled in fosite, do you want me to open the issue there instead? Any configuration would be done on hydra, but handling would happen in fosite.

@apognu
Copy link
Author

apognu commented Sep 2, 2018

And this was apparently discussed already in ory/fosite#273 as wontfix.

Should have searched better beforehand.

@aeneasr
Copy link
Member

aeneasr commented Sep 2, 2018

Flags like --allow-insecure-behaviour are a two-sided sword. On the one side, set ups like yours are much easier to test. On the other, what's used in development often makes it to production. People forget flags like these in Dockerfiles, kubernetes configs, you name it. I was thinking about whitelisting certain domains with a flag but I think it's also a bad idea, because you're apparently mirroring the public TLD but without SSL, so it's incredibly easy to forget that you need to remove this flag later on, and the error won't be caught because it's the same domain.

Is there no other way to work around this for you?

@apognu
Copy link
Author

apognu commented Sep 2, 2018

I understand that letting a way to do insecure thing is globally a bad thing, because people tend to choose the easy way over the secure one. If that's your stance, I totally understand it and will not argue this further.

For my particular situation, we do not mirror the public production domain. We have subdomain, unresolvable from the Internet, routable only from private networks and VPNs, that is used for development machines.

.localhost is a bit annoying because it does resolve to the current machine (as I said, through nss-myhostname, enabled on systemd hosts), and does not even allow a Hydra instance in a Docker container.

If the way of not allowing whitelisting domains is kept, I'll continue using a patched fork that adds my internal domain to fosite for my development host, that is not a big deal. :)

@aeneasr
Copy link
Member

aeneasr commented Sep 2, 2018

If that's your stance, I totally understand it and will not argue this further.

Yes, the "mantra" of this ecosystem is to make bad things hard. Another option is of course to add an SSL termination proxy in front of your API gateway or whatever you use, with a certificate that is either issued by an internal-trusted CA or self-issued (and trusted obviously)

@aeneasr aeneasr added this to the v1.0.0-rc.1 milestone Nov 3, 2018
@aeneasr aeneasr added the bug Something is not working. label Nov 3, 2018
@aeneasr aeneasr self-assigned this Nov 3, 2018
@aeneasr
Copy link
Member

aeneasr commented Nov 3, 2018

It makes sense to fix that and lower developer frustration. It makes sense to have --dangerous-force-http to enable http redirect urls as well.

@aeneasr aeneasr added feat New feature or request. package/cli package/oauth2 and removed bug Something is not working. labels Nov 3, 2018
@aeneasr aeneasr modified the milestones: v1.0.0-rc.1, v1.0.0 Nov 15, 2018
@aeneasr
Copy link
Member

aeneasr commented Nov 15, 2018

We will probably not address this as part of rc.1 but it's on the roadmap for 1.0 stable!

@aeneasr
Copy link
Member

aeneasr commented Apr 11, 2019

Revisiting this, I think it makes sense to have a list of domains whitelisted for development. This would only work when --dangerous-force-http is enabled and probably be a flag like --dangerous-allow-insecure-redirect-urls=http://mydomain.com/callback,http://mydomain2.com/callback. That way we would allow those redirect URLs to be combined with HTTP.

aeneasr added a commit that referenced this issue Apr 11, 2019
This patch enables developers to whitelist insecure redirect URLs while using flag `--dangerous-force-http`.

Closes #1021

Signed-off-by: aeneasr <aeneas@ory.sh>
aeneasr added a commit that referenced this issue Apr 11, 2019
This patch enables developers to whitelist insecure redirect URLs while using flag `--dangerous-force-http`.

Closes #1021

Signed-off-by: aeneasr <aeneas@ory.sh>
@ghost
Copy link

ghost commented Dec 26, 2020

Related issue cropping up when running with dangerous-force-http as an argument passed in using a container context. May not be your issue but I want to link these together for anyone who's digging later: https://github.com/slingshotlabs/reaction-oss-helm-chart/blob/develop/templates/hydra/hydra-deployment.yaml#L38

EDIT

Addressed above with #1354 so great just a config issue for those using dangerous-force-http without dangerous-allow-insecure-redirect-urls when not running using localhost domain.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants