From 7c4689cd0824ee678260ba5d84b25042aad72363 Mon Sep 17 00:00:00 2001 From: Blayne Chard Date: Tue, 21 Jul 2020 16:06:42 +1200 Subject: [PATCH] fix(lambda-api): track api key usage (#943) --- packages/_infra/src/edge/index.ts | 5 +- .../_infra/src/edge/lambda.edge.api.key.ts | 14 +++++- .../src/__test__/validate.test.ts | 28 ++++++----- .../src/__test__/xyz.request.test.ts | 31 ------------ packages/lambda-api-tracker/src/index.ts | 47 +++++++------------ packages/lambda-api-tracker/src/validate.ts | 12 +++-- packages/lambda-xyz/src/__test__/xyz.test.ts | 4 -- packages/lambda-xyz/src/index.ts | 10 +--- packages/lambda-xyz/src/routes/tile.ts | 2 + packages/lambda/package.json | 3 ++ .../src/__test__/api.path.test.ts | 7 ++- packages/lambda/src/__test__/lambda.test.ts | 6 +-- packages/lambda/src/lambda.context.ts | 8 +++- packages/lambda/src/lambda.ts | 6 ++- packages/shared/package.json | 1 - packages/shared/tsconfig.json | 2 +- 16 files changed, 79 insertions(+), 107 deletions(-) rename packages/{shared => lambda}/src/__test__/api.path.test.ts (95%) diff --git a/packages/_infra/src/edge/index.ts b/packages/_infra/src/edge/index.ts index d55ec7ef0..72b71631b 100644 --- a/packages/_infra/src/edge/index.ts +++ b/packages/_infra/src/edge/index.ts @@ -49,8 +49,9 @@ export class EdgeStack extends cdk.Stack { queryString: true, queryStringCacheKeys: ['NOT_A_CACHE_KEY'], }, - // TODO track API keys with viewer requests - // lambdaFunctionAssociations: [lambdaViewerRequest], + lambdaFunctionAssociations: [ + { eventType: cf.LambdaEdgeEventType.VIEWER_REQUEST, lambdaFunction: this.lambda.version }, + ], }, ], }; diff --git a/packages/_infra/src/edge/lambda.edge.api.key.ts b/packages/_infra/src/edge/lambda.edge.api.key.ts index ac2c9b2c5..d98d1fe51 100644 --- a/packages/_infra/src/edge/lambda.edge.api.key.ts +++ b/packages/_infra/src/edge/lambda.edge.api.key.ts @@ -3,6 +3,8 @@ import iam = require('@aws-cdk/aws-iam'); import lambda = require('@aws-cdk/aws-lambda'); import { RetentionDays } from '@aws-cdk/aws-logs'; import { ApiKeyTableArn } from '../api.key.db'; +import { VersionUtil } from '../version'; +import { Env } from '@basemaps/shared'; const CODE_PATH = '../lambda-api-tracker/dist'; /** @@ -10,6 +12,7 @@ const CODE_PATH = '../lambda-api-tracker/dist'; */ export class LambdaApiKeyValidator extends cdk.Construct { public lambda: lambda.Function; + public version: lambda.Version; public constructor(scope: cdk.Stack, id: string) { super(scope, id); @@ -23,16 +26,23 @@ export class LambdaApiKeyValidator extends cdk.Construct { managedPolicies: [{ managedPolicyArn: 'arn:aws:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole' }], }); + const version = VersionUtil.version(); + this.lambda = new lambda.Function(this, 'ApiValidatorFunction', { runtime: lambda.Runtime.NODEJS_10_X, handler: 'index.handler', code: lambda.Code.asset(CODE_PATH), role: lambdaRole, logRetention: RetentionDays.ONE_MONTH, - // Lambda@Edge only allows 128mb of ram - memorySize: 128, + environment: { + [Env.NodeEnv]: Env.get(Env.NodeEnv, 'dev'), + [Env.Hash]: version.hash, + [Env.Version]: version.version, + }, }); + this.version = this.lambda.addVersion(version.hash); + // Allow access to all dynamoDb tables with the same name const dynamoPolicy = new iam.PolicyStatement(); dynamoPolicy.addActions('dynamoDB:getItem', 'dynamoDB:putItem', 'dynamodb:UpdateItem'); diff --git a/packages/lambda-api-tracker/src/__test__/validate.test.ts b/packages/lambda-api-tracker/src/__test__/validate.test.ts index dc565e589..4c286c7c2 100644 --- a/packages/lambda-api-tracker/src/__test__/validate.test.ts +++ b/packages/lambda-api-tracker/src/__test__/validate.test.ts @@ -4,7 +4,6 @@ import o from 'ospec'; import { ValidateRequest } from '../validate'; o.spec('ApiValidate', (): void => { - const dummyApiKey = 'dummy1'; const faultyApiKey = 'fault1'; const mockApiKey = 'mock1'; @@ -16,8 +15,10 @@ o.spec('ApiValidate', (): void => { Aws.apiKey.get = oldGet; }); - function makeContext(): LambdaContext { - return new LambdaContext({} as any, LogConfig.get()); + function makeContext(apiKey: string): LambdaContext { + const ctx = new LambdaContext({} as any, LogConfig.get()); + ctx.apiKey = apiKey; + return ctx; } o('validate should fail on faulty apikey', async () => { @@ -33,7 +34,7 @@ o.spec('ApiValidate', (): void => { minuteCount: 100, } as ApiKeyTableRecord; }; - const result = await ValidateRequest.validate(faultyApiKey, makeContext()); + const result = await ValidateRequest.validate(makeContext(faultyApiKey)); o(result).notEquals(null); if (result == null) throw new Error('Validate returns null result'); @@ -42,15 +43,16 @@ o.spec('ApiValidate', (): void => { o(result.statusDescription).equals('API key disabled'); }); - o('validate should fail on null record', async () => { - Aws.apiKey.get = async (): Promise => null; - const result = await ValidateRequest.validate(dummyApiKey, makeContext()); - o(result).notEquals(null); - if (result == null) throw new Error('Validate returns null result'); + // TODO this should be re-enabled at some stage + // o('validate should fail on null record', async () => { + // Aws.apiKey.get = async (): Promise => null; + // const result = await ValidateRequest.validate(makeContext(dummyApiKey)); + // o(result).notEquals(null); + // if (result == null) throw new Error('Validate returns null result'); - o(result.status).equals(403); - o(result.statusDescription).equals('Invalid API Key'); - }); + // o(result.status).equals(403); + // o(result.statusDescription).equals('Invalid API Key'); + // }); o('validate should fail with rate limit error', async () => { const mockMinuteCount = 1e4; @@ -65,7 +67,7 @@ o.spec('ApiValidate', (): void => { minuteCount: mockMinuteCount, } as ApiKeyTableRecord; }; - const result = await ValidateRequest.validate(mockApiKey, makeContext()); + const result = await ValidateRequest.validate(makeContext(mockApiKey)); o(result).notEquals(null); if (result == null) throw new Error('Validate returns null result'); diff --git a/packages/lambda-api-tracker/src/__test__/xyz.request.test.ts b/packages/lambda-api-tracker/src/__test__/xyz.request.test.ts index d4c9cfd3f..362aa7435 100644 --- a/packages/lambda-api-tracker/src/__test__/xyz.request.test.ts +++ b/packages/lambda-api-tracker/src/__test__/xyz.request.test.ts @@ -73,37 +73,6 @@ o.spec('xyz-request', () => { value: corrId, }, ], - 'x-linz-api-key': [{ key: 'x-linz-api-key', value: '12345' }], - 'x-linz-request-id': [{ key: 'x-linz-request-id', value: String(res.header(HttpHeader.RequestId)) }], - }); - }); - - o('should not cache WMTSCapabilities', async () => { - ValidateRequest.validate = async (): Promise => null; - - const request = req('/v1/tiles/aerial/3857/WMTSCapabilities.xml'); - const res = await handleRequest(request); - - o(res.status).equals(100); - const response = LambdaContext.toResponse(request, res) as CloudFrontRequestResult; - - const corrId = String(res.header(HttpHeader.CorrelationId)); - o(response?.headers).deepEquals({ - referer: [{ key: 'Referer', value: 'from/url' }], - 'user-agent': [{ key: 'User-Agent', value: 'test browser' }], - 'cache-control': [ - { - key: 'cache-control', - value: 'max-age=0', - }, - ], - 'x-linz-correlation-id': [ - { - key: 'x-linz-correlation-id', - value: corrId, - }, - ], - 'x-linz-api-key': [{ key: 'x-linz-api-key', value: '12345' }], 'x-linz-request-id': [{ key: 'x-linz-request-id', value: String(res.header(HttpHeader.RequestId)) }], }); }); diff --git a/packages/lambda-api-tracker/src/index.ts b/packages/lambda-api-tracker/src/index.ts index 2c0493c72..b2bed9063 100644 --- a/packages/lambda-api-tracker/src/index.ts +++ b/packages/lambda-api-tracker/src/index.ts @@ -1,21 +1,20 @@ -import { HttpHeader, LambdaContext, LambdaFunction, LambdaHttpResponse } from '@basemaps/lambda'; -import { Const, LogConfig, tileFromPath, TileType, ProjectionTileMatrixSet } from '@basemaps/shared'; +import { LambdaContext, LambdaFunction, LambdaHttpResponse } from '@basemaps/lambda'; +import { LogConfig, ProjectionTileMatrixSet, tileFromPath, TileType } from '@basemaps/shared'; import { ValidateRequest } from './validate'; -function setTileInfo(ctx: LambdaContext): boolean { +function setTileInfo(ctx: LambdaContext): void { const xyzData = tileFromPath(ctx.action.rest); - if (xyzData?.type === TileType.WMTS) { - return true; - } + if (xyzData == null) return; - if (xyzData?.type === TileType.Image) { - const { x, y, z } = xyzData; + if (xyzData.type === TileType.Image) { + const { x, y, z, ext } = xyzData; ctx.set('xyz', { x, y, z }); + ctx.set('projection', xyzData.projection.code); + ctx.set('extension', ext); const latLon = ProjectionTileMatrixSet.get(xyzData.projection.code).tileCenterToLatLon(xyzData); ctx.set('location', latLon); } - return false; } /** @@ -23,38 +22,24 @@ function setTileInfo(ctx: LambdaContext): boolean { */ export async function handleRequest(req: LambdaContext): Promise { req.set('name', 'LambdaApiTracker'); - req.set('method', req.method); - req.set('path', req.path); - // Extract request information - // ctx.set('clientIp', ctx.evt.clientIp); FIXME + if (LambdaContext.isCloudFrontEvent(req.evt)) { + req.set('clientIp', req.evt.Records[0].cf.request.clientIp); + } + req.set('referer', req.header('referer')); req.set('userAgent', req.header('user-agent')); - const doNotCache = req.action.name === 'tiles' && setTileInfo(req); - - const apiKey = req.query[Const.ApiKey.QueryString]; - if (apiKey == null || Array.isArray(apiKey)) { - return new LambdaHttpResponse(400, 'Invalid API Key'); - } - - // Include the APIKey in the final log entry - req.set(Const.ApiKey.QueryString, apiKey); + if (req.action.name === 'tiles') setTileInfo(req); // Validate the request throwing an error if anything goes wrong req.timer.start('validate'); - const res = await ValidateRequest.validate(apiKey, req); + const res = await ValidateRequest.validate(req); req.timer.end('validate'); - if (res != null) { - return res; - } + if (res != null) return res; - const response = new LambdaHttpResponse(100, 'Continue'); - // Api key will be trimmed from the forwarded request so pass it via a well known header - response.header(HttpHeader.ApiKey, apiKey); - if (doNotCache) response.header(HttpHeader.CacheControl, 'max-age=0'); - return response; + return new LambdaHttpResponse(100, 'Continue'); } export const handler = LambdaFunction.wrap(handleRequest, LogConfig.get()); diff --git a/packages/lambda-api-tracker/src/validate.ts b/packages/lambda-api-tracker/src/validate.ts index 5572dc11c..f40793aff 100644 --- a/packages/lambda-api-tracker/src/validate.ts +++ b/packages/lambda-api-tracker/src/validate.ts @@ -6,16 +6,18 @@ export const ValidateRequest = { * Validate that a API Key is valid * @param apiKey API key to validate */ - async validate(apiKey: string, ctx: LambdaContext): Promise { + async validate(ctx: LambdaContext): Promise { const timer = ctx.timer; + + if (ctx.apiKey == null) return new LambdaHttpResponse(400, 'Invalid API Key'); + // TODO increment the api counter timer.start('validate:db'); - const record = await Aws.apiKey.get(apiKey); + const record = await Aws.apiKey.get(ctx.apiKey); timer.end('validate:db'); - if (record == null) { - return new LambdaHttpResponse(403, 'Invalid API Key'); - } + if (record == null) return null; // Allow invalid keys for now + ctx.log.info({ record }, 'Record'); if (record.lockToIp != null) { diff --git a/packages/lambda-xyz/src/__test__/xyz.test.ts b/packages/lambda-xyz/src/__test__/xyz.test.ts index be3ee733e..68f38154c 100644 --- a/packages/lambda-xyz/src/__test__/xyz.test.ts +++ b/packages/lambda-xyz/src/__test__/xyz.test.ts @@ -85,9 +85,7 @@ o.spec('LambdaXyz', () => { o(z).equals(0); // Validate the session information has been set correctly - o(request.logContext['path']).equals(`/v1/tiles/${tileSetName}/global-mercator/0/0/0.png`); o(request.logContext['tileSet']).equals(tileSetName); - o(request.logContext['method']).equals('get'); o(request.logContext['xyz']).deepEquals({ x: 0, y: 0, z: 0 }); o(round(request.logContext['location'])).deepEquals({ lat: 0, lon: 0 }); }); @@ -109,8 +107,6 @@ o.spec('LambdaXyz', () => { o(z).equals(0); // Validate the session information has been set correctly - o(request.logContext['path']).equals('/v1/tiles/aerial/3857/0/0/0.webp'); - o(request.logContext['method']).equals('get'); o(request.logContext['xyz']).deepEquals({ x: 0, y: 0, z: 0 }); o(round(request.logContext['location'])).deepEquals({ lat: 0, lon: 0 }); }); diff --git a/packages/lambda-xyz/src/index.ts b/packages/lambda-xyz/src/index.ts index 5b2658cce..277ffd8fa 100644 --- a/packages/lambda-xyz/src/index.ts +++ b/packages/lambda-xyz/src/index.ts @@ -1,7 +1,7 @@ import { LambdaContext, LambdaFunction, LambdaHttpResponse, Router } from '@basemaps/lambda'; +import { LogConfig } from '@basemaps/shared'; import { Health, Ping, Version } from './routes/api'; import { TileOrWmts } from './routes/tile'; -import { LogConfig, Const } from '@basemaps/shared'; const app = new Router(); @@ -11,13 +11,7 @@ app.get('version', Version); app.get('tiles', TileOrWmts); export async function handleRequest(req: LambdaContext): Promise { - req.set('name', 'LambdaXyzTiler'); - req.set('method', req.method); - req.set('path', req.path); - - const apiKey = req.query[Const.ApiKey.QueryString]; - if (apiKey != null && !Array.isArray(apiKey)) req.set(Const.ApiKey.QueryString, apiKey); - + req.set('name', 'LambdaTiler'); return await app.handle(req); } diff --git a/packages/lambda-xyz/src/routes/tile.ts b/packages/lambda-xyz/src/routes/tile.ts index 6077f69ef..8e3d1a2d6 100644 --- a/packages/lambda-xyz/src/routes/tile.ts +++ b/packages/lambda-xyz/src/routes/tile.ts @@ -79,6 +79,8 @@ export async function tile(req: LambdaContext, xyzData: TileDataXyz): Promise tiler.tms.maxZoom) return new LambdaHttpResponse(404, `Zoom not found : ${z}`); const latLon = ProjectionTileMatrixSet.get(xyzData.projection.code).tileCenterToLatLon(xyzData); diff --git a/packages/lambda/package.json b/packages/lambda/package.json index e7c2b10a6..e08827f98 100644 --- a/packages/lambda/package.json +++ b/packages/lambda/package.json @@ -11,11 +11,14 @@ "test": "ospec --globs 'build/**/*.test.js' --preload ../../scripts/test.before.js" }, "dependencies": { + "@basemaps/shared": "^4.2.0", "@basemaps/metrics": "^4.0.0", "source-map-support": "^0.5.19", "ulid": "^2.3.0" }, "devDependencies": { + "@basemaps/geo": "^4.1.0", + "@basemaps/tiler": "^4.2.0", "@types/aws-lambda": "^8.10.43" }, "publishConfig": { diff --git a/packages/shared/src/__test__/api.path.test.ts b/packages/lambda/src/__test__/api.path.test.ts similarity index 95% rename from packages/shared/src/__test__/api.path.test.ts rename to packages/lambda/src/__test__/api.path.test.ts index d54f0c148..3d6f9f54d 100644 --- a/packages/shared/src/__test__/api.path.test.ts +++ b/packages/lambda/src/__test__/api.path.test.ts @@ -1,9 +1,8 @@ import { Epsg } from '@basemaps/geo'; -import o from 'ospec'; -import { tileFromPath, TileType } from '../api.path'; -import { LambdaContext } from '@basemaps/lambda'; +import { LogConfig, tileFromPath, TileType } from '@basemaps/shared'; import { ImageFormat } from '@basemaps/tiler'; -import { LogConfig } from '../log'; +import o from 'ospec'; +import { LambdaContext } from '../lambda.context'; o.spec('api.path', () => { function makeContext(path: string): LambdaContext { diff --git a/packages/lambda/src/__test__/lambda.test.ts b/packages/lambda/src/__test__/lambda.test.ts index 57a26de69..f2a8a90e5 100644 --- a/packages/lambda/src/__test__/lambda.test.ts +++ b/packages/lambda/src/__test__/lambda.test.ts @@ -18,7 +18,7 @@ o.spec('LambdaFunction', () => { const testFunc = LambdaFunction.wrap(asyncThrow, FakeLogger()); const spy = o.spy(); - await testFunc({} as any, null as any, spy); + await testFunc({ httpMethod: 'GET' } as any, null as any, spy); o(spy.calls.length).equals(1); const err = spy.args[0]; const res = spy.args[1] as ALBResult; @@ -39,7 +39,7 @@ o.spec('LambdaFunction', () => { const testFunc = LambdaFunction.wrap(asyncThrow, FakeLogger()); const spy = o.spy(); - await testFunc({ Records: [{ cf: { request: { headers: {} } } }] } as any, null as any, spy); + await testFunc({ Records: [{ cf: { request: { method: 'GET', headers: {} } } }] } as any, null as any, spy); o(spy.calls.length).equals(1); const err = spy.args[0]; const res = spy.args[1] as CloudFrontResultResponse; @@ -68,7 +68,7 @@ o.spec('LambdaFunction', () => { }, fakeLogger); const spy = o.spy(); - await testFunc({} as any, null as any, spy); + await testFunc({ httpMethod: 'GET' } as any, null as any, spy); o(spy.calls.length).equals(1); o(spy.args[1]).deepEquals(LambdaContext.toAlbResponse(albOk)); diff --git a/packages/lambda/src/lambda.context.ts b/packages/lambda/src/lambda.context.ts index 27caeb8b4..caa94ddf2 100644 --- a/packages/lambda/src/lambda.context.ts +++ b/packages/lambda/src/lambda.context.ts @@ -5,6 +5,7 @@ import { toAlbHeaders, toCloudFrontHeaders } from './lambda.aws'; import * as qs from 'querystring'; import { HttpHeader } from './header'; import { LambdaHttpResponse } from './lambda.response'; +import { Const } from '@basemaps/shared'; export interface ActionData { version: string; @@ -50,7 +51,11 @@ export class LambdaContext { this.evt = evt; this.id = ulid.ulid(); this.loadHeaders(); - this.apiKey = this.header(HttpHeader.ApiKey); + const apiKey = this.query[Const.ApiKey.QueryString] ?? this.header(HttpHeader.ApiKey); + if (apiKey != null && !Array.isArray(apiKey)) { + this.apiKey = apiKey; + this.set(Const.ApiKey.QueryString, this.apiKey); + } this.correlationId = this.header(HttpHeader.CorrelationId) ?? ulid.ulid(); this.set('correlationId', this.correlationId); this.log = logger.child({ id: this.id }); @@ -88,6 +93,7 @@ export class LambdaContext { return this.evt.queryStringParameters ?? {}; } const query = this.evt.Records[0].cf.request.querystring; + if (query == null || query[0] == null) return {}; return qs.decode(query[0] == '?' ? query.substr(1) : query); } diff --git a/packages/lambda/src/lambda.ts b/packages/lambda/src/lambda.ts index a5a12062e..43b14af86 100644 --- a/packages/lambda/src/lambda.ts +++ b/packages/lambda/src/lambda.ts @@ -1,5 +1,6 @@ import { Callback, Context } from 'aws-lambda'; import { ApplicationJson, HttpHeader } from './header'; +import { Env } from '@basemaps/shared'; import { LambdaContext, LambdaHttpRequestType, LambdaHttpReturnType, LogType } from './lambda.context'; import { LambdaHttpResponse } from './lambda.response'; @@ -39,7 +40,10 @@ export class LambdaFunction { region: process.env['AWS_REGION'], }; - ctx.log.info({ lambda }, 'LambdaStart'); + ctx.set('package', { hash: Env.get(Env.Hash), version: Env.get(Env.Version) }); + ctx.set('method', ctx.method); + ctx.set('path', ctx.path); + ctx.log.debug({ lambda }, 'LambdaStart'); let res: LambdaHttpResponse; try { diff --git a/packages/shared/package.json b/packages/shared/package.json index 16f0e809f..d569a6bd0 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -12,7 +12,6 @@ }, "dependencies": { "@basemaps/geo": "^4.1.0", - "@basemaps/lambda": "^4.0.0", "@basemaps/metrics": "^4.0.0", "@basemaps/tiler": "^4.2.0", "aws-sdk": "^2.508.0", diff --git a/packages/shared/tsconfig.json b/packages/shared/tsconfig.json index 67136a12a..01c9fe57b 100644 --- a/packages/shared/tsconfig.json +++ b/packages/shared/tsconfig.json @@ -5,5 +5,5 @@ "outDir": "./build" }, "include": ["src/**/*"], - "references": [{ "path": "../lambda" }] + "references": [] }