os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
30 stars 30 forks source link

Client session expires after a long idle (exact duration unknown) #169

Closed valp124 closed 2 years ago

valp124 commented 2 years ago

I open a browser session to the server and let it idle for a "while" (it seems like 24 hours will do it, or maybe even 12+), and then I start getting Access denied on any vfs method. It appears that this is caused by the expiration and non-renewal of the session cookie. (Moreover: in the expired session, the cookie isn't even present at all -- it's like the browser deleted it.) The default expiration time, in my case, is 12 hours.

Digging a bit deeper, it turns out that I didn't have http.ping set in my client config.json, which means that the ping would have only happened every six hours (session cookie expiration / 2). I have set it to a low number (10 sec) to test and can see that the session cookie's expiration is getting continuously updated. So that'll hopefully solve the issue -- assuming the client has a chance to ping the server at least once during 12 hours after establishing the session. Remains to be confirmed...

And a note from Anders:

"Change the ping mechanic not to rely on a websocket connection (these are remnants of something else) -- Anders"

valp124 commented 2 years ago

Well, well. I think we have decent evidence now that the fix worked! 24 hours later, the session is active, and I've been watching the expiration date of the cookie and saw that it was getting updated throughout the day (I set the ping to 1/2 hour). Moreover: in one of the open sessions, it looks like the ping restarted even after the VPN disconnected, and even after the computer went to sleep.

So, if this is not a 100% solution, it seems like it'll work for most practical purposes. Of course, there might be a scenario where the cookie will expire, I'm not sure what would cause it. I guess the only way to prevent it is to change the expiration length of the session, but I don't know how to do that πŸ˜›

andersevenrud commented 2 years ago

Sweet! Then this solution is good enough for me :) I've published a new version that defaults to 30m. I'll take care of the other stuff later :)

change the expiration length of the session, but I don't know how to do that

That's quite simple. It can be added to your local server config. Here are the defaults :)

https://github.com/os-js/osjs-server/blob/e5bd5196b827233e1b8e4bfd48cb74a30a20026c/src/config.js#L102

valp124 commented 2 years ago

So that was the good news. The bad news is that I'm looking at a session (unfortunately, I don't remember when I opened it, but I'm fairly certain that it was after I implemented the ping fix), which somehow lost its cookie. (I wish console messages in DevTools had a timestamp!). It may or may not have been related to this ws disconnect (I doubt it):

image

In any event, the question is: Is there any way in the world to regenerate the session cookie without reloading the page? Or is it a theoretical impossibility?

andersevenrud commented 2 years ago

Hm. The websocket comes back up, right ?

I wish console messages in DevTools had a timestamp!

Actually, it can :D Click the wheel in the top right of the devtools pane (not the one under console tab) and enable timestamps under the "console" section.

Is there any way in the world to regenerate the session cookie without reloading the page?

The cookie is always regenerated whenever an HTTP API call is made.

However, creating a session cookie is done exclusively during login.


I suppose one way to solve this is to have some kind of check in the client that automatically logs back in, but that would involve storing the credentials somehow :thinking:


"Change the ping mechanic not to rely on a websocket connection (these are remnants of something else) -- Anders"

https://github.com/os-js/osjs-client/blob/52daef5c7d081385b1ed1d1fa89655bff5d6c662/src/core.js#L343-L349

This is what I meant btw. Here it uses the WS state to determine wherever to ping or not, which is not ideal. If you somehow lose connection or that the "reconnecting" state there is bugged, then you'd definitely not get a fresh cookie on idle.

But I really doubt this is an issue. Would be nice to see if a ping occured before you got a access denied, and when it was on the timeline.

valp124 commented 2 years ago

Actually, it can :D Click the wheel in the top right of the devtools pane (not the one under console tab) and enable timestamps under the "console" section.

Very obvious 😜 I was looking at the "other" wheel.

Ok, great, so looking at the timestamps, I first loaded the page at 9 am yesterday (I think -- there are time but not date stamps); at 11 pm, it momentarily disconnected and reconnected (probably irrelevant); then around 1 am or so, I put the laptop to sleep, and you can see disconnect-reconnect again at 9:45 am (the computer woke up at 8:30 am); then the error I mentioned earlier at 11 am, and then finally at 12:57 pm is when I noticed that the session was lost. This is probably TMI. Anyway, here's the picture of it:

image

No errors on ping, as you can see...

I suppose one way to solve this is to have some kind of check in the client that automatically logs back in, but that would involve storing the credentials somehow πŸ€”

The only thing we'd need to keep around is jwt, and that cookie is still around, btw.

andersevenrud commented 2 years ago

Hm. I don't see the ping in those logs though (it's possible to toggle network log requests in the console).

And this was only on your production server, yes ?

I'll set off a little bit of time this weekend to try to reproduce this myself :thinking:

valp124 commented 2 years ago

Production server, correct. Network messages are on in the console...

image

It might be another one of those "hard to reproduce" situations... I suppose you could try to emulate it by setting the session expiration to a very short time and simulate network dropout (actually, killing the server would be a good use case).

andersevenrud commented 2 years ago

I suppose you could try to emulate it by setting the session expiration to a very short time and simulate network dropout

Indeed.

valp124 commented 2 years ago

Finally had a chance to try setting session cookie's maxAge to a larger value. Strangely, it doesn't seem to work (I'm checking in the Application tab in Dev Tools. My code is:

const maxAge = 60 * 60 * 24 * 7; // a week
...
  session: {
    options: {
      name: 'osjs.sid',
      secret: 'osjs',
      rolling: true,
      resave: false,
      saveUninitialized: false,
      cookie: {
        secure: 'auto',
        maxAge: 1000 * maxAge
      }
    }
  },

Can you spot what I'm doing wrong?

andersevenrud commented 2 years ago

You added this to your server, right ?

valp124 commented 2 years ago

Right, this is on my (localhost) server.

andersevenrud commented 2 years ago

I thought maybe you were inspecting the code you pasted inside devtools, which would most likely mean it was in the wrong place :sweat_smile:

Just checked the code, and it looks like session.options is just piped straight into express-session which according to its docs has cookie.maxAge available which says By default, no maximum age is set.

Your config looks correct to me. (though I recommend only inserting the settings you need, i.e. session.options.maxAge) You still get the default expiration time that I have defined ?

valp124 commented 2 years ago

Yes, I'm just getting the default (12 hours). I did have only the relevant setting there at first, but when it didn't work, I thought that maybe I it's because I needed to include more. So yeah--not working for some reason... The only way to test I can think of is reload the page and look at the cookie's expiration date.

andersevenrud commented 2 years ago

To confirm the server setting you can actually look at the websocket connection. You should get a message with the maxAge value when the connection opens (maybe only on initial connect, so have devtools open and do a refresh while on the network tab).

valp124 commented 2 years ago

Cool! I didn't know that. And yep, maxAge is set to 12 hours... (I wanted to show you a snapshot, but github is being a pig and not pasting it. Believe me that it's set to 43200000.)

andersevenrud commented 2 years ago

Strange. I see that your indentation starts with 2 spaces, so I assume that it's put into the right spot, and not under something else :thinking:

Turning things off and back on again has not resolved the issue either ?

valp124 commented 2 years ago

On and off?

andersevenrud commented 2 years ago

Sorry. Just restarting everything that is.

valp124 commented 2 years ago

Wait, sorry, my bad! I forgot that I was on a different branch. Ok, so after restarting with my change actually in place (duh), the maxAge in the ws messages looks correct. But the cookie still shows only 12 hours Expires / Max-Age. Let me see if I can show you...

andersevenrud commented 2 years ago

Might just have to clear your cookie before you get a proper update on it.

valp124 commented 2 years ago

image

and

image

valp124 commented 2 years ago

Might just have to clear your cookie before you get a proper update on it.

Aha, that was it! Wow. That's counterintuitive. Doesn't help users who've already been there. But whatever, mystery solved!

andersevenrud commented 2 years ago

That's counterintuitive

Yeah. This is probably another quirk in the session adapter that I'm using for express-session. My guess is that it stores the maxAge together with the cookie and just goes with that.

At least it's resolved. Nice :)

valp124 commented 2 years ago

At least it's resolved. Nice :)

Thank you so much again! 😊