You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
Thanks!
The text was updated successfully, but these errors were encountered:
Hi!
While using this code in my project I noticed there is no explicit option to omit both
Max-Age
andExpires
parameters in a cookie handed-out whenSendCookie
parameter is set totrue
making it a session cookie. After looking through the code briefly I foundCookieMaxAge
parameter insideGinJWTMiddleware
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.gin-jwt/auth_jwt.go
Lines 507 to 509 in 22d2e61
Let's assume default
mw.TimeFunc()
implementation, which istime.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 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:
1650899656
with epoch timestampmw.TimeFunc()
returns1650899656
, adding0
to it makesexpireCookie := 1650899656
Time advances by one second now, it's Mon, 25 Apr 2022 15:14:17 GMT or
1650899657
expressed with epoch timestampmw.TimeFunc()
returns1650899657
,maxage
is a result of1650899656 - 1650899657
, which is-1
.According to net/http package documentation passing a negative value as
MaxAge
parameter forhttp.Cookie
struct effectively results in a cookie withMax-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 ofExpires
andMax-Age
attributes.Excerpt from net/http documentation:
Supposing time did not change between
mw.TimeFunc
calls, this would yieldmaxage := 0
, which is correctMaxAge
value to pass tohttp.Cookie
to make theMax-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 likeSessionCookie boolean
overridingCookieMaxAge
andTimeout
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.
Thanks!
The text was updated successfully, but these errors were encountered: