feat: admin api v1 - create url #2213

disKeith · 2023-05-04T04:02:35Z

Problem

This PR closes APIs for Forms to create shortlinks

Solution

This PR implements a new endpoint {{host}}/api/v1/admin/urls which is allows for whitelisted emails defined in ADMIN_API_EMAILS to call the endpoint to create new short URLs and transfer its ownership to a provided email address.

Features:

Added new route to {{host}}/api/v1/admin/urls that uses apiKeyAdminAuthMiddleware for authentication
- Updated apiKeyAdminAuthMiddleware to throw jsonMessage if api key is unauthorized
- Added POST route that additionally validates the required email attribute on top of existing validations
Added AdminApiV1Controller that handles POST requests with shortUrl, longUrl, and email attributes in the JSON body
- createUrl method to find or create a new user with email provided, then creates a new link with the authenticated user before transferring ownership to the parsed email.

Improvements:

Renamed ADMIN_API_EMAIL env variable to ADMIN_API_EMAILS to reflect its use to white list multiple emails
- Added integration-test-admin@open.gov.sg to the white list for integration tests
Updated isAdmin method in ApiKeyAuthService to handle multiple white listed emails

Tests

Unit Test Cases

Create, sanitize and transfer link to target email for admin API
Create and sanitize link with same owner and target email for admin API
Reports bad request with user creation
Reports not found on NotFoundError
Reports bad request on AlreadyExistsError
Reports bad request on Sequelize.ValidationError
Reports bad request on generic error

Integration Test

Added createIntegrationTestAdminUser to create a new integration test admin user which is using the defined integration-test-admin@open.gov.sg email
Tests Cases
- Should not be able to create urls without API key header
- Should not be able to create urls with invalid API key
- Should not be able to create urls with unauthorized API key
- Should be able to create link url with longUrl, shortUrl, and validEmail
- Should not be able to create link url with longUrl and shortUrl, without validEmail
- Should not be able to create link url with longUrl and shortUrl, with invalid email
- Should be able to create link url with longUrl and validEmail, without shortUrl
- Should not be able to create link url with longUrl, without shortUrl and validEmail
- Should not be able to create link url with invalid email
- Should not be able to create link url without longUrl, shortUrl, and email
- Should not be able to create link url with invalid longUrl
- Should not be able to create link url with invalid shortUrl
- Should be able to create link url with longUrl, shortUrl, and same email as admin

test/integration/api/external-v2/Urls.test.ts

halfwhole

mostly lgtm, thanks a lot for this!!

regarding an open question from before on whether this should or shouldn't be external API v2: after considering it more, I think I'd still somewhat (but not strongly) be in favour of it being admin API v1 instead. the functionality provided in this PR to create links under anyone else's account is not just equivalent to creating a link under your own account and then transferring it to someone else's, but involves possibly creating an entirely new account for that someone; plus, their email address need not even be whitelisted! perhaps it's a matter of personal opinion and risk tolerance, but to me this might just be too much power to give to just any public officer who can access Go?

plus, if we're not 100% confident that v2 should be released not just to specific admin users but rolled out to all ordinary public officers (like in v1), then it appears to me that by default they should be kept separate -- if v1 and v2 cater to different groups of people with different permissions and powers, they seem different enough to warrant separating this one out as an admin-only API. documentation-wise, labelling it as a new 'version' might also be misleading, as it implies to our current external API v1 users that there's a new version that they can upgrade to, when in fact it's exclusive to only certain privileged parties like Forms

README.md

src/server/config.ts

src/server/models/user.ts

test/integration/api/external-v2/Urls.test.ts

src/server/api/index.ts

src/server/modules/api/external-v2/ApiV2Controller.ts

src/server/api/external-v2/validators.ts

test/integration/admin/admin-v1/Urls.test.ts

halfwhole

looking good!

src/server/models/user.ts

src/server/admin/admin-v1/index.ts

src/server/admin/index.ts

test/server/mocks/repositories/UserRepository.ts

src/server/modules/api/external-v1/ApiV1Controller.ts

src/server/admin/index.ts

test/integration/admin/admin-v1/Urls.test.ts

src/server/modules/admin/admin-v1/index.ts

test/integration/admin/admin-v1/Urls.test.ts

src/server/admin/index.ts

test/integration/api/admin-v1/Urls.test.ts

+      API_ADMIN_V1_URLS,
+      { longUrl },
+      undefined,
+      'this-is-an-invalid-api-key',


halfwhole

LGTM, great work pushing out such a large feature 🔥 let's test it on staging sometime as we prepare it for prod release!

Some deploy notes:

Before deployment: set ADMIN_API_EMAILS on gov/edu/health staging
Before deployment: set ADMIN_API_EMAILS on gov/edu/health prod
After deployment: remove ADMIN_API_EMAIL on gov/edu/health staging
After deployment: remove ADMIN_API_EMAIL on gov/edu/health prod

src/server/api/admin-v1/validators.ts

src/server/modules/api/external-v1/index.ts

src/server/modules/api/admin-v1/AdminApiV1Controller.ts

src/server/modules/api/admin-v1/__tests__/AdminApiV1Controller.test.ts

* fix: package.json & package-lock.json to reduce vulnerabilities (#2207) The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-XML2JS-5414874 * feat: allow zip files and block password-protected files (#2203) * feat: admin api v1 - create url (#2213) * feat: api v2 endpoint and update admin auth middleware * chore: add and handle external user creation * chore: rename admin api env variable * chore: api v2 unit testing * chore: add integration tests * chore: update test api key string * chore: update unauthorized user error * chore: undo conditional user creation * chore: update admin email env var parsing * chore: port to admin api v1 and update unit and integration tests * chore: remove external user type and add domain validation error msg * chore: remainder port from v2 to admin v1 * chore: add missing return in external v1 * fix: ownership transfer condition and tests * chore: move api structure and update tests * chore: update readme * chore: add email domain validation to schema * chore: fix error type for generic errors * chore: undo types from external v1 * 1.76.0 --------- Co-authored-by: halfwhole <41856541+halfwhole@users.noreply.github.com> Co-authored-by: Snyk bot <snyk-bot@snyk.io>

disKeith added 7 commits April 26, 2023 15:36

feat: api v2 endpoint and update admin auth middleware

285b631

chore: add and handle external user creation

0cd0526

chore: rename admin api env variable

50380c5

chore: api v2 unit testing

80f4b5c

chore: add integration tests

6062ab0

chore: update test api key string

32ca4aa

chore: update unauthorized user error

db00035

disKeith requested review from gweiying and halfwhole May 4, 2023 04:02

github-advanced-security bot found potential problems May 4, 2023

View reviewed changes

test/integration/api/external-v2/Urls.test.ts Fixed Show fixed Hide fixed

disKeith changed the title ~~Feat/backend/create url admin api~~ feat: external api v2 - create url May 4, 2023

halfwhole reviewed May 4, 2023

View reviewed changes

disKeith added 5 commits May 11, 2023 01:42

chore: undo conditional user creation

0ae88d6

chore: update admin email env var parsing

c533284

chore: port to admin api v1 and update unit and integration tests

d7c8b32

chore: remove external user type and add domain validation error msg

2589247

chore: remainder port from v2 to admin v1

fae2318

github-advanced-security bot found potential problems May 10, 2023

View reviewed changes

test/integration/admin/admin-v1/Urls.test.ts Fixed Show fixed Hide fixed

chore: add missing return in external v1

8d54a14

disKeith changed the title ~~feat: external api v2 - create url~~ feat: admin api v1 - create url May 10, 2023

fix: ownership transfer condition and tests

773cd83

halfwhole reviewed May 12, 2023

View reviewed changes

disKeith added 2 commits May 23, 2023 20:10

chore: move api structure and update tests

53bc68a

chore: update readme

ec71b23

github-advanced-security bot found potential problems May 23, 2023

View reviewed changes

test/integration/api/admin-v1/Urls.test.ts

API_ADMIN_V1_URLS,

{ longUrl },

undefined,

'this-is-an-invalid-api-key',

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "this-is-an-invalid-api-key" is used as [authorization header](1).

halfwhole approved these changes May 25, 2023

View reviewed changes

disKeith added 3 commits May 29, 2023 11:40

chore: add email domain validation to schema

fe8169d

chore: fix error type for generic errors

13e92a9

chore: undo types from external v1

b4c75ec

disKeith merged commit 1c527c3 into develop May 29, 2023

halfwhole deleted the feat/backend/create-url-admin-api branch May 29, 2023 04:05

disKeith mentioned this pull request May 29, 2023

[release] 1.76.0 #2222

Merged

22 tasks

wanlingt mentioned this pull request Aug 8, 2023

feat: platform api v1 - get and update webhook settings opengovsg/FormSG#6608

Merged

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: admin api v1 - create url #2213

feat: admin api v1 - create url #2213

disKeith commented May 4, 2023 •

edited

Loading

halfwhole left a comment

halfwhole left a comment

halfwhole left a comment •

edited

Loading

feat: admin api v1 - create url #2213

feat: admin api v1 - create url #2213

Conversation

disKeith commented May 4, 2023 • edited Loading

Problem

Solution

Tests

halfwhole left a comment

Choose a reason for hiding this comment

halfwhole left a comment

Choose a reason for hiding this comment

halfwhole left a comment • edited Loading

Choose a reason for hiding this comment

disKeith commented May 4, 2023 •

edited

Loading

halfwhole left a comment •

edited

Loading