redundant cassandra write in renewCookie

[haskell-monad]

module Brig.User.Auth.Cookie
import qualified Brig.User.Auth.DB.Cookie as DB

-- | Renew the given cookie with a fresh token.
renewCookie old = do
     -- Omit other code -- 
     new <- newCookie u (cookieType old) (cookieLabel old)
     -- Insert new cookie
     DB.insertCookie u new Nothing
     return new

-- | Basic Cookie Management
newCookie u typ label = do
      -- Omit other code -- 
      -- let c = Cookie{..}
      DB.insertCookie u c Nothing
      return c

hoho，It seems to have executed the same insert statement twice (DB.insertCookie) ? No problem?

[fisx]

that's in two different functions: the second is for the case where the client connects without a cookie and needs to obtain one; the first is for the case where a cookie is about to expire and needs to be replaced. if one function gets called, the other doesn't (at least not at that time).

if you are referring to the two calls to insertCookie in renewCookie: one is for storing the new cookie, the second is for storing the old one again with a limited TTL. see the comments in the code.

if there is something we are missing, please re-open the issue with more details.

thanks, though! we appreciate your interest in wire! :-)

[haskell-monad]

newCookie :: UserId -> CookieType -> Maybe CookieLabel -> AppIO (Cookie ZAuth.UserToken)
newCookie u typ label = do
    now <- liftIO =<< view currentTime
    tok <- if typ == PersistentCookie
            then ZAuth.newUserToken u
            else ZAuth.newSessionToken u
    let c = Cookie
          { cookieId      = CookieId (ZAuth.userTokenRand tok)
          , cookieCreated = now
          , cookieExpires = ZAuth.tokenExpiresUTC tok
          , cookieLabel   = label
          , cookieType    = typ
          , cookieSucc    = Nothing
          , cookieValue   = tok
          }
    DB.insertCookie u c Nothing    -- This is the first call to save the newly created Cookie
    return c      -- return the newly created cookie


renewCookie :: Cookie ZAuth.UserToken -> AppIO (Cookie ZAuth.UserToken)
renewCookie old = do
    let t = cookieValue old
    let u = ZAuth.userTokenOf t

    new <- newCookie u (cookieType old) (cookieLabel old) -- Indirectly called the `newCookie` function above, and the `newCookie` function internally saves the newly created cookie.
    -- Insert new cookie
    DB.insertCookie u new Nothing    -- This is the second call, save the newly created cookie again, is this step redundant?               
    
    let old' = old { cookieSucc = Just (cookieId new) }
    ttl <- setUserCookieRenewAge <$> view settings
    DB.insertCookie u old' (Just (DB.TTL (fromIntegral ttl)))   -- the third time is for storing the old one again with a limited TTL
    return new   

I mean: inside the renewCookie function, the newCookie function is called indirectly. At this point, the two functions are executed in one request, and DB.insertCookie is called inside the newCookie function. Save the new cookie and return a new cookie, so I think the first DB.insertCookie in the renewCookie function is redundant. (I think DB.insertCookie has been called a total of 3 times, the first time in the newCookie function, the second and third times in the renewCookie function, the second of which is redundant) @fisx

[fisx]

oops, you're right. thanks for persisting! :-)

i am not worried about this, the second insert is obviously redundant, but doesn't do any harm either. But I now agree we should fix this. Would you like to make a PR? If not, I may get around to doing it eventually.

[fisx]

fixed in #676