Frontend cookies expiry resets on every session data change

[RamiAwar]

See below how every time we have modified session data, we reset the frontend cookie age.

# middleware.py L84

            ...
            headers = MutableHeaders(scope=message)
            header_parts = [
                f'{self.session_cookie}={session_id}',
                f'path={path}',
            ]
            if self.max_age:
                header_parts.append(f'Max-Age={self.max_age}')
            if self.domain:
                header_parts.append(f'Domain={self.domain}')

            header_parts.append(self.security_flags)

I find this a bit strange. Maybe it is something some users want, but I for example don't want this.

As a general solution, I propose optionally setting the max age on every call. Maybe something like this?

class SessionMiddleware:
    def __init__(
        self,
        app: ASGIApp,
        session_cookie: str = "session",
        max_age: int = 14 * 24 * 60 * 60,  # 14 days, in seconds
        +++ auto_refresh_age: bool = False,
        same_site: str = "lax",
        https_only: bool = False,
        autoload: bool = False,
        domain: typing.Optional[str] = None,
        path: typing.Optional[str] = None,
        secret_key: typing.Optional[typing.Union[str, Secret]] = None,
        backend: typing.Optional[SessionBackend] = None,
    ) -> None:
        self.app = app
        if backend is None:
            if secret_key is None:
                raise ImproperlyConfigured("CookieBackend requires secret_key argument.")
            backend = CookieBackend(secret_key, max_age)

        +++self.auto_refresh_age = auto_refresh_age
        self.backend = backend
        self.session_cookie = session_cookie
        self.max_age = max_age

And then later check this flag when updating max-age on the Set-Cookie header:

            if self.auto_refresh_age and self.max_age:
                header_parts.append(f'Max-Age={self.max_age}')

Would you accept such a contrib?

[alex-oleshkevich]

Probably we should not extend max-age at all. If a session suddenly expires for a user, then "remember me" feature must recover and start a new session for him.

Again, it would be nice if we could check how other libs do that.

[alex-oleshkevich]

Another issue, if we don't extend max-age automatically, is that session can expire at any time, erasing all data stored in it. This is definitely not what the users want to have.

[RamiAwar]

You mean the cookie can expire? Or do you mean session?

[alex-oleshkevich]

Cookie. If it expires, we cannot load any session as session ID no longer sent.

[RamiAwar]

Exactly, and then the user is logged out of whatever session they were in.

I don't want the user to be logged in indefinitely, I actually want the frontend cookie to expire after 7 days for example and force the user to relogin.

[alex-oleshkevich]

Seems that PHP works as you described - the session terminates in 24 minutes after creation. It doesn't depend on any user activity. The proposal makes sense to me.

What happens when we don't send max-age back to browser? Will it keep the old one or will use session-scoped cookie?

[RamiAwar]

I also looked at ExpressJS middleware. Same thing. They have 2 options:

• maxAge: Only set once, and then cookie expires after that point
• expires: Specify a date for expiry

If both are set, then last one set takes effect.

They also have a function on the session object to reset the cookie maxAge:

defineMethod(Session.prototype, 'touch', function touch() {
  return this.resetMaxAge();
});

And other functions like regenerate session, reload session without touching cookie, and more. Pretty interesting API tbh, we should use it as inspiration, ex. securing cookies over HTTPs.

What they do and what we should do is that they keep track of the cookie expires, and calculate maxAge on the fly every time you get it:

Store.prototype.createSession = function(req, sess){
  var expires = sess.cookie.expires
  var originalMaxAge = sess.cookie.originalMaxAge

  sess.cookie = new Cookie(sess.cookie);

  if (typeof expires === 'string') {
    // convert expires to a Date object
    sess.cookie.expires = new Date(expires)
  }

  // keep originalMaxAge intact
  sess.cookie.originalMaxAge = originalMaxAge

  req.session = new Session(req, sess);
  return req.session;
};

Still don't have the answer to your question tbh. But I have a solution in mind that doesn't need us to answer it.

[RamiAwar]

Frontend cookie expiry time is sent by the client right? So technically, we can't trust it.

As we're going to be saving the cookie expiry details in the session itself (as-per above implementation in expressjs), it makes sense that we can double check the session frontend expiry every time it is accessed. I don't think it's an expensive operation (datetime.now() - datetime store on session).

This way no matter what the frontend sends, we can double check that their frontend cookie isn't expired. And if it is, we can delete the backend session (no dangling sessions).

With this solution, we could technically send a Set-Cookie header with every response, updating the maxAge accordingly, or just specifying the expiry date again.

But we should have an answer to your question. I've asked myself that a few times as well.

[alex-oleshkevich]

Mozilla says that max-age and expires are for the same purpose, and max-age takes precedence. As for us, we would like to stick to max-age but do not automatically extend it (contrary to how it does today). This feels like a breaking change and therefore scheduled for 2.0.

Also, we would optionally allow users to re-enable it via some function API (like extend_session(request, 60*60)), so users can get it back if they want it. As of flags, I personally dislike too many configuration flags as it looks like bad design.

Well, the topic in not so simple as it pretended. Let's continue our research ;)

[RamiAwar]

Why not request.session.extend()?

[RamiAwar]

Thinking about this again, I don't like the fact that the frontend cookie never expires.

It's not clear in the docs, and I think "max_age" has mislead people to believe that the frontend cookie actually WILL expire.

So this might be a 'breaking' change technically speaking, but from a developer's perspective I see this as a "bugfix" actually, so maybe it shouldn't wait till v2. What do you think?

[alex-oleshkevich]

Why not request.session.extend()?

Starlette defines request.session as dict. Therefore, current Session class is incompatible with it. Anyone who calls request.session.extend will get mypy complaints and have to apply type casts or ignores. In v2 I want to get back to the dict and provide a set of functions that will help users to manage the session.

So this might be a 'breaking' change technically speaking

From business point a view is it a breaking change even if it is a bug fix. Because their apps will start working in the different way than they used to.

[alex-oleshkevich]

Superseded by https://github.com/alex-oleshkevich/starsessions/issues/31

[RamiAwar]

Okay I see your perspective.

I do like a functional format better. Methods hooked to request.session are a little weird to use.