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

User Session #46

Merged
merged 2 commits into from
Dec 11, 2024
Merged

User Session #46

merged 2 commits into from
Dec 11, 2024

Conversation

wkim10
Copy link
Collaborator

@wkim10 wkim10 commented Dec 11, 2024

Description

Added user provider and session, along with middleware to protect routes

Issues

None :)

Screenshots

None :)

Test

Try logging in with different users and console.logging session.user, try navigating to routes that shouldn't be accessible (e.g. going to /private/ when you're not logged in or /public/ when you're logged in)

Possible Downsides

Once it's merged we need to update next-auth.d.ts with the updated VolunteerDetails fields

Additional Documentations

None :)

wkim10 added 2 commits December 11, 2024 05:07

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link
Collaborator

@johnny-t06 johnny-t06 left a comment

Choose a reason for hiding this comment

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

LGTM, great stuff Won. Tested with two accounts and the routes based on logged in or not. Just make sure to remove the console.logs after testing

email: string;
volunteerDetails?: VolunteerDetails | null;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

The 3 interfaces look identical, can we combine them or rely on one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They represent different tokens so I think we need to separate them out like this, I can take a look at modularizing after midyear presentation :)

@wkim10 wkim10 merged commit 43e3c9c into main Dec 11, 2024
1 check passed
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