Skip to content

Add basic auth to integrations api#79

Merged
3mcd merged 2 commits into
mainfrom
kalilsn/basic-auth
Sep 8, 2023
Merged

Add basic auth to integrations api#79
3mcd merged 2 commits into
mainfrom
kalilsn/basic-auth

Conversation

@kalilsn
Copy link
Copy Markdown
Contributor

@kalilsn kalilsn commented Sep 8, 2023

Now that we nearly have a publicly exposed endpoint allowing us to send emails, I think we need to protect the API before someone spams people with our mailgun account. This PR requires all requests to the integrations api (except to the auth endpoint) to send an Authorization: Bearer <api key> header. It doesn't yet add this to the client SDK though! It also defaults to allowing requests if the API_KEY env var isn't set, so nothing will break when this is merged. That's not the default we should ship with!

Issue(s) Resolved

Resolves #44

Test Plan

  1. Set API_KEY=abc123 in your .env.local
  2. Run curl -iX GET localhost:3000/api/v0/integrations/708c6434-37c1-49f7-8fe6-f7e005b865cd/pubs -H 'authorization: Bearer abc123' -H 'content-type:application/json' and you should get a 200 response
  3. Run the same command with a different token than abc123 or without the header and make sure you get an error response

Notes/Context/Gotchas

  • I didn't use a middleware because vercel has decided that you shouldn't be allowed to use the node stdlib in middleware even if it's running in node: Switchable Runtime for Middleware (Allow Node.js APIs in Middleware) vercel/next.js#46722. There don't seem to be any workarounds
  • I'm not sure I'm convinced anymore that overloading the Authorization header for both the api key and the user auth token is a good idea. Seems like it might be confusing!

@kalilsn kalilsn requested a review from 3mcd September 8, 2023 14:59
@3mcd 3mcd changed the title [Draft] Add basic auth to integrations api Add basic auth to integrations api Sep 8, 2023
@3mcd 3mcd force-pushed the kalilsn/basic-auth branch from 1d824c8 to f48c138 Compare September 8, 2023 20:22
@3mcd 3mcd merged commit 4b51145 into main Sep 8, 2023
@3mcd 3mcd deleted the kalilsn/basic-auth branch September 8, 2023 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add basic auth to integration api routes

2 participants