-
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
feat(auth): inspect access during token introspection #2788
Changes from 12 commits
c3cefa5
aafc86d
9544d8e
f2a7b38
f3ca550
c04ab0a
f695813
4aa0a03
eb96853
dcee655
c3bc37b
e8159af
36a284e
b7c909e
90d82ba
fe8175b
345cd5e
017699f
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,209 @@ | ||
import { v4 } from 'uuid' | ||
import { IocContract } from '@adonisjs/fold' | ||
import { Knex } from 'knex' | ||
import { faker } from '@faker-js/faker' | ||
import { | ||
AccessItem, | ||
AccessType, | ||
AccessAction | ||
} from '@interledger/open-payments' | ||
|
||
import { AppServices } from '../app' | ||
import { Access, toOpenPaymentsAccess } from './model' | ||
import { Grant, GrantState, StartMethod, FinishMethod } from '../grant/model' | ||
import { initIocContainer } from '..' | ||
import { Config } from '../config/app' | ||
import { createTestApp, TestContainer } from '../tests/app' | ||
import { truncateTables } from '../tests/tableManager' | ||
import { generateToken, generateNonce } from '../shared/utils' | ||
import { compareRequestAndGrantAccessItems } from './utils' | ||
|
||
describe('Access utilities', (): void => { | ||
let deps: IocContract<AppServices> | ||
let appContainer: TestContainer | ||
let trx: Knex.Transaction | ||
let identifier: string | ||
let grant: Grant | ||
let grantAccessItem: Access | ||
|
||
const receiver: string = | ||
'https://wallet.com/alice/incoming-payments/12341234-1234-1234-1234-123412341234' | ||
|
||
beforeAll(async (): Promise<void> => { | ||
deps = initIocContainer(Config) | ||
appContainer = await createTestApp(deps) | ||
}) | ||
|
||
beforeEach(async (): Promise<void> => { | ||
identifier = `https://example.com/${v4()}` | ||
grant = await Grant.query(trx).insertAndFetch({ | ||
state: GrantState.Processing, | ||
startMethod: [StartMethod.Redirect], | ||
continueToken: generateToken(), | ||
continueId: v4(), | ||
finishMethod: FinishMethod.Redirect, | ||
finishUri: 'https://example.com/finish', | ||
clientNonce: generateNonce(), | ||
client: faker.internet.url({ appendSlash: false }) | ||
}) | ||
|
||
grantAccessItem = await Access.query(trx).insertAndFetch({ | ||
grantId: grant.id, | ||
type: AccessType.OutgoingPayment, | ||
actions: [AccessAction.Read, AccessAction.Create, AccessAction.List], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
}) | ||
}) | ||
|
||
afterEach(async (): Promise<void> => { | ||
await truncateTables(appContainer.knex) | ||
}) | ||
|
||
afterAll(async (): Promise<void> => { | ||
await appContainer.shutdown() | ||
}) | ||
|
||
test('Can compare an access item on a grant and an access item from a request', async (): Promise<void> => { | ||
const requestAccessItem: AccessItem = { | ||
type: 'outgoing-payment', | ||
actions: ['create', 'read', 'list'], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
} | ||
|
||
expect( | ||
compareRequestAndGrantAccessItems( | ||
requestAccessItem, | ||
toOpenPaymentsAccess(grantAccessItem) | ||
) | ||
).toBe(true) | ||
}) | ||
|
||
test('Can compare an access item on a grant and an access item from a request with partial actions', async (): Promise<void> => { | ||
const requestAccessItem: AccessItem = { | ||
type: 'outgoing-payment', | ||
actions: ['read'], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
} | ||
|
||
expect( | ||
compareRequestAndGrantAccessItems( | ||
requestAccessItem, | ||
toOpenPaymentsAccess(grantAccessItem) | ||
) | ||
).toBe(true) | ||
}) | ||
|
||
test('Can compare an access item on a grant and an access item from a request with a subaction of the grant', async (): Promise<void> => { | ||
const grantAccessItemSuperAction = await Access.query(trx).insertAndFetch({ | ||
grantId: grant.id, | ||
type: AccessType.OutgoingPayment, | ||
actions: [AccessAction.ReadAll], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
}) | ||
|
||
const requestAccessItem: AccessItem = { | ||
type: 'outgoing-payment', | ||
actions: ['read'], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
} | ||
|
||
expect( | ||
compareRequestAndGrantAccessItems( | ||
requestAccessItem, | ||
toOpenPaymentsAccess(grantAccessItemSuperAction) | ||
) | ||
) | ||
}) | ||
|
||
test('access comparison fails if grant action items are insufficient', async (): Promise<void> => { | ||
const identifier = `https://example.com/${v4()}` | ||
const receiver = | ||
'https://wallet.com/alice/incoming-payments/12341234-1234-1234-1234-123412341234' | ||
const requestAccessItem: AccessItem = { | ||
type: 'outgoing-payment', | ||
actions: ['create', 'read', 'list'], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
} | ||
|
||
const grant = await Grant.query(trx).insertAndFetch({ | ||
state: GrantState.Processing, | ||
startMethod: [StartMethod.Redirect], | ||
continueToken: generateToken(), | ||
continueId: v4(), | ||
finishMethod: FinishMethod.Redirect, | ||
finishUri: 'https://example.com/finish', | ||
clientNonce: generateNonce(), | ||
client: faker.internet.url({ appendSlash: false }) | ||
}) | ||
|
||
const grantAccessItem = await Access.query(trx).insertAndFetch({ | ||
grantId: grant.id, | ||
type: AccessType.OutgoingPayment, | ||
actions: [AccessAction.Read, AccessAction.Create], | ||
identifier, | ||
limits: { | ||
receiver, | ||
debitAmount: { | ||
value: '400', | ||
assetCode: 'USD', | ||
assetScale: 2 | ||
} | ||
} | ||
}) | ||
|
||
expect( | ||
compareRequestAndGrantAccessItems( | ||
requestAccessItem, | ||
toOpenPaymentsAccess(grantAccessItem) | ||
) | ||
).toBe(false) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||
import { isDeepStrictEqual } from 'util' | ||||||
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. how did I not realize this was in the node standard library? Cool. 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. I just discovered it when completing this PR. I knew there surely was a library that could handle this but I wasn't expecting a node-native solution either! |
||||||
import { AccessItem, AccessAction } from '@interledger/open-payments' | ||||||
|
||||||
export function compareRequestAndGrantAccessItems( | ||||||
requestAccessItem: AccessItem, | ||||||
grantAccessItem: AccessItem | ||||||
): boolean { | ||||||
const { actions: requestAccessItemActions, ...restOfRequestAccessItem } = | ||||||
requestAccessItem | ||||||
const { actions: grantAccessItemActions, ...restOfgrantAccessItem } = | ||||||
grantAccessItem | ||||||
|
||||||
for (const actionItem of requestAccessItemActions) { | ||||||
if ( | ||||||
!grantAccessItemActions.find( | ||||||
(grantAccessAction) => | ||||||
grantAccessAction === actionItem || | ||||||
(actionItem === AccessAction.Read && | ||||||
grantAccessAction === AccessAction.ReadAll) || | ||||||
(actionItem === AccessAction.List && | ||||||
grantAccessAction === AccessAction.ListAll) | ||||||
) | ||||||
) | ||||||
return false | ||||||
} | ||||||
|
||||||
Object.keys(restOfRequestAccessItem).forEach((key) => { | ||||||
const requestAccessItemValue = | ||||||
restOfRequestAccessItem[key as keyof typeof restOfRequestAccessItem] | ||||||
if ( | ||||||
typeof requestAccessItemValue === 'object' && | ||||||
!isDeepStrictEqual( | ||||||
requestAccessItemValue, | ||||||
restOfgrantAccessItem[key as keyof typeof restOfgrantAccessItem] | ||||||
) | ||||||
) { | ||||||
return false | ||||||
} else if ( | ||||||
restOfRequestAccessItem[key as keyof typeof restOfRequestAccessItem] !== | ||||||
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 use
Suggested change
|
||||||
restOfgrantAccessItem[key as keyof typeof restOfgrantAccessItem] | ||||||
) { | ||||||
return false | ||||||
} | ||||||
}) | ||||||
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. I think I wouldn't mind comparing keys explicitly here (ie requestAccessItem.type === grantAccessItem.type), since we don't have too many, but this works as well 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. I decided to update this to compare the keys explicitly. I added a test in light of this comment to ensure that only the requested access rights get returned, and it wouldn't work without explicit key comparison. |
||||||
|
||||||
return true | ||||||
} |
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.
do we need
or does
expect()
just work?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 think it works, updated just to be sure