[oauth2] Refresh flow breaks when id token not returned during refresh

[bplotnick]

Title: Refresh flow breaks when id token not returned during refresh

Description: Authorization Servers are not obligated to refresh the ID Token during token refresh (https://openid.net/specs/openid-connect-core-1_0.html#RefreshTokenResponse).

If an Authorization Server returns an ID Token during the initial flow, but then does not return it during refresh, the following will happen:

• During initial Auth Code flow, Envoy will compute hmac(id_token, refresh_token, access_token, expires) and set the appropriate cookies for each + HMAC cookie
• During refresh, Envoy will compute hmac(refresh_token, access_token, expires) and set the cookies for each + HMAC cookie. However the old ID Token cookie will still be there.
• The next request, Envoy will get cookies for the refreshed refresh_token, access_token, expires, the hmac of these, and the old id_token.
• Envoy will compare the hmac with the hmac of all (including the old ID Token), and conclude that the HMAC does not match, and initiate refresh

And then the whole thing will repeat until the id token expires.

Although i've personally observed the above behavior, I suspect a similar thing will happen if the refresh token is not returned. I don't believe the Authorization Server is obligated to rotate the refresh token.

Here are some options off the top of my head:

1. Clear the ID Token cookie. This feels wrong though because the ID Token isn't expired.
2. Add another cookie or a field on the hmac cookie that says what the hmac was computed from.
3. Keep state of the received ID Token and compute the hmac on the newly received values and the old ID Token. This feels like maybe the best option

Repro steps: You'll need an Authorization Server that does not refresh the ID Token. Or a simple unit test will confirm this behavior.

[phlax]

cc @derekargueta @mattklein123

[derekargueta]

Hey Ben :) Thanks for the bug report, I agree option 3 feels best. When you say "keep state" are you referring to just keeping that cookie untouched for re-use in future HMACs?

[bplotnick]

Hey Ben :) Thanks for the bug report, I agree option 3 feels best. When you say "keep state" are you referring to just keeping that cookie untouched for re-use in future HMACs?

Hey Derek, long time! Yeah exactly - "keep state" wasn't a good way to put it - just use the ID Token received in the request cookie for the HMAC if it is present. I haven't thought through all the implications of this. I think we'll probably want to re-set the cookie to make sure that the cookie expiry is re-set. I think we may want to parse the ID Token for expiration and do the normal authorization code flow if it is expired. The ID Token is guaranteed to be a JWT and parseable unlike the access token. I'm also not sure how this will interact with https://github.com/envoyproxy/envoy/pull/32278.

In terms of scope of this issue - AFAICT, there are a few IdPs that have this behavior, though the biggest ones seem to not suffer from this.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[derekargueta]

nostale

[derekargueta]

I'm starting to wonder if we can solve this by just omitting the id_token from the HMAC, reasoning being that the id_token is guaranteed to be a JWT which can be verified on its own. The HMAC scheme is really only needed because the access token can be an arbitrary string without any authenticity properties.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[bplotnick]

nostale

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions[bot]]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.