tl-its-umich-edu / canvas-course-manager-next

Canvas Course Manager Next: A redesign of the existing CCM application. It extends Canvas features, makes cumbersome features easier to use, and adds new features.
8 stars 9 forks source link

When session expires, user may not be able to initiate new session via LTI launch #300

Closed ssciolla closed 2 years ago

ssciolla commented 2 years ago

Something like this was reported by Dan, but I have also experienced it on occasion as well. Currently, sessions expire after 24 hours. When this occurs, sometimes (or all the time, this needs more testing) an error is thrown in the console but subsequent attempts to relaunch the application via the external tool link in Canvas fail. Instead they see the standard unauthenticated view.

Users should be able initiate a new session by launching the LTI tool again when this occurs. A better error message than the generic authentication failure would also be helpful.

This needs more research.


Test Plan:

  1. Change the value in .env for MAX_AGE_IN_SEC=60 (1 min)
  2. Launch the CCM tool
  3. Open Developer Tools -> Application Tab -> Look for Connect.sid (Expire time)
    • Storage → Cookies → [URL with my ngrok hostname]
    • Filter for "connect.sid"
    • Look at the value in the "Expires / Max-Age" column
  4. Click on "Add U-M Users"
  5. Click the radio button for "1 section at a time"
  6. Wait for the expiration time for "connect.sid" to pass
  7. Click the "SELECT" button. This will throw an error message suggesting that the session might be expired.
  8. Try refresh the browser and launch the tool or simple launch the tool. Launch should be successful
  9. Make sure to change back what you have MAX_AGE_IN_SEC=60 or simply disable the config, restart the server.
melindakraft commented 2 years ago

We are unable to find the perfect storm of replication steps. Dan reports that he had many tabs open and had been in and out of Canvas both as himself and while masquerading. The error message he received was “You were not properly authenticated to the application.” He also stated that since he knows deleting the auth will fix things, he didn't try launching a new session or taking other steps to fix the problem.

This may be an edge case which is solved by other steps (relaunching the tool) that doesn't need further investigation at this time.

ssciolla commented 2 years ago

@pushyamig, do you have time to look at this? I think it happened to @janelilr last week. My hunch is that some cookie has expired, but it is not getting reset properly when a new launch happens. Perhaps the session needs to be destroyed manually after we throw the error, maybe in SessionGuard in auth/session.guard.ts. You should be able to try to reproduce the issue effectively by setting the MAX_AGE_IN_SEC environment variable to something like 120. There may be more complexity hidden here (JWT cookie expiration?), but that's where I'd start.

pushyamig commented 2 years ago

OK. I myself noticed it. We should fix it. I will take a look at it.

pushyamig commented 2 years ago

My Research around this issue is captured here

lsloan commented 2 years ago

I've QA tested this according to the test plan in the description of this issue.

When the maximum age of cookies is set to 60 seconds, performing the specified operation does produce an error, as expected. Refreshing the page gets new cookies and is good for another 60 seconds. So, in that regard, this issue passes QA testing.

image

lsloan commented 2 years ago

However, as the description of this issue also suggests, the error message shown is very general. It would help to have a more specific error message and advice to the user.

lsloan commented 2 years ago

Moved this to "Done", since there wasn't any discussion about it in Slack.

pushyamig commented 2 years ago

@lsloan Sorry I missed the conversation, it will go in the done column. I think your suggestion after testing are great and will look into next phase. Thanks for testing!