Skip to content
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

Updated OpenAPI YAML for CVE v5 #511

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

david-waltermire
Copy link
Collaborator

Description of the Change

In the context #510, this PR updates the OpenAPI YAML for the services for the following endpoints:

  • POST /cve/:id/cna -> Publish the record, expecting a cna container in the JSON body.
  • PUT /cve/:id/cna -> Update the cna container for the record.
  • POST /cve/:id/rejected -> Reject the CVE. Expects a descriptions array with at least one english description and an optional replacedBy array to build the new rejected CVE record.

It integrates the CVE Record Format v5 JSON schema through an integration schema, which is specific to the service implementation.

This is a work in progress for discussion purposes.

@chandanbn
Copy link

I suggest merging PUT and POST into one POST and an "upsert" operation on backend if everything else is valid. See #505.
It reduces the necessity for a client to make two API calls or maintain state on client side.

- $ref: '#/components/parameters/api-secret-header'
- $ref: '#/components/parameters/api-user-header'
- $ref: '#/components/parameters/cve-id-path'
get:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can definitely do this in the future but I think we should avoid trying to do gets for the specific sub resources in order to make this first release. My intuition would be that most use cases would be interested in grabbing the whole record and even then any case that needs the container can pull it from the full record easily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good with me.

description: |
<h2>Access Control</h2>
<p>At least one of the following roles are needed to access the endpoint:</p>
<p>- <b>SECRETARIAT:</b> The user must belong to an Organization with the “SECRETARIAT” role</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that we'll need to specify that CNAs can access this endpoint to post their own records.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I wasn't sure what to include in these text fields.

application/json:
schema:
$ref: '#/components/schemas/errorGeneric'
/cve/{cve-id}/cna/rejected:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it intended to have the action endpoint off the /cna endpoint? For me it's more appropriate to have it off the whole resource (/cve/:cve-id) because that's what is getting rejected. Also, due to the nature of the structure of a rejected record, the containers actually become non-existent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the /cve/{cve-id}/reject endpoint approach better.

content:
application/json:
schema:
$ref: service-api-cve-schema.json#/definitions/rejected
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing the schema is WIP and we'll add the rejected schema once we actually build it.

description: |
<h2>Access Control</h2>
<p>At least one of the following roles are needed to access the endpoint:</p>
<p>- <b>SECRETARIAT:</b> The user must belong to an Organization with the “SECRETARIAT” role</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto adding permission documentation for CNAs

@david-waltermire
Copy link
Collaborator Author

I suggest merging PUT and POST into one POST and an "upsert" operation on backend if everything else is valid. See #505. It reduces the necessity for a client to make two API calls or maintain state on client side.

This is not the ideal REST way of doing this. POST is for creating. PUT is for updating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants