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

add RFC021 vp_token-brearer grant type for oauth2 #265

Merged
merged 28 commits into from
Nov 29, 2023
Merged
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3055386
add RFC021 vp_token-brearer grant type for oauth2
woutslakhorst Sep 12, 2023
ac52094
update vp_formats format
woutslakhorst Sep 12, 2023
ae71774
fixed technical comments from PR
woutslakhorst Sep 14, 2023
ac552e2
some fixes
woutslakhorst Sep 15, 2023
41918d8
fix PE version
woutslakhorst Sep 15, 2023
d6dbbf8
stuff
woutslakhorst Sep 15, 2023
4995470
add scope param to example
woutslakhorst Sep 20, 2023
fdb9f2c
PR feedback
woutslakhorst Sep 20, 2023
a4d5481
PR feedback
woutslakhorst Sep 27, 2023
0d04aa2
PR feedback
woutslakhorst Oct 2, 2023
cc3d2e4
changed use of jti/challenge over nonce and more strict scope usage
woutslakhorst Oct 3, 2023
9c39be2
added error codes
woutslakhorst Oct 9, 2023
60d54cc
PR feedback
woutslakhorst Oct 12, 2023
4716cea
added empty scope
woutslakhorst Oct 12, 2023
d5ed03d
some small privacy related things
woutslakhorst Oct 17, 2023
e70e169
remove parsing detection
woutslakhorst Oct 17, 2023
6e8d31b
Removed scope chapter
woutslakhorst Oct 24, 2023
04021d5
small things
woutslakhorst Oct 26, 2023
3797e92
added client_id to introspection
woutslakhorst Oct 27, 2023
b82d74f
PR feedback
woutslakhorst Nov 3, 2023
3186b22
PR feedback
woutslakhorst Nov 8, 2023
39ee82d
added presentation_submission to introspectin
woutslakhorst Nov 14, 2023
c0b8639
fields as fields
woutslakhorst Nov 16, 2023
8af6a96
PR feedback
woutslakhorst Nov 20, 2023
38a97c8
redefine assertion param
woutslakhorst Nov 23, 2023
67c6ae1
PR feedback
woutslakhorst Nov 27, 2023
062a5e7
jti to nonce
woutslakhorst Nov 28, 2023
221c386
Update rfc/rfc021-vp_token-grant-type.md
woutslakhorst Nov 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions rfc/rfc021-vp_token-grant-type.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ The assertion parameter can either be encoded in JSON or JWT.
The Authorization Server determines the encoding by adding the correct parameters to the Authorization Server metadata [RFC8414]].
The following parameter MUST be added:

* `vp_formats`: An object defining the formats and proof types of Verifiable Presentations and Verifiable Credentials that a Verifier supports as stated by §9 of the OpenID for Verifiable Presentations specification [OIDC4VP].
* `vp_formats`: An object defining the formats and proof types of Verifiable Presentations and Verifiable Credentials that a Verifier supports as stated by §9 of the OpenID for Verifiable Presentations specification [OpenID4VP].

## 4. JWT Format and Processing Requirements

Expand All @@ -117,22 +117,22 @@ The second paragraph describes the requirements that apply to JWT encoded VP's.
4. The `kid` header MUST be a DID URL and MUST resolve to a verificationMethod in the DID Document. The DID part of the DID URL MUST match the `iss` field.
Copy link
Member

Choose a reason for hiding this comment

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

I saw some DID URLs (I think verificationMethod IDs) that just contained a fragment. We might have to support that in (near) future

5. The `sub` field MUST match the `credentialSubject.id` field from all the Verifiable Credentials that are used to request the access token.
6. The `aud` field MUST match the DID of the Authorization Server.
7. The `iat` field MUST be present and contain a valid timestamp. It MUST be before the current time.
8. The `exp` field MUST be present and contain a valid timestamp. It MUST be after the current time.
7. The `iat` field MUST be present and contain a valid unix timestamp. It MUST be before the current time.
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved
8. The `exp` field MUST be present and contain a valid unix timestamp. It MUST be after the current time.
9. The difference between the `exp` and `iat` fields MUST be equal or less than 5 seconds.
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved
10. The `jti` field MUST be present and contain a string that is unique for each access token request.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
10. The `jti` field MUST be present and contain a string that is unique for each access token request.
10. The `jti` field MUST be present and contain a string that is unique for each access token request for the issuer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Better to make it totally unique, that way it's still unique for the issuer.

Copy link
Member

Choose a reason for hiding this comment

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

the vc-data-model maps jti to the VC/VP id value. A value should not have 2 meanings, so we should use nonce here too. We're then using nonce for replay prevention in both data formats which is a nice bonus I'd say.

Copy link
Member

Choose a reason for hiding this comment

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

The security considerations section needs some rewording if this is added


### 4.3 JSON format requirements
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved

1. The assertion MUST be a valid JSON object according to §4.10 the Verifiable Credentials Data Model 1.1 [VC-DATA-MODEL].
2. The proof of the VP MUST be a valid [JSONWebSignature2020] object.
3. The `challenge` field of the JSON object MUST be a string that is unique for each token request.
4. The `domain` field of the JSON object MUST be a DID under control of the Authorization Server.
5. The `verificationMethod` field of the Proof MUST be a DID URL.
6. The `verificationMethod` field of the proof MUST match the `credentialSubject.id` field from all the Verifiable Credentials that are used to request the access token.
7. The `created` field of the proof MUST be present and contain a valid timestamp. It MUST be before the current time.
8. The `expires` field of the proof MUST be present and contain a valid timestamp. It MUST be after the current time.
9. The difference between the `expires` and `created` fields MUST be equal or less than 5 seconds.
2. The proof of the VP MUST be a valid [JSONWebSignature2020] object. Additional requirements for the proof object:
Copy link
Member

Choose a reason for hiding this comment

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

do we need to support JSON-LD?

1. The `challenge` field MUST be a string that is unique for each token request.
2. The `domain` field MUST be a DID under control of the Authorization Server.
Copy link
Member

Choose a reason for hiding this comment

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

How should this be validated? If not validated, why bother?

Copy link
Member Author

Choose a reason for hiding this comment

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

You'll use it as tenant selection

woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved
3. The `verificationMethod` field MUST be a DID URL.
4. The `verificationMethod` field MUST match the `credentialSubject.id` field from all the Verifiable Credentials that are used to request the access token.
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved
5. The `created` field MUST be present and contain a valid ISO8601 formatted date string. It MUST be before the current time.
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved
6. The `expires` field MUST be present and contain a valid ISO8601 formatted date string. It MUST be after the current time.
7. The difference between the `expires` and `created` fields MUST be equal or less than 5 seconds.

### 4.4 Preventing Token Replay

Expand All @@ -148,7 +148,8 @@ In addition to the error response defined in OAuth 2.0 [RFC6749], the Authorizat

* `invalid_verifiable_presentation`: The VP is invalid. This error code is used when the signature is incorrect or when a required field is missing.
Copy link
Member

Choose a reason for hiding this comment

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

should it be "invalid assertion"? (as that reflects the parameter name)

Copy link
Member Author

Choose a reason for hiding this comment

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

removed all custom errors since it creates too much confusion amongst reviewers.

* `invalid_presentation_submission`: The Presentation Submission is invalid. This error code is used when the Presentation Submission is not an answer to the Presentation Definition that corresponds with the requested scope.
* `invalid_verifiable_credentials`: The submitted Verifiable Credentials do not meet the requirements. This error code is used when the Verifiable Credentials aren't corresponding to the Presentation Definition or when the Verifiable Credentials are expired, not trusted or invalid.

An `invalid_request` is returned for any submitted Verifiable Credentials that do not meet the requirements, when the Verifiable Credentials aren't corresponding to the Presentation Definition or when the Verifiable Credentials are expired, not trusted or invalid.
reinkrul marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Kinda ambiguous with "invalid_verifiable_presentation"; if the contained credentials are invalid, it could also be seen as "invalid_verifiable_presentation"?

Also, I think there (ideally) should not be any other "trust" anchors than the one specified by the Presentation Definition. Otherwise, RP and AS still have to agree on a set of trust anchors outside of this specification.

Copy link
Member Author

Choose a reason for hiding this comment

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

We currently decided on putting trust anchors in the PDs, that does not mean that this is mandatory through this RFC. 'Trust' is therefore still required, but undefined how it's solved.

It is also used when the Verifiable Credentials are not issued to the signer of the Verifiable Presentation.

## 5. Presentation Definition endpoint
Expand All @@ -171,13 +172,14 @@ The following example shows a request to the Presentation Definition endpoint:
The Authorization Server MAY support an access token introspection endpoint as defined in OAuth 2.0 [RFC7662].
The introspection endpoint SHOULD map the fields as follows:

* `active`: If the access token is valid.
* `iss`: The issuer of the access token (DID).
* `sub`: The subject of the access token (DID), this is the resource owner. Usually the same as the `iss` field.
Copy link
Member

Choose a reason for hiding this comment

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

this section is not really add anything useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

Entire chapter 6 or specific fields?

Copy link
Member

Choose a reason for hiding this comment

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

May be useful after all, since we don't have a separate introspection RFC, and it adds some claims specific for this RFC (regarding the input descriptors)

* `client_id`: The client (DID) that requested the access token.
* `exp`: The expiration time of the access token.
* `iat`: The time the access token was issued.
* `scope`: The granted scope.
* `vcs`: The Verifiable Credentials that were used to request the access token using the same encoding as used in the access token request.
* `vps`: The Verifiable Presentations that were used to request the access token using the same encoding as used in the access token request.
Copy link
Member

Choose a reason for hiding this comment

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

I think we discussed adding fields specified by input descriptors?

Copy link
Member Author

Choose a reason for hiding this comment

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

both

Copy link
Member

@reinkrul reinkrul Nov 10, 2023

Choose a reason for hiding this comment

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

I'd prefer only to include credentials in the response, that were input for the access token, meaning only those matched by the presentation definition. Why do you think we need the whole VP?

Adding fields later on is a lot easier than removing them (which is breaking)

Copy link
Member Author

Choose a reason for hiding this comment

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

You shouldn't need any of them if all is done correctly. If we're including any data that was used as input then best to include the raw data which are the vps. The vps will also tell you which wallet signed which vcs. Only including vcs won't give you that info. But we can remove it from the spec and just add them to the implementation?

Copy link
Member

Choose a reason for hiding this comment

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

If the vps are always the vp_token (which is true here), then I think we also need to return the presentation_submission to interpret the contents of the vps.

Copy link
Member

Choose a reason for hiding this comment

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

That's also fine. Might want to choose a style short (acronyms), or long form, though


## 7. Security Considerations
woutslakhorst marked this conversation as resolved.
Show resolved Hide resolved

Expand Down Expand Up @@ -206,7 +208,7 @@ Privacy-sensitive data in the scope parameter should be avoided.
* [VC-DATA-MODEL] M. Sporny, D. Longley, D. Chadwick, "Verifiable Credentials Data Model 1.1", W3C Recommendation, 3 March 2022, <https://www.w3.org/TR/vc-data-model/>.
* [PE] D. Buchner, B. Zundel, M. Riedel, K.H. Duffy, "Presentation Exchange 2.0.0", 12 September 2023, <https://identity.foundation/presentation-exchange/spec/v2.0.0/>.
* [JSONWebSignature2020] O. Steel, M. Prorock, C.E. Lehner, "JSON Web Signature 2020", W3C Community Group Report, 21 July 2022, <https://w3c-ccg.github.io/lds-jws2020/>.
* [OIDC4VP] O. Terbu, T. Lodderstedt, K. Yasuda, T. Looker, "OpenID for Verifiable Presentations Draft 18", OpenID connect working group, 21 April 2023, <https://openid.net/specs/openid-4-verifiable-presentations-1_0.html>.
* [OpenID4VP] O. Terbu, T. Lodderstedt, K. Yasuda, T. Looker, "OpenID for Verifiable Presentations Draft 18", OpenID connect working group, 21 April 2023, <https://openid.net/specs/openid-4-verifiable-presentations-1_0.html>.
* [RFC8414] M. Jones, N. Sakimura, J. Bradley, "OAuth 2.0 Authorization Server Metadata", RFC 8414, June 2018, <https://www.rfc-editor.org/info/rfc8414>.
* [RFC7662] J. Richer, "OAuth 2.0 Token Introspection", RFC 7662, October 2015, <https://www.rfc-editor.org/info/rfc7662>.
* [DID] M. Sporny, D. Longley, M. Sabadello, D. Reed, O. Steele, C. Allen, "Decentralized Identifiers (DIDs) v1.0", W3C Recommendation, 19 July 2022, <https://www.w3.org/TR/did-core/>.