Skip to content

Commit 9b1c6ce

Browse files
authored
Add "Fix authorization guard access check" back (#133)
* Revert "Revert "Fix authorization guard access check (#129)" (#131)" This reverts commit 5d9dd45. * Add tests for auth guard and decode * Bump SDK to 0.20.0
1 parent 66cf58b commit 9b1c6ce

File tree

8 files changed

+124
-49
lines changed

8 files changed

+124
-49
lines changed

apps/vault/src/shared/decorator/permission-guard.decorator.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { Reflector } from '@nestjs/core'
44
import { AuthorizationGuard } from '../guard/authorization.guard'
55
import { VaultPermission } from '../type/domain.type'
66

7-
const RequiredPermission = Reflector.createDecorator<VaultPermission[]>()
7+
export const RequiredPermission = Reflector.createDecorator<VaultPermission[]>()
88

99
export function PermissionGuard(...permissions: VaultPermission[]) {
1010
return applyDecorators(

apps/vault/src/shared/guard/__test__/unit/authorization.guard.spec.ts

+88-8
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ const getAccessToken = async (request: unknown, opts: object = {}) => {
119119
return signJwt(payload, enginePrivateJwk, { alg: SigningAlg.EIP191 }, signer)
120120
}
121121

122-
describe('AuthorizationGuard', () => {
122+
describe(AuthorizationGuard.name, () => {
123123
let mockClientService = mock<ClientService>()
124124
let mockConfigService = mock<ConfigService<Config>>()
125125

@@ -134,11 +134,6 @@ describe('AuthorizationGuard', () => {
134134
mockReflector = mock<Reflector>()
135135
})
136136

137-
it('should be defined', () => {
138-
const guard = new AuthorizationGuard(mockClientService, mockConfigService, mockReflector, mockLogger)
139-
expect(guard).toBeDefined()
140-
})
141-
142137
describe('canActivate', () => {
143138
const mockExecutionContext = ({ request }: { request?: unknown } = {}) => {
144139
const mockRequest = request || {
@@ -276,7 +271,7 @@ describe('AuthorizationGuard', () => {
276271
await expect(guard.canActivate(context)).resolves.toEqual(true)
277272
})
278273

279-
it('should throw when token validation is enabled and missing accessToken', async () => {
274+
it('throws when token validation is enabled and missing accessToken', async () => {
280275
expect.assertions(2)
281276
const client = getBaseClient()
282277
mockClientService.findById.mockResolvedValue(client)
@@ -331,7 +326,7 @@ describe('AuthorizationGuard', () => {
331326
await expect(guard.canActivate(context)).rejects.toThrow('No engine key configured')
332327
})
333328

334-
it('should throw when requieBoundTokens is true and token is not bound', async () => {
329+
it('throws when requieBoundTokens is true and token is not bound', async () => {
335330
const userPrivateJwk = secp256k1PrivateKeyToJwk(FIXTURE.UNSAFE_PRIVATE_KEY.Alice)
336331
const client = getBaseClient()
337332
mockClientService.findById.mockResolvedValue(client)
@@ -417,5 +412,90 @@ describe('AuthorizationGuard', () => {
417412

418413
await expect(guard.canActivate(context)).resolves.toEqual(true)
419414
})
415+
416+
it('passes when token contains required permissions', async () => {
417+
const userPrivateJwk = secp256k1PrivateKeyToJwk(FIXTURE.UNSAFE_PRIVATE_KEY.Alice)
418+
const userJwk = secp256k1PublicKeyToJwk(FIXTURE.VIEM_ACCOUNT.Alice.publicKey)
419+
const client = getBaseClient()
420+
mockClientService.findById.mockResolvedValue(client)
421+
422+
// Mock the reflector to return permissions
423+
mockReflector.get.mockReturnValue(['WALLET_READ'])
424+
425+
const guard = new AuthorizationGuard(mockClientService, mockConfigService, mockReflector, mockLogger)
426+
const payload = {
427+
value: 'test-value'
428+
}
429+
430+
const accessToken = await getAccessToken(payload, {
431+
sub: 'user-1',
432+
cnf: userJwk,
433+
access: [
434+
{
435+
resource: 'vault',
436+
permissions: ['WALLET_READ']
437+
}
438+
]
439+
})
440+
const jwsd = await getJwsd({
441+
userPrivateJwk,
442+
requestUrl: '/test',
443+
payload,
444+
accessToken
445+
})
446+
447+
const mockRequest = {
448+
headers: { 'x-client-id': 'test-client', authorization: `GNAP ${accessToken}`, 'detached-jws': jwsd },
449+
body: payload,
450+
url: '/test',
451+
method: 'POST'
452+
}
453+
const context = mockExecutionContext({ request: mockRequest })
454+
455+
await expect(guard.canActivate(context)).resolves.toEqual(true)
456+
})
457+
458+
it('throws when token does not contain required permissions', async () => {
459+
const userPrivateJwk = secp256k1PrivateKeyToJwk(FIXTURE.UNSAFE_PRIVATE_KEY.Alice)
460+
const userJwk = secp256k1PublicKeyToJwk(FIXTURE.VIEM_ACCOUNT.Alice.publicKey)
461+
const client = getBaseClient()
462+
mockClientService.findById.mockResolvedValue(client)
463+
464+
// Mock the reflector to return permissions
465+
mockReflector.get.mockReturnValue(['WALLET_READ'])
466+
467+
const guard = new AuthorizationGuard(mockClientService, mockConfigService, mockReflector, mockLogger)
468+
const payload = {
469+
value: 'test-value'
470+
}
471+
472+
// Token with different permission than required
473+
const accessToken = await getAccessToken(payload, {
474+
sub: 'user-1',
475+
cnf: userJwk,
476+
access: [
477+
{
478+
resource: 'vault',
479+
permissions: ['WALLET_WRITE']
480+
}
481+
]
482+
})
483+
const jwsd = await getJwsd({
484+
userPrivateJwk,
485+
requestUrl: '/test',
486+
payload,
487+
accessToken
488+
})
489+
490+
const mockRequest = {
491+
headers: { 'x-client-id': 'test-client', authorization: `GNAP ${accessToken}`, 'detached-jws': jwsd },
492+
body: payload,
493+
url: '/test',
494+
method: 'POST'
495+
}
496+
const context = mockExecutionContext({ request: mockRequest })
497+
498+
await expect(guard.canActivate(context)).rejects.toThrow('Invalid permissions')
499+
})
420500
})
421501
})

apps/vault/src/shared/guard/authorization.guard.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { Reflector } from '@nestjs/core'
66
import { z } from 'zod'
77
import { ClientService } from '../../client/core/service/client.service'
88
import { Config } from '../../main.config'
9-
import { PermissionGuard } from '../decorator/permission-guard.decorator'
9+
import { RequiredPermission } from '../decorator/permission-guard.decorator'
1010
import { ApplicationException } from '../exception/application.exception'
1111
import { Client, VaultPermission } from '../type/domain.type'
1212

@@ -105,7 +105,7 @@ export class AuthorizationGuard implements CanActivate {
105105

106106
// Get the Permissions (scopes) required from the request decorator, if it exists.
107107
const { request: requestHash } = req.body
108-
const permissions: VaultPermission[] | undefined = this.reflector.get(PermissionGuard, context.getHandler())
108+
const permissions: VaultPermission[] | undefined = this.reflector.get(RequiredPermission, context.getHandler())
109109
const access =
110110
permissions && permissions.length > 0
111111
? [

package-lock.json

+7-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/armory-sdk/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@narval-xyz/armory-sdk",
3-
"version": "0.19.0",
3+
"version": "0.20.0",
44
"license": "MPL-2.0",
55
"publishConfig": {
66
"access": "public"

packages/signature/src/lib/__test__/unit/decode.spec.ts

+22
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe('decodeJwt', () => {
1616
const rawJwt = await signJwt(payload, key, { alg: 'ES256K' })
1717

1818
const jwt = decodeJwt(rawJwt)
19+
1920
expect(jwt).toEqual({
2021
header: {
2122
alg: 'ES256K',
@@ -34,4 +35,25 @@ describe('decodeJwt', () => {
3435
it('throws an error if token is in correct format but not valid base64url encoded data', async () => {
3536
expect(() => decodeJwt('invalid.invalid.invalid')).toThrow(JwtError)
3637
})
38+
39+
it('does not strip arbitrary data from the payload', async () => {
40+
const payloadWithArbitraryData = {
41+
...payload,
42+
foo: 'foo',
43+
bar: 'bar'
44+
}
45+
const rawJwt = await signJwt(payloadWithArbitraryData, key, { alg: 'ES256K' })
46+
47+
const jwt = decodeJwt(rawJwt)
48+
49+
expect(jwt).toEqual({
50+
header: {
51+
alg: 'ES256K',
52+
kid: key.kid,
53+
typ: 'JWT'
54+
},
55+
payload: payloadWithArbitraryData,
56+
signature: rawJwt.split('.')[2]
57+
})
58+
})
3759
})

packages/signature/src/lib/decode.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { JwtError } from './error'
2-
import { Header, Payload } from './schemas'
3-
import { type Jwsd, type Jwt } from './types'
2+
import { Header } from './schemas'
3+
import { Payload, type Jwsd, type Jwt } from './types'
44

55
import { base64UrlToBytes } from './utils'
66

packages/signature/src/lib/schemas.ts

+1-28
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { z } from 'zod'
22
import { addressSchema } from './address.schema'
3-
import { Alg, Curves, KeyTypes, Use } from './types'
3+
import { Alg, Curves, KeyTypes, Payload, Use } from './types'
44

55
// Base JWK Schema
66
export const jwkBaseSchema = z.object({
@@ -186,33 +186,6 @@ export const JwsdHeader = z.object({
186186
)
187187
})
188188

189-
/**
190-
* Defines the payload of JWT.
191-
*
192-
* @param {string} requestHash - The hashed request.
193-
* @param {string} [iss] - The issuer of the JWT.
194-
* @param {number} [iat] - The time the JWT was issued.
195-
* @param {number} [exp] - The time the JWT expires.
196-
* @param {string} sub - The subject of the JWT.
197-
* @param {string} [aud] - The audience of the JWT.
198-
* @param {string[]} [hashWildcard] - A list of paths that were not hashed in the request.
199-
* @param {string} [jti] - The JWT ID.
200-
* @param {Jwk} cnf - The client-bound key.
201-
*
202-
*/
203-
export const Payload = z.object({
204-
sub: z.string().optional(),
205-
iat: z.number().optional(),
206-
exp: z.number().optional(),
207-
iss: z.string().optional(),
208-
aud: z.string().optional(),
209-
jti: z.string().optional(),
210-
cnf: publicKeySchema.optional(),
211-
requestHash: z.string().optional(),
212-
hashWildcard: z.array(z.string()).optional(),
213-
data: z.string().optional()
214-
})
215-
216189
export const Jwt = z.object({
217190
header: Header,
218191
payload: Payload,

0 commit comments

Comments
 (0)