Skip to content

Conversation

@flacial
Copy link
Member

@flacial flacial commented Nov 24, 2022

Closes #1935

Changed files in detail

  • success.tsx: We were using a set of middlewares to populate the user from. We now use the next-auth getSession method to retrieve the session, and handle when a user is already connected to the same Discord account.
  • login.tsx and signup.tsx: We used the Apollo mutations signup and login to start the flow. This has been changed with the next-auth signIn method.
  • logoutContainer: We used the Apollo mutation logout to log out the user. We now use signOut method by next-auth.
  • nextAuth (session callback): Updated the session callback to set the session.user as the user from the database. This callback runs every time we call getSession or useSession. The reason is that the data in the session stays outdated if one of them got updated from the client, such as if the user connected to discord. Created a custom credential provider that'll be used to handle the flow after the user confirm their email on signup flow.

Fixes the bugs in the initial PR #1985:

  1. Lack of email verification
  2. Multiple c0d3 accounts connected to the same Discord account
  3. Exposing sensitive data to the client
  4. Outdated session data when getting redirected to a page immediately after login

Testing

  1. Does login work?
  2. Does logout work?
  3. Does signup work?
  4. Does resetting the password work?
  5. Does setting the user password after email verification work?
  6. Does login with Discord work?
  7. Does connecting to Discord work?

Engineering changes 🔧

  • Use getSession and useSession to get the user's session.

Following

Handle the error thrown when a user is already connected to the same Discord account.

@vercel
Copy link

vercel bot commented Nov 24, 2022

@flacial is attempting to deploy a commit to the c0d3-prod Team on Vercel.

A member of the Team first needs to authorize it.

discordAvatarUrl,
discordUsername,
isConnectedToDiscord: !!user.discordRefreshToken // using this to avoid a second fetch to get Discord username
isConnectedToDiscord: !!user.discordId // using this to avoid a second fetch to get Discord username
Copy link
Member Author

Choose a reason for hiding this comment

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

It has been changed to discordId because with how next-auth work, we can't put sensitive data such as the user Discord refresh token in the session.

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

Merging #2543 (adbf6f1) into master (c0bba05) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2543   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          186       186           
  Lines         3401      3414   +13     
  Branches       915       929   +14     
=========================================
+ Hits          3401      3414   +13     
Impacted Files Coverage Δ
graphql/resolvers/session.ts 100.00% <ø> (ø)
components/AppNav/AppNav.tsx 100.00% <100.00%> (ø)
components/LogoutContainer.tsx 100.00% <100.00%> (ø)
graphql/resolvers/authController.ts 100.00% <100.00%> (ø)
graphql/resolvers/passwordController.ts 100.00% <100.00%> (ø)
helpers/nextAuth.ts 100.00% <100.00%> (ø)
pages/confirm/[token].tsx 100.00% <100.00%> (ø)
pages/discord/success.tsx 100.00% <100.00%> (ø)
pages/login.tsx 100.00% <100.00%> (ø)
pages/review/[lesson].tsx 100.00% <100.00%> (ø)
... and 1 more

@vercel
Copy link

vercel bot commented Nov 29, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
c0d3-app ✅ Ready (Inspect) Visit Preview Dec 8, 2022 at 5:49AM (UTC)

Since we migrated to next-auth, it's a bit hard to cache the session. By sending a request to get the session, the Apollo client will cache and the AppNav in the /curriculum page will display the login-signup buttons in loading-state
const router = useRouter()
const [logoutUser] = useLogoutMutation({
update(cache) {
cache.modify({
Copy link
Member Author

Choose a reason for hiding this comment

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

Expose the cache so we can modify without having to execute a query. There's probably a reason for setting the lessonStatus to an empty array.

@flacial flacial merged commit be42954 into garageScript:master Dec 8, 2022
@flacial flacial self-assigned this Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🦄 Done

Development

Successfully merging this pull request may close these issues.

Next-Auth: Replace GraphQL login/signup with next-auth

2 participants