Not sending auth header would return 500 (worker)

unkeyed / unkey

Open source API management platform

https://go.unkey.com

Other

4k stars 469 forks source link

Not sending auth header would return 500 (worker) #590

Closed RohitLakh closed 10 months ago

RohitLakh commented 11 months ago

Preliminary Checks

[X] I have reviewed the documentation: https://unkey.dev/docs
[X] I have searched for existing issues: https://github.com/unkeyed/unkey/issues
[X] This issue is not a question, general help request, or anything other than a bug report directly related to Unkey. Please ask questions in our Discord community: https://unkey.dev/discord.

Reproduction / Replay Link (Optional)

No response

Issue Summary

The new routes that we have created for CF worker and a zod schema to check for header rather than headers so when we are not sending headers in the request zod doesn't check that.

Steps to Reproduce

Take any CF route
Remove auth header
Send request

Expected behavior

The request should send 400 when no headers are sent

Other information

No response

Screenshots

No response

Version info

- OS: Mac OS
- Node: 18.16.0
- npm: 9.5.1

perkinsjr commented 11 months ago

Our CF code isn't completed so testing against it at this time is probably not ideal. @chronark

RohitLakh commented 11 months ago

True, this came up while adding more routes to the CF worker section. So followed the new contribution guideline to push a fix.

chronark commented 11 months ago

There's a weird behaviour in hono where get and post routes use either header or headers.

I haven't fully understood it yet

RohitLakh commented 11 months ago

So when we are using zod to check the schema we get headers in the autocomplete and while manually testing it with both header and headers, it turns out to be headers to be the check that we need to do using zod.

But when we are accessing the headers in the code to get some data. The documentation says that it will be depreciated and to use header to get the data.

I don't think that will based on the type of request that we have.

chronark commented 11 months ago

so what is the solution here? we use headers in the schema and header in the handler?

RohitLakh commented 11 months ago

Not in every place. Check the PR once.

chronark commented 11 months ago

why not in every place? I see the PR only adds it to some routes for API resources

RohitLakh commented 11 months ago

Ahh didn't add just updated header to headers for the routes where it was missing.

All the other routes were already using headers along with zod.