-
Notifications
You must be signed in to change notification settings - Fork 0
feat: directed identity #26
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
Open
BlairCurrey
wants to merge
5
commits into
main
Choose a base branch
from
blair/ope-183
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
c04c167
feat: directed identity
BlairCurrey d4d6a02
Update openapi/auth-server.yaml
BlairCurrey 02ab4cf
Update openapi/auth-server.yaml
BlairCurrey dffb4c0
Update openapi/auth-server.yaml
BlairCurrey 6f57946
refactor: friendlier mutual exclusivity
BlairCurrey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 1.2.0 | ||
| 1.3.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| openapi: 3.1.0 | ||
| info: | ||
| title: Open Payments Authorization Server | ||
| version: '1.2.0' | ||
| version: '1.3.0' | ||
| license: | ||
| name: Apache 2.0 | ||
| identifier: Apache-2.0 | ||
|
|
@@ -164,6 +164,23 @@ paths: | |
| - read | ||
| identifier: 'http://ilp.interledger-test.dev/bob' | ||
| client: 'https://webmonize.com/.well-known/pay' | ||
| Grant request with directed identity (JWK): | ||
| value: | ||
| access_token: | ||
| access: | ||
| - type: incoming-payment | ||
| actions: | ||
| - create | ||
| - read | ||
| identifier: 'http://ilp.interledger-test.dev/bob' | ||
| client: | ||
| jwk: | ||
| kid: example-key-1 | ||
| alg: EdDSA | ||
| use: sig | ||
| kty: OKP | ||
| crv: Ed25519 | ||
| x: 11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo | ||
| Grant request for subject information: | ||
| value: | ||
| subject: | ||
|
|
@@ -565,15 +582,41 @@ components: | |
| additionalProperties: false | ||
| client: | ||
| title: client | ||
| type: string | ||
| description: |- | ||
| Wallet address of the client instance that is making this request. | ||
| Client identification for grant requests. | ||
|
|
||
| When sending a non-continuation request to the AS, the client instance MUST identify itself by including the client field of the request and by signing the request. | ||
|
|
||
| Can be either: | ||
| - A wallet address string (backwards compatible format) | ||
| - An object with either `jwk` (for directed identity) or `walletAddress` (mutually exclusive) | ||
|
|
||
| When using a wallet address string or the `walletAddress` property: | ||
| A JSON Web Key Set document, including the public key that the client instance will use to protect this request and any continuation requests at the AS and any user-facing information about the client instance used in interactions, MUST be available at the wallet address + `/jwks.json` url. | ||
|
|
||
| When using the `jwk` property (directed identity approach): | ||
| The client instance provides its public key directly in the request, eliminating the need for the AS to fetch it from a wallet address. This approach enhances privacy by not requiring the client to expose a persistent wallet address identifier. The `jwk` property can only be used for non-interactive grant requests (i.e.: incoming payments). | ||
|
|
||
| If sending a grant initiation request that requires RO interaction, the wallet address MUST serve necessary client display information. | ||
| oneOf: | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Go is cleaner with this new way as well. New way: type Client0 = string // for the deprecated string format
type Client1 struct {
WalletAddress string `json:"walletAddress"`
}
type Client2 struct {
Jwk JsonWebKey `json:"jwk"`
}Old way type Client0 = string // for the deprecated string format
type Client1 struct {
Jwk *JsonWebKey `json:"jwk,omitempty"`
WalletAddress *string `json:"walletAddress,omitempty"`
union json.RawMessage
}
type Client10 = interface{}
type Client11 = interface{} |
||
| - type: string | ||
| format: uri | ||
| description: 'DEPRECATED: This string format of the client wallet address is maintained only for backwards compatibility. Migrate to the object form with `jwk` or `walletAddress`.' | ||
| deprecated: true | ||
| - type: object | ||
| required: [walletAddress] | ||
| properties: | ||
| walletAddress: | ||
| type: string | ||
| format: uri | ||
| description: Wallet address of the client instance that is making this request. | ||
| additionalProperties: false | ||
| - type: object | ||
| required: [jwk] | ||
| properties: | ||
| jwk: | ||
| $ref: '#/components/schemas/json-web-key' | ||
| additionalProperties: false | ||
| continue: | ||
| title: continue | ||
| type: object | ||
|
|
@@ -783,6 +826,48 @@ components: | |
| maxItems: 1 | ||
| required: | ||
| - sub_ids | ||
| json-web-key: | ||
| type: object | ||
| properties: | ||
| kid: | ||
| type: string | ||
| alg: | ||
| type: string | ||
| description: 'The cryptographic algorithm family used with the key. The only allowed value is `EdDSA`. ' | ||
| enum: | ||
| - EdDSA | ||
| use: | ||
| type: string | ||
| enum: | ||
| - sig | ||
| kty: | ||
| type: string | ||
| enum: | ||
| - OKP | ||
| crv: | ||
| description: 'The cryptographic curve used with the key. This parameter identifies the elliptic curve (for EC keys) or the Edwards curve (for OKP keys). The only allowed value is `Ed25519`.' | ||
| type: string | ||
| enum: | ||
| - Ed25519 | ||
| x: | ||
| type: string | ||
| pattern: '^[a-zA-Z0-9-_]+$' | ||
| description: The base64 url-encoded public key. | ||
| required: | ||
| - kid | ||
| - alg | ||
| - kty | ||
| - crv | ||
| - x | ||
| title: Ed25519 Public Key | ||
| description: A JWK representation of an Ed25519 Public Key | ||
| examples: | ||
| - kid: key-1 | ||
| alg: EdDSA | ||
| use: sig | ||
| kty: OKP | ||
| crv: Ed25519 | ||
| x: 11qYAYKxCrfVS_7TyWQHOg7hcvPapiMlrwIaaPcHURo | ||
| securitySchemes: | ||
| GNAP: | ||
| name: Authorization | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Before proceeding, I would suggest pulling the updated spec in one of the SDKs and inspect the generated types.
Previous discussions:
Theoretically, the use of
oneOfshould be correct in this case since all properties are exclusive (not present in multiple schemas), but a sanity check would be helpful since we had our fights with the generation for mutually exclusive types.Uh oh!
There was an error while loading. Please reload this page.
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.
Mixed results (see typegen changes here). I will look for a better way.
This top level
oneOflooks fine. How im doing the mutually exclusive object properties (jwk, wallet address) seems like it might be causing some weirdness. In general I'd prefer not to write the spec to satisfy the generator. But in this case it makes me think there is a simpler way even from an open-api perspective.Here is what it gives us (comments stripped for brevity):
The issues:
1.
& (unknown | unknown)is weird, but also completely benign (equivalent to juststring | { ... }). Guessing it's driven by usingoneOffor the required properties.2. both properties of the object are optional. could include none or both, which isn't quite right.
What I would expect, ideally would be more like:
Working on it.
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.
Also, I'm aware we have some sort of
XORts utils in the node client. We might have to massage the types a bit and that's fine. In any case, this part is striking me as perhaps not the best way to represent mutual exclusivity (although I think it should still be valid):Uh oh!
There was an error while loading. Please reload this page.
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.
This should be better:
Spec:
Generated Types:
I also tried to include this, thinking maybe it would set
jwk: neverbut it did not. The generated types were the same as above:Then in the client types we can do this:
EDIT:
Although that technically doesnt use the generated type (just JWK). We could derive it from
ASOperations['post-request']['requestBody']['content']['application/json']['client']but that feels like we are really trying to bend over backwards. In fact it seems easier to forgo ourXORand just do: