Hi!
While using this code in my project I noticed there is no explicit option to omit both Max-Age and Expires parameters in a cookie handed-out when SendCookie parameter is set to true making it a session cookie. After looking through the code briefly I found CookieMaxAge parameter inside GinJWTMiddleware struct, unmentioned in the README. At the first glance, setting it to "0" might seem to be a good idea for this purpose. However, the way the flag is used during cookie generation concerns be a bit.
Let's assume default mw.TimeFunc() implementation, which is time.Now. Max-Age attribute is calculated as duration difference between (time.Now() + CookieMaxAge) and (time.Now()) to the nearest second. Evaluating deductible and deductive is based on two separate calls to mw.TimeFunc() later converted to Unix timestamp. This might cause an inconsistency if system clock flips by one second between those two calls.
Step-by-step scenario:
Current time is Mon, 25 Apr 2022 15:14:16 GMT expressed as 1650899656 with epoch timestamp
mw.TimeFunc() returns 1650899657, maxage is a result of 1650899656 - 1650899657, which is -1.
According to net/http package documentation passing a negative value as MaxAge parameter for http.Cookie struct effectively results in a cookie with Max-Age: 0 attribute. This causes the cookie to be considered as expired [at] the earliest representable date and time, which is significantly different behavior compared to what's achieved with the lack of Expires and Max-Age attributes.
Excerpt from net/http documentation:
// MaxAge=0 means no 'Max-Age' attribute specified.
// MaxAge<0 means delete cookie now, equivalently 'Max-Age: 0'
// MaxAge>0 means Max-Age attribute present and given in seconds
Supposing time did not change between mw.TimeFunc calls, this would yield maxage := 0, which is correct MaxAge value to pass to http.Cookie to make the Max-Age attribute unspecified in the output cookie.
I propose to improve the cookie expiration timeout evaluation method by making sure mw.TimeFunc is called only once. On top of that, I'd see a separate config flag like SessionCookie boolean overriding CookieMaxAge and Timeout as a huge convenience for this kind of use case.
Could you please share your thoughts on my proposal? I'm wiling to implement it if you decide you'd like it in the project.
Hi! While using this code in my project I noticed there is no explicit option to omit both
Max-AgeandExpiresparameters in a cookie handed-out whenSendCookieparameter is set totruemaking it a session cookie. After looking through the code briefly I foundCookieMaxAgeparameter insideGinJWTMiddlewarestruct, unmentioned in the README. At the first glance, setting it to"0"might seem to be a good idea for this purpose. However, the way the flag is used during cookie generation concerns be a bit.https://github.com/appleboy/gin-jwt/blob/22d2e6198bd396d324936a5d990b327a792bffe8/auth_jwt.go#L507-L509
Let's assume default
mw.TimeFunc()implementation, which istime.Now.Max-Ageattribute is calculated as duration difference between (time.Now()+CookieMaxAge) and (time.Now()) to the nearest second. Evaluating deductible and deductive is based on two separate calls tomw.TimeFunc()later converted to Unix timestamp. This might cause an inconsistency if system clock flips by one second between those two calls.Step-by-step scenario:
Current time is Mon, 25 Apr 2022 15:14:16 GMT expressed as
1650899656with epoch timestampmw.TimeFunc()returns1650899656, adding0to it makesexpireCookie := 1650899656Time advances by one second now, it's Mon, 25 Apr 2022 15:14:17 GMT or
1650899657expressed with epoch timestampmw.TimeFunc()returns1650899657,maxageis a result of1650899656 - 1650899657, which is-1.According to net/http package documentation passing a negative value as
MaxAgeparameter forhttp.Cookiestruct effectively results in a cookie withMax-Age: 0attribute. This causes the cookie to be considered as expired [at] the earliest representable date and time, which is significantly different behavior compared to what's achieved with the lack ofExpiresandMax-Ageattributes.Excerpt from net/http documentation:
Supposing time did not change between
mw.TimeFunccalls, this would yieldmaxage := 0, which is correctMaxAgevalue to pass tohttp.Cookieto make theMax-Ageattribute unspecified in the output cookie.I propose to improve the cookie expiration timeout evaluation method by making sure
mw.TimeFuncis called only once. On top of that, I'd see a separate config flag likeSessionCookie booleanoverridingCookieMaxAgeandTimeoutas a huge convenience for this kind of use case.Could you please share your thoughts on my proposal? I'm wiling to implement it if you decide you'd like it in the project.
Thanks!