Add support for oauth2 beind proxy #730
Conversation
This change does use the public_url config value to create the oauth2 callback url. This logic is only setup if the config value has been set. Closes palantir#724
|
Thanks for your interest in palantir/policy-bot, @KnisterPeter! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request. |
server/server.go
Outdated
| oauth2.OnLogin(handler.Login(c.Github, basePath, sessions)), | ||
| } | ||
| if c.Server.PublicURL != "" { | ||
| u, err := url.Parse(c.Server.PublicURL) |
There was a problem hiding this comment.
This is already parsed as publicURL on L73, so I think you can re-use it here. Since the earlier logic also requires that the value is not empty, I think we can unconditionally set the WithRedirectURL parameter.
server/server.go
Outdated
| @@ -218,17 +218,29 @@ func New(c *Config) (*Server, error) { | |||
| Base: basePolicyHandler, | |||
| } | |||
|
|
|||
| oauth2Params := []oauth2.Param{ | |||
| oauth2.ForceTLS(forceTLS), | |||
There was a problem hiding this comment.
The ForceTLS option is only used to generate URLs, so we should be able to remove it now that we're setting a fixed URL.
|
@bluekeyes That's indeed much cleaner 😃 |
|
It seems to me that this prohibits hosting policy-bot at a non root url, such as my-domain.com/policy-bot/ since my path is getting wiped by the default one. Am i missing something? |
|
@fordneild good point, I think that was an oversight here - the |
|
Thanks for the quick fix! |
This change does use the public_url config value to create the oauth2 callback url. This logic is only setup
if the config value has been set.
Closes #724