-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adding AS types & grant requests support to open-payments client #727
Changes from 13 commits
5ae5f3f
deb3081
9b1b87a
17af881
ac71e7c
f0febf0
5aa5370
ca946c8
94f95e5
0a453d5
6eff96c
b219aef
4252c8e
7b6407f
86304fe
f8154ba
1e5a71a
d9d7f7d
d2c0295
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,159 @@ | ||
import nock from 'nock' | ||
import { | ||
createGrantRoutes, | ||
isInteractiveGrant, | ||
isNonInteractiveGrant, | ||
requestInteractiveGrant, | ||
requestNonInteractiveGrant | ||
} from './grant' | ||
import { OpenAPI, HttpMethod, createOpenAPI } from 'openapi' | ||
import config from '../config' | ||
import { | ||
defaultAxiosInstance, | ||
mockInteractiveGrant, | ||
mockInteractiveGrantRequest, | ||
mockNonInteractiveGrant, | ||
mockNonInteractiveGrantRequest, | ||
mockOpenApiResponseValidators, | ||
silentLogger | ||
} from '../test/helpers' | ||
|
||
describe('grant', (): void => { | ||
let openApi: OpenAPI | ||
|
||
beforeAll(async () => { | ||
openApi = await createOpenAPI(config.OPEN_PAYMENTS_AS_OPEN_API_URL) | ||
}) | ||
|
||
const axiosInstance = defaultAxiosInstance | ||
const logger = silentLogger | ||
const baseUrl = 'http://localhost:1000' | ||
const openApiValidators = mockOpenApiResponseValidators() | ||
|
||
describe('createGrantRoutes', (): void => { | ||
test('creates response validator for grant requests', async (): Promise<void> => { | ||
jest.spyOn(openApi, 'createResponseValidator') | ||
|
||
createGrantRoutes({ axiosInstance, openApi, logger }) | ||
expect(openApi.createResponseValidator).toHaveBeenCalledTimes(1) | ||
expect(openApi.createResponseValidator).toHaveBeenCalledWith({ | ||
path: '/', | ||
method: HttpMethod.POST | ||
}) | ||
}) | ||
}) | ||
|
||
describe('requestInteractiveGrant', (): void => { | ||
test('returns interactive grant if passes validation', async (): Promise<void> => { | ||
const interactiveGrant = mockInteractiveGrant() | ||
|
||
nock(baseUrl).post('/').reply(200, interactiveGrant) | ||
|
||
const result = await requestInteractiveGrant( | ||
{ | ||
axiosInstance, | ||
logger | ||
}, | ||
{ | ||
url: `${baseUrl}/`, | ||
request: mockInteractiveGrantRequest() | ||
}, | ||
openApiValidators.successfulValidator | ||
) | ||
expect(result).toStrictEqual(interactiveGrant) | ||
}) | ||
|
||
test('throws if interactive grant does not pass validation', async (): Promise<void> => { | ||
const incorrectInteractiveGrant = mockInteractiveGrant({ | ||
interact: undefined | ||
}) | ||
|
||
nock(baseUrl).post('/').reply(200, incorrectInteractiveGrant) | ||
|
||
expect( | ||
requestInteractiveGrant( | ||
{ | ||
axiosInstance, | ||
logger | ||
}, | ||
{ | ||
url: `${baseUrl}/`, | ||
request: mockInteractiveGrantRequest() | ||
}, | ||
openApiValidators.successfulValidator | ||
) | ||
).rejects.toThrow('Could not validate interactive grant') | ||
}) | ||
}) | ||
|
||
describe('requestNonInteractiveGrant', (): void => { | ||
test('returns non-interactive grant if passes validation', async (): Promise<void> => { | ||
const nonInteractiveGrant = mockNonInteractiveGrant() | ||
|
||
nock(baseUrl).post('/').reply(200, nonInteractiveGrant) | ||
|
||
const result = await requestNonInteractiveGrant( | ||
{ | ||
axiosInstance, | ||
logger | ||
}, | ||
{ | ||
url: `${baseUrl}/`, | ||
request: mockNonInteractiveGrantRequest() | ||
}, | ||
openApiValidators.successfulValidator | ||
) | ||
expect(result).toStrictEqual(nonInteractiveGrant) | ||
}) | ||
|
||
test('throws if non-interactive grant does not pass validation', async (): Promise<void> => { | ||
const incorrectNonInteractiveGrant = mockNonInteractiveGrant({ | ||
access_token: undefined | ||
}) | ||
|
||
nock(baseUrl).post('/').reply(200, incorrectNonInteractiveGrant) | ||
|
||
expect( | ||
requestNonInteractiveGrant( | ||
{ | ||
axiosInstance, | ||
logger | ||
}, | ||
{ | ||
url: `${baseUrl}/`, | ||
request: mockNonInteractiveGrantRequest() | ||
}, | ||
openApiValidators.successfulValidator | ||
) | ||
).rejects.toThrow('Could not validate non-interactive grant') | ||
}) | ||
}) | ||
|
||
describe('isInteractiveGrant', (): void => { | ||
test('returns true if has interact property', async (): Promise<void> => { | ||
expect(isInteractiveGrant(mockInteractiveGrant())).toBe(true) | ||
}) | ||
|
||
test('returns false if has access_token property', async (): Promise<void> => { | ||
const grant = mockInteractiveGrant() | ||
|
||
grant['access_token'] = { value: 'token' } | ||
|
||
expect(isInteractiveGrant(grant)).toBe(false) | ||
}) | ||
}) | ||
|
||
describe('isNonInteractiveGrant', (): void => { | ||
test('returns true if has access_token property', async (): Promise<void> => { | ||
expect(isNonInteractiveGrant(mockNonInteractiveGrant())).toBe(true) | ||
}) | ||
|
||
test('returns false if has interact property', async (): Promise<void> => { | ||
const grant = mockNonInteractiveGrant() | ||
|
||
grant['interact'] = { redirect: 'http://example.com/redirect' } | ||
|
||
expect(isNonInteractiveGrant(grant)).toBe(false) | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,117 @@ | ||||||||||
import { HttpMethod, ResponseValidator } from 'openapi' | ||||||||||
import { RouteDeps } from '.' | ||||||||||
import { | ||||||||||
getASPath, | ||||||||||
InteractiveGrant, | ||||||||||
InteractiveGrantRequest, | ||||||||||
NonInteractiveGrant, | ||||||||||
NonInteractiveGrantRequest | ||||||||||
} from '../types' | ||||||||||
import { post } from './requests' | ||||||||||
|
||||||||||
interface RequestGrantArgs<T> { | ||||||||||
url: string | ||||||||||
request: T | ||||||||||
} | ||||||||||
|
||||||||||
export interface GrantRoutes { | ||||||||||
requestInteractiveGrant( | ||||||||||
args: RequestGrantArgs<InteractiveGrantRequest> | ||||||||||
): Promise<InteractiveGrant> | ||||||||||
requestNonInteractiveGrant( | ||||||||||
args: RequestGrantArgs<NonInteractiveGrantRequest> | ||||||||||
): Promise<NonInteractiveGrant> | ||||||||||
} | ||||||||||
|
||||||||||
export const createGrantRoutes = (deps: RouteDeps): GrantRoutes => { | ||||||||||
const { axiosInstance, openApi, logger } = deps | ||||||||||
|
||||||||||
const requestGrantValidator = openApi.createResponseValidator< | ||||||||||
InteractiveGrant | NonInteractiveGrant | ||||||||||
>({ | ||||||||||
path: getASPath('/'), | ||||||||||
method: HttpMethod.POST | ||||||||||
}) | ||||||||||
return { | ||||||||||
requestInteractiveGrant: ( | ||||||||||
args: RequestGrantArgs<InteractiveGrantRequest> | ||||||||||
) => | ||||||||||
requestInteractiveGrant( | ||||||||||
{ axiosInstance, logger }, | ||||||||||
args, | ||||||||||
requestGrantValidator | ||||||||||
), | ||||||||||
requestNonInteractiveGrant: ( | ||||||||||
args: RequestGrantArgs<NonInteractiveGrantRequest> | ||||||||||
) => | ||||||||||
requestNonInteractiveGrant( | ||||||||||
{ axiosInstance, logger }, | ||||||||||
args, | ||||||||||
requestGrantValidator | ||||||||||
) | ||||||||||
} | ||||||||||
} | ||||||||||
|
||||||||||
export const requestInteractiveGrant = async ( | ||||||||||
deps: Pick<RouteDeps, 'axiosInstance' | 'logger'>, | ||||||||||
args: RequestGrantArgs<InteractiveGrantRequest>, | ||||||||||
validateOpenApiResponse: ResponseValidator< | ||||||||||
InteractiveGrant | NonInteractiveGrant | ||||||||||
> | ||||||||||
) => { | ||||||||||
const { axiosInstance, logger } = deps | ||||||||||
const { url } = args | ||||||||||
|
||||||||||
const grant = await post( | ||||||||||
{ axiosInstance, logger }, | ||||||||||
{ url: args.url, body: args.request }, | ||||||||||
validateOpenApiResponse | ||||||||||
) | ||||||||||
|
||||||||||
if (!isInteractiveGrant(grant)) { | ||||||||||
const errorMessage = 'Could not validate interactive grant' | ||||||||||
logger.error({ url, grant }, errorMessage) | ||||||||||
throw new Error(errorMessage) | ||||||||||
} | ||||||||||
|
||||||||||
return grant | ||||||||||
} | ||||||||||
|
||||||||||
export const requestNonInteractiveGrant = async ( | ||||||||||
deps: Pick<RouteDeps, 'axiosInstance' | 'logger'>, | ||||||||||
args: RequestGrantArgs<NonInteractiveGrantRequest>, | ||||||||||
validateOpenApiResponse: ResponseValidator< | ||||||||||
InteractiveGrant | NonInteractiveGrant | ||||||||||
> | ||||||||||
) => { | ||||||||||
const { axiosInstance, logger } = deps | ||||||||||
const { url } = args | ||||||||||
|
||||||||||
const grant = await post( | ||||||||||
{ axiosInstance, logger }, | ||||||||||
{ url: args.url, body: args.request }, | ||||||||||
validateOpenApiResponse | ||||||||||
) | ||||||||||
|
||||||||||
if (!isNonInteractiveGrant(grant)) { | ||||||||||
const errorMessage = 'Could not validate non-interactive grant' | ||||||||||
logger.error({ url, grant }, errorMessage) | ||||||||||
throw new Error(errorMessage) | ||||||||||
} | ||||||||||
|
||||||||||
return grant | ||||||||||
} | ||||||||||
|
||||||||||
export const isInteractiveGrant = ( | ||||||||||
grant: InteractiveGrant | NonInteractiveGrant | ||||||||||
): grant is InteractiveGrant => | ||||||||||
!('access_token' in grant) && | ||||||||||
'interact' in grant && | ||||||||||
!!(grant as InteractiveGrant).interact | ||||||||||
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. Could this just be
Suggested change
I'm assuming the OpenAPI validator would disallow 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. Yeah I wasn't sure how lenient the validator was with |
||||||||||
|
||||||||||
export const isNonInteractiveGrant = ( | ||||||||||
grant: InteractiveGrant | NonInteractiveGrant | ||||||||||
): grant is NonInteractiveGrant => | ||||||||||
!('interact' in grant) && | ||||||||||
'access_token' in grant && | ||||||||||
!!(grant as NonInteractiveGrant).access_token |
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.
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:@sabineschaller or @njlie or @adrianhopebailie might be able to correct me on this.
A compromise could be to do:
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.
requestNonInteractiveGrant
seems fine as is, since the request should just fail if the AS requires RO interaction for that grant.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 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
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.
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.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'm still leaning towards
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).
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.
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.
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'm ok also keeping:
But maybe requiring the
interact
field to be specified isn't unreasonable? 🤔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 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