-
Notifications
You must be signed in to change notification settings - Fork 17
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
Use a Convex function for Next.js auth checks #129
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -3,7 +3,7 @@ import GitHub from "@auth/core/providers/github"; | |||
import { convexAuth } from "@convex-dev/auth/server"; | |||
import { internal } from "./_generated/api"; | |||
|
|||
export const { auth, signIn, signOut, store } = convexAuth({ | |||
export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one thing I don't like about this is that it expands the required members of the auth:
namespace.
Maybe instead I can bundle it in the auth
object that's already unpacked/exported here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't seem to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think this is kind of the only way to do this (we want a query, and need to add a new export for it)
If we made auth into a component, then we'd hide away some of the installation, but that's hard for other reasons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(oops forgot to click submit yesterday)
@@ -3,7 +3,7 @@ import GitHub from "@auth/core/providers/github"; | |||
import { convexAuth } from "@convex-dev/auth/server"; | |||
import { internal } from "./_generated/api"; | |||
|
|||
export const { auth, signIn, signOut, store } = convexAuth({ | |||
export const { auth, signIn, signOut, store, isAuthenticated } = convexAuth({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think this is kind of the only way to do this (we want a query, and need to add a new export for it)
If we made auth into a component, then we'd hide away some of the installation, but that's hard for other reasons
9ee4cc3
to
7e42e6c
Compare
@@ -0,0 +1,33 @@ | |||
import { test, expect, Page } from "@playwright/test"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome
test-nextjs/playwright.config.ts
Outdated
@@ -77,5 +77,7 @@ export default defineConfig({ | |||
: "npm run dev:frontend", | |||
url: "http://127.0.0.1:3000", | |||
reuseExistingServer: !process.env.CI, | |||
stdout: "ignore", // Set to "pipe" if you're debugging a failing test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it too loud? if this cost is just scrolling past logs seems useful to just have it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It adds about 100 lines of log output to a successful test run. But I guess others working on this might not even know to come in here and tweak this to debug a failing test - you're probably right that it's better to have on by default.
If the auth token doesn't validate on the Convex backend, then it's considered invalid and thus unauthenticated.
`isAuthenticatedNextJs` now checks w/ the Convex backend too.
We now unpack the isAuthenticated function in all of them.
5ae141b
to
4f4a69f
Compare
Suggestion from review.
Thanks for the reviews! |
If the auth token doesn't validate on the Convex
backend, then it's considered invalid and thus
unauthenticated.
The old check just validated that there was some token
cookie but not that it was valid for use.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.