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

feat(frameworks): add @auth/fastify #9587

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

Conversation

hillac
Copy link
Contributor

@hillac hillac commented Jan 9, 2024

☕️ Reasoning

This PR is a port of @rexfordessilfie's @auth/express to Fastify. It uses similar translation layer with Web API Request and Response to fastify types. I opted to make it a plugin.

Implementation

  • Introduce a toWebRequest helper that converts an FastifyRequest into a web
  • Introduce a toFastifyReply helper that converts a web Request into an FastifyReply
  • Introduce an FastifyAuth(authConfig) initializer which returns a fastify plugin function of type FastifyPluginAsync to fulfill authentication requests. Under the hood, it:
  • It calls toWebRequest(request) to get a web Request
  • Forwards the web request to Auth (from @auth/core) to get back a web Response
  • Forwards the web response along with Fastify's reply to toFastifyReply(response, reply) to respond to fulfill the request

Tests

  • Tests the toWebRequest and toFastifyReply helpers to ensure that they forward all headers, and body and in the right format (depending on content-type)
  • Tests the full login flow of a credentials provider
  • Tests the getSession by mocking the session

Documentation

I still haven't finished converting the express docs to fastify, just the main example is done so far.

Notes

  • In the docs, I've added the trust proxy for the https issue. I've yet to test if this is actually required in fastify.

  • For the async handlers, I've used return body instead of reply.send(body). I'm not sure whats preferred.

  • The body is unknown type in FastifyRequest so I checked typeof req.body === 'object' && req.body !== null in encodeRequestBody. I'm not sure if this is ok. In order to test encodeUrlEncoded, I had to add checks for the body type.

  • For the response tests, the fastify injector returned the body as a string, so I had to stringify the expected value in the test equality. It also added ; charset=utf-8 to the end of the content type header, so I stripped that off for the test equality. I'm not sure if this is an issue.

  • @fastify/formbody might need to be a peer dependency, I'm not sure.

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

📌 Resources

Ported express package to fastify and added the integration test for login.
Copy link

vercel bot commented Jan 9, 2024

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

Name Status Preview Comments Updated (UTC)
auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 19, 2024 0:29am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2024 0:29am

Copy link

vercel bot commented Jan 9, 2024

@hillac is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@hillac
Copy link
Contributor Author

hillac commented Jan 9, 2024

@ianschmitz How does this compare to the method you used?

@ianschmitz
Copy link

@ianschmitz How does this compare to the method you used?

I didn't get a chance to take it all the way to production, so I don't think I have a ton of feedback to give

@ndom91
Copy link
Member

ndom91 commented Jan 15, 2024

So first of all, thanks for contributing this! I'm working on a fastify application as we speak and had been putting off integrating Auth.js 😂

Anyway, I did some testing and it seems it's having an issue with esm/cjs and @auth/core. Error message printed when starting:

{"level":50,"time":1705315403580,"pid":1221767,"hostname":"ndo4","err":
{"type":"Error","message":"No \"exports\" main defined in /opt/ndomino/sveltekasten-
rss/node_modules/@auth/core/package.json","stack":"Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: 
No \"exports\" main defined in /opt/ndomino/sveltekasten-
rss/node_modules/@auth/core/package.json\n    at __node_internal_captureLargerStackTrace 
(node:internal/errors:497:5)\n    at new NodeError (node:internal/errors:406:5)\n    at 
exportsNotFound (node:internal/modules/esm/resolve:268:10)\n    at packageExportsResolve 
(node:internal/modules/esm/resolve:542:13)\n    at resolveExports 
(node:internal/modules/cjs/loader:547:36)\n    at Module._findPath 
(node:internal/modules/cjs/loader:621:31)\n    at Module._resolveFilename 
(node:internal/modules/cjs/loader:1034:27)\n    at a._resolveFilename 
(/opt/ndomino/sveltekasten-
rss/node_modules/.pnpm/tsx@4.7.0/node_modules/tsx/dist/cjs/index.cjs:1:1729)\n    at 
Module._load (node:internal/modules/cjs/loader:901:27)\n    at Module.require 
(node:internal/modules/cjs/loader:1115:19)\n    at require 
(node:internal/modules/helpers:130:18)\n   

Looks like its trying to resolve an import via esm/resolve helper, then falling back to cjs loader and then failing.. ./node_modules/@auth/core does contain the full package and normal package.json with the export map as expected, of course theres no main export, but it should pick up the import key 🤔

My fastify application is rather simple/straightforward atm and all ESM as far as I can tell

  • "type": "module" in root package.json
  • using import everywhere
  • Running it with tsx watch src/index.ts

Here's my entrypoint file, I've checked out your branch, built the fastify adapter and pnpm link-ed it into my project to get this:

import Fastify from "fastify"
import { dirname, join } from "path"
import { fileURLToPath } from "url"
import { updateJob } from "@jobs/cron-update"
import autoLoad from "@fastify/autoload"
import formbodyParser from "@fastify/formbody"

import GitHub from "@auth/fastify/providers/github"
import { FastifyAuth } from "@auth/fastify"

const fastify = Fastify({ logger: { level: "warn" } })
const _dirname = dirname(fileURLToPath(import.meta.url))

fastify.register(formbodyParser)

fastify.register(
  FastifyAuth({
    providers: [
      GitHub({
        clientId: process.env.GITHUB_ID,
        clientSecret: process.env.GITHUB_SECRET,
      }),
    ],
  }),
  { prefix: "/api/auth" },
)

fastify.register(autoLoad, {
  dir: join(_dirname, "routes"),
})

fastify.register(autoLoad, {
  dir: join(_dirname, "plugins"),
})
;(async function () {
  const port = process.env.PORT ? parseInt(process.env.PORT) : 8000
  try {
    await fastify.listen({ port, host: "0.0.0.0" })
    console.log(`
  🚀 Server ready at: http://0.0.0.0:${port}
  ⌛ Next cron run at: ${updateJob.nextRun()}
  `)
  } catch (err) {
    fastify.log.error(err)
    process.exit(1)
  }
})()

Am I missing anything? What did your example / development application look like and how did you run it? Maybe this is just a tsx issue 🤔

@hillac
Copy link
Contributor Author

hillac commented Jan 15, 2024

@ndom91 Here is a demo: https://github.com/hillac/authjs-fastify-demo
I'm not 100% if I've done the auth decorator in the idiomatic way, but it works for me. I'm on node 20.9.0.

Also, as was discussed with @auth/express, it might be nicer for users if the auth decorator is part of @auth/fastify in the plugin. I left it out for now to copy @auth/express.

@ndom91
Copy link
Member

ndom91 commented Jan 15, 2024

@hillac okay great, thanks for the repo example. Turns out I was just having some issues with the fastify autoload plugin. Seems to all work now!

@ndom91
Copy link
Member

ndom91 commented Jan 15, 2024

Regarding your question about the decorator - while I agree, we should try to get as much as possible in the plugin itself, this "authenticate" decorator seems like it would potentially be very different from user to user, no? 🤔

@ndom91
Copy link
Member

ndom91 commented Jan 15, 2024

Also looks like the toFastifyReply behaviour was correct when using an async cb - https://fastify.dev/docs/latest/Reference/Routes/#promise-resolution

packages/frameworks-fastify/src/index.ts Outdated Show resolved Hide resolved
@hillac
Copy link
Contributor Author

hillac commented May 12, 2024

I think it's ready, just waiting for a review I guess.

@ndom91
Copy link
Member

ndom91 commented May 12, 2024

Hey yeah this looks pretty good already! We'd really appreciate some prep in the new docs for Fastify as well

Can you add support for a fastify docs tabs for in the /docs/../Code/index.tsx component? (see: https://github.com/nextauthjs/next-auth/blob/main/docs%2Fcomponents%2FCode%2Findex.tsx).

Also maybe then add fastify example tabs to some of the initial setup docs / code examples in pages like /docs/pages/getting-started/installation.mdx and session-management/*.mdx? Once you add support for the fastify Code tab component (i.e. Code.Fastify), itll be super straight forward to add a fastify Tab in those docs pages

@hillac
Copy link
Contributor Author

hillac commented May 14, 2024

The information in the getting started pages seems kind of redundant to the information already in the api reference page.

Copy link

socket-security bot commented May 24, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher

🚮 Removed packages: npm/@typescript-eslint/eslint-plugin@v6.19.1, npm/@typescript-eslint/parser@v6.19.1

View full report↗︎

@hillac
Copy link
Contributor Author

hillac commented May 24, 2024

Ok, everything's ready

@hillac
Copy link
Contributor Author

hillac commented Jun 5, 2024

Is anything else needed to get this through?

@stonelotus
Copy link

Is anything else needed to get this through?

any updates on this?

@hillac
Copy link
Contributor Author

hillac commented Jun 24, 2024

@ndom91 @balazsorban44 @ThangHuuVu Anything else needed to get this merged?

Copy link
Contributor

@rexfordessilfie rexfordessilfie 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 work here @hillac! I hope you're able to get more attention and a merge soon.

@hillac hillac deleted the branch nextauthjs:main July 1, 2024 00:37
@hillac hillac closed this Jul 1, 2024
@hillac hillac deleted the main branch July 1, 2024 00:37
@hillac hillac restored the main branch July 1, 2024 00:51
@hillac hillac reopened this Jul 1, 2024
@hillac
Copy link
Contributor Author

hillac commented Jul 1, 2024

Oops, didn't realize renaming a branch would close the PR

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

Successfully merging this pull request may close these issues.

None yet

10 participants