p5.js-web-editor icon indicating copy to clipboard operation
p5.js-web-editor copied to clipboard

Evaluate redundant redirects between client and server

Open lindapaiste opened this issue 1 year ago • 4 comments

Increasing Access

It is confusing to work with code where redirections can be be initiated from multiple places.

Feature enhancement details

Ref: https://github.com/processing/p5.js-web-editor/pull/2987#issuecomment-1925915203

We should look through the https://github.com/processing/p5.js-web-editor/blob/develop/client/routes.jsx file and see which redirects are not actually needed because they are already handled by the server in https://github.com/processing/p5.js-web-editor/blob/develop/server/routes/server.routes.js.

Specifically, we should check over our usages of userIsNotAuthenticated, userIsAuthenticated, userIsAuthorized, and createRedirectWithUsername. I have a hunch that we can actually delete all four of those functions. Let's make a list of all routes which use some form of redirection and whether each route's redirect logic duplicates what is already implemented on the server.

lindapaiste avatar Feb 04 '24 21:02 lindapaiste

Logged-out only:

Logged-in only:

Logged-in only, redirects to URL with username:

Only for current user:

  • "/:username/assets" client and server (we return a 404 for another user's assets, which might not be 100% correct)

Viewable to anyone:

  • "/"
  • "/reset-password/:reset_password_token" (but maybe should make this logged-out only)
  • "/verify" (maybe should be logged-in only?)
  • "/projects/:project_id"
  • "/:username/full/:project_id"
  • "/full/:project_id"
  • "/:username/sketches/:project_id/add-to-collection" (we only link to this URL if logged in, but there's no verification on the URL itself)
  • "/:username/sketches/:project_id"
  • "/:username/sketches"
  • "/:username/collections/:collection_id"
  • "/:username/collections"
  • "/about"
  • "/privacy-policy"
  • "/terms-of-use"
  • "/code-of-conduct"

lindapaiste avatar Feb 05 '24 01:02 lindapaiste

Proposed changes to server:

  • [ ] redirect "/sketches" and "/assets" to the URL with the username
  • [ ] redirect "/reset-password" and "/reset-password/:reset_password_token" to "/account" if the user is logged in (since the account page allows changing the password)

Proposed changes to client:

  • [ ] remove all auth checks from routes.jsx
  • [ ] delete /utils/auth.js file
  • [ ] delete /components/createRedirectWithUsername.jsx file

lindapaiste avatar Feb 05 '24 01:02 lindapaiste

@lindapaiste can you assign this issue to me

adityagarg06 avatar Feb 05 '24 03:02 adityagarg06

@lindapaiste can i work on this issue

Keshav-0907 avatar Feb 05 '24 04:02 Keshav-0907