Move originatedBy and suppliedBy properties back to Element

[rnjudge]

This PR updated the Range of suppliedBy from Element to Artifact. This needs to be reverted to the Range of Element. This issue was discovered and discussed at the Security profile call on July 26th. A summary of the discussion:

• Core profile and Security profile both include a suppliedBy property
• the Security profile uses suppliedBy for Vulnerability Elements, with the original intent to use the Element's suppliedBy property
• Recent creation of the Artifact class in the core model caused this duplication
• Discussed resolving this by having vulnerability elements inherit from artifact elements
  • Realized that this only solves half the problem, it will not resolve the use of suppliedBy in the relationships defined by the security profile (see suppliedBy used here in the Security profile. )
• Recommendation: move the originatedBy and suppliedBy properties back to the Element class
• Justification: they are being used as properties on the many relationships defined in the security profile, and likely to be used as properties on relationships in other profiles as well, e.g., licensing or AI

[goneall]

The logic makes sense to me - I agree with the proposed change.

[davaya]

There are two potential approaches:

1. put every property that any subclass uses in Element
2. put properties in the subclasses that use them

Annotation and Agent/Person/Organization don't have originatedBy and suppliedBy - the concept doesn't apply to things that aren't supplied. Elements describing things that are supplied, like Artifact and its subclasses, do have supply-related properties. Software defines a SoftwareArtifact class with more properties (contentIdentifier, primaryPurpose, etc.) that other Software classes inherit from. It is not a duplication for multiple classes to use the same property.

Security already defines the VulnAssessmentRelationship class that other relationships inherit from. I suggest adding originatedBy and suppliedBy properties to VulnAssmentRelationship to avoid polluting elements where they don't make sense.

[goneall]

related to #437

[goneall]

@davaya makes a good argument for just adding the property to the Relationship subclasses used by security.

Reconsidering my previous opinion.

[sbarnum]

From my read of Rose's initial comments above she likely intended to propose changing the domain (not the range) of the originatedBy and suppliedBy properties back to Element rather than Artifacts?

The domain is the class that the property describes (is on) while the range is the type (class) of the value that property holds.

It reads to me as though Rose is not talking about adding these properties to subclasses of VulnAssessmentRelationship but rather to the Vulnerability class itself.
If this is the only class in addition to Artifact where these properties are asserted as relevant then I would agree with @davaya and @goneall that it would be cleaner for now to just add them directly to Vulnerability.
If it starts to get to be a lot more places then it would likely make more sense to generalize them onto Element for now.

[rnjudge]

From my read of Rose's initial comments above she likely intended to propose changing the domain (not the range) of the originatedBy and suppliedBy properties back to Element rather than Artifacts?

The domain is the class that the property describes (is on) while the range is the type (class) of the value that property holds.

This is correct! Sorry for the confusion.

It reads to me as though Rose is not talking about adding these properties to subclasses of VulnAssessmentRelationship but rather to the Vulnerability class itself.

We (the security team) wants to add these as properties to Elements, not just Vulnerabilities.

[jeff-schutt]

Discussed in the security team meeting yesterday and agreed that:

1. the Vulnerability class can inherit from the Artifact class -> updated the SubclassOf in update Vulnerability.md #473
2. the VulnAssessmentRelationship class can reuse the same property -> updated the definition in Update suppliedBy.md #472

This addresses both uses for the suppliedBy property: on the Vulnerability class and the VulnAssessmentRelationship class or subclasses.

[goneall]

I'm wondering if we also need to update the suppliedBy property to core/suppliedBy in

spdx-3-model/model/Security/Classes/VulnAssessmentRelationship.md

Line 30 in 97af34f

- suppliedBy

[goneall]

I'm wondering if we also need to update the suppliedBy property to core/suppliedBy in

@zvr - can you confirm that the spec parser can handle this?

[goneall]

@jeff-schutt @rnjudge - Can we close this?

[jeff-schutt]

@goneall Yes, looks like #488 is ready to be merged and this issue can be marked as completed.