-
Notifications
You must be signed in to change notification settings - Fork 37
Move issuance pending from Deferred Credential Error Response to Deferred Credential Response and move interval parameter to Credential Response #492
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
…rred Credential Response, move interval to Credential Response
Co-authored-by: Timo Glastra <timo@animo.id>
Co-authored-by: Stefan Charsley <charsleysa@gmail.com>
awoie
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.
generally fine with the direction but we should consider interval being mandatory.
Co-authored-by: Tobias Looker <tobias.looker@mattr.global>
Co-authored-by: Joseph Heenan <joseph@authlete.com>
| Deferred Credential Response may either contain the requested Credentials or further defer the issuance: | ||
|
|
||
| * If the Credential Issuer is able to issue the requested Credentials, the Deferred Credential Response MUST use the `credentials` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 200 (see Section 15.3.3 of [@!RFC9110]). | ||
| * If the Credential Issuer still requires more time, the Deferred Credential Response MAY use the `interval` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 202 (see Section 15.3.3 of [@!RFC9110]). |
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.
| * If the Credential Issuer still requires more time, the Deferred Credential Response MAY use the `interval` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 202 (see Section 15.3.3 of [@!RFC9110]). | |
| * If the Credential Issuer still requires more time, the response MUST use the HTTP status code 202 (see Section 15.3.3 of [@!RFC9110]). |
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.
I think we're significantly under defining the "still requires more time" response now, it must include interval right?
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.
yeah, I caught the same in the meantime :) However my suggestion was to require interval in the new response. WDYT?
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.
@paulbastian I'm good with that.
Co-authored-by: Tobias Looker <tobias.looker@mattr.global>
| * If the Credential Issuer is able to issue the requested Credentials, the Deferred Credential Response MUST use the `credentials` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 200 (see Section 15.3.3 of [@!RFC9110]). | ||
| * If the Credential Issuer still requires more time, the Deferred Credential Response MAY use the `interval` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 202 (see Section 15.3.3 of [@!RFC9110]). | ||
|
|
||
| The Deferred Credential Response MAY use the `notification_id` parameter as defined in (#credential-response). |
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.
I think this should be in the "is able to issue" paragraph?
| Deferred Credential Response may either contain the requested Credentials or further defer the issuance: | ||
|
|
||
| * If the Credential Issuer is able to issue the requested Credentials, the Deferred Credential Response MUST use the `credentials` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 200 (see Section 15.3.3 of [@!RFC9110]). | ||
| * If the Credential Issuer still requires more time, the Deferred Credential Response MAY use the `interval` parameter as defined in (#credential-response) and MUST respond with the HTTP status code 202 (see Section 15.3.3 of [@!RFC9110]). |
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.
I think we're significantly under defining the "still requires more time" response now, it must include interval right?
| ## Deferred Credential Response {#deferred-credential-response} | ||
|
|
||
| The Deferred Credential Response uses the `credentials` and `notification_id` parameters as defined in (#credential-response). | ||
| A Deferred Credential Response may either contain the requested Credentials or further defer the issuance: |
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 response can also indicate that there are no credentials for the holder (for a variety of reasons) and that the wallet should not poll any longer. I think it's worth adding normative description of what this response looks like. One of the following probably:
- 204
- 200 with an empty response
- 200 & and empty
credentialsarray - 400 & error
credential_request_deniedor a new error code such asno_deferred_credential
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.
I think this is out of scope for this PR. I'll convert it into a new issue instead.
|
@paulbastian This PR brings even closer alignment between the credential and deferred endpoints. I think WG should reconsider merging them.
|
Yes. I'm pushing in the same direction. Consider it a preparation step. |
+1 to this. Having two versions of the same thing lead to exactly this sort of skew. It'll be valuable for future implementors to also know that the message for the request/response are necessarily, simply with slightly different restriction. |
Closes #1
Closes #226
intervalparameter from Deferred Credential Response Error to the Credential Response