Allow redirect configuring

[arturpfb]

Adds a REDIR_URI setting that operates by using it if it exists, else following the existent pattern of request.build_absolute_uri(reverse("django_auth_adfs:callback"))

It can be pretty useful in cases where there is a proxy or gateway before the application (where we lose the identification of is_secure). Today, the suggested fix for this situation is by creating on this proxy/gateway a header with this identification, to be used by SECURE_PROXY_SSL_HEADER. This is not possible in some situations (as was my case on the company I work, where it would require actions of multiple other teams that are responsible for the gateway).

Other problem that is solved by this is when the host/netloc of the request that is used to parse the redirect uri is different from a desired one (due to an app with multiple domain names, proxy/gateway configuration, etc)

This pr has the possibility to solve #327 and #303

[codecov]

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (378f141) 86.1% compared to head (f03deda) 86.0%.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #330     +/-   ##
========================================
- Coverage    86.1%   86.0%   -0.1%     
========================================
  Files          11      11             
  Lines         556     561      +5     
========================================
+ Hits          479     483      +4     
- Misses         77      78      +1     

Files	Coverage Δ
django_auth_adfs/config.py	87.3% <83.3%> (-0.2%)	⬇️

[JonasKs]

I'd like us to discuss alternatives to this setting, since it's against what Django itself recommends, and I don't really feel comfortable adding features we cannot recommend using.

1. Gunicorn specifically recommends this setting through nginx, but also has settings to add them directly.
2. Uvicorn has settings to add this with the --proxy-headers flag
3. Maybe a custom middleware for you is enough?

class ModifyRequestMiddleware(MiddlewareMixin):
    def process_request(self, request):
        request.META['HTTP_X_FORWARDED_PROTO'] = 'https'

[arturpfb]

Hm... Yeah, going against django premisses is pretty bad indeed.

Exploring a little bit more the situation I'm in, it's something like:

The Gunicorn/Uvicorn path is kinda impossible due to my current environment (the gateway/proxy/balancer I cited)

This middleware solution looks promissing for the https part of the problem, but I'm still stuck on the domain name one.

In the pr description, I haven't explained it completely, but in my situation, my app has a domain name specific to how requests are handled inside the cluster it is currently. This is how the redirect uri is built currently, so I get something like http://appname-appname/sufix/oauth2/callback?code=..., where I need it to be something like https://PrettyAndClientAccessibleDomain/oauth2/callback?code=....

I need some way to indicate what is the real domain-name, before all this, but currently, there is none. It could go in a way similar to how I built this solution (I found that there is a header indicating the original domain name pre-request), but the redirect-uri would need to be mutated anyway, so this solution would go a little beyond in fixing both problems simmultaneously.

And I've made it opptional, so this option is available in cases where it is a must, but in all others, the default, automatic path can be used.

[JonasKs]

How do you run your server?

[arturpfb]

It is a gunicorn app. This runs inside a kubernetes cluster, with a gateway and a reverse proxy on the way between the client and the app. Both the gateway and the reverse-proxy are managed by other teams.

A fellow co-worker indicated that we can probably make it work completely with the middleware suggestion you made. We'll try it, seems pretty promising.
Sorry about my limited django knowledge, I'm just recently starting to use it.

[JonasKs]

I'd request the K8s team to ensure their proxy forwards the header, but that's just me.

An alternative is to use sslify, I've heard people use that for Heroku back in the days.

[JonasKs]

Did you find a solution? I'm still hesitant on this change.

[arturpfb]

Sorry for disappearing for so long, went on a vacation trip and got some time without a computer.
Updating on the problem I brought, internal do my company, we got it to work purely with middlewares, following your hint (Thanks a lot for that!!! <3)
On this PR's subject, we can discuss potential ways, but I agree that the way used in here wasn't very django-ish. Do you think something in this direction, but built differently, would be a great addition, or we should just close this PR?

[JonasKs]

That makes me happy - glad it got resolved. 😊

I think I’d like this closed for now - I’d like to keep this library as close to how Django documents stuff to be done. Thanks a lot for the PR, though - it sparked a good discussion.

[arturpfb]

Agreed my friend. Thanks a lot!

[alex-atkins]

@arturpfb what was your solution? I have almost the exact same situation and the same issue. Strange thing is that I did not have this problem when deployed with gunicorn and wsgi, but I'm testing gunicorn with asgi / uvicorn and this issue appeared. Only change made was this switch.

[arturpfb]

Hey @alex-atkins !
While on this infrastructure, we used the proposal of @JonasKs of having a middleware.
Basically it did something along the lines of the following, before proceeding to the treatment of the request itself:

  def __call__(self, request):
       setattr(request, "_dont_enforce_csrf_checks", True)
       response = self.get_response(request)
       return response

Later we migrated to an infra where the header was correctly redirected.
Try something like the code above, and tell us if it works for you.
(we use gunicorn + wsgi, so maybe it won't work the same for asgi + uvicorn, but there is a good probability that it will)