/login returns 404 for GET and 500 for POST

[olizilla]

Using incorrect http method

Here we send GET /login lbut the route is defined as POST /login so we end up returning a 404, even tho the error message is also warning us that we will need a bearer key...

curl -I "https://api.nft.storage/login"
HTTP/2 404 

curl "https://api.nft.storage/login"
{"ok":false,"error":{"code":"EXPECTED_BEARER_STRING","message":"API Key is missing, make sure the `Authorization` header has a value in the following format `Bearer {api key}`."}}

If we added a bearer key we'd still get a 404 so we should be returning a 405 Method Not Allowed.

Not providing authorization header

If we send POST /login but without auth credentials:

curl -I -X POST "https://api.nft.storage/login"
HTTP/2 500 

curl -X POST "https://api.nft.storage/login"
{"ok":false,"error":{"code":"SyntaxError","message":"Unexpected end of JSON input"}}

This should return a 401 Unauthorized, and the response body should be as in the preview example

{"ok":false,"error":{"code":"EXPECTED_BEARER_STRING","message":"API Key is missing, make sure the `Authorization` header has a value in the following format `Bearer {api key}`."}}

related, it looks like we fixed this once before https://github.com/nftstorage/nft.storage/pull/95

[olizilla]

we convert the error code here https://github.com/nftstorage/nft.storage/blob/f503a4cca62b8fb9c39ecc3dd629370a08cbb82f/packages/api/src/errors.js#L73-L77

why it not return i wonder...

[olizilla]

Ah ha! Method not match! GET /login isn't a thing. Only POST /login

[olizilla]

oh wat, POST is 500

curl -I -X POST "https://api.nft.storage/login"
HTTP/2 500 

curl -X POST "https://api.nft.storage/login"
{"ok":false,"error":{"code":"SyntaxError","message":"Unexpected end of JSON input"}}

[olizilla]

👀 a request for GET /login is handled by theGET /:cid route, which includes the withAuth middleware-ish fn which then calls validate, which fails when it tries to parse the auth header.

[olizilla]

root route catch-alls considered harmful

[olizilla]

@hugomrdias there isn't an obvious easy fix here. Like if we pass a regex to regexparam instead of the '/:cid' route like /^\/(?<cid>ba\S+|Qm\S+)/i we then match on the 404 catch all route. I think we need to tweak the router to check for a route-but-not-method match, but I know you just extracted a standalone router, and wondered what the fate of the nft.storage router is before i dig in any further.

[hugomrdias]

Just kill it and use the new router I extracted.

[hugomrdias]

Hopefully I did decent job and it's easy to swap, and there not catch all just a not found hook