Status Attestation Responses #221
Conversation
|
Please note I tried to be more clear on the use of error codes:
I wanted to enforce the concept that the error code MUST NOT revert the meaning of the http response (which is a failure). |
| * - *400 Bad Request* | ||
| - *invalid_request* | ||
| - The request is not valid due to the lack or incorrectness of one or more parameters. (:rfc:`6749#section-5.2`). | ||
| - Error code and description |
There was a problem hiding this comment.
is this intended to be here?
if yes, why it is not in the similar errors below?
There was a problem hiding this comment.
I don't understand this comment.
There was a problem hiding this comment.
Ok, maybe I understood what you meant.
The response body is required only for the cases in which it effectively adds information for the user. That is, there's no value in sending "server error" as a payload to a 500 Server Error response.
Also, 5xx errors are not always handled by the application but rather by gateways and proxies, so it may be impractical to customize them. As it brings no value, I'd rather not consider the body.
There was a problem hiding this comment.
I agree with @balanza , the only status code that makes sense to include a body in is the 400 Bad Request status code. Having said that, probably we could remove the body column saying that only the 400 (Bad Request) status Code MUST include a body encoded in application/json format and MUST contain the following parameters: [...].
@peppelinux WDYT?
There was a problem hiding this comment.
no, since we have server_error in RFC 6749
https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
There was a problem hiding this comment.
Yes but here probably we can omit it as in the RFC6749 I read:
(This error code is needed because a 500 Internal Server Error HTTP status code cannot be returned to the client via an HTTP redirect.)
Am I missing something?
Anyway, if we don't reach a consensus now, I suggest to put this PR to the next release if you agree.
There was a problem hiding this comment.
I can add the error payload if we feel it's worth it, it doesn't affect the way the communication works.
Let me just point out that:
- RFC6749 does not design the endpoint to use multiple http codes as error, that's why it needs to differentiate by payload
- In RFC6749, the http response's content type is
application/x-www-form-urlencoded, we are usingapplication/json - 5xx will usually be generated by systems other than the resource server (ex: load balancers, api gateways, firewalls, etc), so it's probable we will have payload not in the expected format.
| - One or more attributes contained in the Digital Credential are changed. The *error_description* field MUST contain a list of updated attributes. | ||
|
|
||
| Below a non-normative example of an HTTP Response with an error. |
There was a problem hiding this comment.
There was a problem hiding this comment.
They are the same snippets but they apply to different endpoints.
I think it's ok to have examples when you expect them, even if they are duplicated
There was a problem hiding this comment.
I can approve and merge as it is
in the future we'll try to make some editorials to make the text shorter and use more references
| - *server_error* | ||
| - |
There was a problem hiding this comment.
this is RFC6749
https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2.1
| @@ -359,48 +381,63 @@ The *Credential status endpoint* MUST return a response with a *HTTP status code | |||
| - It MUST contain the Status Attestation as a signed JWT. | |||
| - `[OAuth Status Attestation draft 01] <https://datatracker.ietf.org/doc/draft-demarco-status-attestations/01/>`_. | |||
|
|
|||
| If the Digital Credential could not be found by the Issuer, an HTTP Response with status code 404 (Not Found) MUST be returned. In all other cases the Issuer MUST return an HTTP Response Error using *application/json* as the content type, and including the following parameters: | |||
There was a problem hiding this comment.
It is simply moved below, @balanza is it right?
There was a problem hiding this comment.
Details on every possible response have been moved in the response table: https://github.com/italia/eudi-wallet-it-docs/pull/221/files/fdd6d6fa93100f45a71fbab1dc082a5f85c176fa#diff-d3bfad4aa613161f55b96d232893081a0ac30d5127aca04dda54925b43aceb1eR401
Rationale: it gives the same information in a structured way, using fewer words. Implementers may like it.
Refactor the descriptions of the HTTP responses for easier reads.
Key changes:
Solves #219