supporting openstax oauth, a non-openid-compliant oauth2 provider

[reedstrm]

Thanks for vouch, I was able to get it working with our local, home-grown oauth2 provider, but hit two issues. (Bear with me, I'm new to golang)

Took me a bit to realize that I had to provide one of the listed providers, since there's no default fallback for getUserInfo(). So, go ahead and use oidc. This lead to the first, biggest problem, is that our code is oauth2, not OpenID, so vouch gets through all of the authentication hand-shaking steps, but when going to grab the UserInfo, it falls down on doing the code to token exchange, here:

vouch-proxy/handlers/handlers.go

Line 528 in 5f336c4

ptokens.PIdToken = providerToken.Extra("id_token").(string)

Since our oauth2 server doesn't sent an id_token (or any Extra, for that matter), this line blows up. As far as I can tell, the PIdToken is never used elsewhere in vouch-proxy, so I commented this line out, and make it to the next problem. :-) (BTW, I see value in upgrading our server to provide this JWT token in any case, so this problem may go away, for me. Otherwise, I don't know enough go to test if the providerToken has an Extra member, sorry. )

Also, the error for that one looks almost exactly like what was seen here:
#20 (no real hint where the actual problem is)

The next problem seems to be the usual - everyone's UserInfo is different. In our case, emails are nested inside a contacts hash, we provide no top-level email member in the json. We do however, provide a username at the toplevel. So this code:

vouch-proxy/pkg/structs/structs.go

Lines 27 to 29 in 5f336c4

	// PrepareUserData implement PersonalData interface
	func (u *User) PrepareUserData() {
	u.Username = u.Email

stomps on the username. Which caused the attempt to store in the bold db to fail.
For that fix, I just conditionalized it:

https://github.com/reedstrm/vouch-proxy/blob/21b0770605956be2ee68510589af21751fe86d0a/pkg/structs/structs.go#L27-L32

// PrepareUserData implement PersonalData interface
func (u *User) PrepareUserData() {
	if u.Username == "" {
	    u.Username = u.Email
    }
}

If we decide to use vouch in production, I think I'll just go ahead and define an openstax provider, and encapsulate the necessary getUserInfo changes there.

Just thought I'd share my experience with y'all and get any feedback you might have.

[bnfinet]

thanks for diving into Vouch Proxy @reedstrm and sharing your experience

Testing u.Username before overwriting it seems like the correct behavior. Care to offer a quick PR?

Not specific to this issue, but generally related, I've thought we should probably be using u.Sub or u.Subject instead of u.Username as per https://tools.ietf.org/html/rfc7519#section-4.1.2

wrt ptokens.PIdToken I think we have to leave it in for oidc but I do understand how that does not work for your use case.

I'd be happy to accept a PR for openstax. I wonder if it could be generalized to something like generic_oauth and perhaps an additional configuration element such as oauth.subject_field could be used to define the field which should be used to get the username/email/subject from. That might offer broader utility for other oauth2 implementations.

[reedstrm]

Sure, I'll cut a branch w/ just the don't clobber on it. As to the other, not sure I have sufficiently broad experience to generalize to "other oath2 implementations" I've really only worked with the one.
<wavy lines 3 hours later>
So, that wasn't too painful. I'll cut two PRs: one w/ the don't clobber and test for valid config provider name. The other w/ adding an OpenStax provider (note that's not OpenStack, the cloud software components, but OpenStax, the Open Education Resources (and free) college textbooks.)

[reedstrm]

fixes in PRs #140 and #141

[bnfinet]

#141 has been merged!