feat: bearer token authentication for public APIs

[wanlingt]

Problem

Closes FRM-906

Solution

This PR allows admins with a valid API token to access public APIs. API tokens are in the format env_version_userId_token.

Breaking Changes

• No - this PR is backwards compatible

Features:

• Created a new base route for public APIs (/api/public/v1)
  Public APIs will not share the same namespace as internal session-based APIs (/api/v3) as the external APIs will have different functions (e.g. updating multiple form fields at one go)
• The hash of the API token is stored in the user collection as ApiKeyHash
• Add public API endpoint for listing forms (GET /api/public/v1/admin/forms)
• Added new error types for public APIs

Tests

Test API

Test 1: API works

• a. Generate a random string using crypto.randomBytes(32).toString("base64")
• b. find your user id, and format an API key as test_v1_${userId}_${randomString}
• c. Store a hash of that key (using 1 salt round) in the user collection, adding it using this structure:

  "apiToken" :{
      "keyHash": "${hashedApiKey}",
      "createdAt": ISODate("2023-06-01T00:00:00.000Z"),
      "lastUsedAtAt": ISODate("2023-06-01T00:00:00.000Z")
      }

(instructions on how to generate the api key and hash are also here)

• d. Send an API request as such:

curl --request GET \
  --url http://form.gov.sg/api/public/v1/admin/forms \
  --header 'Authorization: Bearer ${apiKey}' \

You should receive a 200 OK response, with the full list of forms associated with your user.

Test 2: Without authorisation header

• Repeat the same steps as above.
• In Step 2c, send a curl request without an authorisation header

curl --request GET \
  --url http://form.gov.sg/api/public/v1/admin/forms \

• You should receive a 400 Bad Request response, with the message "Authorisation header is missing"

Test 3: With wrong credentials

• Repeat the same steps as above.
• In Step 2c, send a curl request with a different API token

curl --request GET \
  --url http://form.gov.sg/api/public/v1/admin/forms \
  --header 'Authorization: Bearer test_v1_${userId}_${WRONG_API_TOKEN}' \

• You should receive a 401 Unauthorized response

Regression Test

Test 4: Regression test for handleListDashboardForms

• Log in. You should see the full list of forms in your dashboard, i.e. the GET request to http://form.gov.sg/api/v3/admin/forms should return with a 200 OK

Deploy Notes

New environment variables:

• API_KEY_VERSION: current API version
• PUBLIC_API_RATE_LIMIT: Per-minute, per-IP, per-instance request limit for public APIs

[LinHuiqing]

Should this be exposed in the code or should we load this in as an env var instead? From the point of view of exposing our protocol, I think loading env vars is slightly more secure. It'll also be easier for us to tweak if we want to change it.

[LinHuiqing]

Oh! Also I think we should set more rounds since this is for the API key which is permanent, rather than something ephemeral like our OTP!

[timotheeg]

Should this be exposed in the code or should we load this in as an env var instead? From the point of view of exposing our protocol, I think loading env vars is slightly more secure. It'll also be easier for us to tweak if we want to change it.

I'm not too worried about this. The strength of the encryption is more important than hiding the number of rounds (i.e. security through obscurity on the algo). Also With bcrypt, the number of rounds is encoded in the hash, so it is visible information to anyone who gets their hand on the hash (though obviously, we should do everything we can to protect against that!).

I also think that changing the number of rounds would be a very infrequent operation, so there's no need to have it a tunable.

I think we should set more rounds since this is for the API key which is permanent, rather than something ephemeral like our OTP!

Agreed. I increased the value to 2, which performs 4 rounds. We could do more, but we need to be mindful of processing cost. In the current POC, we compute the hash on every API call (as opposed to one time on login for session-based access).

At POC staged, I'm not overly worried about this.

[kenjin-work]

Random thought: Is implementing expiring bearer tokens (e.g. after a year) a common practice? Or is that not required here?

[timotheeg]

Random thought: Is implementing expiring bearer tokens (e.g. after a year) a common practice? Or is that not required here?

Good comment @kenjin-work !

It's common enough, and it indeed removes risk exposure for keys, especially which are not actively in use. But it's not a must-have practice, and many small SaaS do not have expiry on their token (e.g. BetterUptime or Snyk to name just 2 of our vendors). Removing keys can be disruptive to active automations, and so it typically requires some additional mechanisms to inform users (likely with multiple reminders!) that their key will expire in, say, 60 days, 30 days, 10 days, 2 days, 1 day, BOOM 😅

This API initiative is on POC/MVP mode right now, With the primary goal of showing what the authentication model and API namespace can be. We are not even considering building the UI to generate/revoke/regenerate the API tokens yet 😅 .

So we (cc @wanlingt as feature owner) can add requirements as we go to make the feature as robust as can. For example, just like we track lastLoginTime for UI sessions, so too we should track lastUsageTime for the token, and indeed we should add a tokenCreationTime too, so we are ready to add an expiration mechanism later on if we want.

[timotheeg]

Just a note on this: this works because the session middleware is currently running for all routes served by express, and that is sort of wrong.

We should activate the session middleware only for routes which require it (typically under the API router), and for the new "external" APIs, our auth middleware should take responsibility to setup a compatible session object. Something as small as:

Suggested change

	req.session.user = { _id: user.id }
	if (!req.session) req.session = {}
	req.session.user = { _id: user.id }

[timotheeg]

I added some minor changes, but I realized one important thing must be changed: retrieving forms atm returns the entire user object 😱. We need a filter process to make sure any non-intended fields is not returned to the client. The comment applies for both the session base API, and bearer token auth.

[timotheeg]

I added the API endpoint to retrieve submission count and submissions themselves (from our ndjson stream controller), and here is a gist that shows how it can be used to load submissions from the server (all const values formids, privateKey, apiKey are from my local env, so obviously they need to be changed accordingly to run yourselve.

One more note: I'm not sure we need to place the public API endpoints within a namespace /admin 🤔 , considering we have bearer token auth, public APIs should necessarily be for admins.

[timotheeg]

Also to note / possibly something for discussion.

The apiToken is now a structure that includes createdAt and lastUsedAt fields, and that will be useful for some instrumentation of the feature.

We might need an additional date field regeneratedAt, so if a key is revoked to create a new one, we don't lose stats of when admins first started to use an API key 🤔

[timotheeg]

LGTM. Good enough for demo even in staging / prod.

Important note though: before we start adding keys into users, we MUST change the query that retrieve forms, so the api key hash information NEVER leaves the DB or server.

ATM, the auto fetch of user objects for form->admin and form->collaborators return the api token structure as a user property. We need to return the bare minimum fields for users.