Add high-level API actions for POA&Ms, SAPs, and SARs #63

nuttercd · 2022-08-08T19:24:38Z

This will close #56

Also update `$ref` for poam, sap, and sar paths

openapi.yaml

laurelmay

Need need OAuth2 scopes and to fix existing ones.

Good job copying the catalog :D but we need to update some words.

openapi.yaml

tuckerzp · 2022-08-09T19:22:33Z

This is a good start! Just make sure to look over your changes in the Swagger Editor before pushing. That helps a lot to see the places where info doesn't make sense or match up.

tuckerzp

Still looks like some of the tags are incorrectly labeled as Catalog

openapi.yaml

tuckerzp · 2022-08-11T19:23:56Z

openapi.yaml

+  - name: OSCAL System Assessment Plan
+    externalDocs:
+      description: Find out more
+      url: https://pages.nist.gov/OSCAL/concepts/layer/assessment/assessment-plan/
+  - name: OSCAL System Assessment Result


So I know this will take a little bit to change, but should we drop the "System"?

https://github.com/EasyDynamics/OSCAL/tree/main/json/schema https://pages.nist.gov/OSCAL/reference/latest/complete/json-outline/

The files are just named Assessment Plan & Assessment Result. So I wonder if it is needed? That or we should change the name to Security Assessment ...

@kylelaker & @brian-easyd what do you think?

I'd prefer that align with the NIST terminology here; and drop the first word. "Assessment Plan" and "Assessment Results" to match the documentation and schema.

And when people casually refer to the meaning of SAP and SAR, it is a Security Assessment Plan and Security Assessment Result(s) in my neck of the woods. Do people actually call it System Assessment Plan in your experience?

Fun fact, and I agree with Dave Waltermire on this: we call them AP and AR and don't mention the S because, in 800-53 land, they can be security assessments, privacy assessments, or other. They aren't strictly always security assessments, and labeling them that way prejudices right from the start.

Also, hi, super excited to see this PR and the discussion here. :-)

@aj-stein-nist Thanks! I think "System Assessment Plan" was some copying from "System Security Plan" and maybe a misunderstanding of it :D One thing we have noticed is that the specification does include Security but we can open an issue upstream to discuss that if it'd be useful.

I definitely like the idea of not including the "S" to cover the fact it can be for more than one thing!

@kylelaker @aj-stein-nist - In FedRAMP parlance (and most Agency implementations of FISMA) they are:

Security Assessment Plan (SAP); and

Security Assessment Report (SAR) [NOTE: Not "result", but "report".]

"System" is not typically used.

That said, Dave and I debated the AP, AR name and he made the very valid point that while these are the correct terms for government use, OSCAL is more than just for the Federal government. He asserted that "Assessment Plan" and "Assessment Result" were more friendly to all frameworks.

openapi.yaml

Bronstrom

This is looking really close to completion! I have just a couple minor suggestions.

openapi.yaml

Bronstrom · 2022-08-16T15:58:57Z

openapi.yaml

+          description: ID of plan of action and milestone to replace.
+          required: true
+          schema:
+            type: string
+      requestBody:
+        description: Plan of action and milestone object to be replaced.


Suggested change

description: ID of plan of action and milestone to replace.

required: true

schema:

type: string

requestBody:

description: Plan of action and milestone object to be replaced.

description: ID of plan of action and milestone to replace

required: true

schema:

type: string

requestBody:

description: Plan of action and milestone object to be replaced

For consistency purposes, we should delete any periods in the descriptions. There are a couple other places where this occurs

I believe prior descriptions had periods in them. Should we keep or remove periods in descriptions?

Probably best to remove those too, even though they aren't part of this issue.

laurelmay · 2022-08-16T16:23:30Z

openapi.yaml

+            write:aps: modify assessment plans in your account
+            read:aps: read your assessment plans parties
+            write:ars: modify assessment results in your account
+            read:ars: read your assessment results in your account


Here and elsewhere, I'd personally prefer to see us use assessmentPlan, assessment-plan, assessmentResults, or assessment-results as appropriate. More like Component Definitions than SSPs and POA&Ms.

Thoughts?

I like the use of assessmentPlan as it will match componentDefinitions

laurelmay · 2022-08-16T18:26:28Z

openapi.yaml

+            write:assessmentPlan: modify assessment plans in your account
+            read:assessmentPlan: read your assessment plans


Suggested change

write:assessmentPlan: modify assessment plans in your account

read:assessmentPlan: read your assessment plans

write:assessmentPlans: modify assessment plans in your account

read:assessmentPlans: read your assessment plans

Fix here and wherever else it's used

Also removes periods

Bronstrom

Nice work @nuttercd!

tuckerzp

I think this is almost perfect! Thank you @nuttercd for working on this so far. I am excited to get it in soon.

tuckerzp · 2022-08-22T18:38:48Z

openapi.yaml

+    get:
+      tags:
+        - OSCAL Plan of Action and Milestone
+      summary: Returns all OSCAL plan of action and milestones


I feel like Plans of action and milestones sounds better as the plural here.

Suggested change

summary: Returns all OSCAL plan of action and milestones

summary: Returns all OSCAL plans of action and milestones

tuckerzp · 2022-08-22T18:41:55Z

openapi.yaml

+    post:
+      tags:
+        - OSCAL Plan of Action and Milestone
+      summary: Adds a new OSCAL plan of action and milestone


Should milestone always be plural? That is how it appears on the spec . At the very least, we should be consistent on this.

Suggested change

summary: Adds a new OSCAL plan of action and milestone

summary: Adds a new OSCAL plan of action and milestones

tuckerzp

Thank you so much for you work on this @nuttercd! I am very glad to get this in.

tuckerzp · 2022-08-25T13:56:42Z

@kylelaker Looking over everything again, things look good to me. I wanted to give you a chance to look over at things before I dismissed your old review.

There is some inconsistency with periods that we should address.

tuckerzp · 2022-08-25T14:30:02Z

openapi.yaml

      description: |
-        Removes the given catalog from the given profile.
+        Removes the given catalog from the given profile

-        This should also result in references under other parts of the schema being removed.
+        This should also result in references under other parts of the schema being removed


I do not think that every description needs to have periods removed.

I will review this again after remaining comments are addressed.

…/oscal-rest into feature/add-poams-saps-sars

laurelmay

Thanks for your work on this! I think it's a good start for these types!

tuckerzp and others added 6 commits August 5, 2022 11:26

Add new tags

95b8fdf

Add temp info

eded859

Better define new OSCAL Types

74b27e0

Fix OSCAL type path errors, and ID's.

1707e82

Fix linting indentation errors

65fd05b

Fix indentation

a83315e

easy-dynamics-oscal-automation bot requested a review from a team August 8, 2022 19:24

Create component schemas for poam, sap, and sar

684e173

Also update `$ref` for poam, sap, and sar paths

laurelmay previously requested changes Aug 9, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

laurelmay reviewed Aug 9, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

openapi.yaml Outdated Show resolved Hide resolved

openapi.yaml Outdated Show resolved Hide resolved

openapi.yaml Outdated Show resolved Hide resolved

tuckerzp suggested changes Aug 9, 2022

View reviewed changes

openapi.yaml Show resolved Hide resolved

tuckerzp reviewed Aug 9, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

Address comments

849cb0d

nuttercd force-pushed the feature/add-poams-saps-sars branch from 30c9f08 to 849cb0d Compare August 10, 2022 15:33

tuckerzp suggested changes Aug 10, 2022

View reviewed changes

Remove "catalog" from unnecessarry places

5ce57bd

tuckerzp suggested changes Aug 10, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

Remove uneeded catalog from schemas

4442738

tuckerzp suggested changes Aug 11, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

openapi.yaml Show resolved Hide resolved

Remove pluralization of tags

724d891

tuckerzp reviewed Aug 11, 2022

View reviewed changes

openapi.yaml Show resolved Hide resolved

Fix path tags to match the new singualar tags.

db0bfe6

tuckerzp reviewed Aug 11, 2022

View reviewed changes

openapi.yaml Outdated Show resolved Hide resolved

Replace SAP and SAR with AP an AR

a389233

Bronstrom requested changes Aug 16, 2022

View reviewed changes

laurelmay reviewed Aug 16, 2022

View reviewed changes

Rename aps/ars to assessmentPlan/assessmentResults

54b9b6e

laurelmay reviewed Aug 16, 2022

View reviewed changes

Change assessmentPlan to assessmentPlans

4da5b2c

Also removes periods

Bronstrom approved these changes Aug 16, 2022

View reviewed changes

tuckerzp suggested changes Aug 22, 2022

View reviewed changes

Fix pluralization of POA&Ms

5c3ad0d

tuckerzp previously approved these changes Aug 25, 2022

View reviewed changes

Merge branch 'develop' into feature/add-poams-saps-sars

9715e6d

tuckerzp requested a review from laurelmay August 25, 2022 13:55

tuckerzp reviewed Aug 25, 2022

View reviewed changes

easy-dynamics-oscal-automation bot requested a review from a team August 25, 2022 14:51

nuttercd added 2 commits August 25, 2022 10:52

Add periods to multiline descriptions

308445f

Merge branch 'feature/add-poams-saps-sars' of github.com:EasyDynamics…

f7afc4c

…/oscal-rest into feature/add-poams-saps-sars

laurelmay approved these changes Aug 25, 2022

View reviewed changes

laurelmay merged commit 0fae9ad into develop Aug 25, 2022

laurelmay deleted the feature/add-poams-saps-sars branch August 25, 2022 23:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add high-level API actions for POA&Ms, SAPs, and SARs #63

Add high-level API actions for POA&Ms, SAPs, and SARs #63

nuttercd commented Aug 8, 2022

laurelmay left a comment

tuckerzp commented Aug 9, 2022 •

edited

Loading

tuckerzp left a comment

tuckerzp Aug 11, 2022

laurelmay Aug 11, 2022

aj-stein-nist Aug 11, 2022

laurelmay Aug 11, 2022

brian-comply0 Aug 29, 2022

Bronstrom left a comment

Bronstrom Aug 16, 2022

nuttercd Aug 16, 2022

Bronstrom Aug 16, 2022

laurelmay Aug 16, 2022

nuttercd Aug 16, 2022

laurelmay Aug 16, 2022

Bronstrom left a comment

tuckerzp left a comment

tuckerzp Aug 22, 2022 •

edited

Loading

tuckerzp Aug 22, 2022 •

edited

Loading

tuckerzp left a comment

tuckerzp commented Aug 25, 2022 •

edited

Loading

tuckerzp Aug 25, 2022

laurelmay left a comment

		write:assessmentPlan: modify assessment plans in your account
		read:assessmentPlan: read your assessment plans

	summary: Returns all OSCAL plan of action and milestones
	summary: Returns all OSCAL plans of action and milestones

	summary: Adds a new OSCAL plan of action and milestone
	summary: Adds a new OSCAL plan of action and milestones

Add high-level API actions for POA&Ms, SAPs, and SARs #63

Add high-level API actions for POA&Ms, SAPs, and SARs #63

Conversation

nuttercd commented Aug 8, 2022

laurelmay left a comment

Choose a reason for hiding this comment

tuckerzp commented Aug 9, 2022 • edited Loading

tuckerzp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bronstrom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bronstrom left a comment

Choose a reason for hiding this comment

tuckerzp left a comment

Choose a reason for hiding this comment

tuckerzp Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

tuckerzp Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

tuckerzp left a comment

Choose a reason for hiding this comment

tuckerzp commented Aug 25, 2022 • edited Loading

Choose a reason for hiding this comment

laurelmay left a comment

Choose a reason for hiding this comment

tuckerzp commented Aug 9, 2022 •

edited

Loading

tuckerzp Aug 22, 2022 •

edited

Loading

tuckerzp Aug 22, 2022 •

edited

Loading

tuckerzp commented Aug 25, 2022 •

edited

Loading