OSCAL icon indicating copy to clipboard operation
OSCAL copied to clipboard

App development would be easier if group id's were required rather than optional in Catalog and Profile

Open fsuits opened this issue 2 years ago • 12 comments

User Story:

As an OSCAL application developer, coding logic would be simpler and clearer if group id's in Catalog and Profile groups were required rather than optional.

Goals:

Groups are present in Catalogs and in Profiles, and in each case the schema specifies a required title and an optional group_id. Since groups may be nested, and since the id is intended to allow direct reference to the specific group in a document, it makes sense for the id to be required rather than optional. This adds a small burden to the document creator - but would simplify application developers since they would not need to do special handling when the group id is not present - e.g. create an internal id on the fly during processing.

Dependencies:

No dependencies other than a change in the schema. There are presumably few use cases where the group id's are not already specified in a catalog or profile.

Acceptance Criteria

  • [ ] The schemas for Catalog and Profile are updated to require group id's in addition to group title.
  • [ ] All OSCAL website and readme documentation affected by the changes in this issue have been updated. Changes to the OSCAL website can be made in the docs/content directory of your branch.
  • [ ] A Pull Request (PR) is submitted that fully addresses the goals of this User Story. This issue is referenced in the PR.
  • [ ] The CI-CD build process runs without any reported errors on the PR. This can be confirmed by reviewing that all checks have passed in the PR.

fsuits avatar Jun 07 '22 04:06 fsuits

As a consumer, you'll probably need to handle older data, which may not have the group id. So I'm not sure its really going to save effort. If it had been required from 1.0, then I can see the advantage, but it seems limited value now.

bradh avatar Jun 07 '22 05:06 bradh

@fsuits should these IDs be required and unique or simply required?

hahsan-ti avatar Jun 07 '22 12:06 hahsan-ti

@hahsanti The group ids are currently optional, but must be unique when provided.

One driver to allow groups without ids is to allow the catalog author to decide which groups are intended to be addressed by an id and which groups to not allow addressing in this way.

@fsuits This would be a backwards compatibility breaking change, so we could only consider this for an OSCAL 2.0.0 release.

david-waltermire avatar Jun 07 '22 13:06 david-waltermire

One could imagine the evolution of a REST API to include things like adding a new control to a group, i.e.:

POST /catalogs/{catalogId}/groups/{groupId}/controls

and that existing group could not be referenced without a group ID.

rgauss avatar Jun 10 '22 14:06 rgauss

One could imagine the evolution of a REST API to include things like adding a new control to a group, i.e.:

POST /catalogs/{catalogId}/groups/{groupId}/controls

and that existing group could not be referenced without a group ID.

What would it mean for the API to support the following, given precious little details and just what-if-ing on the fly?

POST /catalogs/{catalogId}/controls: all controls returned, each control returned with a groupId being defined as a string or null. Whether or not the control is grouped (with or without a group with a given groupId, return isGrouped as true or false, so it would true if grouped but the group does not have an ID).

POST /catalogs/{catalogId}/groups: return all controls all IDs in array elements group per group in groups and assign "id": null for the property of a given group if no id is provided.

{ 
  "groups": 
    [
       {"id": "group1.a", "control-ids": ["g1a-1", "g1a-2"] },
       {"id": null, "control-ids": ["gnoid-3", "gnoid-4"] }
    ] 
} 

POST /catalogs/{catalogId}/groups/{groupId}/controls: probably what you'd expect, full control data properly grouped.

It seems that your proposed endpoints around {groups}/{groupId} are not implemented at all yet and the OSCAL catalog model could kind of supports my middle of the road approach. Is there a reason not to have a continuum with the current option group ID?

aj-stein-nist avatar Jun 10 '22 19:06 aj-stein-nist

At a high level I think we want the REST API GET endpoints (which I think is what you mean above) to directly align with the OSCAL data models as much as possible, almost like a JSON path expression. With that in mind...

GET /catalogs/{catalogId}/controls: I would expect to return an array of only the 'top-level' controls in that catalog, i.e. with no group.

GET /catalogs/{catalogId}/groups: I would expect to return an array of the 'top-level' groups including the control objects (or subgroups) and other data like title, params, props, within each group.

GET /catalogs/{catalogId}/groups/{groupId}/controls: I would expect to return an array of just the control objects under that specific group.

When we look at the scenario of adding a new control to an existing group, we need to reference that group by a unique identifier. With an optional group ID there could be two groups with "id": null as you describe and there's no way to indicate which group to add it to.

It seems that your proposed endpoints around {groups}/{groupId} are not implemented at all yet and the OSCAL catalog model could kind of supports my middle of the road approach. Is there a reason not to have a continuum with the current option group ID?

Correct, not in the specification yet. We wanted to validate the approach with the community before going 'too deep' on the spec.

To be clear, I was just using REST APIs and this scenario as an example of where we might need to reference a group, which is a clear indicator that it needs an ID.

rgauss avatar Jun 10 '22 20:06 rgauss

At a high level I think we want the REST API GET endpoints (which I think is what you mean above) to directly align with the OSCAL data models as much as possible, almost like a JSON path expression. With that in mind...

Makes sense. I (personally, not speaking for the team) can agree with that.

Correct, not in the specification yet. We wanted to validate the approach with the community before going 'too deep' on the spec.

Understandable and totally agree with the approach.

To be clear, I was just using REST APIs and this scenario as an example of where we might need to reference a group, which is a clear indicator that it needs an ID.

Yes, also completely reasonable and understandable. I think you did reframe the wording around this scenario.

GET /catalogs/{catalogId}/groups: I would expect to return an array of the 'top-level' groups including the control objects (or subgroups) and other data like title, params, props, within each group.

You described this better than I did. I know this is kludgey, but until 2.0 time frame, what would stop the API specification and developers from allowing {groupId} to be either a relevant UUID in the respective group object or perhaps the 0-based index of a group without a group ID (so if there are three groups and the second had no idea, you could do /catalogs/{catalogId}/groups/1/controls because you previously examined /catalogs/{catalogId}/groups and know that group has no explicit ID? OSCAL v2.0.0 is a long-time away (perhaps more than 3-6 months; I think Dave will have a better estimate). For that reason I ask if this is not a reasonable approach. Also worth examining (and I will follow up in the EasyDynamics/oscal-rest repo if it does come up, not here in this issue); what about other optional @id fields that impact API surface. Is this, even in catalog the only glaring example? If so, I am pleasantly surprised. :-)

aj-stein-nist avatar Jun 10 '22 20:06 aj-stein-nist

You described this better than I did. I know this is kludgey, but until 2.0 time frame, what would stop the API specification and developers from allowing {groupId} to be either a relevant UUID in the respective group object or perhaps the 0-based index of a group without a group ID (so if there are three groups and the second had no idea, you could do /catalogs/{catalogId}/groups/1/controls because you previously examined /catalogs/{catalogId}/groups and know that group has no explicit ID?

The issue there is that we can't rely on order of JSON array elements in many implementations. It's entirely possible that the response of /catalogs/{catalogId}/groups/1/controls would differ from one request to the next.

I think the best short-term solution would be for the REST API to essentially layer additional constraints on the OSCAL models and state that group IDs are required/will be generated. We're doing something similar already by declaring that UUIDs must be scoped to the entire implementing system rather than just the OSCAL document.

what about other optional @id fields that impact API surface. Is this, even in catalog the only glaring example? If so, I am pleasantly surprised. :-)

I haven't looked in detail, but yes, I would assume there are other instances of this problem.

rgauss avatar Jun 13 '22 13:06 rgauss

You described this better than I did. I know this is kludgey, but until 2.0 time frame, what would stop the API specification and developers from allowing {groupId} to be either a relevant UUID in the respective group object or perhaps the 0-based index of a group without a group ID (so if there are three groups and the second had no idea, you could do /catalogs/{catalogId}/groups/1/controls because you previously examined /catalogs/{catalogId}/groups and know that group has no explicit ID?

The issue there is that we can't rely on order of JSON array elements in many implementations. It's entirely possible that the response of /catalogs/{catalogId}/groups/1/controls would differ from one request to the next.

Just to be clear, I called it a kludge not because I thought it was an elegant and fully complete design. :-) I meant it as more of a workaround if the OSCAL model will not change short term around a required group ID.

Is the intent that your catalogs could be dynamically generated? If we look at them is static or semi-permanent database of control items, I had not really thought about your point about your point re serialization and deserialization in different runtimes and their implementations re arrays, so point well made!

aj-stein-nist avatar Jun 13 '22 14:06 aj-stein-nist

With regard to the question of other optional id's, I believe the only optional id's are for groups, either in catalogs or profiles, and for parts. Parts are very granular and usually in a well defined context - so I can understand it would be somewhat overkill to require an id for them, while at the same time it might be convenient to allow one.

In contrast, groups are relatively coarse-grained and each control has a well defined path in a catalog that allows direct access. It will either be in the catalog's list itself, with no group at all, or follow a path of catalog->group->group->group->control->control->control - in the most general case. Without group id's this path becomes hard to specify - unless you assign group id's on the fly.

I created this issue from the perspective of a developer, since it is cleaner to rely on certain attributes to be present and not deal with situations of available/not_available and corresponding fallback logic. I'm not clear on the use case of wanting to prevent direct access to a group - and since groups have titles (that may not be unique) you can still use the title somewhat as a group id - at least for narrowing the search of where a control lives.

My preference as a developer is to have group id's be unique and required - while titles are completely flexible. This would help any part of code, or a REST interface, where the direct path to access the group or a control is needed without a search.

A specific need in trestle is the ability to map a catalog's group structure to a filesystem directory, where subgroups are simply named subdirectories based on the group id. Without a required id, temporary ones would be needed.

fsuits avatar Jun 16 '22 03:06 fsuits

We discussed this on the 6/24 model review and agreed that group identifiers need to remain optional to avoid a backwards compatibility breaking change. This can be revisited in OSCAL 2.0.0.

david-waltermire avatar Jul 29 '22 21:07 david-waltermire

At the 11/30 Triage Meeting: The team decided to pursue this ticket in the next version of OSCAL.

Arminta-Jenkins-NIST avatar Nov 30 '23 19:11 Arminta-Jenkins-NIST