Skip to content

Conversation

@zgavin1
Copy link
Contributor

@zgavin1 zgavin1 commented Mar 12, 2025

Description

NextJS has a cool middleware feature we can leverage.

If I'm logged in and try to visit the login index page, redirect me to the dashboard. If I'm logged out and I try to access /dashboard or /coaching-session/... route, redirect me to the login index page.

This is not meant to be an all-encompassing route protection system. Just a simple "optimistic' light weight check against the cookie set by our login/logout API.

This does log a user out if their session expires from the backend.

Screenshots / Videos Showing UI Changes (if applicable)

Testing Strategy

describe how you or someone else can test and verify the changes

Concerns

Exposes bug #79 by the use of localStorage but not responsible for the bug.

@zgavin1 zgavin1 marked this pull request as ready for review March 12, 2025 18:07
@zgavin1 zgavin1 requested a review from jhodapp March 12, 2025 18:07
@zgavin1 zgavin1 self-assigned this Mar 12, 2025
@jhodapp jhodapp added the enhancement Improves existing functionality or feature label Mar 12, 2025
@jhodapp jhodapp added this to the 1.0-beta1 milestone Mar 12, 2025
Copy link
Member

@jhodapp jhodapp left a comment

Choose a reason for hiding this comment

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

Looks great Zach, thank you. A few things line to have a look at.

@zgavin1
Copy link
Contributor Author

zgavin1 commented Mar 12, 2025

Clicking around a bit more than I did earlier. This might be a little premature. Reverting to draft for now

@zgavin1 zgavin1 marked this pull request as draft March 12, 2025 19:15
@zgavin1 zgavin1 force-pushed the optimistic_nextjs_middleware_auth_check branch from ec7284f to 466e086 Compare March 14, 2025 17:55
@zgavin1 zgavin1 requested a review from jhodapp March 14, 2025 20:49
@jhodapp jhodapp marked this pull request as ready for review March 15, 2025 23:15
Copy link
Member

@jhodapp jhodapp left a comment

Choose a reason for hiding this comment

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

Looks great, works as expected.

There is a newly exposed bug from moving to localStorage where the coaching_session_id contained in the URL vs the one in the AuthStore are used differently on the coaching session page and becomes evident when you try and open the same coaching session URL on a new tab of the same browser. But this isn't your fault, it's just a bug outside the scope of this PR.

@zgavin1 zgavin1 merged commit 41df5c6 into main Mar 16, 2025
4 checks passed
@zgavin1 zgavin1 deleted the optimistic_nextjs_middleware_auth_check branch March 16, 2025 15:21
@github-project-automation github-project-automation bot moved this from Review to ✅ Done in Refactor Coaching Platform Mar 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improves existing functionality or feature

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants