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 additional trusted hosts during callback redirect #355

Open
axieum opened this issue Nov 29, 2024 · 8 comments
Open

Allow additional trusted hosts during callback redirect #355

axieum opened this issue Nov 29, 2024 · 8 comments

Comments

@axieum
Copy link

axieum commented Nov 29, 2024

During development, our Django backend runs at http://localhost:8000/ while the frontend is at http://localhost:5173/.

The following host verification code restricts the redirect to the current request's host.

url_is_safe = url_has_allowed_host_and_scheme(
url=redirect_to,
allowed_hosts=[request.get_host()],
require_https=request.is_secure(),
)
redirect_to = redirect_to if url_is_safe else '/'

Could it be possible to merge in Django's ALLOWED_HOSTS setting so we can redirect them back to the original client that may be at a different host?

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@tim-schilling
Copy link
Member

I'm curious why specifying the redirect via state doesn't work as a workaround?

@axieum
Copy link
Author

axieum commented Dec 2, 2024

At present, the view decodes the redirect URI from the state but since the redirect is to a different host than the Django application itself, it replaces it with /.

This issue requires that we can add more allowed redirect hosts, so we can redirect away to the frontend application that may be at a different host.

@axieum
Copy link
Author

axieum commented Dec 3, 2024

Could it be possible to merge in Django's ALLOWED_HOSTS setting so we can redirect them back to the original client that may be at a different host?

Instead of using Django's setting, I think a dedicated setting in the AUTH_ADFS namespace would be more secure and flexible.

@tim-schilling
Copy link
Member

I think using Django's ALLOWED_HOSTS setting would be reasonable. It feels overly redundant to create a separate setting. I would be open to creating a get_* method on the view to fetch it so it's easier for other devs to restrict/customize it in the future. @JonasKs thoughts?

@JonasKs
Copy link
Member

JonasKs commented Dec 15, 2024

I haven’t written Django frontends for probably 5 years, but why would they be hosted on a different URL/port? Is this because of use of e.g. DRF and single page frontends?

If so, the frontend should fetch tokens using PKCE flow, and send the token as a header to the backend.

@axieum
Copy link
Author

axieum commented Dec 15, 2024

Originally I had Remix frontend handle SSO and then forward the ADFS JWT with every API call using the provided AdfsAccessTokenAuthentication authentication backend. However, I noticed it was making lots of database calls with each request to sync the Django user (and groups) with the JWT contents.

So now we are looking to piggyback off the existing Django session cookie since it's accessible by both the backend and frontend.

  • example.com/api › Django ASGI
  • example.com › Remix

It's just that during local development, it runs on a different port at localhost which isn't allowed redirect destination.

  • localhost:8000 › Django ASGI
  • localhost:5173 › Remix

For now we are logging into Django to set the session cookie, and then manually going back to the Remix app. Works a treat.

As for third-party's using the Django API - they go through an API gateway which handles their Oauth2 token exchanging, etc.

@JonasKs
Copy link
Member

JonasKs commented Dec 15, 2024

I see. I'm a bit unsure of the security implications of merging a SPA into ALLOWED_HOSTS, as it's used to protect from host header attacks, @tim-schilling. Maybe there is none?

Since this is a dev-problem, an alternative could be to allow any configured host when DEBUG is on? I'd prefer not to introduce extra settings and pit falls to production environments. Most security incidents are due to configuration issues.

@tim-schilling
Copy link
Member

I was imagining this being useful for a services based approach where you authenticate against login.domain.com and then are redirected to app.domain.com.

I'm not sure about the security implications of that. I would imagine if you're in charge of both hosts, it should be fine. I don't see these concerns as possible if that's the case:

Django uses the Host header provided by the client to construct URLs in certain cases. While these values are sanitized to prevent Cross Site Scripting attacks, a fake Host value can be used for Cross-Site Request Forgery, cache poisoning attacks, and poisoning links in emails.

Additionally, it looks like request.get_host() is the piece that validates the host against ALLOWED_HOSTS. Forwarding those along makes sense in some cases.

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

No branches or pull requests

3 participants