Synchronise expiry of id token cookie with JWT exp claim #34117
Conversation
|
Hi @michaelsauter, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
michaelsauter
force-pushed
the
feature/sync-id-token-expiry-with-cookie
branch
from
bb2b501
to
9bb888b
Compare
|
This needs a clang-format (if you click through a few things from the "prechecks precheck format" details you get to here eventually which provides a diff you could apply). |
Yes sorry, realised after submitting the PR. I did not get to update it yesterday but will do so today. |
michaelsauter
force-pushed
the
feature/sync-id-token-expiry-with-cookie
branch
from
9bb888b
to
f543c25
Compare
The expiry of the id token cookie should match the exp claim in the id token JWT, not the expiry of the access token. Some identity providers (definitely Microsoft Entra ID, probably others) issue id tokens with different expiry than the access tokens. This change is largely based on the work done in envoyproxy#32278 which supports expiry of the refresh token based on the exp claim of the refresh token if it is a JWT. Closes envoyproxy#32485. Signed-off-by: Michael Sauter <mail@michaelsauter.net>
michaelsauter
force-pushed
the
feature/sync-id-token-expiry-with-cookie
branch
from
f543c25
to
dedccce
Compare
Signed-off-by: Michael Sauter <mail@michaelsauter.net>
Signed-off-by: Michael Sauter <mail@michaelsauter.net>
Signed-off-by: Michael Sauter <mail@michaelsauter.net>
|
This should be ready for review now. |
| if (!id_token.empty()) { | ||
| ::google::jwt_verify::Jwt jwt; | ||
| if (jwt.parseFromString(id_token) == ::google::jwt_verify::Status::Ok && jwt.exp_ != 0) { | ||
| const std::chrono::seconds expirationFromJwt = std::chrono::seconds{jwt.exp_}; |
| }; | ||
| filter_->decodeHeaders(request_headers, false); | ||
|
|
||
| const std::string idToken = |
Signed-off-by: Michael Sauter <mail@michaelsauter.net>
michaelsauter
force-pushed
the
feature/sync-id-token-expiry-with-cookie
branch
from
b3353ed
to
bb38fb4
Compare
Signed-off-by: Michael Sauter <mail@michaelsauter.net>
|
@lizan Thanks for approving! I just resolved the merge conflict on the changelog, this should be ready to merge now. |
The expiry of the id token cookie should match the exp claim in the id token JWT, not the expiry of the access token. Some identity providers (definitely Microsoft Entra ID, probably others) issue id tokens with different expiry than the access tokens.
This change is largely based on the work done in #32278 which supports expiry of the refresh token based on the exp claim of the refresh token if it is a JWT.
Closes #32485.
Note: #32278 changed the httpOnly setting of the refresh token cookie without discussion around it. I decided to "revert" that change so that the httpOnly is controlled again by the runtime guard. Let me know if you would like to change this and remove the runtime guard completely? None of these cookies should be accessible by the client ...