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

Issue #621 #622

Merged
merged 3 commits into from
May 18, 2022
Merged

Issue #621 #622

merged 3 commits into from
May 18, 2022

Conversation

ElectricNroff
Copy link
Contributor

Identify the Bug

#621

Description of the Change

It changes three letters in two parts of the documentation.

Alternate Designs

An environment variable such as MY_API_ORG_URL could have been introduced, to avoid writing the same URL twice. Credentials could have been explicitly shown on the command line, instead of assuming that the user's .curlrc file is being used (the former would have cauaed an https://cwe.mitre.org/data/definitions/214.html problem).

Possible Drawbacks

A user might be confused because the example CNA name, mitre, already exists in the pre-population data set. (This pull request does not change the name.)

Verification Process

Checked both https://github.com/CVEProject/cve-services/blob/dev/src/controller/org.controller/index.js and https://github.com/CVEProject/cve-services/blob/dev/docs/openapi.yml for the correct route.

Release Notes

The Docker README was changed to refer to the /api/org route; fixes #621

@slubar
Copy link
Contributor

slubar commented Apr 11, 2022

@ElectricNroff the /api/org endpoint is for Secretariat only, so the result of the curl is an error message. If the point is to make sure that the server is running, doing an unauthenticated call such as /api/cve would be better

@ElectricNroff
Copy link
Contributor Author

The documentation that I'm trying to fix applies to a scenario where the user has just installed the CVE Services software, knows how to authenticate as the Secretariat, and has already understood step 6 of the "Setup for Local Penetration Testing" section. In other words, before getting to the later examples ("Try It Out" and "To use curl to add a CNA"), the user is already supposed to know that the three CVE-API headers are always required when authenticating. If the user doesn't understand this, then certainly the "To use curl to add a CNA" documentation isn't going to work for them. (In other words, it's not especially useful to change the "Try It Out" command to be unauthenticated, given that both the previous curl example and the next curl example require authentication.) Indeed it would be possible to write more expansive documentation, to cover the case where the user doesn't know anything about the three CVE-API headers, but that's not what I was trying to do in the pull request.

Whether the result of the curl is an error message depends entirely on context. If the user is familiar with curl, they may know that any required headers can either be placed directly on the command line (as in step 6), or can be placed in a .curlrc file. So, the user can choose to follow the structure of step 6, and insert the three "-H" command-line options within the commands shown in the "Try It Out" and "To use curl to add a CNA" sections. Or, the user can first create a .curlrc file with:

-H "CVE-API-ORG: mitre"
-H "CVE-API-USER: admin2@mitre.org"
-H "CVE-API-KEY: API_KEY"

and then run the commands exactly as shown in the "Try It Out" and "To use curl to add a CNA" sections.

@slubar
Copy link
Contributor

slubar commented Apr 12, 2022

@ElectricNroff please add the auth headers to the curl command so that it is more representative of what the user needs to do

@slubar slubar merged commit 21320be into CVEProject:dev May 18, 2022
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.

docker/README.md uses /api/cna instead of /api/org
2 participants