covidwatchorg / covidwatch-cloud-functions Goto Github PK

This will send a header with the current version.
Additionally the logic will either send a status ok, or an upgrade command.

Later we can add code to check the user-agent and version etc.

For now just short circuit to upgrade command so we can integrate / test this in the app code bases.
Ok command:
Custom Header:
"X-API-Version": {{current version}}

{
    "status": "200",
    "message": "App version ok"
}

Upgrade command:
Custom Header:
"X-API-Version": {{current version}}

{
    "status": "426",
    "message": "Please upgrade your client"
}

This is the tracking issue for adding integration tests.

Integration tests require the following components:

• An instance of the Firestore emulator
• An instance of the dev app (i.e. cd functions && go run ./cmd)
• A test program which exercises the dev app

We have the following constraints:

• We need to be able to clear the Firestore emulator after each integration test
• We need to be able to run the dev app with different environment variables set, so we need to be able to kill and restart it for different integration tests

Integration tests to be added:

• Test that ALLOW_EMPTY_CHALLENGE_SOLUTION is properly respected, and that challenges are enforced when it is not set (#54)

Currently, when a new pending report is uploaded, the upload token is allocated by generating a random number and retrying until an unallocated token is found. This works for testing, but is not what we need in the long run.

The actual token allocation scheme must retain the following property: When a new token is allocated, the numerically smallest token which is not currently allocated is chosen.

For the current version, we may be able to rely on Firestore's strong consistency model to make this easier. However, we should keep in mind that if we migrate off of Firestore in the future and onto another NoSQL database, that database may not have the same consistency model, and so we may need to design another scheme, possibly incorporating a SQL database into our infrastructure in order to support token allocation.

Use the net/http.MaxBytesReader function to limit the size of incoming requests in order to protect against DoS attacks.

Blocked on googleapis/google-cloud-go#1978

There was a period of time during which Actions were taking a long time to execute due to a large backlog. The problem seems to have been largely resolved. However, if it returns, one potential mitigation would be to disable Actions for draft PRs and encourage developers to leave PRs in a draft state until they are intending to merge.

Update the file COVID-Watch.postman_collection.json:

• Rename to Covid-Watch.postman_collection.json (to match stylization on covid-watch.org)
• Update with new API endpoints (this is most easily done by editing through the Postman app)

If we run the queries in the Emulator the firestore emulator should report what indexes are needed.

Add these to the documentation and the backend.

The firebase installation docs recommend using a curl|bash approach for beginners. Unfortunately, the firebase tool that this installs can end up using a different version of NPM than the version of NPM used to build/run the project. In order to avoid this, it's better to install using npm install -g firebase-tools, which is another of the documented installation methods. We should make it clear in the README that this is the preferred method.

Currently, we have a number of constants - such as the proof of work challenge's work factor, the token expiration period, etc - which are constants in the code. These should instead be runtime constants which are loaded from a config file, environment variable, etc.

Create a new markdown document (API.md? ENDPOINTS.md?) that documents each API endpoint and provides example requests and responses (including for error conditions).

The /validate endpoint is privileged, and it should only be possible to call it from within the private network.

This is part of the larger effort in #53.

Behaviors to test:

• When ALLOW_EMPTY_CHALLENGE_SOLUTION is not set, challenge solutions are enforced
• When ALLOW_EMPTY_CHALLENGE_SOLUTION is not set:
  • When the provided challenge solution is empty, the request is allowed to continue
  • When the provided challenge solution is non-empty, the solution is enforced

When an Apple/Google report is uploaded, we should validate that it matches the protobuf Key message described here. When we offer up keys for download, we need to serialize these keys in a File message, which will require us to first parse the uploaded Keys. Validating these on upload will allow us to avoid wasting database space on invalid Keys which we won't be able to serve later anyway.

Run the following commands in GitHub Actions that block merging of PRs:

• Run find functions -type f -name '*.go' -exec gofmt -l {} \;, and verify that its output is empty (this checks to make sure that all *.go files are properly formatted)
• cd functions && go vet

When I try to run npm build or npm test in the functions directory, I get errors like these:

> functions@ pretest /mnt/c/Users/drolle/Repos/covid19risk/covidwatch-cloud-functions/functions
> tsc --build tsconfig.test.json

../../../../node_modules/@types/jest/index.d.ts:35:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'beforeEach' must be of type 'HookFunction', but here has type 'Lifecycle'.

35 declare var beforeEach: jest.Lifecycle;
               ~~~~~~~~~~

  node_modules/@types/mocha/index.d.ts:2734:13
    2734 declare var beforeEach: Mocha.HookFunction;
                     ~~~~~~~~~~
    'beforeEach' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:37:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'afterEach' must be of type 'HookFunction', but here has type 'Lifecycle'.

37 declare var afterEach: jest.Lifecycle;
               ~~~~~~~~~

  node_modules/@types/mocha/index.d.ts:2752:13
    2752 declare var afterEach: Mocha.HookFunction;
                     ~~~~~~~~~
    'afterEach' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:38:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'describe' must be of type 'SuiteFunction', but here has type 'Describe'.

38 declare var describe: jest.Describe;
               ~~~~~~~~

  node_modules/@types/mocha/index.d.ts:2768:13
    2768 declare var describe: Mocha.SuiteFunction;
                     ~~~~~~~~
    'describe' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:40:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'xdescribe' must be of type 'PendingSuiteFunction', but here has type 'Describe'.

40 declare var xdescribe: jest.Describe;
               ~~~~~~~~~

  node_modules/@types/mocha/index.d.ts:2789:13
    2789 declare var xdescribe: Mocha.PendingSuiteFunction;
                     ~~~~~~~~~
    'xdescribe' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:41:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'it' must be of type 'TestFunction', but here has type 'It'.

41 declare var it: jest.It;
               ~~

  node_modules/@types/mocha/index.d.ts:2803:13
    2803 declare var it: Mocha.TestFunction;
                     ~~
    'it' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:43:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'xit' must be of type 'PendingTestFunction', but here has type 'It'.

43 declare var xit: jest.It;
               ~~~

  node_modules/@types/mocha/index.d.ts:2824:13
    2824 declare var xit: Mocha.PendingTestFunction;
                     ~~~
    'xit' was also declared here.

../../../../node_modules/@types/jest/index.d.ts:44:13 - error TS2403: Subsequent variable declarations must have the same type.  Variable 'test' must be of type 'TestFunction', but here has type 'It'.

44 declare var test: jest.It;
               ~~~~

  node_modules/@types/mocha/index.d.ts:2817:13
    2817 declare var test: Mocha.TestFunction;
                     ~~~~
    'test' was also declared here.


Found 7 errors.

When a request is made to the /report endpoint, the response should include the expiration time of the allocated upload token. The main difficulty is to make sure that the phone agrees with the server on when the time is (taking into consideration disagreeing clocks, time zones, etc). Some considerations:

• We can solve the time zone problem by reporting the time in UTC.
• We can partially solve the disagreeing clocks problem by reporting a delta (that is, the duration of the expiration period) rather than a timestamp. However, this may be broken by the phone changing its clock after the phone has stored the response from the server and calculated the expiration time in terms of its local clock.

Automatically run tests in GitHub and enable the "Require status checks to pass before merging" rule to require that these tests pass before merging to master.

• rename createPermissionNumber to approvePermissionNumber
• Change to new Request / Response types below

Request:

{
    "token": "1234567"
}

• in a transaction
  • query diagnosis_permission_number for token
    if result count == 1:
    diagnosis_permission_number_id = uuid of returned result
    else if result count == 0 or > 1:
    - if > 1, log an error because > 1 should never happen
    break, reject transaction
  • find diagnosis_permission_number for token
  • find positive_diagnosis record by positive_diagnosis_id
  • update positive_diagnosis record to {"status": "approved"}
  • update diagnosis_permission_number record to remove "token", "positive_diagnosis_id", "created_timestamp", while adding "approved_timestamp:":
    Example updated diagnosis_permission_number

      {
          "permission_key": "arbitrarystring",
          "approved_timestamp": {{ server_timestamp }}
      }
  

  • this allows the key to be used for future reports but not re-used to token auth

If success:
Response:

{
    "status": "200"
    "message": "Report Approved"
}

If fail:
Response:

{
    "status": "404",
    "message": "Token not found"
}

We need to add a service account or equivalent secure connection between the "permission validator" cloud function endpoint and the Permission Portal backend code.

Currently the old end point has been set to internal access only:
https://us-central1-covidwatch-354ce.cloudfunctions.net/createPermissionNumber

We need to ensure that this function can be invoked but only from the Permission Portal.

All responses should include a field with API version in the header, starting with 1 for example:

Custom Header:
"X-API-Version": 1

JSON Payload:

{
    "status": "201",
    "message": "Description"
}

The "Get all live signed reports as public should succeed" test sometimes fails and sometimes succeeds, implying that there is some accidental nondeterminism.

Concretely, when it fails, it fails with:

Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (.../covidwatch-cloud-functions/functions/.mocha/test/integrated.test.js)

So that we can leverage Cloud Endpoints and the ESPv2 nginx proxy we should define an openapi 2 spec.

See here: https://cloud.google.com/endpoints/docs/openapi

Draft a new README. Using @madhava's template, it should have

• Outline
• Setup
  • Go installation, environment, IDE
  • build and run
  • emulator setup
• API endpoints
  • purpose, procedure (i.e generate challenge first, use it for subsequent requests)
  • sample requests and expected response ( we have a Postman file containing sample requests that we use for integration test, maybe we should have something similar )
• Deployment
  • setup real Firebase / Firestore accounts
  • transfer the functions to Cloud Functions
• ToDos

Schedule functions to perform various cleanup actions:

• Remove expired challenges (once per hour, to be reevaluated if the challenge collection becomes a DoS vector)
• Remove expired upload tokens (once per day)

See this Stack Overflow post for advice on how to implement these in Go.

The unit test "should not let anyone write a singed_reports" fails which means anyone can write into the database. This is considering that in the code Firestore is initiated with null credentials. Maybe there's an issue in the way assertion interprets the response (i.e 200 but with fail message). I'm looking into it now.

See the other repos for examples:
https://github.com/covid19risk/

Adding Badges to the README.md would also be nice!

Apple recommends for the mobile app to request Exposure Notification Configuration Data from the server (https://developer.apple.com/documentation/exposurenotification/building_an_app_to_notify_users_of_covid-19_exposure).

This means we need a simple getExposureConfiguration method that returns a JSON-encoded ENExposureConfiguration

(https://developer.apple.com/documentation/exposurenotification/enexposureconfiguration).

https://imgur.com/a/m00Dotq

Replace the old /submitDiagnosis endpoint with a new /report endpoint.

The only method supported on /report is POST. There are two forms of request to /report:

Initial Report

When the first report is uploaded, no upload key is included. Initial report upload is rate limited using a proof of work challenge.

Request

{
   "challenge" : {
      "challenge" : {
         "nonce" : "0e0e6fd368aac433f4b59ce218233385",
         "work_factor" : 1024
      },
      "solution" : {
         "nonce" : "15b59b443d8c662473e1534189e46f17"
      }
   },
   "report" : {
      "data" : "YtO6A+YhTu2ne8yqIywl3myGv3ZUICgvDcIMlm5y5TZMxERkg5bLKj+I"
   }
}

The object at challenge.challenge is a challenge which was previously obtained via a GET request to /challenge.

data is a Base64-encoded report.

Behavior

1. Validate that data is not too long
2. Validate the solution to the challenge, including validating that the challenge has not expired
3. Allocate a new upload token, generate a new upload key, and store these along with the report data in the database
4. Respond with the upload token and upload key

Responses

On success, respond with status code 200 and the following body:

{
   "upload_token": "123-456-9",
    "upload_key": "02f9a1d73a3d5dc00a42200002f52172",
}

On data too long, respond with status code 400 and the following body:

{
    "message": "report data too large"
}

On invalid challenge solution, respond with status code 400 and the following body:

{
    "message": "invalid solution to proof of work challenge"
}

On expired challenge, respond with status code 400 and the following body:

{
    "message": "proof of work challenge expired"
}

Subsequent Report

When subsequent reports are uploaded, the upload key generated after the first report is used to authorize the upload, and no proof of work challenge is required.

Request

{
   "upload_key": "02f9a1d73a3d5dc00a42200002f52172",
   "report" : {
      "data" : "YtO6A+YhTu2ne8yqIywl3myGv3ZUICgvDcIMlm5y5TZMxERkg5bLKj+I"
   }
}

Behavior

1. Validate that data is not too long
2. Check that the upload key exists in the database
3. Publish the report

Response

On success, respond with status code 200 and the following body:

{}

On data too long, respond with status code 400 and the following body:

{
    "message": "report data too large"
}

On invalid upload key, respond with status code 403 and the following body:

{
    "message": "invalid upload key"
}

Currently it seems that prettier is not included as a devDependency and there is no prettier config checked in. As a result there's no consistency in the file formatting - if I make a change to a file I'm getting the whole thing lighting up in the diff because the default prettier config is different to how the previous developer formatted the file.

What I would suggest as a way to solve this issue and simplify the whole build/lint/test process at the same time is that we could use tsdx to manage those tasks, because it handles both tsc and babel and ships with prettier included.