Skip to content
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

Revocation and Status Assertions flows reviewed #308

Merged

Conversation

SaraConsoliACN
Copy link
Contributor

@SaraConsoliACN SaraConsoliACN commented Jun 6, 2024

Title

Revocation and Status Attestation flows reviewed

Content

Content review based on the new version of the specifications defined in https://github.com/peppelinux/draft-demarco-oauth-status-attestations

resolve #265

"jti": "6f204f7e-e453-4dfd-814e-9d155319408c"
"credential_hash": $CREDENTIAL-HASH,
"credential_hash_alg": "sha-256",
"msg": "Revoked"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msg?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaraConsoliACN please, as discussed during our editor call, provide another parameter name with a good semantic

docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
Comment on lines 644 to 646
* - **msg**
- Message returned from the Credential Issuer after revocation. It MUST be set with the value "OK".
- This specification.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

????

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this msg is not defined in the status assertion draft.

docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
SaraConsoliACN and others added 4 commits June 10, 2024 10:16
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
@peppelinux peppelinux requested a review from fmarino-ipzs June 12, 2024 08:06
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
* - **credential_pop**
- It MUST contain a JWT proof of possession of the cryptographic key the Credential to be revoked shall be bound to. See Section :ref:`Credential Proof of Possession <sec_revocation_credential_pop>` for more details.
- `[OAuth Status Attestation draft 01] <https://datatracker.ietf.org/doc/draft-demarco-status-attestations/01/>`_
* - **revocation_assertion_requests**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this parameter name must be changed

the scope of the request is to request the revocation of credentials or wallet instance attestations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the parameter name is changed with revocation_requests

docs/en/revocation-lists.rst Outdated Show resolved Hide resolved
docs/en/revocation-lists.rst Show resolved Hide resolved
SaraConsoliACN and others added 2 commits June 17, 2024 17:16
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
@peppelinux peppelinux marked this pull request as draft June 17, 2024 15:18
@peppelinux peppelinux requested review from grausof and hevelius June 17, 2024 15:18
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
@peppelinux
Copy link
Member

this PR must cover the wallet instance revocation request use case and the pid provider revocation request use case

"jti": "6f204f7e-e453-4dfd-814e-9d155319408c"
"credential_hash": $CREDENTIAL-HASH,
"credential_hash_alg": "sha-256",
"msg": "Revoked"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SaraConsoliACN please, as discussed during our editor call, provide another parameter name with a good semantic

@peppelinux peppelinux added this to the 0.8.0 milestone Jun 26, 2024
SaraConsoliACN and others added 7 commits July 25, 2024 12:26
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
@m-basili
Copy link
Collaborator

@peppelinux @SaraConsoliACN, I made last commits to align with the current draft of status assertion, as a note credential status validity is still in a PR and is not included in the draft 2 yet.

SaraConsoliACN and others added 4 commits July 30, 2024 11:12
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
RFC 7519 reference retained only in the standards.rst file.
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
@peppelinux peppelinux merged commit 47efdc1 into italia:versione-corrente Jul 31, 2024
4 of 5 checks passed
@SaraConsoliACN SaraConsoliACN deleted the SaraConsoliACN-statusAssertion branch July 31, 2024 10:05
@SaraConsoliACN
Copy link
Contributor Author

fixes #265

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5 participants