OIDC ID Token, Authorization Headers, Refreshing and Verification

[JoelSpeed]

This PR replaces: bitly/oauth2_proxy#621

Description

This PR includes the commits from/replaces #534 and #620 and adds proper OIDC token validation. Please see both original PRs for descriptions and conversations about the PRs.

This now fixes #530

To summarise:

#534: Adds the ability to expose the raw OIDC token to applications behind the proxy
#620: Implement background refreshing of OIDC tokens when cookie_refresh is set > 0
Verification: Implement ValidateSessionState for the OIDC provider to properly verify the id_token. The default providers implementation does not work with OIDC and was causing issues with refreshing

This PR has become quite large but, having opened #620, I realised there was a bug if the cookie_refresh period was shorter than the id_token lifetime (Thanks @jhohertz), to fix this, I needed to correctly implement ValidateSessionState for OIDC which then in turn needs the changes from #534

So the resulting PR is a combination of fixes that adds a bunch of new stuff to the OIDC provider

Motivation and Context

This makes the OIDC implementation complete in terms of refreshing and token verification. It also allows the OIDC ID tokens to be exposed via an authorization header which can then be proxied into the Kubernetes Dashboard.

How Has This Been Tested?

This has been running in our production for around 6 months now and we haven't seen any issues with it. OAuth2 Proxy plus dex with the OAuth2 Proxy in nginx auth_request mode

Checklist:

• My change requires a change to the documentation or CHANGELOG.
• I have updated the documentation/CHANGELOG accordingly.
• I have created a feature (non-master) branch for my PR.

[loshz]

LGTM - just a few minor comments.

[luispollo]

Hello @JoelSpeed. Thanks for contributing this! Do you have a Docker image built from this PR that is publicly accessible, like you did for the previous repo?

[jhohertz]

Just wanted to jump in and day we've been using some of these patches from @JoelSpeed for around 6 months as well with great results, and am excited to see some new life in the project on his repositories.

Let me know anything I can do to help move things forward.

[luispollo]

@JoelSpeed Never mind, I see the Dockerfile in the repo. 😝

[JoelSpeed]

@syscll I've addressed your comments.

I didn't change the fmt.Errorf to an errors.New though as the codebase is littered with a mix of them and I personally prefer fmt.Errorf as I see it used a lot more often, WDYT?

[zentale]

@luispollo small remark, ca-certificates package is not installed on the image of this branch. (master is not merged) This causes tls to not work.
Thx for the work guys.

[loshz]

LGTM 👍

[tlawrie]

@JoelSpeed the refresh token is working however I am experiencing the issue where by multiple requests made at the same time (due to reactjs calls) are causing a failure.

One of the calls will make it through to refresh the token, however the other duplicate calls will fail with

createSessionState() - rawIDToken: 
createSessionState() - ok: false
RefreshSessionIfNeeded() - Error: unable to update session: token response did not contain an id_token

Which means I have multiple calls happening at the same time, and one call succeeds with the refresh. The rests make the refresh call to not get a good response as the refresh token has already been redeemed.

I implemented what is suggested here (https://gist.github.com/JoelSpeed/9f4dbf6f79f6498d12ccd6ff0bc096e2) lines 41-53 as you have previously mentioned back last year.

[JoelSpeed]

@tlawrie We've been running that fix for a while now and have seen the problem pretty much disappear. It would be good to think up a long term proper solution for this but the only way I can see that happening is by having some central state storage which would defeat the lovely horizontal scalability of the current design. Welcome to ideas on that!

[tlawrie]

@JoelSpeed it might be my implementation of the fix. I currently have the following in the ingress and ingress-controller

Ingress

"ingress.kubernetes.io/configuration-snippet": "
    proxy_pass_request_body     off;
    proxy_cache                   authentication;
    proxy_cache_key               $cookie_bmrg_auth_proxy_dev;
    proxy_cache_valid             202 401 3s;
    proxy_cache_lock              on;
    proxy_ignore_headers          Set-Cookie;",

Ingress Controller

"http-snippet": "proxy_cache_path        /var/run/cache levels=1:2 keys_zone=authentication:10m inactive=3s;\n",

It could also be that something strange is happening in the oidc provider when connected to our provider that is causing the empty ID Token. Looking at the code in RedeemRefreshToken() it merges the context with the new refreshtoken etc. I am not sure how the context.Background() works. Is it possible that it can have an empty ID Token?

[tamalsaha]

@JoelSpeed , I wonder why the idToken is set as Authorization header? Should not this be set as X-Auth-Id-Token or something like that?