Crash when token refresh fails because access was revoked

[kevinschumacher]

Specifically seeing this with the Google provider, not sure if it impacts other providers as well.

When oauth2-proxy tries to refresh the authentication token, if Google responds with

400 from "https://www.googleapis.com/oauth2/v3/token" 
{
 "error": "invalid_grant",
 "error_description": "Token has been revoked."
}

there is a

2016/01/21 02:53:16 server.go:1775: http: panic serving 127.0.0.1:37688: runtime error: invalid memory address or nil pointer dereference

I have oauth2-proxy configured behind nginx, so I just see 502 bad gateway responses from nginx. Not sure what oauth2-proxy is serving (I imagine a 50X error)

A workaround we've been using is to clear the oauth2proxy auth cookie, and go sign in again. This takes you through the app authorization process again.

I would expect oauth2proxy instead to either:

1. redirect to the sign-in page to start the flow again or,
2. just jump straight into the authentication flow to authorize the app again.

In my particular situation, IT policy is to revoke access to my Google account for any applications which are not on an approved whitelist. Our application is not yet on the whitelist, so I see this often.

Relevant log entries:

2016/01/21 02:53:16 oauthproxy.go:458: 127.0.0.1:37687 refreshing 28h8m11s old session cookie for Session{[email protected] token:true expires:2016-01-19 23:45:05 +0000 UTC refresh_token:true} (refresh after 1h0m0s)
2016/01/21 02:53:16 oauthproxy.go:463: 127.0.0.1:37687 removing session. error refreshing access token got 400 from "https://www.googleapis.com/oauth2/v3/token" {
 "error": "invalid_grant",
 "error_description": "Token has been revoked."
}
 Session{[email protected] token:true expires:2016-01-19 23:45:05 +0000 UTC refresh_token:true}
2016/01/21 02:53:16 server.go:1775: http: panic serving 127.0.0.1:37687: runtime error: invalid memory address or nil pointer dereference
goroutine 2609 [running]:
net/http.func·011()
    /usr/local/Cellar/go/1.4.2/libexec/src/net/http/server.go:1130 +0xbb
main.(*OauthProxy).Proxy(0xc20806c780, 0x7f6e70e41a20, 0xc20825c880, 0xc2082b6820)
    /Users/jehiah/projects/tmp_build/src/github.com/bitly/oauth2_proxy/oauthproxy.go:478 +0x7bb
main.(*OauthProxy).ServeHTTP(0xc20806c780, 0x7f6e70e41a20, 0xc20825c880, 0xc2082b6820)
    /Users/jehiah/projects/tmp_build/src/github.com/bitly/oauth2_proxy/oauthproxy.go:375 +0x347
main.loggingHandler.ServeHTTP(0x7f6e70e3fdb0, 0xc208036008, 0x7f6e70e40728, 0xc20806c780, 0x1, 0x7f6e70e419e8, 0xc20824e6e0, 0xc2082b6820)
    /Users/jehiah/projects/tmp_build/src/github.com/bitly/oauth2_proxy/logging_handler.go:82 +0x153
main.(*loggingHandler).ServeHTTP(0xc2080fa330, 0x7f6e70e419e8, 0xc20824e6e0, 0xc2082b6820)
    <autogenerated>:13 +0xbe
net/http.serverHandler.ServeHTTP(0xc20805c840, 0x7f6e70e419e8, 0xc20824e6e0, 0xc2082b6820)
    /usr/local/Cellar/go/1.4.2/libexec/src/net/http/server.go:1703 +0x19a
net/http.(*conn).serve(0xc20824e640)
    /usr/local/Cellar/go/1.4.2/libexec/src/net/http/server.go:1204 +0xb57
created by net/http.(*Server).Serve
    /usr/local/Cellar/go/1.4.2/libexec/src/net/http/server.go:1751 +0x35e

[jehiah]

ouch. no good. Are you interested in trying to open a fix for this @kevinschumacher ?

[kevinschumacher]

Well, I've been looking at it for a few minutes, and I'm not really sure (I've never worked with go before)

I see here in providers.google.redeemRefreshToken(...) there is a check for non-200 codes, and, I guess token and duration will be nil. So then up the callstack in RefreshSessionIfNeeded the err gets passed back up.

Here

oauth2_proxy/oauthproxy.go

Line 533 in d5a332c

if ok, err := p.provider.RefreshSessionIfNeeded(session); err != nil {

I get that err isn't nil and ok is false, so can just add an else to start the auth flow?

Is it as simple as

    p.OAuthStart(rw, req)

when err != nil and !ok ?

Would that suffice?

Obviously this is pretty easy to test but I don't even have the build tools installed yet.

[jehiah]

no worries. I'm pretty sure this bug is already fixed by #117

Are you using version 2.0 instead of 2.0.1? (also a new 2.1 release is due out shortly)

[kevinschumacher]

I'm running 2.0.1 and this bug persists

[jehiah]

oops sorry. you are correct. This was actually fixed by #139 and will be included in the next release (due out shortly #193 )