Skip to content

Evaluate redundant redirects between client and server #2992

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

Closed
lindapaiste opened this issue Feb 4, 2024 · 5 comments · Fixed by #2993
Closed

Evaluate redundant redirects between client and server #2992

lindapaiste opened this issue Feb 4, 2024 · 5 comments · Fixed by #2993

Comments

@lindapaiste
Copy link
Collaborator

Increasing Access

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

Feature enhancement details

Ref: #2987 (comment)

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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

Sorry, something went wrong.

@adityagarg06
Copy link
Contributor

@lindapaiste can you assign this issue to me

@Keshav-0907
Copy link
Contributor

@lindapaiste can i work on this issue

@PiyushChandra17
Copy link
Contributor

@raclim I would like to work on this issue. Can you please assign me this issue? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment