peppelinux / draft-demarco-oauth-status-assertions

OAuth 2.0 Status Assertions for Digital Credentials
Other
4 stars 4 forks source link

breaking change: credential_pop is an array #36

Closed SaraConsoliACN closed 1 month ago

SaraConsoliACN commented 2 months ago

This PR:

peppelinux commented 1 month ago

It is not yet clear in the text what happens if a Status Assertion Request fails due to a single or subset of clredentials. For example in the text we have the following sentence:

this would tend to introduce a json object in the response with an array of statuses

marinaado commented 1 month ago

It is not yet clear in the text what happens if a Status Assertion Request fails due to a single or subset of clredentials. For example in the text we have the following sentence:

In cases where a Status Assertion request is made for a Digital Credential that does not exist, has expired, been revoked, or is in any way invalid, or if the HTTP Request is compromised by missing or incorrect parameters, the Credential Issuer is required to respond with an HTTP Response. This response should have a status code of 400 and use application/json as the content type, including the following parameters:

Does this mean that if the request includes Credential PoP A and Credential PoP B and the Credential B is invalid, the Credential Issuer must give an error? Probably the response should contain only the status assertion for Credential A without giving an error response?

We have two error codes in the spec - one for invalid request and the second for invalid credential. How would you differentiate between the two errors ? Can it happen that there is error in request for Credential PoP B while the request for Credential PoP A is correct? If this cannot happen then your proposal is ok.

peppelinux commented 1 month ago

It is not yet clear in the text what happens if a Status Assertion Request fails due to a single or subset of clredentials. For example in the text we have the following sentence:

this would tend to introduce a json object in the response with an array of statuses

or, differently

regarding the response of the status assertions, instead of using a complex json object WDYT about returning an array of JWT/CWT where each JWT/CWT matches the one provided within the request and in the same order

where

the header parameter typ would say if it is a statu sassertion or a status assertion error?

WDYT @fmarino-ipzs @OR13 ? using the power of the JWT/CWT objects we would avoid the json object envelope with unsigned (meta)data

peppelinux commented 1 month ago

It is not yet clear in the text what happens if a Status Assertion Request fails due to a single or subset of clredentials. For example in the text we have the following sentence:

In cases where a Status Assertion request is made for a Digital Credential that does not exist, has expired, been revoked, or is in any way invalid, or if the HTTP Request is compromised by missing or incorrect parameters, the Credential Issuer is required to respond with an HTTP Response. This response should have a status code of 400 and use application/json as the content type, including the following parameters:

Does this mean that if the request includes Credential PoP A and Credential PoP B and the Credential B is invalid, the Credential Issuer must give an error? Probably the response should contain only the status assertion for Credential A without giving an error response?

We have two error codes in the spec - one for invalid request and the second for invalid credential. How would you differentiate between the two errors ? Can it happen that there is error in request for Credential PoP B while the request for Credential PoP A is correct? If this cannot happen then your proposal is ok.

@marinaado WDYT about this conversation? https://github.com/peppelinux/draft-demarco-oauth-status-attestations/pull/36#discussion_r1598713649

OR13 commented 1 month ago

I think the following is a good solution for now"

HTTP POST /status 
content-type: application/json
{
  "status_assertion_requests" : ["${base64url(json({typ: (some pop for status-assertion)+jwt, ...}))}.payload.signature", ... ]
}
... Response :
{
  "status_assertions" : ["${base64url(json({typ: status-assertion+jwt, ...}))}.payload.signature", ... ]
}
peppelinux commented 1 month ago

Short recap

in the request:

in the response:

peppelinux commented 1 month ago

I think the following is a good solution for now"

HTTP POST /status 
content-type: application/json
{
  "status_assertion_requests" : ["${base64url(json({typ: (some pop for status-assertion)+jwt, ...}))}.payload.signature", ... ]
}
... Response :
{
  "status_assertions" : ["${base64url(json({typ: status-assertion+jwt, ...}))}.payload.signature", ... ]
}

@SaraConsoliACN the non normative examples must be updated with the example provided here by Orie