Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Add support for v1.5 API #663

Closed
flagxor opened this issue Jun 23, 2020 · 10 comments
Closed

Add support for v1.5 API #663

flagxor opened this issue Jun 23, 2020 · 10 comments
Assignees
Labels
kind/enhancement New feature or feature request.

Comments

@flagxor
Copy link
Contributor

flagxor commented Jun 23, 2020

TL;DR

v1.5 adds some additional fields attached to keys that need to be handled by the Key server.
Limited support for change of key status + revocation is supported in the API shape.

Design Goals

Additional fields need to be selected by the HA app:

  • report_type - unambiguous source of the diagnosis
  • days_since_onset_of_symptoms - offset to the date on which symptoms first appeared

The HA also has the opportunity to make a single revision to a previously published key:

  • Changes are placed in revised_keys

Only some changes are allowed (enforced by the client, but server can also check):
Recursive -> Confirmed Test, Revoked, Clinical Diagnosis, Self-Report
Self-Report -> Confirmed Test, Revoked, Clinical Diagnosis
Clinical Diagnosis -> Confirmed Test, Revoked

To support these fields well the Diagnosis Key server will need to:

  • Allow the app to provide an optional days since first onset and attach this to keys
  • Allow the app to suggest a reportType, but limit some report types to those certified by a verification server

Revision + revocation will require dispensing some secret to the client to allow later revalidation to change a previously released key.
Revision + revocation support in the reference server is of secondary priority to supporting the new key fields.

Transmission risk is deprecated, but can be co-present with the other fields to support v1.0 clients.

@mikehelmick
Copy link
Contributor

I'll take this on next week.

@mikehelmick mikehelmick added the kind/enhancement New feature or feature request. label Jun 23, 2020
@mikehelmick
Copy link
Contributor

Is revoked in this case, a user initiated action or does that need to be run through a verification server?

mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 7, 2020
* remove free form pha claims
* introduce v1.5 fields to verification claims
* clairfy naming
* mark transmission risk overrides as deprecated, but still used

Part of google#680
Part of google#663
@mikehelmick
Copy link
Contributor

mikehelmick commented Jul 7, 2020

Some high level notes

  • In this reference sever we will not support uploads of "recursive" or "self-report" at this time, but instead continue to recommend the use of the verification server protocol.
  • Symptom onset date will be accepted and then assigned by the server. This field can also be present on the verification certificate. Ideally that is where the field will come from.
  • Report type must come from a verification certificate, we won't let apps assign this on their own
  • For revised keys:
    • the verification system itself doesn't know that it is revising a key.
    • For HA verification there are 2 paths we can support
      • likely -> confirmed
      • likely -> negative
      • if a duplicate key is encountered and the verification certificate indicates this status change, it will be recorded
    • This is the 3rd allowable change only (see original comment)
    • revisions must come from the same health authority ID

Execution plan

  • Change verification certificate claims to support v1.5 fields
  • Enable same day / early key release in server
  • Change the exposure table to add the following fields: health_authority_id, report_type, days_since_symptom_onset, revised_report_type, revised_at (timestamp)
  • Add support in the exposure API to accept symptom onset date only. This means that a confirmed or likely diagnosis can only come from a verification certificate.
  • Add the new fields to the verification server for token + certificate processing
  • Process expanded fields in the verification certificate. Write new fields to database
  • Add support for key revision in the exposure API
  • Add support for publishing revised keys in export files, based off of the new fields in the exposure table.
  • export padding needs to take into account new fields during generation
  • generation - server needs to generate v1.5 metadata
  • generation - export generation needs v1.5 metadata and revised keys

google-oss-robot pushed a commit that referenced this issue Jul 7, 2020
* remove free form pha claims
* introduce v1.5 fields to verification claims
* clairfy naming
* mark transmission risk overrides as deprecated, but still used

Part of #680
Part of #663
@mikehelmick
Copy link
Contributor

Updates to verification server for new claims:
google/exposure-notifications-verification-server#101

@mh-
Copy link
Contributor

mh- commented Jul 9, 2020

The specification still states "The header must be exactly “EK Export v1” right-padded to 16 bytes with UTF-8 whitespaces."
Do you intend to keep that / how do you plan to do versioning for this interface?

@mikehelmick
Copy link
Contributor

@flagxor @gurayAlsac - Can you answer the question about the header string?

mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 9, 2020
* Allow for v1.5+ early key release. Multiple keys can be provided that all have the same start interval
* Add configuration params for this
* Add tests for new pieces. 100% coverage on exposure model transform.
* Add documentation.
* add same day release as an optional feature to the generate server

Fixes google#705
Part of google#663
@mikehelmick
Copy link
Contributor

When stable, I plan to promote the API version from v1alpha1 to v1beta1.

google-oss-robot pushed a commit that referenced this issue Jul 9, 2020
* Allow for v1.5+ early key release. Multiple keys can be provided that all have the same start interval
* Add configuration params for this
* Add tests for new pieces. 100% coverage on exposure model transform.
* Add documentation.
* add same day release as an optional feature to the generate server

Fixes #705
Part of #663
mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 11, 2020
* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on google#663
mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 11, 2020
* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on google#663
google-oss-robot pushed a commit that referenced this issue Jul 13, 2020
* Work on v1.5

* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on #663

* review comments

* got/want fix

* tabs
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* remove free form pha claims
* introduce v1.5 fields to verification claims
* clairfy naming
* mark transmission risk overrides as deprecated, but still used

Part of google#680
Part of google#663
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* Allow for v1.5+ early key release. Multiple keys can be provided that all have the same start interval
* Add configuration params for this
* Add tests for new pieces. 100% coverage on exposure model transform.
* Add documentation.
* add same day release as an optional feature to the generate server

Fixes google#705
Part of google#663
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* Work on v1.5

* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on google#663

* review comments

* got/want fix

* tabs
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* Work on v1.5

* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on google#663

* review comments

* got/want fix

* tabs
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* remove free form pha claims
* introduce v1.5 fields to verification claims
* clairfy naming
* mark transmission risk overrides as deprecated, but still used

Part of google#680
Part of google#663
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* Allow for v1.5+ early key release. Multiple keys can be provided that all have the same start interval
* Add configuration params for this
* Add tests for new pieces. 100% coverage on exposure model transform.
* Add documentation.
* add same day release as an optional feature to the generate server

Fixes google#705
Part of google#663
krazykid pushed a commit to krazykid/exposure-notifications-server that referenced this issue Jul 13, 2020
* Work on v1.5

* Add key revision columns to exposure table
* This includes capturing the health authority ID (if provided)
  * Keys can only be revised by the same health authority ID
* Implement setting of Exposure fields based on the claims from the verification certificate
  * If a ReportType is present, set it in the Exposure.ReportType
  * Optionally, backfill the transmission risk, based on the report type
* Calculate the days +/- sypmtom onset
  * the symptom onset interval can be provided in the API or from the verification certificate
* stub out key revision
  * existing keys are read from the DB if they match the input
  * TODO: merge existing and input keys
  * TODO: implement transactional key revision
* IterateExposures
  * add ability to select revised keys

More work on google#663

* review comments

* got/want fix

* tabs
mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 14, 2020
* Publish API changes
 * Transform still validates the incoming request
 * If any of the uploaded keys have previously been seen, join and figure out changes
 * perform upsert over the new keys

More work on google#663
@gurayAlsac
Copy link
Contributor

@flagxor @gurayAlsac - Can you answer the question about the header string?

We won't be changing the header string for this backwards compatible proto change. The header string would only change for a larger file format change (e.g., using a different proto, or something other than a proto)

mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 15, 2020
* Add new TEK metadata fields to generated exports
* Add revised keys to output

Work on google#663
google-oss-robot pushed a commit that referenced this issue Jul 15, 2020
* Implement ExposureKey revisions

* Publish API changes
 * Transform still validates the incoming request
 * If any of the uploaded keys have previously been seen, join and figure out changes
 * perform upsert over the new keys

More work on #663

* review comments
@mikehelmick mikehelmick mentioned this issue Jul 15, 2020
mikehelmick added a commit to mikehelmick/exposure-notifications-server that referenced this issue Jul 15, 2020
* Add new TEK metadata fields to generated exports
* Add revised keys to output

Work on google#663
google-oss-robot pushed a commit that referenced this issue Jul 15, 2020
* v1.5 export files

* Add new TEK metadata fields to generated exports
* Add revised keys to output

Work on #663

* fix build errors

* fix out of bounds w/ empty data

* comments on new errors
@mikehelmick
Copy link
Contributor

/close

Remaining work is in #717

@google-oss-robot
Copy link

@mikehelmick: Closing this issue.

In response to this:

/close

Remaining work is in #717

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement New feature or feature request.
Projects
None yet
Development

No branches or pull requests

5 participants