Refactor Panoptes authentication in all packages #4803

eatyourgreens · 2023-06-13T15:00:04Z

Refactor usePanoptesUser to get the current user from the stored Panoptes JWT (#4766.) Fall back to auth.checkCurrent(), which gets a new session and token from the Panoptes API, if we don't have a stored token.

Refactor the classifier to authenticate in ClassifierContainer, waiting until sign-in is complete and all user data has loaded before rendering the Classifier component (#4845 and #4875.) Move all the workflow validation up to ClassifierContainer, so that we validate the workflow ID (from the URL) when the user has loaded, then pass down null to Classifier if they don't have permission to view that workflow.

Test ClassifierContainer, checking that:

inactive workflows only load when you have permission to view them.
the mock API for /subjects/queued is only called once.

Fix a bug, in the project app, where updating the project user in the project app's store will unmount and mount the classifier (#4946.)

Fix a bug where a project_roles request is continually made (closes #4975).

The Classifier component uses MobX observable data, but isn't wrapped in observer, so I've also fixed that here.

Please request review from @zooniverse/frontend team or an individual member of that team.

Package

app-content-pages
app-project
lib-classifier

Linked Issue and/or Talk Post

How to Review

This PR reflects on the Panoptes JSON Web Token to get the logged-in user for the project app, the content pages app and the classifier, which should be faster than pinging the API for the current user session and won’t ask Panoptes OAuth to generate a new token (zooniverse/panoptes-javascript-client#53.) If there's no stored token yet, it falls back to auth.checkCurrent(), which is currently used for all user checks.

You should be able to use any project, logged-in or not, and continue to use Admin Mode without any changes to the app's behaviour.

On page load, there should only be one request to /api/me to get the user resource.

Subjects should only be fetched once, with your access token, after you log in (#4875.)

Apps will still lose your stored session when they unload eg. when going from PFE to FEM on the same origin, or when going from the project app to the content pages app. That can be tested in staging, on the https://frontend.preview.zooniverse.org origin, after this PR merges.

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

Tests are passing locally and on Github
Documentation is up to date and changelog has been updated if appropriate
You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)
FEM works in a mobile browser

General UX

Example Staging Project: i-fancy-cats

All pages of a FEM project load: Home Page, Classify Page, and About Pages
Can submit a classification
Can sign-in and sign-out
The component is accessible
- Can be used with a screen reader BBC guide to VoiceOver
- Can be used from the keyboard WebAIM guide to keyboard testing
- It is passing accessibility checks in the storybook

Refactoring

The PR creator has described the reason for refactoring
The refactored component(s) continue to work as expected

coveralls · 2023-06-15T08:26:28Z

coverage: 81.95% (-0.1%) from 82.064% when pulling 0d5f5d0 on panoptes-token-auth into 42c767a on master.

eatyourgreens · 2023-06-17T10:52:33Z

Adding the 'bug' label to this because it fixes a small bug in the classifier. When you are already logged in, on the Classify page, usePanoptesUser in the classifier returns null when the Classifier component first loads (#4845.)

eatyourgreens · 2023-06-18T11:16:01Z

You can try out a production build of this branch here:
https://fe-project-branch.preview.zooniverse.org/projects/zooniverse/gravity-spy

Refactor `usePanoptesUser` to get the current user from the stored Panoptes JWT. Fall back to `auth.checkCurrent()`, which is slower, if we don't have a stored token.

Check Panoptes user state in `ClassifierContainer`. Defer rendering `Classifier` while that check is taking place.

Setting the UPP loading state to 'loading' will unmount the classifier, so avoid that after the page has loaded and the classifier is mounted.

Wait for all user session data to load before rendering the classifier. Refactor the user hooks to use the latest SWR API.

goplayoutside3

Looks good! I re-tested each of the Issues tagged here. The dev classifier runs as expected, as do the Zooniverse content pages and classifier storybook.

packages/app-project/stores/User/User.spec.js

packages/lib-classifier/src/components/Classifier/Classifier.js

packages/lib-classifier/src/components/Classifier/ClassifierContainer.js

packages/lib-classifier/src/components/Classifier/ClassifierContainer.spec.js

packages/lib-classifier/src/components/Classifier/hooks/usePanoptesUserSession.js

packages/lib-classifier/src/hooks/usePanoptesAuth.js

goplayoutside3 · 2023-07-11T19:32:27Z

packages/lib-classifier/src/store/SubjectViewerStore/SubjectViewerStore.js

@@ -39,7 +39,7 @@ const SubjectViewer = types

  .views(self => ({
    get disableImageToolbar () {
-      const subject = getRoot(self).subjects.active
+      const subject = tryReference(() => getRoot(self).subjects?.active)


Question: Just for my understanding, what does the change here implement?

subjects.active is a reference (it's actually the subject ID.) References should be accessed via tryReference to avoid errors.

https://mobx-state-tree.js.org/API/#tryreference

The mock store errored on this line during cleanup, because subjects.active isn't always a valid reference when the store is destroyed.

Co-authored-by: Delilah C. <[email protected]>

…ntainer.js Co-authored-by: Delilah C. <[email protected]>

Co-authored-by: Delilah C. <[email protected]>

packages/lib-classifier/src/components/Classifier/ClassifierContainer.spec.js

packages/lib-classifier/src/hooks/usePanoptesUser.js

eatyourgreens added refactor Refactoring existing code performance labels Jun 13, 2023

eatyourgreens requested a review from a team June 13, 2023 15:00

eatyourgreens force-pushed the panoptes-token-auth branch 4 times, most recently from 77bb390 to 0b53f9c Compare June 15, 2023 08:21

eatyourgreens marked this pull request as ready for review June 15, 2023 08:21

eatyourgreens changed the title ~~Get the user from Panoptes JWT tokens~~ Project app: Get the user from the Panoptes JWT Jun 15, 2023

eatyourgreens force-pushed the panoptes-token-auth branch 3 times, most recently from 6040321 to acc8d1c Compare June 15, 2023 21:27

eatyourgreens changed the title ~~Project app: Get the user from the Panoptes JWT~~ Authentication: Get the user from the Panoptes JWT Jun 15, 2023

eatyourgreens force-pushed the panoptes-token-auth branch 2 times, most recently from 83361af to 39de677 Compare June 16, 2023 11:16

eatyourgreens marked this pull request as draft June 16, 2023 11:35

eatyourgreens force-pushed the panoptes-token-auth branch 2 times, most recently from 67b8745 to a381330 Compare June 16, 2023 11:54

eatyourgreens marked this pull request as ready for review June 16, 2023 11:54

eatyourgreens force-pushed the panoptes-token-auth branch 3 times, most recently from 8097c25 to 287d5ce Compare June 17, 2023 06:11

eatyourgreens added the bug Something isn't working label Jun 17, 2023

eatyourgreens force-pushed the panoptes-token-auth branch from 287d5ce to 9855db1 Compare June 18, 2023 10:55

eatyourgreens force-pushed the panoptes-token-auth branch 2 times, most recently from 2d43bc2 to 39ac06e Compare June 20, 2023 15:44

eatyourgreens mentioned this pull request Jun 21, 2023

Wait before refreshing the subject queue #4848

Merged

12 tasks

eatyourgreens added 12 commits July 11, 2023 16:07

Get the user from the Panoptes JWT

6139f92

Refactor `usePanoptesUser` to get the current user from the stored Panoptes JWT. Fall back to `auth.checkCurrent()`, which is slower, if we don't have a stored token.

Update session auth in the classifier

d6bb803

Update session auth in the content pages app

2461f2f

Classifier: add user loading state

b3c2d01

Check Panoptes user state in `ClassifierContainer`. Defer rendering `Classifier` while that check is taking place.

Move decodeJWT to the API client

f6712f9

Maintain UPP when user refreshes in the background

5faccd8

Setting the UPP loading state to 'loading' will unmount the classifier, so avoid that after the page has loaded and the classifier is mounted.

Remove duplicate populateQueue on user change

11702e3

move project preferences into ClassifierContainer

7ec5e02

Move project roles into ClassifierContainer

2c3634e

Move allowed workflows up to ClassifierContainer

fb9ef22

Refactor user loading state

9ddea08

Wait for all user session data to load before rendering the classifier. Refactor the user hooks to use the latest SWR API.

Test admin mode in the classifier container

0f6b5cb

eatyourgreens force-pushed the panoptes-token-auth branch from afd5258 to 0f6b5cb Compare July 11, 2023 15:07

goplayoutside3 approved these changes Jul 11, 2023

View reviewed changes

github-actions bot added the approved This PR is approved for merging label Jul 11, 2023

eatyourgreens and others added 4 commits July 11, 2023 22:50

Update packages/lib-classifier/src/hooks/usePanoptesAuth.js

9b63a47

Co-authored-by: Delilah C. <[email protected]>

Update packages/lib-classifier/src/components/Classifier/ClassifierCo…

602f736

…ntainer.js Co-authored-by: Delilah C. <[email protected]>

Update packages/app-project/stores/User/User.spec.js

de49938

Co-authored-by: Delilah C. <[email protected]>

Update packages/lib-classifier/src/components/Classifier/Classifier.js

272187e

Co-authored-by: Delilah C. <[email protected]>

eatyourgreens commented Jul 11, 2023

View reviewed changes

packages/lib-classifier/src/components/Classifier/ClassifierContainer.spec.js Outdated Show resolved Hide resolved

packages/lib-classifier/src/hooks/usePanoptesUser.js Outdated Show resolved Hide resolved

Clean up ClassifierContainer tests

baba30e

eatyourgreens force-pushed the panoptes-token-auth branch 2 times, most recently from e42ab19 to df1e425 Compare July 12, 2023 08:33

eatyourgreens enabled auto-merge (squash) July 12, 2023 08:46

Use an SWR-like API for all Panoptes user hooks

be05d50

eatyourgreens force-pushed the panoptes-token-auth branch from df1e425 to be05d50 Compare July 12, 2023 08:49

eatyourgreens disabled auto-merge July 12, 2023 09:00

update ClassifierContainer propTypes

0d5f5d0

eatyourgreens merged commit 7d19264 into master Jul 12, 2023

eatyourgreens deleted the panoptes-token-auth branch July 12, 2023 09:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor Panoptes authentication in all packages #4803

Refactor Panoptes authentication in all packages #4803

eatyourgreens commented Jun 13, 2023 •

edited

Loading

coveralls commented Jun 15, 2023 •

edited

Loading

eatyourgreens commented Jun 17, 2023 •

edited

Loading

eatyourgreens commented Jun 18, 2023

goplayoutside3 left a comment

goplayoutside3 Jul 11, 2023

eatyourgreens Jul 11, 2023

eatyourgreens Jul 11, 2023 •

edited

Loading

Refactor Panoptes authentication in all packages #4803

Refactor Panoptes authentication in all packages #4803

Conversation

eatyourgreens commented Jun 13, 2023 • edited Loading

Package

Linked Issue and/or Talk Post

How to Review

Checklist

General

General UX

Refactoring

coveralls commented Jun 15, 2023 • edited Loading

eatyourgreens commented Jun 17, 2023 • edited Loading

eatyourgreens commented Jun 18, 2023

goplayoutside3 left a comment

Choose a reason for hiding this comment

goplayoutside3 Jul 11, 2023

Choose a reason for hiding this comment

eatyourgreens Jul 11, 2023

Choose a reason for hiding this comment

eatyourgreens Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

eatyourgreens commented Jun 13, 2023 •

edited

Loading

coveralls commented Jun 15, 2023 •

edited

Loading

eatyourgreens commented Jun 17, 2023 •

edited

Loading

eatyourgreens Jul 11, 2023 •

edited

Loading