Implement refreshing within OIDC provider

[JoelSpeed]

Allows tokens to be refreshed within the OIDC provider.

This means that users will no longer have to re-authenticate when the ID Token expires but instead the OAuth proxy will refresh automatically in the background.

Make sure to add offline_access to the request scope when using this (else you won't request a refresh token from the upstream provider).

I'd also recommend setting the cookie_refresh parameter to the same duration as your token lifetime.

Can be used in conjunction with #534 using the branch https://github.com/pusher/oauth2_proxy/tree/oidc-refresh

[jhohertz]

I'm testing this now in a build with #534 and #464 applied as well, container here: viafoura/oauth2-proxy:oidc-1

So far so good, waiting for my token to time out so I can see it refresh. :)

[JoelSpeed]

Probably should have mentioned that I also built an image 😅

We have a branch at Pusher (https://github.com/pusher/oauth2_proxy/tree/kubernetes) that I'm maintaining which has all of my PRs in (#464, #534, #620) and I've published an image on quay: quay.io/joelspeed/oauth2_proxy:kubernetes-2

We are now running all three PRs in production and I've been testing locally with Dex/OAuth Proxy with 1-minute tokens.
A regular call to ouath2/auth takes about 10ms and I'm seeing the refresh calls take around 100ms, if anyone is interested

[jhohertz]

Nice. I checked my merged branch against yours and see some minor variation, not sure if there's a change missing somewhere. For some reason I can't get a good diff view in github, so I'm uploading a patch here, just wanted to run by you in case it means something is missing from one of the PRs?

variation.diff.txt

[JoelSpeed]

@jhohertz I added a commit to the end of #464 to add some documentation which you are missing,

You're also using the copy of this branch pusher/oidc-refresh-bitly which won't work in conjunction with #534, my plan was to wait and see which of this and 534 gets merged first and fix the other one up appropriately, you need the commits from pusher/oidc-refresh

And the final change is a line I never meant to commit that I... err... erased from history (I'm bad, I know 😆)

If I were you I would just take the pusher/kubernetes branch and push it over the top of yours

[jhohertz]

@JoelSpeed : Thanks for taking the time to explain. Now on a build from your kubernetes branch, will let you know if I run into any issues.

[jhohertz]

@JoelSpeed I seem to be having issues with refresh, though I suspect it may be a bad configuration on my part vs. a problem w/ the code. From the logs (w/ token/email obfuscated):

2018/06/20 18:26:00 oauthproxy.go:727: 100.100.0.5:33598 ("10.2.8.52") refreshing 1h18m1s old session cookie for Session{email:[email protected] user:joe token:true id_token:true expires:2018-06-21 17:07:58 +0000 UTC refresh_token:true} (refresh after 1h0m0s)
2018/06/20 18:26:00 internal_util.go:60: GET ?access_token=abcdefghijkl...
2018/06/20 18:26:00 internal_util.go:61: token validation request failed: Get ?access_token=abcdefghijklmnopqrstuvwxy: unsupported protocol scheme ""

I have dex (with your google PRs) running as the oidc provider.

My guess is one of the url params of the proxy I am not setting is required for this? (redeem? unsure what the right endpoint on dex for this would be)

[JoelSpeed]

@jhohertz just to double check, do you have offline_access set as part of your scope???

And also what timings have you got for the cookie_refresh setting on the oauth proxy and the token expiry on dex?

[jhohertz]

I do have offline _access in my scope, here's the flags I set for oauth2-proxy:

          - --cookie-secure=true
          - --cookie-expire=168h0m
          - --cookie-refresh=60m
          - --cookie-domain=.{{ cluster_name }}
          - --whitelist-domain=.{{ cluster_name }}
          - --email-domain={{ gapps_domain_name }}
          - --set-authorization-header=true
          - --http-address=0.0.0.0:4180
          - --provider=oidc
          - --client-id=kubernetes
          - --client-secret={{ dex_oauth2_client_secret }}
          - --oidc-issuer-url=https://dex.{{ cluster_name }}
          - --scope=openid email profile groups offline_access
          - --skip-provider-button=true

For Dex... it looks like I must be using defaults, as I am not setting any timeout values in my config. I will have a look.

[JoelSpeed]

@jhohertz I've been having a look at this and have found the problem.

If you don't actually need to refresh, the RefreshSessionIfNeeded returns false, nil, but this then triggers a ValidateSessionState which the OIDC provider doesn't override.

The default ValidateSessionState won't work with OIDC and so causes a failure (there is no validate URL for OIDC)

To fix this (properly) I could override the ValidateSessionState for the OIDC provider, but to make that work properly, I need the changes from #534 since you need to have the id_token to verify that session properly.

The alternative that I have come up with is a bit of a cheeky hack,

Replacing the start of RefreshSessionIfNeeded with:

	// Can't refresh without the refresh token
	if s == nil || s.RefreshToken == "" {
		return false, nil
	}

	// If id_token hasn't expired, no need to refresh, just verify
	if s.ExpiresOn.After(time.Now()) {
		ctx := context.Background()
		_, err := p.Verifier.Verify(ctx, s.IdToken)
		if err != nil {
			return false, fmt.Errorf("unable to verify id_token: %v", err)
		}
		return true, nil
	}

But not without the verification step if not using #534

Any maintainers around to weigh in on this?

I think I might create a branch that does this properly, but I may have to merge #534 and #620 into a bigger OIDC rewrite PR

[JoelSpeed]

An alternative quick fix is to make sure your cookie_refresh == Dex token lifetime (by default 24h)

[JoelSpeed]

Closing this in favour of #621

Thanks @jhohertz for pointing out the refresh bug here