[#152] Test: created integration test for supabase/middleware#402
[#152] Test: created integration test for supabase/middleware#402alexappleget wants to merge 21 commits intodevelopfrom
Conversation
alexappleget
requested review from
HoldUpFjord,
jennifertieu,
kepsteen and
shashilo
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
mathematiCode
left a comment
mathematiCode
left a comment
There was a problem hiding this comment.
I think it's fine to mock the page redirects, but I do think we should have an actual test database with test users to make sure they can only access the appropriate pages. Otherwise, really nice job on this!
…tegration-test-for-supabase-middleware
|
|
||
| describe('Middleware', () => { | ||
| it('redirects unauthenticated user back to /', async () => { | ||
| const request = new NextRequest('https://localhost:4000/dashboard'); |
There was a problem hiding this comment.
We're testing local or staging?
There was a problem hiding this comment.
also make the base URL an environment variable or at a minimum a constant and then you can do something like this.
| const request = new NextRequest('https://localhost:4000/dashboard'); | |
| const request = new NextRequest(`${new URL('/dashboard', BASE_APP_URL)}`); |
There was a problem hiding this comment.
I can't make this change yet as the staging/production url has not been set up yet on vercel
|
|
||
| describe('Middleware', () => { | ||
| it('redirects unauthenticated user back to /', async () => { | ||
| const request = new NextRequest('https://localhost:4000/dashboard'); |
There was a problem hiding this comment.
also make the base URL an environment variable or at a minimum a constant and then you can do something like this.
| const request = new NextRequest('https://localhost:4000/dashboard'); | |
| const request = new NextRequest(`${new URL('/dashboard', BASE_APP_URL)}`); |
| const { | ||
| data: { user }, | ||
| } = await supabase.auth.getUser(); | ||
| const accessToken = request.cookies.get('sb-access-token')?.value; |
There was a problem hiding this comment.
From what I’ve read, @supabase/ssr doesn’t include the createMiddlewareClient helper like the older @supabase/auth-helpers-nextjs package did. So manually reading the cookie to get the access token is the correct approach here.
That said, does this mean middleware-based auth hasn’t been working since we switched to @supabase/ssr? Just wondering if we had a gap in session handling before this change.
There was a problem hiding this comment.
When I was writing the test, I had to log in via the email way so that way we can use test users and not the google auth. When doing so, I noticed that the test kept throwing null for the user and that it couldn't find it. After digging, I found that the issue was happening here with fetching the user. So i added in that the helper function could grab the accesstoken itself, and if so then it fetches the user. Thinking about it, I do believe supabase had a gap in the auth and my test caught it.
There was a problem hiding this comment.
auth was working for google auth, but i believe we need to clean up our auth context as well. it's not handling things correctly
…tegration-test-for-supabase-middleware
…r others if needed
4 participants
Closes #152
All tests pass and I double checked by forcing them to fail to make sure it was testing correctly.
And since I touched the middleware file to move it into its own folder and write a test for it, I handled the eslint errors for the
middleware.tsas well.IMPORTANT: I edited the
middleware.tsfile because it was needed to even make any of these tests pass and be accurate.