-
Notifications
You must be signed in to change notification settings - Fork 34
feat(lead verifier): verification API support #376
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
Conversation
242e420 to
c2da15a
Compare
| } | ||
|
|
||
| supportedMediaTypes = append(supportedMediaTypes, supportedCompositeEvidenceMediaTypes...) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 410 should say, evidence mediaType not one of Veraison supported Evidence or Composite Evidence Media Types
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started reviewing it, will complete the review tomorrow!!!
| // attestation result. | ||
| attestationResult, err := o.Verifier.ProcessEvidence(tenantID, session.Nonce, | ||
| evidence, mediaType) | ||
| var attestationResult []byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please correct line 434 to add a bit of description about forwarding the evidence either to a Component Verifier or a Lead Verifier, based on the received evidence media type
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I have made few minor comments for you to have a look!
Also raised a documentation issue in https://github.com/veraison/docs repo due to this change...
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| } | ||
|
|
||
| isSupported, err := o.Verifier.IsSupportedMediaType(mediaType) | ||
| isSupportedMediaType, err := o.Verifier.IsSupportedMediaType(mediaType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please review comment on Line 319 and 320, we need to correct this to reflect the current state!
// Here we only deal with CMW records and tags. Collection CMWs are
// dealt with in the non-exceptional path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The content of the comment is not invalidated by the changes in this PR.
Add transparent support for the "lead verifier" mode where the usual challenge-response API accepts verification requests for composite evidence and dispatches them to the CE handler endpoint in VTS. The existing verification API logic is extended to: 1. Advertise collection types during content negotiation, as well as via the discovery interface. 2. Recognise requests for collection types that do not have an associated scheme plugin, and forward them to the CE handler. Signed-off-by: Thomas Fossati <[email protected]>
c2da15a to
db99ab5
Compare
yogeshbdeshpande
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGMT!
Add transparent support for the "lead verifier" mode where the usual challenge-response API accepts verification requests for composite evidence and dispatches them to the CE handler endpoint in VTS.
The existing verification API logic is extended to:
Fix #368