-
Notifications
You must be signed in to change notification settings - Fork 4.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
oauth2: ID Token JWT expiry may not be synchronised with IdToken cookie expiry #32485
Comments
FYI as a workaround, one can add a Go filter between the OAuth2 and JWT filters in the chain, which adjusts the package main
import (
"errors"
"fmt"
"regexp"
"strings"
"time"
"github.com/envoyproxy/envoy/contrib/golang/common/go/api"
"github.com/golang-jwt/jwt/v5"
)
type filter struct {
api.PassThroughStreamFilter
callbacks api.FilterCallbackHandler
config *Config
}
// *filter.DecodeHeaders, *filter.DecodeData, *filter.DecodeTrailers, *filter.EncodeData and *filter.EncodeTrailers
// are omitted for brevity, they all just return api.Continue.
// *filter.OnLog, *filter.OnLogDownstreamStart, *filter.OnLogDownstreamPeriodic and *filter.OnDestroy
// are omitted for brevity, they are all just empty.
func (f *filter) EncodeHeaders(header api.ResponseHeaderMap, endStream bool) api.StatusType {
setCookies := header.Values("set-cookie")
if len(setCookies) == 0 {
return api.Continue
}
header.Del("set-cookie")
for key, value := range setCookies {
if !strings.HasPrefix(value, "IdToken=") {
continue
}
parts := strings.Split(value, ";")
_, cookieValue, _ := strings.Cut(parts[0], "=")
expiry, err := readIDTokenExpiry(cookieValue)
if err != nil {
api.LogErrorf("read IdToken expiry: %s", err)
break
}
newMaxAge := int(time.Until(expiry.Time).Seconds())
api.LogDebugf("fixing max-age - new lifetime: %d", newMaxAge)
setCookies[key] = replaceMaxAgeInSetCookieValue(value, newMaxAge)
break
}
for _, v := range setCookies {
header.Add("set-cookie", v)
}
return api.Continue
}
func readIDTokenExpiry(idToken string) (*jwt.NumericDate, error) {
unverifiedToken, _, err := new(jwt.Parser).ParseUnverified(idToken, jwt.MapClaims{})
if err != nil {
return nil, fmt.Errorf("parse the JWT: %s", err)
}
if unverifiedClaims, ok := unverifiedToken.Claims.(jwt.MapClaims); ok {
expiry, err := unverifiedClaims.GetExpirationTime()
if err != nil {
return nil, fmt.Errorf("get expiry: %s", err)
}
return expiry, nil
}
return nil, errors.New("invalid JWT token")
}
func replaceMaxAgeInSetCookieValue(value string, maxAge int) string {
return regexp.MustCompile(`Max-Age=([0-9]+)`).ReplaceAllString(value, fmt.Sprintf("Max-Age=%d", maxAge))
} |
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. |
I'd appreciate if this would be kept open. Without applying the workaround, which is quite cumbersome, the bug directly affects users as they can be logged out for some time without an easy way to login again until the timeout is reached. |
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. |
not stale |
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.
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>
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>
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>
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. |
Title: The expiry of the IdToken cookie may be longer than the expiry of the JWT stored inside the cookie
Description:
This issue is related to the oauth2 and jwt filters. The authn server sets the
expires_in
field of the token response in sync with the expiry of the access token. The oauth2 filter however uses this value to set the expiry of all cookies, including the IdToken cookie. However, authn providers are not obliged to use the same lifetime for access tokens and ID tokens. For example, Microsoft Entra does not: https://learn.microsoft.com/en-us/entra/identity-platform/configurable-token-lifetimes. By default, Entra ID tokens have a lifetime of 60 minutes, whereas Entra access tokens have a lifetime of somewhere in between 60 and 90 minutes. The effect of this is that the IdToken cookie expires sometime after the JWT contained in it. As a result, after 60 minutes pass, the JWT filter errors withJwt is expired
and the user must delete the cookies or wait until the cookie expires as well.This is related to #30053, because the root cause of both issues is the same: the
expires_in
field of the token response must only be used to determine expiry of the access token, not of any other tokens. The fix should also roughly be the same for both issues: if the IdToken cookie expiry is set from the JWT expiry field, the problem is solved.Repro steps:
Configure your OAuth2 authorization server to hand out access tokens and ID tokens with different lifetime. In envoy, make use of the ID token in the jwt filter, e.g. like this:
After the ID token expires, envoy responds with
Jwt is expired
until the cookies / access token expire as well.Other comments:
Another way to solve the issue would be to introduce a separate configuration field, named e.g.
max_expiry
. Say this is set to 60 minutes, then all cookies would not have an expiry of more than 60 minutes. This would fix the issue as well, although it sounds a bit like a hack to me. In addition,max_expiry
would have to exclude the refresh token, as otherwise #30053 would happen again.The text was updated successfully, but these errors were encountered: