Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: logged in means authenticated & authorized #709

Merged
merged 8 commits into from
Feb 12, 2024
Merged

Conversation

jimniels
Copy link
Collaborator

@jimniels jimniels commented Sep 25, 2023

Problem

The app renders based on whether the isAuthenticated variable from the auth0 client is true, e.g. "is the user logged in? ok render their dashboard"

However, with auth0 being authenticated does not mean being authorized.

Authenticated means we know who they are. Authorized means they have a token to do stuff in the app.

So for a user who has stayed logged in to the app for longer than 24hrs, we still know who they are (isAuthenticated from auth0 remains as true) however their authorization for making changes has expired (the token needs to be refreshed).

Today, this results in a flash of weird behavior where, if the user has been logged in for longer than 24 hours and they come back to the app, the app will render for a split second (because isAuthenticated=true) however they'll see an error because the auth token has expired, so anywhere we called authClient.getToken() throws an error and renders an error for the user. Behind the scenes, the auth0 client is redirecting the user to get a new token, but they see a flash of the app, then get redirect to auth0, then bounce back to the app with their content now visible.

There are two cases where this happens:

Case 1: the user reloads the app when the token has expired. In this case, the app renders for a split second with an error, the user is redirected to auth0 (note the URL change), a new token is granted, then they're redirected back to the app

CleanShot.2024-02-01.at.16.40.41.mp4

Case 2: the user comes back to the tab after the token has expired. In this case, we don't know that the token has expired yet because the app only tries to use the token when an API call is made. You can see in the video that only once they navigate to a different tab does the auth0 client try to get the token and realize that it's expired. At that point, the user sees the error for a split second after navigating, the whole app redirects to auth0, gets a new token, then navigates back to the app and the user sees their data.

CleanShot.2024-02-01.at.16.59.20.mp4

Solution

To address these issues, we ensure that we always have an access token. If we don't we redirect.

We do this by:

  1. Checking on the initial render of the app, before rendering, are we both authenticated and authorized? (Solves Case no. 1 above)
  2. Whenever the browser window regains focus, we check for the presence of the token (Solves Case no. 2 above).

This means that if a user refreshes the page, the app won't render until we first have an authorization token (if we don't the screen stays blank, it redirects to auth0, we get a token, then it redirects back and renders the app).

CleanShot.2024-02-01.at.16.57.13.mp4

Additionally, when the tab regains focus (switching back to Quadratic from another tab or window), we automatically check for a token in the background and, if we don't have one, we get a new one (you can see in the video that the URL changes as it goes to auth0, gets a token, then comes back to quadratic — they'll only see this when their token has expired).

CleanShot.2024-02-01.at.17.13.22.mp4

To Test

This should be tested from both the dashboard and from the app.

From dashboard

  • Log in and view your files
  • Edit the access token in localStorage so it's expiration date is in the past (or delete it)
  • Refocus the window (from the devtools)
  • See that the app navigates you to auth0 and re-authenticates you

If you really want to test this fine-grained:

  • Open the devtools and find the expiration date of the token
  • Use https://www.epochconverter.com/ to find out a time a few mins in the future.
  • Replace the expiration date of the token with this new one a few mins in the future
  • Navigate away from the app for a few mins
  • Come back to the app and see that it gets you a new token (and refresh the page)

From the app

  • Log in and view a file
  • Edit the access token in localStorage so it's expiration date is in the past (or delete it)
  • Refocus the window (from the devtools, or from another window)
  • See that the app navigates you to auth0 and re-authenticates you
CleanShot.2024-02-02.at.09.10.01.mp4

Unauthenticated

Part of this functionality works by checking for an auth token when the window regains focuses. We want that to always happen on the dashboard, but not always in the sheet (because an unauthenticated user may be looking at a sheet that's shared publicly).

  • Share a file publicly
  • View that shared link in an incognito browser
  • Ensure that the app loads and, as you switch from one window or tab back to quadratic, you don't get redirected to auth0.

Notes:

How to change the expiration date:

CleanShot 2023-09-25 at 15 20 59@2x

Or you can delete it (as seen in the video).

@vercel
Copy link

vercel bot commented Sep 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
quadratic ✅ Ready (Inspect) Visit Preview Feb 12, 2024 10:52pm

@davidkircos davidkircos temporarily deployed to quadratic-api-dev-pr-709 September 25, 2023 23:20 Inactive
@jimniels jimniels temporarily deployed to quadratic-api-dev-pr-709 September 26, 2023 16:25 Inactive
Base automatically changed from next to main November 13, 2023 19:06
@jimniels jimniels marked this pull request as draft November 20, 2023 18:04
@jimniels jimniels had a problem deploying to quadratic-api-dev-pr-709 February 1, 2024 22:06 Failure
@jimniels jimniels had a problem deploying to quadratic-api-dev-pr-709 February 1, 2024 22:07 Failure
Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (220bc37) 89.09% compared to head (4e4b739) 90.14%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #709      +/-   ##
==========================================
+ Coverage   89.09%   90.14%   +1.04%     
==========================================
  Files         155      129      -26     
  Lines       25314    23207    -2107     
==========================================
- Hits        22554    20919    -1635     
+ Misses       2760     2288     -472     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jimniels jimniels temporarily deployed to quadratic-api-dev-pr-709 February 1, 2024 23:36 Inactive
@davidkircos davidkircos temporarily deployed to quadratic-api-dev-pr-709 February 2, 2024 03:34 Inactive
@@ -54,7 +55,7 @@ export const router = createBrowserRouter(
}
}

initializeAnalytics({ isAuthenticated, user });
initializeAnalytics(user);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a user means the person is authenticated. We don't need both.

@davidkircos davidkircos temporarily deployed to quadratic-api-dev-pr-709 February 12, 2024 22:46 Inactive
@davidkircos davidkircos merged commit 9b74f7f into main Feb 12, 2024
11 of 12 checks passed
@davidkircos davidkircos deleted the next-fix-auth branch February 12, 2024 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants