[#38] [Backend] As a user I can login with an OAuth provider

[tednguyendev]

Close #38

What happened 👀

• Add API endpoint to login with Google
• Store session in the cookie
• Store user in the database
• Use the New app Directory (Beta) of Next.js 13
• Use Route Handler of Next.js 13
• Use NextAuth to oAuth

Insight 📝

Reuse old structure Next.js code

This PR mostly reuse the code from PR #2 for convenience.

NextAuth

When compare NextAuth to Passport, we can easily see that Passport is more battle-tested, support more oAuth providers, more popular(21.5k stars on Github, compared to 16.7k of NextAuth), but I see NextAuth is more suitable for our POC, since:

• It's build for Next.js, hence more compatible and always keep up-to-date with Next.js, and it's compatible with Route Handler
  • Meanwhile, seems like the composition Passport and Route Handler and next-connect are not compatible yet. And it's also because the Route Handlerof Next.js 13 can already replace some parts of the next-connect now.
• A lot easier to setup oAuth, which only needs to setup envs, setup the provider credentials, and a route.js file,
• Easy to set up middleware

Proof Of Work 📹

• Login flow:

CleanShot.2023-05-23.at.15.18.13-converted.mp4

• Stored Cookie:

[olivierobert]

While re-using existing code is a good idea, I am unsure if we should still keep using next-connect with Next.JS 13.

I used that package as 1) passport is THE battle-tested oAuth library in the Node community, and 2) API routes in Next.JS 12 (with the Pages router) were quite "poor" (we had to check on the HTTP verb, e.g., `if req == 'POST').

However, Next.JS 13 (with the App router) has better server-side components, including routes handlers. So we can do defining GET and POST endpoints with function GET() {} and function POST() {}. So I believe we can do more backend logic similarly to what we do with Express (see this article to do oAuth without even passport).

I am not against using https://next-auth.js.org/ if it is easier (e.g, easier to test) and better (e.g., more secure, bug-free). I want to make sure we understand our options.

[olivierobert]

This package is now outdated as there is a newer 1.0.0 version that has been in the works and released: https://github.com/hoangvvo/next-connect/releases/tag/v1.0.0

[tednguyendev]

While re-using existing code is a good idea, I am unsure if we should still keep using next-connect with Next.JS 13.

I used that package as 1) passport is THE battle-tested oAuth library in the Node community, and 2) API routes in Next.JS 12 (with the Pages router) were quite "poor" (we had to check on the HTTP verb, e.g., `if req == 'POST').

However, Next.JS 13 (with the App router) has better server-side components, including routes handlers. So we can do defining GET and POST endpoints with function GET() {} and function POST() {}. So I believe we can do more backend logic similarly to what we do with Express (see this article to do oAuth without even passport).

I am not against using https://next-auth.js.org/ if it is easier (e.g, easier to test) and better (e.g., more secure, bug-free). I want to make sure we understand our options.

Yes I see.

I just updated the PR description. What do you think about this?

And there's also this one thing. When using passport authenticate, we are using it as authService.authenticate()(req, res), which needs both request and response, but since the syntax of Route Handlers is export async function GET(request: Request) {}, we don't have the response here. This is why I want to use the API Route temporarily for just this PR, which usePassport. Do you know if there are any other ways for us to do this if we still want to use Route Handlers?

And seems like it's indeed theNextAuth is easier to use with Route Handler .

The Nextjs and it's ecosystem are still new to me so I appreciate your help a lot :D.

[tednguyendev]

Hi @olivierobert, I decided to go with the NextAuth and reimplemented the flow. Please help me re-review this POC 🚀

[olivierobert]

Looks great overall 👍 Just two minor issues to fix before it can be merged.

[olivierobert]

Minor typo issue:

Suggested change

	it('return null', async () => {
	it('returns null', async () => {

[tednguyendev]

I just fixed it!