From 52e4f55732309f9c78615baff5d06f95d18a65c1 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Fri, 2 Sep 2022 11:59:35 +0200 Subject: [PATCH 01/15] feat(auth): allow non-interactive grants for incoming payments --- .../20220504161932_create_grants_table.js | 16 +- packages/auth/src/access/types.ts | 4 +- packages/auth/src/config/app.ts | 4 +- packages/auth/src/grant/model.ts | 12 +- packages/auth/src/grant/routes.test.ts | 269 +++++------------- packages/auth/src/grant/routes.ts | 59 +++- packages/auth/src/grant/service.test.ts | 44 ++- packages/auth/src/grant/service.ts | 16 +- 8 files changed, 192 insertions(+), 232 deletions(-) diff --git a/packages/auth/migrations/20220504161932_create_grants_table.js b/packages/auth/migrations/20220504161932_create_grants_table.js index b08ef6bb5e..2d2a15fe36 100644 --- a/packages/auth/migrations/20220504161932_create_grants_table.js +++ b/packages/auth/migrations/20220504161932_create_grants_table.js @@ -5,18 +5,18 @@ exports.up = function (knex) { table.string('state').notNullable() table.specificType('startMethod', 'text[]').notNullable() - table.string('continueToken').unique() - table.string('continueId').unique() + table.string('continueToken').notNullable().unique() + table.string('continueId').notNullable().unique() table.integer('wait') - table.string('finishMethod').notNullable() - table.string('finishUri').notNullable() - table.string('clientNonce').notNullable() + table.string('finishMethod') + table.string('finishUri') + table.string('clientNonce') table.string('clientKeyId').notNullable() - table.string('interactId').notNullable().unique() - table.string('interactRef').notNullable().unique() - table.string('interactNonce').notNullable().unique() + table.string('interactId').unique() + table.string('interactRef').unique() + table.string('interactNonce').unique() table.timestamp('createdAt').defaultTo(knex.fn.now()) table.timestamp('updatedAt').defaultTo(knex.fn.now()) diff --git a/packages/auth/src/access/types.ts b/packages/auth/src/access/types.ts index e2575ab27c..fd18a0466a 100644 --- a/packages/auth/src/access/types.ts +++ b/packages/auth/src/access/types.ts @@ -19,7 +19,7 @@ interface BaseAccessRequest { interval?: string } -interface IncomingPaymentRequest extends BaseAccessRequest { +export interface IncomingPaymentRequest extends BaseAccessRequest { type: AccessType.IncomingPayment limits?: never } @@ -58,7 +58,7 @@ export function isAction(actions: Action[]): actions is Action[] { return true } -function isIncomingPaymentAccessRequest( +export function isIncomingPaymentAccessRequest( accessRequest: IncomingPaymentRequest ): accessRequest is IncomingPaymentRequest { return ( diff --git a/packages/auth/src/config/app.ts b/packages/auth/src/config/app.ts index 6a0165dd58..fd559daf1e 100644 --- a/packages/auth/src/config/app.ts +++ b/packages/auth/src/config/app.ts @@ -38,5 +38,7 @@ export const Config = { accessTokenExpirySeconds: envInt('ACCESS_TOKEN_EXPIRY_SECONDS', 10 * 60), // Default 10 minutes databaseCleanupWorkers: envInt('DATABASE_CLEANUP_WORKERS', 1), accessTokenDeletionDays: envInt('ACCESS_TOKEN_DELETION_DAYS', 30), - introspectionHttpsig: process.env.INTROSPECTION_HTTPSIG === 'true' + introspectionHttpsig: process.env.INTROSPECTION_HTTPSIG === 'true', + incomingPaymentInteraction: + process.env.INCOMING_PAYMENT_INTERACTION === 'true' } diff --git a/packages/auth/src/grant/model.ts b/packages/auth/src/grant/model.ts index 2d6b335cbd..4fe1760e4b 100644 --- a/packages/auth/src/grant/model.ts +++ b/packages/auth/src/grant/model.ts @@ -51,12 +51,12 @@ export class Grant extends BaseModel { public continueId!: string public wait?: number - public finishMethod!: FinishMethod - public finishUri!: string - public clientNonce!: string // client-generated nonce for post-interaction hash + public finishMethod?: FinishMethod + public finishUri?: string + public clientNonce?: string // client-generated nonce for post-interaction hash public clientKeyId!: string - public interactId!: string - public interactRef!: string - public interactNonce!: string // AS-generated nonce for post-interaction hash + public interactId?: string + public interactRef?: string + public interactNonce?: string // AS-generated nonce for post-interaction hash } diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index a18d3f0655..8c35a16c04 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -12,7 +12,7 @@ import { Config, IAppConfig } from '../config/app' import { initIocContainer } from '..' import { AppServices } from '../app' import { truncateTables } from '../tests/tableManager' -import { GrantRoutes } from './routes' +import { GrantRoutes, validateGrantRequest } from './routes' import { Action, AccessType } from '../access/types' import { Access } from '../access/model' import { Grant, StartMethod, FinishMethod, GrantState } from '../grant/model' @@ -50,7 +50,6 @@ export const TEST_CLIENT_KEY = { const BASE_GRANT_ACCESS = { type: AccessType.IncomingPayment, actions: [Action.Create, Action.Read, Action.List], - locations: ['https://example.com'], identifier: `https://example.com/${v4()}` } @@ -75,7 +74,6 @@ const BASE_GRANT_REQUEST = { { type: AccessType.IncomingPayment, actions: [Action.Create, Action.Read, Action.List], - locations: ['https://example.com'], identifier: `https://example.com/${v4()}` } ] @@ -142,7 +140,7 @@ describe('Grant Routes', (): void => { await appContainer.shutdown() }) - describe('Grant validation', (): void => { + describe('Grant request validation', (): void => { const expDate = new Date() expDate.setTime(expDate.getTime() + 1000 * 60 * 60) @@ -164,34 +162,7 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = incomingPaymentGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.response).toSatisfyApiSpec() - expect(ctx.status).toBe(201) - - scope.isDone() + expect(validateGrantRequest(incomingPaymentGrantRequest)).toBe(true) }) test('Valid outgoing payment grant', async (): Promise => { @@ -208,34 +179,7 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = outgoingPaymentGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.response).toSatisfyApiSpec() - expect(ctx.status).toBe(201) - - scope.isDone() + expect(validateGrantRequest(outgoingPaymentGrantRequest)).toBe(true) }) test('Valid account grant', async (): Promise => { @@ -251,34 +195,7 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = accountGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.response).toSatisfyApiSpec() - expect(ctx.status).toBe(201) - - scope.isDone() + expect(validateGrantRequest(accountGrantRequest)).toBe(true) }) test('Valid quote grant', async (): Promise => { @@ -294,34 +211,7 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = quoteGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.response).toSatisfyApiSpec() - expect(ctx.status).toBe(201) - - scope.isDone() + expect(validateGrantRequest(quoteGrantRequest)).toBe(true) }) test('Cannot create outgoing payment grant with unexpected limit payload', async (): Promise => { @@ -338,37 +228,11 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = outgoingPaymentGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.status).toBe(400) - - scope.isDone() + expect(validateGrantRequest(outgoingPaymentGrantRequest)).toBe(false) }) test('Cannot create account grant with unexpected limit payload', async (): Promise => { - const incomingPaymentGrantRequest = { + const accountGrantRequest = { ...BASE_GRANT_REQUEST, access_token: { access: [ @@ -381,37 +245,11 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = incomingPaymentGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.status).toBe(400) - - scope.isDone() + expect(validateGrantRequest(accountGrantRequest)).toBe(false) }) test('Cannot create quote grant with unexpected limit payload', async (): Promise => { - const incomingPaymentGrantRequest = { + const quoteGrantRequest = { ...BASE_GRANT_REQUEST, access_token: { access: [ @@ -424,33 +262,7 @@ describe('Grant Routes', (): void => { } } - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = incomingPaymentGrantRequest - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.status).toBe(400) - - scope.isDone() + expect(validateGrantRequest(quoteGrantRequest)).toBe(false) }) }) @@ -576,6 +388,7 @@ describe('Grant Routes', (): void => { ) ctx.request.body = BASE_GRANT_REQUEST + ctx.request.body.access_token.access[0].type = AccessType.OutgoingPayment await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() @@ -596,6 +409,57 @@ describe('Grant Routes', (): void => { scope.isDone() }) + + test('Can get a software-only authorization grant', async (): Promise => { + const expDate = new Date() + expDate.setTime(expDate.getTime() + 1000 * 60 * 60) + + const nbfDate = new Date() + nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) + + const scope = nock(KEY_REGISTRY_ORIGIN) + .get(KID_PATH) + .reply(200, { + ...TEST_CLIENT_KEY.jwk, + exp: Math.round(expDate.getTime() / 1000), + nbf: Math.round(nbfDate.getTime() / 1000), + revoked: false + }) + + const ctx = createContext( + { + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + }, + url, + method + }, + {} + ) + + ctx.request.body = BASE_GRANT_REQUEST + + await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() + // expect(ctx.response).toSatisfyApiSpec() + expect(ctx.status).toBe(200) + expect(ctx.body).toEqual({ + access_token: { + value: expect.any(String), + manage: expect.any(String), + access: BASE_GRANT_REQUEST.access_token.access, + continue: { + access_token: { + value: expect.any(String) + }, + uri: expect.any(String) + }, + expiresIn: 600 + } + }) + + scope.isDone() + }) }) // TODO: validate that routes satisfy API spec @@ -1117,10 +981,15 @@ describe('Grant Routes', (): void => { { actions: expect.arrayContaining(['create', 'read', 'list']), identifier: access.identifier, - locations: expect.arrayContaining(['https://example.com']), type: 'incoming-payment' } ]), + continue: { + access_token: { + value: expect.any(String) + }, + uri: expect.any(String) + }, expiresIn: 600 } }) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index 4f60a242d1..1b8d6feb6b 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -2,15 +2,20 @@ import * as crypto from 'crypto' import { URL } from 'url' import { AppContext } from '../app' import { GrantService, GrantRequest } from './service' -import { GrantState } from './model' +import { Grant, GrantState } from './model' import { Access } from '../access/model' import { ClientService } from '../client/service' import { BaseService } from '../shared/baseService' -import { isAccessRequest } from '../access/types' +import { + IncomingPaymentRequest, + isAccessRequest, + isIncomingPaymentAccessRequest +} from '../access/types' import { IAppConfig } from '../config/app' import { AccessTokenService } from '../accessToken/service' import { AccessService } from '../access/service' import { accessToBody } from '../shared/utils' +import { AccessToken } from '../accessToken/model' interface ServiceDependencies extends BaseService { grantService: GrantService @@ -66,7 +71,8 @@ export function createGrantRoutes({ } } -function validateGrantRequest( +// exported for testing +export function validateGrantRequest( grantRequest: GrantRequest ): grantRequest is GrantRequest { if (typeof grantRequest.access_token !== 'object') return false @@ -102,6 +108,27 @@ async function createGrantInitiation( return } + if ( + body.access_token.access + .map((acc) => { + return isIncomingPaymentAccessRequest(acc as IncomingPaymentRequest) + }) + .every((el) => el === true) && + !deps.config.incomingPaymentInteraction + ) { + const grant = await grantService.issueNoInteractionGrant(body) + const accessToken = await deps.accessTokenService.create(grant.id) + const access = await deps.accessService.getByGrant(grant.id) + ctx.status = 200 + ctx.body = createGrantBody( + deps.config.authServerDomain, + grant, + access, + accessToken + ) + return + } + const grant = await grantService.initiateGrant(body) ctx.status = 201 ctx.body = { @@ -384,12 +411,32 @@ async function continueGrant( const access = await accessService.getByGrant(grant.id) // TODO: add "continue" to response if additional grant request steps are added - ctx.body = { + ctx.body = createGrantBody( + config.authServerDomain, + grant, + access, + accessToken + ) +} + +function createGrantBody( + domain: string, + grant: Grant, + access: Access[], + accessToken: AccessToken +) { + return { access_token: { value: accessToken.value, - manage: config.authServerDomain + `/token/${accessToken.managementId}`, + manage: domain + `/token/${accessToken.managementId}`, access: access.map((a: Access) => accessToBody(a)), - expiresIn: accessToken.expiresIn + expiresIn: accessToken.expiresIn, + continue: { + access_token: { + value: grant.continueToken + }, + uri: domain + `continue/${grant.continueId}` + } } } } diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index f0980fc971..4eab34e7be 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -96,7 +96,7 @@ describe('Grant Service', (): void => { } describe('create', (): void => { - test('Can create a grant', async (): Promise => { + test('Can initiate a grant', async (): Promise => { const grantRequest: GrantRequest = { ...BASE_GRANT_REQUEST, access_token: { @@ -125,11 +125,44 @@ describe('Grant Service', (): void => { startMethod: expect.arrayContaining([StartMethod.Redirect]) }) - const dbAccessGrant = await Access.query(trx) + const dbAccessGrant = (await Access.query(trx) .where({ grantId: grant.id }) - .first() + .first()) as Access + + expect(dbAccessGrant.type).toEqual(AccessType.IncomingPayment) + }) + test('Can issue a grant without interaction', async (): Promise => { + const grantRequest: GrantRequest = { + ...BASE_GRANT_REQUEST, + access_token: { + access: [ + { + ...BASE_GRANT_ACCESS, + type: AccessType.IncomingPayment + } + ] + } + } + + const grant = await grantService.issueNoInteractionGrant(grantRequest) + + expect(grant).toMatchObject({ + state: GrantState.Granted, + continueId: expect.any(String), + continueToken: expect.any(String), + finishMethod: FinishMethod.Redirect, + finishUri: BASE_GRANT_REQUEST.interact.finish.uri, + clientKeyId: BASE_GRANT_REQUEST.client.key.jwk.kid, + startMethod: expect.arrayContaining([StartMethod.Redirect]) + }) + + const dbAccessGrant = (await Access.query(trx) + .where({ + grantId: grant.id + }) + .first()) as Access expect(dbAccessGrant.type).toEqual(AccessType.IncomingPayment) }) @@ -164,7 +197,10 @@ describe('Grant Service', (): void => { expect(fetchedGrant?.id).toEqual(grant.id) }) test('Can fetch a grant by its interaction information', async (): Promise => { - const fetchedGrant = await grantService.getByInteraction(grant.interactId) + const fetchedGrant = await grantService.getByInteraction( + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + grant.interactId! + ) expect(fetchedGrant?.id).toEqual(grant.id) expect(fetchedGrant?.interactId).toEqual(grant.interactId) }) diff --git a/packages/auth/src/grant/service.ts b/packages/auth/src/grant/service.ts index 5cb9d0e76a..bd605e8661 100644 --- a/packages/auth/src/grant/service.ts +++ b/packages/auth/src/grant/service.ts @@ -11,6 +11,7 @@ import { AccessService } from '../access/service' export interface GrantService { get(grantId: string): Promise initiateGrant(grantRequest: GrantRequest): Promise + issueNoInteractionGrant(grantRequest: GrantRequest): Promise getByInteraction(interactId: string): Promise getByInteractionSession( interactId: string, @@ -76,7 +77,9 @@ export async function createGrantService({ return { get: (grantId: string) => get(grantId), initiateGrant: (grantRequest: GrantRequest, trx?: Transaction) => - initiateGrant(deps, grantRequest, trx), + initiateGrant(deps, grantRequest, true, trx), + issueNoInteractionGrant: (grantRequest: GrantRequest, trx?: Transaction) => + initiateGrant(deps, grantRequest, false, trx), getByInteraction: (interactId: string) => getByInteraction(interactId), getByInteractionSession: (interactId: string, interactNonce: string) => getByInteractionSession(interactId, interactNonce), @@ -115,6 +118,7 @@ async function rejectGrant( async function initiateGrant( deps: ServiceDependencies, grantRequest: GrantRequest, + interaction: boolean, trx?: Transaction ): Promise { const { accessService, knex } = deps @@ -132,15 +136,17 @@ async function initiateGrant( const grantTrx = trx || (await Grant.startTransaction(knex)) try { const grant = await Grant.query(grantTrx).insert({ - state: GrantState.Pending, + state: interaction ? GrantState.Pending : GrantState.Granted, startMethod: start, finishMethod: finish?.method, finishUri: finish?.uri, clientNonce: finish?.nonce, clientKeyId: kid, - interactId: v4(), - interactRef: v4(), - interactNonce: crypto.randomBytes(8).toString('hex').toUpperCase(), // TODO: factor out nonce generation + interactId: interaction ? v4() : undefined, + interactRef: interaction ? v4() : undefined, + interactNonce: interaction + ? crypto.randomBytes(8).toString('hex').toUpperCase() + : undefined, // TODO: factor out nonce generation continueId: v4(), continueToken: crypto.randomBytes(8).toString('hex').toUpperCase() }) From 96767a98d042bad1c41c0ab3ba58fe457f4bd6fc Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Fri, 2 Sep 2022 13:06:20 +0200 Subject: [PATCH 02/15] test(auth): remove tests that are caught by request validator --- packages/auth/src/grant/routes.test.ts | 53 -------------------------- 1 file changed, 53 deletions(-) diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index 8c35a16c04..b26ce4293d 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -146,8 +146,6 @@ describe('Grant Routes', (): void => { const nbfDate = new Date() nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) - const url = '/' - const method = 'POST' test('Valid incoming payment grant', async (): Promise => { const incomingPaymentGrantRequest: GrantRequest = { @@ -213,57 +211,6 @@ describe('Grant Routes', (): void => { expect(validateGrantRequest(quoteGrantRequest)).toBe(true) }) - - test('Cannot create outgoing payment grant with unexpected limit payload', async (): Promise => { - const outgoingPaymentGrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.OutgoingPayment, - limits: { wrongLimitField: 'wrong' } - } - ] - } - } - - expect(validateGrantRequest(outgoingPaymentGrantRequest)).toBe(false) - }) - - test('Cannot create account grant with unexpected limit payload', async (): Promise => { - const accountGrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.Account, - limits: OUTGOING_PAYMENT_LIMIT - } - ] - } - } - - expect(validateGrantRequest(accountGrantRequest)).toBe(false) - }) - - test('Cannot create quote grant with unexpected limit payload', async (): Promise => { - const quoteGrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.Quote, - limits: OUTGOING_PAYMENT_LIMIT - } - ] - } - } - - expect(validateGrantRequest(quoteGrantRequest)).toBe(false) - }) }) describe('/create', (): void => { From c11c566798ae7e985d45848408398ea63da54837 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Fri, 2 Sep 2022 13:07:29 +0200 Subject: [PATCH 03/15] test(auth): fix typing --- packages/auth/src/grant/service.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index 4eab34e7be..ccad4eadf4 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -182,7 +182,7 @@ describe('Grant Service', (): void => { const fetchedGrant = await grantService.getByContinue( continueId, continueToken, - interactRef + interactRef as string ) expect(fetchedGrant?.id).toEqual(grant.id) expect(fetchedGrant?.continueId).toEqual(continueId) From add955205de442f5618cb8f524203eecce14387d Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Fri, 2 Sep 2022 13:20:26 +0200 Subject: [PATCH 04/15] fix(auth): add not nullable field continueId to grant --- packages/auth/src/accessToken/service.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/auth/src/accessToken/service.test.ts b/packages/auth/src/accessToken/service.test.ts index 37ff81db22..0331c3de14 100644 --- a/packages/auth/src/accessToken/service.test.ts +++ b/packages/auth/src/accessToken/service.test.ts @@ -207,21 +207,21 @@ describe('Access Token Service', (): void => { await token.$query(trx).patch({ expiresIn: 1000000 }) const result = await accessTokenService.revoke(token.managementId) expect(result).toBeUndefined() - token = await AccessToken.query(trx).findById(token.id) + token = (await AccessToken.query(trx).findById(token.id)) as AccessToken expect(token).toBeUndefined() }) test('Can revoke even if token has already expired', async (): Promise => { await token.$query(trx).patch({ expiresIn: -1 }) const result = await accessTokenService.revoke(token.managementId) expect(result).toBeUndefined() - token = await AccessToken.query(trx).findById(token.id) + token = (await AccessToken.query(trx).findById(token.id)) as AccessToken expect(token).toBeUndefined() }) test('Can revoke even if token has already been revoked', async (): Promise => { await token.$query(trx).delete() const result = await accessTokenService.revoke(token.id) expect(result).toBeUndefined() - token = await AccessToken.query(trx).findById(token.id) + token = (await AccessToken.query(trx).findById(token.id)) as AccessToken expect(token).toBeUndefined() }) }) @@ -234,6 +234,7 @@ describe('Access Token Service', (): void => { grant = await Grant.query(trx).insertAndFetch({ ...BASE_GRANT, continueToken: crypto.randomBytes(8).toString('hex').toUpperCase(), + continueId: v4(), interactId: v4(), interactRef: crypto.randomBytes(8).toString('hex').toUpperCase(), interactNonce: crypto.randomBytes(8).toString('hex').toUpperCase() @@ -261,9 +262,9 @@ describe('Access Token Service', (): void => { await token.$query(trx).patch({ expiresIn: -1 }) const result = await accessTokenService.rotate(token.managementId) expect(result.success).toBe(true) - token = await AccessToken.query(trx).findOne({ + token = (await AccessToken.query(trx).findOne({ managementId: result.success && result.managementId - }) + })) as AccessToken expect(token.value).not.toBe(originalTokenValue) }) test('Cannot rotate nonexistent token', async (): Promise => { From a438f8e13fdfc49c9dba6d77ba6c74d36d5b403a Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Fri, 2 Sep 2022 13:33:56 +0200 Subject: [PATCH 05/15] fix(auth): create grant request test suite --- packages/auth/src/grant/routes.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index b26ce4293d..8641658c28 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -334,8 +334,18 @@ describe('Grant Routes', (): void => { {} ) - ctx.request.body = BASE_GRANT_REQUEST - ctx.request.body.access_token.access[0].type = AccessType.OutgoingPayment + ctx.request.body = { + ...BASE_GRANT_REQUEST, + access_token: { + access: [ + { + type: AccessType.OutgoingPayment, + actions: [Action.Create, Action.Read, Action.List], + identifier: `https://example.com/${v4()}` + } + ] + } + } await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() From 0c3f58fef5ed749278eec86c097be5632880478b Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Mon, 5 Sep 2022 10:58:38 +0200 Subject: [PATCH 06/15] fix(auth): unnecessary typecast, check for incoming payment type --- packages/auth/src/accessToken/service.test.ts | 10 ++++++---- packages/auth/src/grant/routes.ts | 4 ++-- packages/auth/src/grant/service.test.ts | 16 +++++++++------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/auth/src/accessToken/service.test.ts b/packages/auth/src/accessToken/service.test.ts index 0331c3de14..8cd9af3410 100644 --- a/packages/auth/src/accessToken/service.test.ts +++ b/packages/auth/src/accessToken/service.test.ts @@ -207,15 +207,17 @@ describe('Access Token Service', (): void => { await token.$query(trx).patch({ expiresIn: 1000000 }) const result = await accessTokenService.revoke(token.managementId) expect(result).toBeUndefined() - token = (await AccessToken.query(trx).findById(token.id)) as AccessToken - expect(token).toBeUndefined() + await expect( + AccessToken.query(trx).findById(token.id) + ).resolves.toBeUndefined() }) test('Can revoke even if token has already expired', async (): Promise => { await token.$query(trx).patch({ expiresIn: -1 }) const result = await accessTokenService.revoke(token.managementId) expect(result).toBeUndefined() - token = (await AccessToken.query(trx).findById(token.id)) as AccessToken - expect(token).toBeUndefined() + await expect( + AccessToken.query(trx).findById(token.id) + ).resolves.toBeUndefined() }) test('Can revoke even if token has already been revoked', async (): Promise => { await token.$query(trx).delete() diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index 1b8d6feb6b..c77ae7857b 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -109,12 +109,12 @@ async function createGrantInitiation( } if ( + !deps.config.incomingPaymentInteraction && body.access_token.access .map((acc) => { return isIncomingPaymentAccessRequest(acc as IncomingPaymentRequest) }) - .every((el) => el === true) && - !deps.config.incomingPaymentInteraction + .every((el) => el === true) ) { const grant = await grantService.issueNoInteractionGrant(body) const accessToken = await deps.accessTokenService.create(grant.id) diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index ccad4eadf4..0c1e13f90d 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -158,13 +158,15 @@ describe('Grant Service', (): void => { startMethod: expect.arrayContaining([StartMethod.Redirect]) }) - const dbAccessGrant = (await Access.query(trx) - .where({ - grantId: grant.id - }) - .first()) as Access - - expect(dbAccessGrant.type).toEqual(AccessType.IncomingPayment) + await expect( + Access.query(trx) + .where({ + grantId: grant.id + }) + .first() + ).resolves.toMatchObject({ + type: AccessType.IncomingPayment + }) }) }) From d938fce74ad5351f0e92cb06ea923011b04cf845 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Mon, 5 Sep 2022 10:58:54 +0200 Subject: [PATCH 07/15] feat(auth): add envBool function --- packages/auth/src/config/app.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/auth/src/config/app.ts b/packages/auth/src/config/app.ts index fd559daf1e..959fe7d5a7 100644 --- a/packages/auth/src/config/app.ts +++ b/packages/auth/src/config/app.ts @@ -10,6 +10,10 @@ function envInt(name: string, value: number): number { return envValue == null ? value : parseInt(envValue) } +function envBool(name: string, value: boolean): boolean { + const envValue = process.env[name] + return envValue == null ? value : envValue === 'true' +} export type IAppConfig = typeof Config export const Config = { @@ -38,7 +42,6 @@ export const Config = { accessTokenExpirySeconds: envInt('ACCESS_TOKEN_EXPIRY_SECONDS', 10 * 60), // Default 10 minutes databaseCleanupWorkers: envInt('DATABASE_CLEANUP_WORKERS', 1), accessTokenDeletionDays: envInt('ACCESS_TOKEN_DELETION_DAYS', 30), - introspectionHttpsig: process.env.INTROSPECTION_HTTPSIG === 'true', - incomingPaymentInteraction: - process.env.INCOMING_PAYMENT_INTERACTION === 'true' + introspectionHttpsig: envBool('INTROSPECTION_HTTPSIG', false), + incomingPaymentInteraction: envBool('INCOMING_PAYMENT_INTERACTION', false) } From d5d1c525c5b68f3567135f7a3870e7fbb2c50c4d Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Mon, 5 Sep 2022 11:44:21 +0200 Subject: [PATCH 08/15] fix(auth): remove another unneccessary typecast --- packages/auth/src/grant/service.test.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index 0c1e13f90d..cb20f47199 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -125,13 +125,15 @@ describe('Grant Service', (): void => { startMethod: expect.arrayContaining([StartMethod.Redirect]) }) - const dbAccessGrant = (await Access.query(trx) - .where({ - grantId: grant.id - }) - .first()) as Access - - expect(dbAccessGrant.type).toEqual(AccessType.IncomingPayment) + await expect( + Access.query(trx) + .where({ + grantId: grant.id + }) + .first() + ).resolves.toMatchObject({ + type: AccessType.IncomingPayment + }) }) test('Can issue a grant without interaction', async (): Promise => { const grantRequest: GrantRequest = { From c9b0d17f03fa232fd8d88b5af0ab16b33db0234b Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Thu, 22 Sep 2022 13:25:48 +0200 Subject: [PATCH 09/15] fix(auth): remove unneccessary validation function --- packages/auth/src/grant/routes.test.ts | 140 +------------------------ packages/auth/src/grant/routes.ts | 22 +--- 2 files changed, 2 insertions(+), 160 deletions(-) diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index 8641658c28..c84e2ee4ca 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -12,11 +12,10 @@ import { Config, IAppConfig } from '../config/app' import { initIocContainer } from '..' import { AppServices } from '../app' import { truncateTables } from '../tests/tableManager' -import { GrantRoutes, validateGrantRequest } from './routes' +import { GrantRoutes } from './routes' import { Action, AccessType } from '../access/types' import { Access } from '../access/model' import { Grant, StartMethod, FinishMethod, GrantState } from '../grant/model' -import { GrantRequest } from '../grant/service' import { AccessToken } from '../accessToken/model' export const KEY_REGISTRY_ORIGIN = 'https://openpayments.network' @@ -53,21 +52,6 @@ const BASE_GRANT_ACCESS = { identifier: `https://example.com/${v4()}` } -const OUTGOING_PAYMENT_LIMIT = { - sendAmount: { - value: '1000000000', - assetCode: 'usd', - assetScale: 9 - }, - receiveAmount: { - value: '2000000000', - assetCode: 'usd', - assetScale: 9 - }, - expiresAt: new Date().toISOString(), - receiver: 'https://wallet.com/alice' -} - const BASE_GRANT_REQUEST = { access_token: { access: [ @@ -140,79 +124,6 @@ describe('Grant Routes', (): void => { await appContainer.shutdown() }) - describe('Grant request validation', (): void => { - const expDate = new Date() - expDate.setTime(expDate.getTime() + 1000 * 60 * 60) - - const nbfDate = new Date() - nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) - - test('Valid incoming payment grant', async (): Promise => { - const incomingPaymentGrantRequest: GrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.IncomingPayment - } - ] - } - } - - expect(validateGrantRequest(incomingPaymentGrantRequest)).toBe(true) - }) - - test('Valid outgoing payment grant', async (): Promise => { - const outgoingPaymentGrantRequest: GrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.OutgoingPayment, - limits: OUTGOING_PAYMENT_LIMIT - } - ] - } - } - - expect(validateGrantRequest(outgoingPaymentGrantRequest)).toBe(true) - }) - - test('Valid account grant', async (): Promise => { - const accountGrantRequest: GrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.Account - } - ] - } - } - - expect(validateGrantRequest(accountGrantRequest)).toBe(true) - }) - - test('Valid quote grant', async (): Promise => { - const quoteGrantRequest: GrantRequest = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - ...BASE_GRANT_ACCESS, - type: AccessType.Quote - } - ] - } - } - - expect(validateGrantRequest(quoteGrantRequest)).toBe(true) - }) - }) - describe('/create', (): void => { const url = '/' const method = 'POST' @@ -257,55 +168,6 @@ describe('Grant Routes', (): void => { expect(ctx.body).toEqual({ error: 'invalid_request' }) }) - test('Cannot initiate grant with invalid grant request', async (): Promise => { - const expDate = new Date() - expDate.setTime(expDate.getTime() + 1000 * 60 * 60) - - const nbfDate = new Date() - nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) - - const scope = nock(KEY_REGISTRY_ORIGIN) - .get(KID_PATH) - .reply(200, { - ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), - revoked: false - }) - - const ctx = createContext( - { - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - url, - method - }, - {} - ) - - ctx.request.body = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - type: 'unknown-type', - actions: [Action.Create, Action.Read, Action.List] - } - ] - } - } - - await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - expect(ctx.status).toBe(400) - expect(ctx.body).toEqual({ - error: 'invalid_request' - }) - - scope.isDone() - }) - test('Can initiate a grant request', async (): Promise => { const expDate = new Date() expDate.setTime(expDate.getTime() + 1000 * 60 * 60) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index c77ae7857b..bfae59e9e4 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -1,14 +1,13 @@ import * as crypto from 'crypto' import { URL } from 'url' import { AppContext } from '../app' -import { GrantService, GrantRequest } from './service' +import { GrantService } from './service' import { Grant, GrantState } from './model' import { Access } from '../access/model' import { ClientService } from '../client/service' import { BaseService } from '../shared/baseService' import { IncomingPaymentRequest, - isAccessRequest, isIncomingPaymentAccessRequest } from '../access/types' import { IAppConfig } from '../config/app' @@ -71,20 +70,6 @@ export function createGrantRoutes({ } } -// exported for testing -export function validateGrantRequest( - grantRequest: GrantRequest -): grantRequest is GrantRequest { - if (typeof grantRequest.access_token !== 'object') return false - const { access_token } = grantRequest - if (typeof access_token.access !== 'object') return false - for (const access of access_token.access) { - if (!isAccessRequest(access)) return false - } - - return grantRequest.interact?.start !== undefined -} - async function createGrantInitiation( deps: ServiceDependencies, ctx: AppContext @@ -102,11 +87,6 @@ async function createGrantInitiation( const { body } = ctx.request const { grantService, config } = deps - if (!validateGrantRequest(body)) { - ctx.status = 400 - ctx.body = { error: 'invalid_request' } - return - } if ( !deps.config.incomingPaymentInteraction && From 172d0dc5f56e14dc242bfa05751be1afcd702409 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Thu, 22 Sep 2022 13:40:24 +0200 Subject: [PATCH 10/15] chore(auth): update reference to API spec --- packages/auth/src/config/app.ts | 2 +- packages/auth/src/grant/routes.test.ts | 28 +++++++++++++------------- packages/auth/src/grant/routes.ts | 16 +++++++-------- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/auth/src/config/app.ts b/packages/auth/src/config/app.ts index 959fe7d5a7..41de8e7761 100644 --- a/packages/auth/src/config/app.ts +++ b/packages/auth/src/config/app.ts @@ -29,7 +29,7 @@ export const Config = { ), authServerSpec: envString( 'AUTH_SERVER_SPEC', - 'https://raw.githubusercontent.com/interledger/open-payments/53236cc92a070ec98a7ab31c6ed0f6fc50a98041/auth-server-open-api-spec.yaml' + 'https://raw.githubusercontent.com/interledger/open-payments/16cff7a2605cb90d0e5b5d34ba32c8263d694f2b/auth-server-open-api-spec.yaml' ), identityServerDomain: envString( 'IDENTITY_SERVER_DOMAIN', diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index c84e2ee4ca..a3b9cbf2e2 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -211,7 +211,7 @@ describe('Grant Routes', (): void => { await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() - expect(ctx.status).toBe(201) + expect(ctx.status).toBe(200) expect(ctx.body).toEqual({ interact: { redirect: expect.any(String), @@ -260,20 +260,20 @@ describe('Grant Routes', (): void => { ctx.request.body = BASE_GRANT_REQUEST await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() - // expect(ctx.response).toSatisfyApiSpec() + expect(ctx.response).toSatisfyApiSpec() expect(ctx.status).toBe(200) expect(ctx.body).toEqual({ access_token: { value: expect.any(String), manage: expect.any(String), access: BASE_GRANT_REQUEST.access_token.access, - continue: { - access_token: { - value: expect.any(String) - }, - uri: expect.any(String) - }, expiresIn: 600 + }, + continue: { + access_token: { + value: expect.any(String) + }, + uri: expect.any(String) } }) @@ -803,13 +803,13 @@ describe('Grant Routes', (): void => { type: 'incoming-payment' } ]), - continue: { - access_token: { - value: expect.any(String) - }, - uri: expect.any(String) - }, expiresIn: 600 + }, + continue: { + access_token: { + value: expect.any(String) + }, + uri: expect.any(String) } }) }) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index bfae59e9e4..a7eb10ce65 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -110,7 +110,7 @@ async function createGrantInitiation( } const grant = await grantService.initiateGrant(body) - ctx.status = 201 + ctx.status = 200 ctx.body = { interact: { redirect: config.authServerDomain + `/interact/${grant.interactId}`, @@ -410,13 +410,13 @@ function createGrantBody( value: accessToken.value, manage: domain + `/token/${accessToken.managementId}`, access: access.map((a: Access) => accessToBody(a)), - expiresIn: accessToken.expiresIn, - continue: { - access_token: { - value: grant.continueToken - }, - uri: domain + `continue/${grant.continueId}` - } + expiresIn: accessToken.expiresIn + }, + continue: { + access_token: { + value: grant.continueToken + }, + uri: domain + `continue/${grant.continueId}` } } } From dee2671f849993b94f0fcfe5794183a677c0c099 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Thu, 22 Sep 2022 13:53:32 +0200 Subject: [PATCH 11/15] test(auth): avoid typecasting --- packages/auth/src/accessToken/service.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/auth/src/accessToken/service.test.ts b/packages/auth/src/accessToken/service.test.ts index 8cd9af3410..330da71806 100644 --- a/packages/auth/src/accessToken/service.test.ts +++ b/packages/auth/src/accessToken/service.test.ts @@ -223,8 +223,9 @@ describe('Access Token Service', (): void => { await token.$query(trx).delete() const result = await accessTokenService.revoke(token.id) expect(result).toBeUndefined() - token = (await AccessToken.query(trx).findById(token.id)) as AccessToken - expect(token).toBeUndefined() + await expect( + AccessToken.query(trx).findById(token.id) + ).resolves.toBeUndefined() }) }) @@ -264,10 +265,11 @@ describe('Access Token Service', (): void => { await token.$query(trx).patch({ expiresIn: -1 }) const result = await accessTokenService.rotate(token.managementId) expect(result.success).toBe(true) - token = (await AccessToken.query(trx).findOne({ + const rotatedToken = await AccessToken.query(trx).findOne({ managementId: result.success && result.managementId - })) as AccessToken - expect(token.value).not.toBe(originalTokenValue) + }) + expect(rotatedToken).toBeDefined() + expect(rotatedToken?.value).not.toBe(originalTokenValue) }) test('Cannot rotate nonexistent token', async (): Promise => { const result = await accessTokenService.rotate(v4()) From aed10d4ddb40426ac562f7bcd381e5ffa04c5966 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Thu, 22 Sep 2022 14:41:47 +0200 Subject: [PATCH 12/15] feat(auth): check that on non-incoming-payment grant request, interact is present --- .../20220504161932_create_grants_table.js | 4 +- packages/auth/src/grant/routes.test.ts | 93 ++++++++++++------- packages/auth/src/grant/routes.ts | 12 ++- packages/auth/src/grant/service.test.ts | 12 +-- packages/auth/src/grant/service.ts | 32 +++---- 5 files changed, 92 insertions(+), 61 deletions(-) diff --git a/packages/auth/migrations/20220504161932_create_grants_table.js b/packages/auth/migrations/20220504161932_create_grants_table.js index 2d2a15fe36..0a35a91cbd 100644 --- a/packages/auth/migrations/20220504161932_create_grants_table.js +++ b/packages/auth/migrations/20220504161932_create_grants_table.js @@ -2,8 +2,8 @@ exports.up = function (knex) { return knex.schema.createTable('grants', function (table) { table.uuid('id').notNullable().primary() - table.string('state').notNullable() - table.specificType('startMethod', 'text[]').notNullable() + table.string('state') + table.specificType('startMethod', 'text[]') table.string('continueToken').notNullable().unique() table.string('continueId').notNullable().unique() diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index a3b9cbf2e2..ec56970141 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -56,7 +56,7 @@ const BASE_GRANT_REQUEST = { access_token: { access: [ { - type: AccessType.IncomingPayment, + type: AccessType.OutgoingPayment, actions: [Action.Create, Action.Read, Action.List], identifier: `https://example.com/${v4()}` } @@ -128,6 +128,15 @@ describe('Grant Routes', (): void => { const url = '/' const method = 'POST' + const expDate = new Date() + expDate.setTime(expDate.getTime() + 1000 * 60 * 60) + + const nbfDate = new Date() + nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) + + const exp = Math.round(expDate.getTime() / 1000) + const nbf = Math.round(nbfDate.getTime() / 1000) + test('accepts json only', async (): Promise => { const ctx = createContext( { @@ -169,18 +178,12 @@ describe('Grant Routes', (): void => { }) test('Can initiate a grant request', async (): Promise => { - const expDate = new Date() - expDate.setTime(expDate.getTime() + 1000 * 60 * 60) - - const nbfDate = new Date() - nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) - const scope = nock(KEY_REGISTRY_ORIGIN) .get(KID_PATH) .reply(200, { ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), + exp, + nbf, revoked: false }) @@ -196,18 +199,7 @@ describe('Grant Routes', (): void => { {} ) - ctx.request.body = { - ...BASE_GRANT_REQUEST, - access_token: { - access: [ - { - type: AccessType.OutgoingPayment, - actions: [Action.Create, Action.Read, Action.List], - identifier: `https://example.com/${v4()}` - } - ] - } - } + ctx.request.body = BASE_GRANT_REQUEST await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() @@ -230,18 +222,12 @@ describe('Grant Routes', (): void => { }) test('Can get a software-only authorization grant', async (): Promise => { - const expDate = new Date() - expDate.setTime(expDate.getTime() + 1000 * 60 * 60) - - const nbfDate = new Date() - nbfDate.setTime(nbfDate.getTime() - 1000 * 60 * 60) - const scope = nock(KEY_REGISTRY_ORIGIN) .get(KID_PATH) .reply(200, { ...TEST_CLIENT_KEY.jwk, - exp: Math.round(expDate.getTime() / 1000), - nbf: Math.round(nbfDate.getTime() / 1000), + exp, + nbf, revoked: false }) @@ -256,8 +242,22 @@ describe('Grant Routes', (): void => { }, {} ) - - ctx.request.body = BASE_GRANT_REQUEST + const body = { + access_token: { + access: [ + { + type: AccessType.IncomingPayment, + actions: [Action.Create, Action.Read, Action.List], + identifier: `https://example.com/${v4()}` + } + ] + }, + client: { + display: TEST_CLIENT_DISPLAY, + key: TEST_CLIENT_KEY + } + } + ctx.request.body = body await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() expect(ctx.response).toSatisfyApiSpec() @@ -266,7 +266,7 @@ describe('Grant Routes', (): void => { access_token: { value: expect.any(String), manage: expect.any(String), - access: BASE_GRANT_REQUEST.access_token.access, + access: body.access_token.access, expiresIn: 600 }, continue: { @@ -277,6 +277,35 @@ describe('Grant Routes', (): void => { } }) + scope.isDone() + }) + test('Fails to initiate a grant w/o interact field', async (): Promise => { + const scope = nock(KEY_REGISTRY_ORIGIN) + .get(KID_PATH) + .reply(200, { + ...TEST_CLIENT_KEY.jwk, + exp, + nbf, + revoked: false + }) + + const ctx = createContext( + { + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + }, + url, + method + }, + {} + ) + + ctx.request.body = { ...BASE_GRANT_REQUEST, interact: undefined } + + await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() + expect(ctx.status).toBe(400) + scope.isDone() }) }) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index a7eb10ce65..5620205c86 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -96,7 +96,7 @@ async function createGrantInitiation( }) .every((el) => el === true) ) { - const grant = await grantService.issueNoInteractionGrant(body) + const grant = await grantService.create(body) const accessToken = await deps.accessTokenService.create(grant.id) const access = await deps.accessService.getByGrant(grant.id) ctx.status = 200 @@ -109,7 +109,15 @@ async function createGrantInitiation( return } - const grant = await grantService.initiateGrant(body) + if (!body.interact) { + ctx.status = 400 + ctx.body = { + error: 'interaction_required' + } + return + } + + const grant = await grantService.create(body) ctx.status = 200 ctx.body = { interact: { diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index cb20f47199..ea0c20a7e0 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -109,7 +109,7 @@ describe('Grant Service', (): void => { } } - const grant = await grantService.initiateGrant(grantRequest) + const grant = await grantService.create(grantRequest) expect(grant).toMatchObject({ state: GrantState.Pending, @@ -145,19 +145,17 @@ describe('Grant Service', (): void => { type: AccessType.IncomingPayment } ] - } + }, + interact: undefined } - const grant = await grantService.issueNoInteractionGrant(grantRequest) + const grant = await grantService.create(grantRequest) expect(grant).toMatchObject({ state: GrantState.Granted, continueId: expect.any(String), continueToken: expect.any(String), - finishMethod: FinishMethod.Redirect, - finishUri: BASE_GRANT_REQUEST.interact.finish.uri, - clientKeyId: BASE_GRANT_REQUEST.client.key.jwk.kid, - startMethod: expect.arrayContaining([StartMethod.Redirect]) + clientKeyId: BASE_GRANT_REQUEST.client.key.jwk.kid }) await expect( diff --git a/packages/auth/src/grant/service.ts b/packages/auth/src/grant/service.ts index bd605e8661..eb08c06973 100644 --- a/packages/auth/src/grant/service.ts +++ b/packages/auth/src/grant/service.ts @@ -10,8 +10,7 @@ import { AccessService } from '../access/service' export interface GrantService { get(grantId: string): Promise - initiateGrant(grantRequest: GrantRequest): Promise - issueNoInteractionGrant(grantRequest: GrantRequest): Promise + create(grantRequest: GrantRequest, trx?: Transaction): Promise getByInteraction(interactId: string): Promise getByInteractionSession( interactId: string, @@ -37,7 +36,7 @@ export interface GrantRequest { access: AccessRequest[] } client: ClientInfo - interact: { + interact?: { start: StartMethod[] finish?: { method: FinishMethod @@ -76,10 +75,8 @@ export async function createGrantService({ } return { get: (grantId: string) => get(grantId), - initiateGrant: (grantRequest: GrantRequest, trx?: Transaction) => - initiateGrant(deps, grantRequest, true, trx), - issueNoInteractionGrant: (grantRequest: GrantRequest, trx?: Transaction) => - initiateGrant(deps, grantRequest, false, trx), + create: (grantRequest: GrantRequest, trx?: Transaction) => + create(deps, grantRequest, trx), getByInteraction: (interactId: string) => getByInteraction(interactId), getByInteractionSession: (interactId: string, interactNonce: string) => getByInteractionSession(interactId, interactNonce), @@ -115,17 +112,16 @@ async function rejectGrant( }) } -async function initiateGrant( +async function create( deps: ServiceDependencies, grantRequest: GrantRequest, - interaction: boolean, trx?: Transaction ): Promise { const { accessService, knex } = deps const { access_token: { access }, - interact: { start, finish }, + interact, client: { key: { jwk: { kid } @@ -136,15 +132,15 @@ async function initiateGrant( const grantTrx = trx || (await Grant.startTransaction(knex)) try { const grant = await Grant.query(grantTrx).insert({ - state: interaction ? GrantState.Pending : GrantState.Granted, - startMethod: start, - finishMethod: finish?.method, - finishUri: finish?.uri, - clientNonce: finish?.nonce, + state: interact ? GrantState.Pending : GrantState.Granted, + startMethod: interact?.start, + finishMethod: interact?.finish?.method, + finishUri: interact?.finish?.uri, + clientNonce: interact?.finish?.nonce, clientKeyId: kid, - interactId: interaction ? v4() : undefined, - interactRef: interaction ? v4() : undefined, - interactNonce: interaction + interactId: interact ? v4() : undefined, + interactRef: interact ? v4() : undefined, + interactNonce: interact ? crypto.randomBytes(8).toString('hex').toUpperCase() : undefined, // TODO: factor out nonce generation continueId: v4(), From bb9954627072bedd0dd71f329c25d8c84f3f4951 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Thu, 22 Sep 2022 17:46:10 +0200 Subject: [PATCH 13/15] refactor(aut): add Brandon's suggestions --- packages/auth/src/grant/routes.ts | 27 +++++++++++++++---------- packages/auth/src/grant/service.test.ts | 10 ++++----- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index 5620205c86..5286615e59 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -100,12 +100,12 @@ async function createGrantInitiation( const accessToken = await deps.accessTokenService.create(grant.id) const access = await deps.accessService.getByGrant(grant.id) ctx.status = 200 - ctx.body = createGrantBody( - deps.config.authServerDomain, + ctx.body = createGrantBody({ + domain: deps.config.authServerDomain, grant, access, accessToken - ) + }) return } @@ -399,20 +399,25 @@ async function continueGrant( const access = await accessService.getByGrant(grant.id) // TODO: add "continue" to response if additional grant request steps are added - ctx.body = createGrantBody( - config.authServerDomain, + ctx.body = createGrantBody({ + domain: config.authServerDomain, grant, access, accessToken - ) + }) } -function createGrantBody( - domain: string, - grant: Grant, - access: Access[], +function createGrantBody({ + domain, + grant, + access, + accessToken +}: { + domain: string + grant: Grant + access: Access[] accessToken: AccessToken -) { +}) { return { access_token: { value: accessToken.value, diff --git a/packages/auth/src/grant/service.test.ts b/packages/auth/src/grant/service.test.ts index ea0c20a7e0..5cc3b1fdcd 100644 --- a/packages/auth/src/grant/service.test.ts +++ b/packages/auth/src/grant/service.test.ts @@ -1,3 +1,4 @@ +import assert from 'assert' import crypto from 'crypto' import { Knex } from 'knex' import { v4 } from 'uuid' @@ -180,11 +181,12 @@ describe('Grant Service', (): void => { describe('continue', (): void => { test('Can fetch a grant by its continuation information', async (): Promise => { const { continueId, continueToken, interactRef } = grant + assert.ok(interactRef) const fetchedGrant = await grantService.getByContinue( continueId, continueToken, - interactRef as string + interactRef ) expect(fetchedGrant?.id).toEqual(grant.id) expect(fetchedGrant?.continueId).toEqual(continueId) @@ -199,10 +201,8 @@ describe('Grant Service', (): void => { expect(fetchedGrant?.id).toEqual(grant.id) }) test('Can fetch a grant by its interaction information', async (): Promise => { - const fetchedGrant = await grantService.getByInteraction( - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - grant.interactId! - ) + assert.ok(grant.interactId) + const fetchedGrant = await grantService.getByInteraction(grant.interactId) expect(fetchedGrant?.id).toEqual(grant.id) expect(fetchedGrant?.interactId).toEqual(grant.interactId) }) From 428cd925e2e8fffb0d3f6dff8ff811ff83ea3cf4 Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Mon, 26 Sep 2022 10:13:48 +0200 Subject: [PATCH 14/15] feat(auth): create grant and access token within trx --- packages/auth/src/accessToken/service.ts | 2 +- packages/auth/src/grant/routes.ts | 31 +++++++++++++++--------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/accessToken/service.ts b/packages/auth/src/accessToken/service.ts index be0ad4f31e..adf5741fc0 100644 --- a/packages/auth/src/accessToken/service.ts +++ b/packages/auth/src/accessToken/service.ts @@ -136,7 +136,7 @@ async function createAccessToken( grantId: string, opts?: AccessTokenOpts ): Promise { - return AccessToken.query(deps.knex).insert({ + return AccessToken.query(opts?.trx || deps.knex).insert({ value: crypto.randomBytes(8).toString('hex').toUpperCase(), managementId: v4(), grantId, diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index 5286615e59..d5b117e79e 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -96,17 +96,26 @@ async function createGrantInitiation( }) .every((el) => el === true) ) { - const grant = await grantService.create(body) - const accessToken = await deps.accessTokenService.create(grant.id) - const access = await deps.accessService.getByGrant(grant.id) - ctx.status = 200 - ctx.body = createGrantBody({ - domain: deps.config.authServerDomain, - grant, - access, - accessToken - }) - return + const trx = await Grant.startTransaction() + try { + const grant = await grantService.create(body, trx) + const accessToken = await deps.accessTokenService.create(grant.id, { + trx + }) + await trx.commit() + const access = await deps.accessService.getByGrant(grant.id) + ctx.status = 200 + ctx.body = createGrantBody({ + domain: deps.config.authServerDomain, + grant, + access, + accessToken + }) + return + } catch (err) { + await trx.rollback() + throw err + } } if (!body.interact) { From bd2c4c91037ed5885700e3cd5c8012edacff5aad Mon Sep 17 00:00:00 2001 From: Sabine Schaller Date: Tue, 27 Sep 2022 10:48:01 +0200 Subject: [PATCH 15/15] fix(auth): deal with internal server error on grant and token creation --- packages/auth/src/grant/routes.test.ts | 48 ++++++++++++++++++++++++++ packages/auth/src/grant/routes.ts | 27 ++++++++------- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/packages/auth/src/grant/routes.test.ts b/packages/auth/src/grant/routes.test.ts index ec56970141..ad9124bed7 100644 --- a/packages/auth/src/grant/routes.test.ts +++ b/packages/auth/src/grant/routes.test.ts @@ -17,6 +17,7 @@ import { Action, AccessType } from '../access/types' import { Access } from '../access/model' import { Grant, StartMethod, FinishMethod, GrantState } from '../grant/model' import { AccessToken } from '../accessToken/model' +import { AccessTokenService } from '../accessToken/service' export const KEY_REGISTRY_ORIGIN = 'https://openpayments.network' export const KID_PATH = '/keys/base-test-key' @@ -96,6 +97,7 @@ describe('Grant Routes', (): void => { let knex: Knex let grantRoutes: GrantRoutes let config: IAppConfig + let accessTokenService: AccessTokenService let grant: Grant beforeEach(async (): Promise => { @@ -114,6 +116,7 @@ describe('Grant Routes', (): void => { knex = await deps.use('knex') appContainer = await createTestApp(deps) jestOpenAPI(await deps.use('openApi')) + accessTokenService = await deps.use('accessTokenService') }) afterEach(async (): Promise => { @@ -279,6 +282,51 @@ describe('Grant Routes', (): void => { scope.isDone() }) + test('Does not create grant if token issuance fails', async (): Promise => { + jest + .spyOn(accessTokenService, 'create') + .mockRejectedValueOnce(new Error()) + const scope = nock(KEY_REGISTRY_ORIGIN) + .get(KID_PATH) + .reply(200, { + ...TEST_CLIENT_KEY.jwk, + exp, + nbf, + revoked: false + }) + + const ctx = createContext( + { + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json' + }, + url, + method + }, + {} + ) + const body = { + access_token: { + access: [ + { + type: AccessType.IncomingPayment, + actions: [Action.Create, Action.Read, Action.List], + identifier: `https://example.com/${v4()}` + } + ] + }, + client: { + display: TEST_CLIENT_DISPLAY, + key: TEST_CLIENT_KEY + } + } + ctx.request.body = body + + await expect(grantRoutes.create(ctx)).resolves.toBeUndefined() + expect(ctx.status).toBe(500) + scope.isDone() + }) test('Fails to initiate a grant w/o interact field', async (): Promise => { const scope = nock(KEY_REGISTRY_ORIGIN) .get(KID_PATH) diff --git a/packages/auth/src/grant/routes.ts b/packages/auth/src/grant/routes.ts index d5b117e79e..5c1e0a9c55 100644 --- a/packages/auth/src/grant/routes.ts +++ b/packages/auth/src/grant/routes.ts @@ -97,25 +97,28 @@ async function createGrantInitiation( .every((el) => el === true) ) { const trx = await Grant.startTransaction() + let grant: Grant + let accessToken: AccessToken try { - const grant = await grantService.create(body, trx) - const accessToken = await deps.accessTokenService.create(grant.id, { + grant = await grantService.create(body, trx) + accessToken = await deps.accessTokenService.create(grant.id, { trx }) await trx.commit() - const access = await deps.accessService.getByGrant(grant.id) - ctx.status = 200 - ctx.body = createGrantBody({ - domain: deps.config.authServerDomain, - grant, - access, - accessToken - }) - return } catch (err) { await trx.rollback() - throw err + ctx.status = 500 + return } + const access = await deps.accessService.getByGrant(grant.id) + ctx.status = 200 + ctx.body = createGrantBody({ + domain: deps.config.authServerDomain, + grant, + access, + accessToken + }) + return } if (!body.interact) {