Allow multiple hostnames #10
Conversation
* Enable ProxyHeader handler to get accurate scheme/hosts/etc * Figure out hostname from hosts header * Add tests for above * Support legacy REDIRECT_URI
There was a problem hiding this comment.
This is awesome! Thank you! I especially appreciate your attention to backwards compatibility, and your addition of tests! 10/10 PR and a great explanation of your use case.
I did some manual testing and discovered a really minor bug up above. If you don't have the bandwidth to fix it I'm happy to just merge and take care of it but wanted to give you the opportunity.
PS Sorry for the delay in getting back to you, I've been out of the country for a couple weeks.
| @@ -38,6 +58,7 @@ func authorize(w http.ResponseWriter, r *http.Request) { | |||
|
|
|||
| tmpl := template.Must(template.ParseFiles("static/index.html")) | |||
| data := AuthorizePage{ | |||
| SelfRoot: SelfRoot(r), | |||
| Authorized: true, | |||
| URL: url, | |||
There was a problem hiding this comment.
url (defined above) is still being populated by the REDIRECT_URI env var, so when a user attempts to authorize the against a Plaxt instance that only has ALLOWED_HOSTNAME they won't get a usable URL.
I think you can just drop SelfRoot(r) in the place of the Getenv and everything will be groovy.
There was a problem hiding this comment.
:P i had to re-read that like 3 times. I made a wrong assumption of what you were saying. You are saying get rid of the os.Getenv from the url := declaration, not drop SelfRoot from the struct.
Yea! i can do it
Once i figure out more about how interfaces work, i'll see if i can't make more tests for the handlers (cause of trakt.AuthRequest).
I have an inhouse k8s cluster I use for deploying random apps. I have an internal hostname I use for testing that everything works right. The existing codebase didn't allow me to support my internal and external hostname.
I don't really think it is beneficial to anyone but me, but I wanted the practice writing go code, and writing tests.