more robust ClearSessionCookie()

[ploxiln]

default domain changed from request Host to blank, recently
try to clear cookies for both

The combination of #466 and #376 causes this problem where the cookie which could not be parsed also was not cleared, and login never worked:

2017-12-12 23:47:54.321 oauthproxy.go:608: 127.0.0.1:49562 ("158.106.212.130") could not decode session state: expected 2 chunks got 1
2017-12-12 23:47:56.665 github.go:178: Found Github Organization:"PhysicalGraph" Team:"platform-video-pizza-box" (Name:"Platform: Video Pizza Box")
2017-12-12 23:47:56.766 github.go:239: got 200 from "https://api.github.com/user/emails" [{"email":"pierce.lopez@gmail.com","primary":true,"verified":true,"visibility":"public"},...]
2017-12-12 23:47:56.882 github.go:288: got 200 from "https://api.github.com/user" {"login":"ploxiln","id":649835,"avatar_url":...}
2017-12-12 23:47:56.882 oauthproxy.go:563: 127.0.0.1:49564 ("158.106.212.130") authentication complete Session{email:pierce.lopez@gmail.com user:ploxiln token:true}
2017-12-12 23:47:56.994 oauthproxy.go:608: 127.0.0.1:49564 ("158.106.212.130") could not decode session state: expected 2 chunks got 1

This is an admittedly very cludgy fix which only applies if you use the default cookie domain.

My browser which couldn't re-sign-in was able to re-sign-in after this. The Firefox devtools showed the two cookies with the same name in an awkward and invalid-looking way, but curl confirmed that the cookies were being set as desired:

< HTTP/1.1 403 Forbidden
< Server: nginx
< Date: Wed, 13 Dec 2017 01:18:26 GMT
< Content-Type: text/html; charset=utf-8
< Transfer-Encoding: chunked
< Connection: keep-alive
< Keep-Alive: timeout=60
< Set-Cookie: _oauth2_proxy=; Path=/; Expires=Wed, 13 Dec 2017 00:18:26 GMT; HttpOnly; Secure
< Set-Cookie: _oauth2_proxy=; Path=/; Domain=avdocs.st-av.net; Expires=Wed, 13 Dec 2017 00:18:26 GMT; HttpOnly; Secure
<

cc @reedloden @clobrano @hlhendy

[reedloden]

That is indeed an ugly hack for backwards compatibility to clear out cookies with the domain set, but makes sense in wanting to get rid of both cookies (and as for the two Set-Cookie responses, that's expected and looks fine). My Go sucks, so I can't speak to the pointer math on variable copying, though.

[ploxiln]

Thanks for taking a look :)

Now that I think about it, an alternative work-around would be to just change the cookie name when going through this upgrade.

[ploxiln]

test failure looks spurious ... and the PASS is odd too:

2017/12/13 01:26:52 watcher.go:57: reloading after event: "/tmp/test_auth_emails_135722942": REMOVE
2017/12/13 01:26:52 validator.go:42: failed opening authenticated-emails-file="/tmp/test_auth_emails_135722942", open /tmp/test_auth_emails_135722942: no such file or directory
--- PASS: TestValidatorOverwriteEmailListViaCopyingOver (0.05s)
exit status 1
FAIL	github.com/bitly/oauth2_proxy	0.161s
testing github.com/bitly/oauth2_proxy/api
go vet github.com/bitly/oauth2_proxy/api

[ploxiln]

So anyway, I think this would be a good idea to have in the next release, because it fixes the default case (no cookie-domain being specified in the config), which otherwise will stop working for a while when upgrading an existing oauth2_proxy setup from v2.2 to v2.3. Then it could be removed before the next release (it's only needed for the cookie-expire duration).