fix: extractCookie from GraphiQLHeader (#6894)

[zaiyou12]

Implement a fix for this issue: #6894

Steps to reproduce

1. Follow dbAuth document
2. Setup GraphiQL header
3. Open http://localhost:8911/graphql in browser
4. See the following error:

api | 21:44:52 🌲 incoming request POST xxx /graphql
api | 21:44:52 🐛 Parsing request to extract GraphQL parameters
api | 21:44:52 🌲 request completed 29ms
api | 21:44:52 🐛 Processing GraphQL Parameters
api | 21:44:52 🚨 graphql-server Error building context. Error: Exception in getAuthenticationContext: Cannot read properties of undefined (reading 'id')

Solution description:

event.headers.cookie || event.headers.Cookie overwrites cookieFromGraphiqlHeader in below, even if user has generated graphiql headers. (It will return event.headers.cookie, not cookieFromGraphiqlHeader)

redwood/packages/auth-providers-api/src/dbAuth/shared.ts

Lines 19 to 21 in 61ec0fe

	return (
	event.headers.cookie || event.headers.Cookie || cookieFromGraphiqlHeader
	)

When graphiql headers has recognized, it should be returned immediately to prevent overwriting issue.

[replay-io]

6 replays were recorded for 3ea4f93.

4 Failed

• requireAuth graphql checks
• useAuth hook, auth redirects checks
• RBAC: Admin user should be able to delete contacts
• RBAC: Should not be able to delete contact as non-admin user

2 Passed

• Smoke test with dev server
• Smoke test with rw serve

View test run on Replay ↗︎

[dthyresson]

Thanks @zaiyou12 for finding that and fixing.

Could you add a test in packages/auth-providers-api/src/dbAuth/__tests__/shared.test.js so that we don't get a regression -- test that when graphiql headers has recognized, it should be returned immediately to prevent overwriting issue?

[zaiyou12]

Sure, I've updated commit with the test code

[dthyresson]

Thanks much @zaiyou12 for the tests! I'll have a look and also run by @cannikin as this touches dbAuth.

[dthyresson]

Had a few questions and will let @cannikin also review. Thanks!

[cannikin]

I wasn't involved in adding the GraphiQL header stuff to dbAuth, so let me know if my comment about that logic is way off base, maybe I just don't understand how it works! 😅

[zaiyou12]

I've fixed all the issues. Also, I added tests to make sure it works well under normal case when not using GraphiQLHeader.

[dthyresson]

Thanks @zaiyou12 ... I grabbed this PR and did a little refactoring on the tests and added some tests from the shared utilities.

But, I cannot push to your remote:

  ! [remote rejected]     zaiyou12/main -> zaiyou12/main (permission denied)

Could I get permissions to push?

We'll be able to merge once I can. Thanks!

[zaiyou12]

Hi @dthyresson , thank you for the updates. I’ve checked “Allow edits by maintainers” option and invited you to my repo as collaborator. I hope it will works. Thank you!

[dthyresson]

@zaiyou12 Sorry, when I pushed changes to your branch, I got a new PR with both your and my updates here: #6969

I'll close this PR and track the newer one. Sorry about that.

[zaiyou12]

It's okay. Thank you for your help!