Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(backend): request grant to query incoming payment receiver #779

Merged
merged 12 commits into from
Dec 6, 2022
Merged
1 change: 1 addition & 0 deletions infrastructure/local/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ services:
REDIS_URL: redis://redis:6379/0
QUOTE_URL: http://fynbos/quotes
BYPASS_SIGNATURE_VALIDATION: "true"
PAYMENT_POINTER_URL: https://backend/.well-known/pay
depends_on:
- tigerbeetle
- database
Expand Down
10 changes: 7 additions & 3 deletions infrastructure/local/peer-docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ services:
dockerfile: ./packages/auth/Dockerfile
restart: always
networks:
rafiki:
local_rafiki:
wilsonianb marked this conversation as resolved.
Show resolved Hide resolved
ports:
- "4006:3006"
environment:
Expand All @@ -27,7 +27,7 @@ services:
- "4000:80"
- "4001:3001"
networks:
rafiki:
local_rafiki:
environment:
NODE_ENV: development
LOG_LEVEL: debug
Expand All @@ -51,13 +51,14 @@ services:
REDIS_URL: redis://redis:6379/1
QUOTE_URL: http://local-bank/quote
BYPASS_SIGNATURE_VALIDATION: "true"
PAYMENT_POINTER_URL: https://peer-backend/.well-known/pay
local-bank:
build:
context: ../..
dockerfile: ./packages/mock-account-provider/Dockerfile
restart: always
networks:
rafiki:
local_rafiki:
ports:
- '3031:80'
environment:
Expand All @@ -69,3 +70,6 @@ services:
- ./seed.peer.yml:/workspace/seed.peer.yml
depends_on:
- peer-backend
networks:
local_rafiki:
external: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this after noticing 👇 while testing:

$ docker compose -f infrastructure/local/peer-docker-compose.yml ps
service "peer-auth" refers to undefined network rafiki: invalid compose project

However, I am now sometimes getting the following when starting the localenv:

network local_rafiki declared as external, but could not be found

I wonder if that can be avoided by splitting these into separate/subsequent docker compose commands:

rafiki/package.json

Lines 20 to 21 in 632bdfb

"localenv": "docker compose -f ./infrastructure/local/docker-compose.yml -f ./infrastructure/local/peer-docker-compose.yml",
"localenv:build": "docker compose -f ./infrastructure/local/docker-compose.yml -f ./infrastructure/local/peer-docker-compose.yml -f ./infrastructure/local/build-override.yml build",

22 changes: 2 additions & 20 deletions packages/auth/src/access/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
export enum AccessType {
Account = 'account',
IncomingPayment = 'incoming-payment',
OutgoingPayment = 'outgoing-payment',
Quote = 'quote'
Expand All @@ -8,15 +7,15 @@ export enum AccessType {
export enum Action {
Create = 'create',
Read = 'read',
ReadAll = 'read-all',
List = 'list',
ListAll = 'list-all',
Complete = 'complete'
}

interface BaseAccessRequest {
actions: Action[]
locations?: string[]
identifier?: string
interval?: string
wilsonianb marked this conversation as resolved.
Show resolved Hide resolved
}

export interface IncomingPaymentRequest extends BaseAccessRequest {
Expand All @@ -29,11 +28,6 @@ interface OutgoingPaymentRequest extends BaseAccessRequest {
limits?: OutgoingPaymentLimit
}

interface AccountRequest extends BaseAccessRequest {
type: AccessType.Account
limits?: never
}

interface QuoteRequest extends BaseAccessRequest {
type: AccessType.Quote
limits?: never
Expand All @@ -42,7 +36,6 @@ interface QuoteRequest extends BaseAccessRequest {
export type AccessRequest =
| IncomingPaymentRequest
| OutgoingPaymentRequest
| AccountRequest
| QuoteRequest

export function isAccessType(accessType: AccessType): accessType is AccessType {
Expand Down Expand Up @@ -78,16 +71,6 @@ function isOutgoingPaymentAccessRequest(
)
}

function isAccountAccessRequest(
accessRequest: AccountRequest
): accessRequest is AccountRequest {
return (
accessRequest.type === AccessType.Account &&
isAction(accessRequest.actions) &&
!accessRequest.limits
)
}

function isQuoteAccessRequest(
accessRequest: QuoteRequest
): accessRequest is QuoteRequest {
Expand All @@ -104,7 +87,6 @@ export function isAccessRequest(
return (
isIncomingPaymentAccessRequest(accessRequest as IncomingPaymentRequest) ||
isOutgoingPaymentAccessRequest(accessRequest as OutgoingPaymentRequest) ||
isAccountAccessRequest(accessRequest as AccountRequest) ||
isQuoteAccessRequest(accessRequest as QuoteRequest)
)
}
Expand Down
1 change: 0 additions & 1 deletion packages/auth/src/grant/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ describe('Grant Service', (): void => {

const BASE_GRANT_ACCESS = {
actions: [Action.Create, Action.Read, Action.List],
locations: ['https://example.com'],
identifier: `https://example.com/${v4()}`
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
exports.up = function (knex) {
return knex.schema.createTable('authServers', function (table) {
table.uuid('id').notNullable().primary()
table.string('url').notNullable().unique()
table.timestamp('createdAt').defaultTo(knex.fn.now())
table.timestamp('updatedAt').defaultTo(knex.fn.now())
})
}

exports.down = function (knex) {
return knex.schema.dropTableIfExists('authServers')
}
23 changes: 23 additions & 0 deletions packages/backend/migrations/20221012013413_create_grants_table.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
exports.up = function (knex) {
return knex.schema.createTable('grants', function (table) {
table.uuid('id').notNullable().primary()
table.uuid('authServerId').notNullable()
table.foreign('authServerId').references('authServers.id')
table.string('continueId').nullable()
table.string('continueToken').nullable()
table.string('accessToken').nullable().unique()
table.string('accessType').notNullable()
table.specificType('accessActions', 'text[]')

table.timestamp('expiresAt').nullable()

table.timestamp('createdAt').defaultTo(knex.fn.now())
table.timestamp('updatedAt').defaultTo(knex.fn.now())

table.unique(['authServerId', 'accessType', 'accessActions'])
})
}
Comment on lines +1 to +19
Copy link
Member

Choose a reason for hiding this comment

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

Is it sufficient to not store the entire access array of the grant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is for now since the RS is only requesting read-all incoming payment grants, and the new GrantService only supports creating grants of a single access type.
Do we anticipate the RS requesting multi-type grants that we should consider future-proofing for?


exports.down = function (knex) {
return knex.schema.dropTableIfExists('grants')
}
22 changes: 18 additions & 4 deletions packages/backend/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import { createAssetService } from './asset/service'
import { createAccountingService } from './accounting/service'
import { createPeerService } from './peer/service'
import { createAuthService } from './open_payments/auth/service'

import { createAuthServerService } from './open_payments/authServer/service'
import { createGrantService } from './open_payments/grant/service'
import { createPaymentPointerService } from './open_payments/payment_pointer/service'
import { createSPSPRoutes } from './spsp/routes'
import { createPaymentPointerKeyRoutes } from './paymentPointerKey/routes'
Expand Down Expand Up @@ -176,6 +177,19 @@ export function initIocContainer(
authOpenApi: await deps.use('authOpenApi')
})
})
container.singleton('authServerService', async (deps) => {
return await createAuthServerService({
logger: await deps.use('logger'),
knex: await deps.use('knex')
})
})
container.singleton('grantService', async (deps) => {
return await createGrantService({
authServerService: await deps.use('authServerService'),
logger: await deps.use('logger'),
knex: await deps.use('knex')
})
})
container.singleton('paymentPointerService', async (deps) => {
const logger = await deps.use('logger')
const assetService = await deps.use('assetService')
Expand Down Expand Up @@ -218,8 +232,9 @@ export function initIocContainer(
})
})
container.singleton('paymentPointerRoutes', async (deps) => {
const config = await deps.use('config')
return createPaymentPointerRoutes({
config: await deps.use('config')
authServer: config.authServerGrantUrl
})
})
container.singleton('paymentPointerKeyRoutes', async (deps) => {
Expand Down Expand Up @@ -248,9 +263,8 @@ export function initIocContainer(
const config = await deps.use('config')
return await createReceiverService({
logger: await deps.use('logger'),
// TODO: https://github.com/interledger/rafiki/issues/583
accessToken: config.devAccessToken,
connectionService: await deps.use('connectionService'),
grantService: await deps.use('grantService'),
incomingPaymentService: await deps.use('incomingPaymentService'),
openPaymentsUrl: config.openPaymentsUrl,
paymentPointerService: await deps.use('paymentPointerService'),
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/open_payments/auth/grant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ interface AmountJSON {
assetScale: number
}

// TODO: replace with open-payments generated types
export enum AccessType {
IncomingPayment = 'incoming-payment',
OutgoingPayment = 'outgoing-payment',
Expand Down
9 changes: 9 additions & 0 deletions packages/backend/src/open_payments/authServer/model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { BaseModel } from '../../shared/baseModel'

export class AuthServer extends BaseModel {
public static get tableName(): string {
return 'authServers'
}

public url!: string
}
50 changes: 50 additions & 0 deletions packages/backend/src/open_payments/authServer/service.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { IocContract } from '@adonisjs/fold'
import { faker } from '@faker-js/faker'
import { Knex } from 'knex'

import { AuthServer } from './model'
import { AuthServerService } from './service'
import { initIocContainer } from '../../'
import { AppServices } from '../../app'
import { Config } from '../../config/app'
import { createTestApp, TestContainer } from '../../tests/app'
import { truncateTables } from '../../tests/tableManager'

describe('Auth Server Service', (): void => {
let deps: IocContract<AppServices>
let appContainer: TestContainer
let authServerService: AuthServerService
let knex: Knex

beforeAll(async (): Promise<void> => {
deps = await initIocContainer(Config)
appContainer = await createTestApp(deps)
knex = await deps.use('knex')
authServerService = await deps.use('authServerService')
})

afterEach(async (): Promise<void> => {
await truncateTables(knex)
})

afterAll(async (): Promise<void> => {
await appContainer.shutdown()
})

describe('getOrCreate', (): void => {
test('Auth server can be created or fetched', async (): Promise<void> => {
const url = faker.internet.url()
await expect(
AuthServer.query(knex).findOne({ url })
).resolves.toBeUndefined()
const authServer = await authServerService.getOrCreate(url)
await expect(authServer).toMatchObject({ url })
await expect(AuthServer.query(knex).findOne({ url })).resolves.toEqual(
authServer
)
await expect(authServerService.getOrCreate(url)).resolves.toEqual(
authServer
)
})
})
})
40 changes: 40 additions & 0 deletions packages/backend/src/open_payments/authServer/service.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { UniqueViolationError } from 'objection'

import { AuthServer } from './model'
import { BaseService } from '../../shared/baseService'

export interface AuthServerService {
getOrCreate(url: string): Promise<AuthServer>
}

type ServiceDependencies = BaseService

export async function createAuthServerService(
deps_: ServiceDependencies
): Promise<AuthServerService> {
const deps: ServiceDependencies = {
...deps_,
logger: deps_.logger.child({
service: 'AuthServerService'
})
}
return {
getOrCreate: (url) => getOrCreateAuthServer(deps, url)
}
}

async function getOrCreateAuthServer(
deps: ServiceDependencies,
url: string
): Promise<AuthServer> {
try {
return await AuthServer.query(deps.knex).insertAndFetch({
url
})
} catch (err) {
if (err instanceof UniqueViolationError) {
return await AuthServer.query(deps.knex).findOne({ url })
}
throw err
}
}
43 changes: 43 additions & 0 deletions packages/backend/src/open_payments/grant/model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Model, QueryContext } from 'objection'

import { AccessType, AccessAction } from '../auth/grant'
import { AuthServer } from '../authServer/model'
import { BaseModel } from '../../shared/baseModel'

export class Grant extends BaseModel {
Copy link
Contributor

Choose a reason for hiding this comment

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

Anything we can do here (naming wise) to make it more explicit what the difference between Grant and GrantReference is?

Copy link
Contributor Author

@wilsonianb wilsonianb Nov 28, 2022

Choose a reason for hiding this comment

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

What about GrantReference -> ResourceGrant (or (PaymentPointer)SubresourceGrant)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also tempted to backtrack on this: #535 (comment)

We could instead store clientId (soon to be client payment pointer #737) on subresources.
Rename GrantReference as OutgoingPaymentGrants (since it's only really needed to lock on limit checking 👇) and drop the clientId column

//lock grant
await deps.grantReferenceService.lock(grant.grant, deps.knex)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Renaming is the right move since we only store references to outgoing payment grants. I'll have to look into the other PR to understand why we can drop clientId.

Copy link
Contributor Author

@wilsonianb wilsonianb Nov 30, 2022

Choose a reason for hiding this comment

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

At the moment, we store references for grants for incoming payments, quotes, and outgoing payments because it is how we associate clientId with those resources.

public static get tableName(): string {
return 'grants'
}

static get virtualAttributes(): string[] {
return ['expired']
}

static relationMappings = {
authServer: {
relation: Model.BelongsToOneRelation,
modelClass: AuthServer,
join: {
from: 'grants.authServerId',
to: 'authServers.id'
}
}
}

public authServerId!: string
public continueId?: string
public continueToken?: string
public accessToken?: string
public accessType!: AccessType
public accessActions!: AccessAction[]
public expiresAt?: Date

public get expired(): boolean {
return !!this.expiresAt && this.expiresAt <= new Date()
}

$afterFind(queryContext: QueryContext): void {
super.$afterFind(queryContext)
delete this['authServer']
}
}
Loading