Adding AS types & grant requests support to open-payments client #727

mkurapov · 2022-11-04T13:40:20Z

Changes proposed in this pull request

Generate open payment AS open APIs types, export them
Enable open payments client functionality for interactive and non-interactive grant creation (using AS spec under the hood)

Context

Continued work for the open payments client, roadmap item: Open Payments: Develop a ts/js client sdk roadmap#20.

Checklist

Related issues linked using fixes #number
Tests added/updated
Documentation added
Make sure that all checks pass

mkurapov · 2022-11-04T13:41:57Z

packages/open-payments/src/config.ts

+  OPEN_PAYMENTS_RS_OPEN_API_URL:
+    'https://raw.githubusercontent.com/interledger/open-payments/1e3e118d8b22c5d2942f972e28ebf0f0114d04f5/openapi/resource-server.yaml',
+  OPEN_PAYMENTS_AS_OPEN_API_URL:
+    'https://raw.githubusercontent.com/interledger/open-payments/1e3e118d8b22c5d2942f972e28ebf0f0114d04f5/openapi/auth-server.yaml',


Originally, I was thinking of doing something like

Suggested change

OPEN_PAYMENTS_RS_OPEN_API_URL:

'https://raw.githubusercontent.com/interledger/open-payments/1e3e118d8b22c5d2942f972e28ebf0f0114d04f5/openapi/resource-server.yaml',

OPEN_PAYMENTS_AS_OPEN_API_URL:

'https://raw.githubusercontent.com/interledger/open-payments/1e3e118d8b22c5d2942f972e28ebf0f0114d04f5/openapi/auth-server.yaml',

COMMIT_HASH: '1e3e118d8b22c5d2942f972e28ebf0f0114d04f5',

OPEN_PAYMENTS_RS_OPEN_API_URL:

'https://raw.githubusercontent.com/interledger/open-payments/<commitHash>/openapi/resource-server.yaml',

OPEN_PAYMENTS_AS_OPEN_API_URL:

'https://raw.githubusercontent.com/interledger/open-payments/<commitHash>/openapi/auth-server.yaml',

and then replace the <commitHash> whenever OPEN_PAYMENTS_AS_OPEN_API_URL was grabbed, but I thought overkill with only two configs

mkurapov · 2022-11-04T13:42:27Z

packages/open-payments/src/types.ts

@@ -1,9 +1,36 @@
-import { components, paths as Paths } from './generated/types'
+import {


decided to keep them in a single file, we don't have too many types

mkurapov · 2022-11-04T13:42:51Z

packages/open-payments/tsconfig.json

@@ -7,5 +7,5 @@
    "declaration": true
  },
  "include": ["src/**/*"],
-  "exclude": ["**/*.test.ts", "src/scripts/*", "src/test/*"]


we don't have a src/scripts folder

mkurapov · 2022-11-04T13:46:49Z

packages/open-payments/src/client/grant.ts

+  return {
+    requestInteractiveGrant: (
+      args: RequestGrantArgs<InteractiveGrantRequest>
+    ) =>
+      post(
+        { axiosInstance, logger },
+        { url: args.url, body: args.request },
+        createInteractiveGrantValidator
+      ),
+    requestNonInteractiveGrant: (
+      args: RequestGrantArgs<NonInteractiveGrantRequest>
+    ) =>
+      post(
+        { axiosInstance, logger },
+        { url: args.url, body: args.request },
+        createNonInteractiveGrantValidator
+      )
+  }


At first, I wanted to do something tricky with types & method overloading, where we have one grant.create method that determined whether an "interactive" or "non-interactive" grant arguments were passed in, and returned with the correct response, but I decided something more explicit is much better to read and use.

mkurapov · 2022-11-04T13:47:43Z

packages/open-payments/src/client/grant.ts

+  const createInteractiveGrantValidator =
+    openApi.createResponseValidator<InteractiveGrant>({
+      path: getASPath('/'),
+      method: HttpMethod.POST
+    })
+  const createNonInteractiveGrantValidator =
+    openApi.createResponseValidator<NonInteractiveGrant>({
+      path: getASPath('/'),
+      method: HttpMethod.POST
+    })


unfourtunately, needed both to pass type checking when given as arguments to the request*Grant methods

If we keep requestInteractiveGrant/requestNonInteractiveGrant separate, I think we'll need custom validators that include the openApi.createResponseValidator as well as a subsequent type guard that the response is the expected InteractiveGrant or NonInteractiveGrant. (In which case, we might be able to have a single openApi.createResponseValidator for both?)
As is, I think the OpenAPI validator would allow a NonInteractiveGrant response for requestInteractiveGrant and vice versa.

wilsonianb · 2022-11-04T14:20:11Z

packages/open-payments/src/client/grant.ts

+  requestInteractiveGrant(
+    args: RequestGrantArgs<InteractiveGrantRequest>
+  ): Promise<InteractiveGrant>


My concern with splitting requestInteractiveGrant/requestNonInteractiveGrant is that I don't think we've standardized what grant requests are interactive. And thus, a client might not be expected to know when to use either.
For example, Rafiki has made incoming-payment-only grants non-interactive, but I don't know that that's required:

feat(auth): allow non-interactive grants for incoming payments #588

@sabineschaller or @njlie or @adrianhopebailie might be able to correct me on this.

A compromise could be to do:

Suggested change

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant>

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant | NonInteractiveGrant>

requestNonInteractiveGrant seems fine as is, since the request should just fail if the AS requires RO interaction for that grant.

I separated these, since at some point, I thought the client need to know that they need to pass in interact ?
https://github.com/interledger/open-payments/blob/main/openapi/auth-server.yaml#L93-L94

Given our mix of grants for different resources needing interact or not, I've been assuming a client might just always assume a grant request requires interaction, but also be able to handle getting back a NonInteractiveGrant response.

I'm still leaning towards

Suggested change

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant>

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant | NonInteractiveGrant>

and clients can utilize isInteractiveGrant/isNonInteractiveGrant.

I got the impression from Adrian that their policy agent AS might be determining what grant requests require interaction (vs having it Open Payments standardized).

Ah, I see what you mean, it's more of a configurable, per-instance thing instead of standard, makes sense.

Going to make changes to assume client will always require interaction, but call might return either or.

I'm ok also keeping:

requestNonInteractiveGrant( args: RequestGrantArgs<NonInteractiveGrantRequest> ): Promise<NonInteractiveGrant>

But maybe requiring the interact field to be specified isn't unreasonable? 🤔

Yeah even in the demo Postman example, there is an interact field present when requesting the incoming payment grant -> which returns a non-interactive one.
We could run with a single call to make things simple for now. Going to play around w it

packages/open-payments/src/config.ts

wilsonianb · 2022-11-07T19:25:55Z

packages/open-payments/src/client/grant.ts

+  const createInteractiveGrantValidator =
+    openApi.createResponseValidator<InteractiveGrant>({
+      path: getASPath('/'),
+      method: HttpMethod.POST
+    })
+  const createNonInteractiveGrantValidator =
+    openApi.createResponseValidator<NonInteractiveGrant>({
+      path: getASPath('/'),
+      method: HttpMethod.POST
+    })


If we keep requestInteractiveGrant/requestNonInteractiveGrant separate, I think we'll need custom validators that include the openApi.createResponseValidator as well as a subsequent type guard that the response is the expected InteractiveGrant or NonInteractiveGrant. (In which case, we might be able to have a single openApi.createResponseValidator for both?)
As is, I think the OpenAPI validator would allow a NonInteractiveGrant response for requestInteractiveGrant and vice versa.

wilsonianb · 2022-11-07T19:31:27Z

packages/open-payments/src/client/grant.ts

+  ): Promise<NonInteractiveGrant>
+}
+
+export const createGrantRoutes = (clientDeps: ClientDeps): GrantRoutes => {


We'll also need the grant continuation route, in order to support the full interactive grant flow.
https://github.com/interledger/open-payments/blob/62c4b4a9875e3adaa21f89f597e88db43016fe0b/openapi/auth-server.yaml#L163-L171

# Conflicts: # packages/open-payments/src/client/index.ts # packages/open-payments/src/client/requests.test.ts # packages/open-payments/src/client/requests.ts # packages/open-payments/src/client/signatures.ts # packages/open-payments/src/index.ts

mkurapov · 2022-11-17T15:33:51Z

Going to add grant continuation as a separate PR

packages/open-payments/src/client/incoming-payment.ts

wilsonianb · 2022-11-17T15:27:19Z

packages/open-payments/src/client/grant.ts

+  !('access_token' in grant) &&
+  'interact' in grant &&
+  !!(grant as InteractiveGrant).interact


Could this just be

Suggested change

!('access_token' in grant) &&

'interact' in grant &&

!!(grant as InteractiveGrant).interact

!!(grant as InteractiveGrant).interact

I'm assuming the OpenAPI validator would disallow access_token + interact.
ditto below

Yeah I wasn't sure how lenient the validator was with oneOf in the spec, but this can be less strict

wilsonianb · 2022-11-17T15:33:55Z

packages/open-payments/src/client/grant.ts

+  requestInteractiveGrant(
+    args: RequestGrantArgs<InteractiveGrantRequest>
+  ): Promise<InteractiveGrant>


I'm still leaning towards

Suggested change

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant>

requestInteractiveGrant(

args: RequestGrantArgs<InteractiveGrantRequest>

): Promise<InteractiveGrant | NonInteractiveGrant>

and clients can utilize isInteractiveGrant/isNonInteractiveGrant.

I got the impression from Adrian that their policy agent AS might be determining what grant requests require interaction (vs having it Open Payments standardized).

mkurapov · 2022-11-17T18:10:06Z

packages/open-payments/src/types.ts

+export const isInteractiveGrant = (
+  grant: InteractiveGrant | NonInteractiveGrant
+): grant is InteractiveGrant => !!(grant as InteractiveGrant).interact
+
+export const isNonInteractiveGrant = (
+  grant: InteractiveGrant | NonInteractiveGrant
+): grant is NonInteractiveGrant => !!(grant as NonInteractiveGrant).access_token


could live in src/client/grants.ts or src/type-guard.ts but decided to leave them here

mkurapov · 2022-11-17T18:10:35Z

packages/open-payments/src/types.ts

+export type GrantRequest = {
+  access_token: ASOperations['post-request']['requestBody']['content']['application/json']['access_token']
+  client: ASOperations['post-request']['requestBody']['content']['application/json']['client']
+  interact?: ASOperations['post-request']['requestBody']['content']['application/json']['interact']


@wilsonianb single grant request with optional interact in line with the Open Payments API

I was thinking we'd require interact
#727 (comment)
#727 (comment)
but I'm ok as is. The grant request would just throw if interaction is required and interact wasn't specified.

Was going back and forth but less errors and throws are better here IMO. Made it required.

wilsonianb · 2022-11-17T18:29:26Z

packages/open-payments/src/types.ts

+export type GrantRequest = {
+  access_token: ASOperations['post-request']['requestBody']['content']['application/json']['access_token']
+  client: ASOperations['post-request']['requestBody']['content']['application/json']['client']
+  interact?: ASOperations['post-request']['requestBody']['content']['application/json']['interact']


I was thinking we'd require interact
#727 (comment)
#727 (comment)
but I'm ok as is. The grant request would just throw if interaction is required and interact wasn't specified.

wilsonianb · 2022-11-17T18:29:49Z

packages/open-payments/src/client/grant.ts

+}
+
+export interface GrantRoutes {
+  requestGrant(


Suggested change

requestGrant(

request(

or

Suggested change

requestGrant(

requestAccess(

https://datatracker.ietf.org/doc/html/draft-ietf-gnap-core-protocol#section-2

yup, had request in my follow-up PR, will do it here instead

wilsonianb · 2022-11-17T20:50:33Z

commit 300! 🎉

mkurapov added 2 commits November 4, 2022 11:53

feat(open-payments): adding AS types

5ae5f3f

feat(open-payments): adding support for grant requests

deb3081

github-actions bot added pkg: open-payments type: source Changes business logic type: tests Testing related labels Nov 4, 2022

mkurapov commented Nov 4, 2022

View reviewed changes

wilsonianb reviewed Nov 4, 2022

View reviewed changes

wilsonianb mentioned this pull request Nov 7, 2022

feat(backend): add services for managing RS grants #693

Closed

4 tasks

mkurapov added 3 commits November 7, 2022 16:44

Merge branch 'main' into 633/mk/adding-AS-types-to-client

9b1b87a

chore(open-payments): update open-payments spec commit version

17af881

chore(open-payments): update AS request/response types

ac71e7c

mkurapov marked this pull request as ready for review November 7, 2022 16:51

mkurapov requested review from wilsonianb, njlie and sabineschaller November 7, 2022 16:54

chore(open-payments): rename AS generated types file

f0febf0

wilsonianb reviewed Nov 7, 2022

View reviewed changes

chore(open-payments): merge bw-httpsig

5aa5370

github-actions bot added the pkg: backend Changes in the backend package. label Nov 16, 2022

mkurapov changed the base branch from main to bw-httpsig November 16, 2022 23:04

chore(open-payments): update types, fix tests

ca946c8

github-actions bot added pkg: auth Changes in the GNAP auth package. type: localenv Local playground labels Nov 16, 2022

Merge branch 'main' into 633/mk/adding-AS-types-to-client

94f95e5

github-actions bot added pkg: map Changes in mock-account-provider package pkg: openapi labels Nov 16, 2022

Base automatically changed from bw-httpsig to main November 17, 2022 14:11

feat(open-payments) adding tests for new grant fetching methods

b219aef

github-actions bot removed pkg: auth Changes in the GNAP auth package. pkg: openapi pkg: map Changes in mock-account-provider package type: localenv Local playground type: ci Changes to the CI labels Nov 17, 2022

github-actions bot removed the pkg: backend Changes in the backend package. label Nov 17, 2022

chore(open-payments): rename dependency var

7b6407f

mkurapov requested a review from wilsonianb November 17, 2022 15:32

mkurapov changed the title ~~633/mk/adding as types to client~~ Adding AS types & grant requests support to open-payments client Nov 17, 2022

wilsonianb reviewed Nov 17, 2022

View reviewed changes

mkurapov added 3 commits November 17, 2022 18:56

feat(open-payments): unify the grant request call

86304fe

chore(open-payments): simplify type

f8154ba

feat(open-payments): move around type guards

1e5a71a

mkurapov commented Nov 17, 2022

View reviewed changes

chore(open-payments): remove duplicate fns

d9d7f7d

mkurapov requested a review from wilsonianb November 17, 2022 18:12

wilsonianb previously approved these changes Nov 17, 2022

View reviewed changes

feat(open-payments): address comments

d2c0295

mkurapov dismissed wilsonianb’s stale review via d2c0295 November 17, 2022 19:50

mkurapov requested a review from wilsonianb November 17, 2022 19:51

wilsonianb approved these changes Nov 17, 2022

View reviewed changes

mkurapov merged commit 461fb76 into main Nov 17, 2022

mkurapov deleted the 633/mk/adding-AS-types-to-client branch November 17, 2022 20:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding AS types & grant requests support to open-payments client #727

Adding AS types & grant requests support to open-payments client #727

mkurapov commented Nov 4, 2022 •

edited

Loading

mkurapov Nov 4, 2022

mkurapov Nov 4, 2022

mkurapov Nov 4, 2022

mkurapov Nov 4, 2022

mkurapov Nov 4, 2022

wilsonianb Nov 7, 2022

wilsonianb Nov 4, 2022

wilsonianb Nov 7, 2022

mkurapov Nov 7, 2022

wilsonianb Nov 7, 2022

wilsonianb Nov 17, 2022

mkurapov Nov 17, 2022

wilsonianb Nov 17, 2022

mkurapov Nov 17, 2022 •

edited

Loading

wilsonianb Nov 7, 2022

wilsonianb Nov 7, 2022

mkurapov commented Nov 17, 2022

wilsonianb Nov 17, 2022

mkurapov Nov 17, 2022

wilsonianb Nov 17, 2022

mkurapov Nov 17, 2022

mkurapov Nov 17, 2022 •

edited

Loading

wilsonianb Nov 17, 2022 •

edited

Loading

mkurapov Nov 17, 2022

wilsonianb Nov 17, 2022 •

edited

Loading

wilsonianb Nov 17, 2022

mkurapov Nov 17, 2022 •

edited

Loading

wilsonianb commented Nov 17, 2022

		@@ -1,9 +1,36 @@
		import { components, paths as Paths } from './generated/types'
		import {

Adding AS types & grant requests support to open-payments client #727

Adding AS types & grant requests support to open-payments client #727

Conversation

mkurapov commented Nov 4, 2022 • edited Loading

Changes proposed in this pull request

Context

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov commented Nov 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

wilsonianb Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wilsonianb Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkurapov Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

wilsonianb commented Nov 17, 2022

mkurapov commented Nov 4, 2022 •

edited

Loading

mkurapov Nov 17, 2022 •

edited

Loading

mkurapov Nov 17, 2022 •

edited

Loading

wilsonianb Nov 17, 2022 •

edited

Loading

wilsonianb Nov 17, 2022 •

edited

Loading

mkurapov Nov 17, 2022 •

edited

Loading