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 multiple callback_urls for all backends #79

Closed
dubit0 opened this issue Feb 25, 2019 · 3 comments · Fixed by #161
Closed

Allow multiple callback_urls for all backends #79

dubit0 opened this issue Feb 25, 2019 · 3 comments · Fixed by #161

Comments

@dubit0
Copy link

dubit0 commented Feb 25, 2019

In principle one vouch proxy instance can be used to authenticate users on different vhosts. Putting the nginx config for vouch in a separate file and including it for different vhosts adds a lot of convenience. Such setup currently almost works and only fails at the callback since only a single callback url can be specified in the vouch config. Only the google backend already allows for multiple callback_urls. As can be seen below, it chooses the callback url that matches the identified domain.

if cfg.GenOAuth.Provider == cfg.Providers.Google {
// If the provider is Google, find a matching redirect URL to use for the client
domain := domains.Matches(r.Host)
log.Debugf("looking for redirect URL matching %v", domain)
for i, v := range cfg.GenOAuth.RedirectURLs {
if strings.Contains(v, domain) {
log.Debugf("redirect value matched at [%d]=%v", i, v)
cfg.OAuthClient.RedirectURL = v
break
}
}

To enable using one vouch proxy on different vhosts, it would be great to move the code out of the if-statement and apply this logic to all backends.

@bnfinet
Copy link
Member

bnfinet commented Mar 19, 2019

Yeah that seems like it'd be good.

domains.Matches in that block is specific to the vouch.domains config element.

func Matches(s string) string {

The test might be better against the originally provided URL stored in the session variable.

@bnfinet
Copy link
Member

bnfinet commented Jul 1, 2019

As per #133 @libussa do you have working code? Care to submit a PR?

If not @jayb1122 perhaps you'd care to work on this?

@libussa
Copy link
Contributor

libussa commented Jul 1, 2019

@bnfinet I do, I just moved the if logic. I didn't however do what you suggested (against the originally provided URL stored in the session variable)

I can submit a PR if you still want it

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

Successfully merging a pull request may close this issue.

3 participants