-
-
Notifications
You must be signed in to change notification settings - Fork 2
[BACK-3857] Add API docs for user consent records #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,7 +405,7 @@ paths: | |
get: | ||
operationId: VerifyDeviceToken | ||
summary: Verify Device Token | ||
description: "Checks the validity of Apple's DeviceCheck token by calling Apple's server to verify it. For more details, see Apple's [developer documentation](https://developer.apple.com/documentation/devicecheck)." | ||
description: 'Checks the validity of Apple''s DeviceCheck token by calling Apple''s server to verify it. For more details, see Apple''s [developer documentation](https://developer.apple.com/documentation/devicecheck).' | ||
requestBody: | ||
$ref: '#/components/requestBodies/VerifyDeviceToken' | ||
responses: | ||
|
@@ -680,6 +680,144 @@ paths: | |
- sessionToken: [] | ||
tags: | ||
- Internal | ||
/v1/consents: | ||
get: | ||
summary: List Consents | ||
tags: | ||
- Internal | ||
- Consent | ||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/consents.v1.yaml | ||
operationId: ListConsents | ||
description: 'Retrieves the list of all different consent types and their content. ' | ||
requestBody: | ||
content: {} | ||
parameters: [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks like also also also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and |
||
x-internal: true | ||
parameters: [] | ||
'/v1/consents/{consentType}': | ||
parameters: | ||
- $ref: '#/components/parameters/consentType' | ||
get: | ||
summary: Get Latest Consent By Type | ||
tags: | ||
- Consent | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for this and all of these requests below we should add security: as we have done in the rest of the docs. edit: For this particular one /v1/consents ooks like the security there might be a server token but the rest seem to work with a session token edit 2: Somehow the sessionToken authorization is indeed showing up on redocly. Either sorcery or I'm looking at something outdated? You may be able to disregard this... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's likely a global security setting. Added it explicitly. |
||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/consent.v1.yaml | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GET /v1/consents/big_data_donation_project results in this response: { which is not a single object as referenced here, but an array of objects. we should return the latest consent object directly I think or switch to the consents model (though the latter makes less sense for what I think this endpoint is intended for) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
operationId: GetLatestConsentByType | ||
description: Returns the latest consent version for the given type | ||
'/v1/consents/{consentType}/versions': | ||
parameters: | ||
- $ref: '#/components/parameters/consentType' | ||
get: | ||
summary: Get Consent Versions | ||
tags: | ||
- Consent | ||
- Internal | ||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/consents.v1.yaml | ||
operationId: GetConsentVersions | ||
description: Returns a list of all consent versions for a given type | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this endpoint? Given that version is an integer, it feels safe to assume that if the latest version is 4, then the list of versions is 4, 3, 2, and 1, right? If the latest version of a consent is 5, can a user consent to version 3? I think the API as it is right now would allow this, but why should the system allow that? Passing a version with a CreateRecord request makes sense, so we can double check that if they're creating a record for version 4, but version 5 is the newest, we can refuse and return an error. Is there some reason I don't understand for letting someone consent to an older version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This endpoint is only for admin purposes. I believe it will be useful down the road to display all consents versions of a given type e.g. in Orca. |
||
x-internal: true | ||
'/v1/users/{userId}/consents': | ||
parameters: | ||
- $ref: '#/components/parameters/userId' | ||
get: | ||
summary: Get User Consent Records | ||
tags: [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add Consent tag |
||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/records.v1.yaml | ||
operationId: GetUserConsentRecords | ||
description: Returns a list of all user consent records. By default only the most recent consent record for a given consent type will be returned. This can be overriden by setting the `latest` query parameter to `false`. | ||
parameters: | ||
- schema: | ||
type: boolean | ||
enum: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What other values would ever be added to this enum? Would this be better as a boolean flag? That is, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was changed to |
||
- 'true' | ||
- 'false' | ||
default: 'true' | ||
in: query | ||
name: latest | ||
description: Whether to return all versions or only the most recent one. | ||
- schema: | ||
type: string | ||
in: query | ||
name: type | ||
post: | ||
summary: Create User Consent Record | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can a user grant consent only the latest version of a consent? What happens if they try to grant consent to older version? This may need a bit more description of the various workflows - are these correct interpretations of the proposed API?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if by definition any new version of consent in case #2 is a superset of the original consent? If we revoke the past consent, does that mean we need to purge content they shared while that previous consent was in effect? IANAL so might be best to check with Jake. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How we interpret revocation dates and whether we purge the data depends on the type of the consent and the actual content. Currently, I believe we state that date shared with BDDP accounts is not purged even after the consent is revoked. |
||
operationId: CreateUserConsentRecord | ||
responses: | ||
'200': | ||
description: Created | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/record.v1.yaml | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
description: Creates a user consent record | ||
requestBody: | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/recordcreate.v1.yaml | ||
'/v1/users/{userId}/consents/{recordId}': | ||
parameters: | ||
- $ref: '#/components/parameters/userId' | ||
- $ref: '#/components/parameters/recordId' | ||
get: | ||
summary: Get User Consent Record | ||
tags: [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add Consent tag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The rest of the requests also don't have consent tags so they're popped out of the consent folder. I would add those too if you want them all under the "Consent" folder in the API docs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/record.v1.yaml | ||
operationId: GetUserConsentRecord | ||
description: Retrieves a consent record by ID for a given user. | ||
patch: | ||
summary: Update User Consent Record | ||
operationId: UpdateUserConsentRecord | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In most places we use Update<X> with PUT requests, which may or may not obey PUT semantics. Now we're adding PATCH endpoints called Update<X> (which will hopefully only implement PATCH semantics). I'm worried this will get confusing when implementing the handler functions. Ideally (IMO), PATCH => Update<X> and PUT => Replace<X>, but that's too big of a change for no really good reason. Is it clearer if we have PATCH => Patch<X> or just something other than Update<X>, so it's clear what the handler's expected semantics are? Mostly just thinking out loud here, I don't think I feel strongly enough to ask for anything here to be changed. Do you have thoughts? To summarize, I would like it if I knew that a handler named Update<X> was supposed to behave in a certain way, and if we have PUT Update<X> that does multiple different things already (some obey traditional PUT some don't), and then we add PATCH Update<X> it just all becomes more confusing than it needs to be. |
||
responses: | ||
'200': | ||
description: OK | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/record.v1.yaml | ||
description: '' | ||
requestBody: | ||
content: | ||
application/json: | ||
schema: | ||
$ref: ./auth/models/consent/recordupdate.v1.yaml | ||
delete: | ||
summary: 'Revoke User Consent Record' | ||
operationId: RevokeConsent | ||
responses: | ||
'204': | ||
description: OK | ||
description: Revokes a consent record | ||
components: | ||
securitySchemes: | ||
basicAuth: | ||
|
@@ -941,3 +1079,22 @@ components: | |
required: | ||
- code | ||
- reason | ||
parameters: | ||
consentType: | ||
name: consentType | ||
in: path | ||
required: true | ||
schema: | ||
$ref: ./auth/models/consent/type.v1.yaml | ||
userId: | ||
name: userId | ||
in: path | ||
required: true | ||
schema: | ||
$ref: ./common/models/tidepooluserid.yaml | ||
recordId: | ||
name: recordId | ||
in: path | ||
required: true | ||
schema: | ||
$ref: ./auth/models/consent/recordId.v1.yaml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
title: Consent | ||
description: Consent | ||
type: object | ||
properties: | ||
type: | ||
$ref: ./type.v1.yaml | ||
version: | ||
type: integer | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggest checking with Jake on whether we can rely on this version being a simple integer. We may need to accommodate an arbitrary string. I can see the value of an integer version for sorting, but we could accomplish that using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked to Jake - using an integer is ok |
||
example: 1 | ||
x-stoplight: | ||
id: 13aoxi0q1a5dq | ||
minimum: 1 | ||
readOnly: true | ||
content: | ||
type: string | ||
contentType: | ||
type: string | ||
enum: | ||
- markdown | ||
createdTime: | ||
type: string | ||
format: date-time | ||
readOnly: true | ||
required: | ||
- type | ||
- version | ||
- content | ||
- contentType | ||
- createdTime |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,21 @@ | ||||||
title: consents.v1 | ||||||
x-stoplight: | ||||||
id: o02joytb2fxay | ||||||
type: object | ||||||
properties: | ||||||
data: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
x-stoplight: | ||||||
id: iepviv3jm8m9m | ||||||
type: array | ||||||
uniqueItems: true | ||||||
items: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
$ref: ./consent.v1.yaml | ||||||
x-stoplight: | ||||||
id: 22oi76x2nbqnp | ||||||
count: | ||||||
type: integer | ||||||
x-stoplight: | ||||||
id: jsfxo40v5vj0g | ||||||
required: | ||||||
- data | ||||||
- count |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
title: metadata.tbddp.v1 | ||
x-stoplight: | ||
id: 95q0hhj045suc | ||
type: object | ||
properties: | ||
supportedOrganizations: | ||
type: array | ||
uniqueItems: true | ||
description: >- | ||
The list of supported organizations when the consent grant is for the | ||
Tidepool Big Data Donation Project | ||
items: | ||
x-stoplight: | ||
id: whfmccxwfj7zx | ||
type: string | ||
enum: | ||
- ADCES Foundation | ||
- Beyond Type 1 | ||
- Children With Diabetes | ||
- The Diabetes Link | ||
- Diabetes Youth Families (DYF) | ||
- DiabetesSisters | ||
- The diaTribe Foundation | ||
- Breakthrough T1D |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
title: metadata.v1 | ||
x-stoplight: | ||
id: 8ctwq6xjg496g | ||
oneOf: | ||
- $ref: ./metadata.tbddp.v1.yaml |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
title: record.v1 | ||
x-stoplight: | ||
id: nvtrxa89tps30 | ||
type: object | ||
properties: | ||
id: | ||
$ref: ./recordId.v1.yaml | ||
status: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than having a grab bag status field, which can be abused in future changes to the model, consider simply having a "revoked" field that's a timestamp. If the timestamp is a non-zero value, then it has been revoked. |
||
type: string | ||
x-stoplight: | ||
id: m5tc1errbouci | ||
enum: | ||
- active | ||
- revoked | ||
readOnly: true | ||
ageGroup: | ||
type: string | ||
x-stoplight: | ||
id: u5whtndrytunl | ||
enum: | ||
- <12 | ||
- 13-17 | ||
- 18+ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the API actually expects "<13", "13-17", ">=18" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
ownerName: | ||
type: string | ||
x-stoplight: | ||
id: 2cw4335ateapa | ||
description: The name of the account owner | ||
parentGuardianName: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The grantor can either be the owner of the data or the parent/guardian. Depending on the age group we might need to capture both names (e.g. when the owner is <18). |
||
type: string | ||
x-stoplight: | ||
id: 94pqubqvzwigs | ||
description: The name of the parent or legal guardian granting the consent. Required if ageGroup is '<12' or '13-17'. | ||
grantorType: | ||
type: string | ||
x-stoplight: | ||
id: zc8e4q8t37yig | ||
enum: | ||
- owner | ||
- parent/guardian | ||
type: | ||
$ref: ./type.v1.yaml | ||
x-stoplight: | ||
id: jub0w6n4sx03g | ||
version: | ||
type: integer | ||
x-stoplight: | ||
id: s4pt6ptt7ez4n | ||
metadata: | ||
$ref: ./metadata.v1.yaml | ||
x-stoplight: | ||
id: u4fzjwa9iivzp | ||
grantTime: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than repeating Time in each of these fields, consider just naming them Is Assuming that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The I think both grant time and status are useful for downstream consumers because it takes off the responsibility from explaining the implied meaning of the fields of the model. This will be useful to the data science team. |
||
type: string | ||
x-stoplight: | ||
id: z984vbbyzt0n5 | ||
format: date-time | ||
readOnly: true | ||
revocationTime: | ||
type: string | ||
x-stoplight: | ||
id: 9hx9eo1oucsak | ||
format: date-time | ||
readOnly: true | ||
createdTime: | ||
type: string | ||
x-stoplight: | ||
id: r7ypd0pfkbhkv | ||
format: date-time | ||
readOnly: true | ||
updatedTime: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did current response looks like this: { there is also a userId being sent back as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
type: string | ||
x-stoplight: | ||
id: ohlc8mndn74ov | ||
format: date-time | ||
readOnly: true | ||
required: | ||
- id | ||
- status | ||
- ageGroup | ||
- ownerName | ||
- grantorType | ||
- type | ||
- version | ||
- grantTime | ||
- createdTime | ||
- updatedTime |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
title: recordId.v1 | ||
x-stoplight: | ||
id: 5aevzr0n4qx6k | ||
type: string | ||
minLength: 1 | ||
readOnly: true |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
$ref: ./record.v1.yaml | ||
x-stoplight: | ||
id: saa37zeaoirkr | ||
examples: | ||
- type: tbddp | ||
version: 1 | ||
metadata: | ||
supportedOrganizations: | ||
- ADCES Foundation |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,21 @@ | ||||||
title: records.v1 | ||||||
x-stoplight: | ||||||
id: 0tuzybru8eee5 | ||||||
type: object | ||||||
properties: | ||||||
data: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Or... if you're looking for a more descriptive term in general, you might consider "grants", since "record" is a very generic term. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentionally generic so it can be consistent across endpoints. |
||||||
type: array | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
x-stoplight: | ||||||
id: whbezmg3q7pxh | ||||||
uniqueItems: true | ||||||
items: | ||||||
$ref: ./record.v1.yaml | ||||||
x-stoplight: | ||||||
id: u1b4u8fpl1ed6 | ||||||
count: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this useful? Isn't it just a data.length? Do we really need to put this in the response when it's so easy for a client to calculate it itself? PS- I believe a count appears in the list of consent types too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not the length of the data array. Count is the total number of documents matching the query. |
||||||
type: integer | ||||||
x-stoplight: | ||||||
id: leqbw9m8u2lku | ||||||
required: | ||||||
- data | ||||||
- count |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
title: recordupdate.v1 | ||
x-stoplight: | ||
id: c8oq1dn4m4uak | ||
type: object | ||
properties: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this include There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If so, then the required properties could be either There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Need to think about this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metadata is the only updatable attribute now. Consent can be revoked by sending a DELETE request. |
||
metadata: | ||
$ref: ./metadata.v1.yaml | ||
x-stoplight: | ||
id: 2hnhmpwbyck12 | ||
required: | ||
- metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to be able to restrict the list of consent types that could be accepted by a user? Or can everyone consent to everything? Put another way: does this API require a valid access token?
I could see a potential use-case for restricting this due to kind of an invitation-only or restricted clinical trial. However, this creates a problem with defining who can see which consent types, and how does that work with people who do not yet have a user account. Similar to invitation workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest checking in with Jake and Product team for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list endpoint is for internal/admin use only. We could make this endpoint public and add restrictions in the future. The frontend should be responsible for fetching a particular consent by type so it can be rendered.