From db15457ce4eb533ec822cf93c3ed83eafe9e64d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Mon, 19 Sep 2022 09:43:46 +0200 Subject: [PATCH] feat(files): add validating remote identifiers --- packages/auth/src/Bootstrap/Container.ts | 10 +++++- packages/auth/src/Bootstrap/Types.ts | 1 + .../Controller/ValetTokenController.spec.ts | 18 +++++++++- .../src/Controller/ValetTokenController.ts | 21 ++++++++++-- .../Domain/Validator/UuidValidator.spec.ts | 34 +++++++++++++++++++ .../src/Domain/Validator/UuidValidator.ts | 10 ++++++ .../Domain/Validator/ValidatorInterface.ts | 3 ++ packages/common/src/Domain/index.ts | 2 ++ packages/files/src/Bootstrap/Container.ts | 2 ++ packages/files/src/Bootstrap/Types.ts | 1 + .../ValetTokenAuthMiddleware.spec.ts | 31 ++++++++++++++++- .../Controller/ValetTokenAuthMiddleware.ts | 17 ++++++++++ 12 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 packages/common/src/Domain/Validator/UuidValidator.spec.ts create mode 100644 packages/common/src/Domain/Validator/UuidValidator.ts create mode 100644 packages/common/src/Domain/Validator/ValidatorInterface.ts diff --git a/packages/auth/src/Bootstrap/Container.ts b/packages/auth/src/Bootstrap/Container.ts index bdc9392a4..5aec81320 100644 --- a/packages/auth/src/Bootstrap/Container.ts +++ b/packages/auth/src/Bootstrap/Container.ts @@ -130,7 +130,14 @@ import { RedisOfflineSubscriptionTokenRepository } from '../Infra/Redis/RedisOff import { CreateOfflineSubscriptionToken } from '../Domain/UseCase/CreateOfflineSubscriptionToken/CreateOfflineSubscriptionToken' import { AuthenticateOfflineSubscriptionToken } from '../Domain/UseCase/AuthenticateOfflineSubscriptionToken/AuthenticateOfflineSubscriptionToken' import { SubscriptionCancelledEventHandler } from '../Domain/Handler/SubscriptionCancelledEventHandler' -import { ContentDecoder, ContentDecoderInterface, ProtocolVersion } from '@standardnotes/common' +import { + ContentDecoder, + ContentDecoderInterface, + ProtocolVersion, + Uuid, + UuidValidator, + ValidatorInterface, +} from '@standardnotes/common' import { GetUserOfflineSubscription } from '../Domain/UseCase/GetUserOfflineSubscription/GetUserOfflineSubscription' import { ApiGatewayOfflineAuthMiddleware } from '../Controller/ApiGatewayOfflineAuthMiddleware' import { UserEmailChangedEventHandler } from '../Domain/Handler/UserEmailChangedEventHandler' @@ -559,6 +566,7 @@ export class ContainerConfigLoader { container .bind(TYPES.StatisticsStore) .toConstantValue(new RedisStatisticsStore(periodKeyGenerator, container.get(TYPES.Redis))) + container.bind>(TYPES.UuidValidator).to(UuidValidator) if (env.get('SNS_TOPIC_ARN', true)) { container diff --git a/packages/auth/src/Bootstrap/Types.ts b/packages/auth/src/Bootstrap/Types.ts index c5a9cff90..c9743ae8e 100644 --- a/packages/auth/src/Bootstrap/Types.ts +++ b/packages/auth/src/Bootstrap/Types.ts @@ -189,6 +189,7 @@ const TYPES = { UserSubscriptionService: Symbol.for('UserSubscriptionService'), AnalyticsStore: Symbol.for('AnalyticsStore'), StatisticsStore: Symbol.for('StatisticsStore'), + UuidValidator: Symbol.for('UuidValidator'), } export default TYPES diff --git a/packages/auth/src/Controller/ValetTokenController.spec.ts b/packages/auth/src/Controller/ValetTokenController.spec.ts index 8695a0eeb..7ffdffb79 100644 --- a/packages/auth/src/Controller/ValetTokenController.spec.ts +++ b/packages/auth/src/Controller/ValetTokenController.spec.ts @@ -4,18 +4,23 @@ import { Request, Response } from 'express' import { results } from 'inversify-express-utils' import { ValetTokenController } from './ValetTokenController' import { CreateValetToken } from '../Domain/UseCase/CreateValetToken/CreateValetToken' +import { Uuid, ValidatorInterface } from '@standardnotes/common' describe('ValetTokenController', () => { let createValetToken: CreateValetToken + let uuidValidator: ValidatorInterface let request: Request let response: Response - const createController = () => new ValetTokenController(createValetToken) + const createController = () => new ValetTokenController(createValetToken, uuidValidator) beforeEach(() => { createValetToken = {} as jest.Mocked createValetToken.execute = jest.fn().mockReturnValue({ success: true, valetToken: 'foobar' }) + uuidValidator = {} as jest.Mocked> + uuidValidator.validate = jest.fn().mockReturnValue(true) + request = { body: { operation: 'write', @@ -42,6 +47,17 @@ describe('ValetTokenController', () => { expect(await result.content.readAsStringAsync()).toEqual('{"success":true,"valetToken":"foobar"}') }) + it('should not create a valet token if the remote resource identifier is not a valid uuid', async () => { + uuidValidator.validate = jest.fn().mockReturnValue(false) + + const httpResponse = await createController().create(request, response) + const result = await httpResponse.executeAsync() + + expect(createValetToken.execute).not.toHaveBeenCalled() + + expect(result.statusCode).toEqual(400) + }) + it('should create a read valet token for read only access session', async () => { response.locals.readOnlyAccess = true request.body.operation = 'read' diff --git a/packages/auth/src/Controller/ValetTokenController.ts b/packages/auth/src/Controller/ValetTokenController.ts index 15dc1fbd6..0f7b4226a 100644 --- a/packages/auth/src/Controller/ValetTokenController.ts +++ b/packages/auth/src/Controller/ValetTokenController.ts @@ -11,12 +11,15 @@ import { CreateValetTokenPayload } from '@standardnotes/responses' import TYPES from '../Bootstrap/Types' import { CreateValetToken } from '../Domain/UseCase/CreateValetToken/CreateValetToken' -import { ErrorTag } from '@standardnotes/common' +import { ErrorTag, Uuid, ValidatorInterface } from '@standardnotes/common' import { ValetTokenOperation } from '@standardnotes/security' @controller('/valet-tokens', TYPES.ApiGatewayAuthMiddleware) export class ValetTokenController extends BaseHttpController { - constructor(@inject(TYPES.CreateValetToken) private createValetKey: CreateValetToken) { + constructor( + @inject(TYPES.CreateValetToken) private createValetKey: CreateValetToken, + @inject(TYPES.UuidValidator) private uuidValitor: ValidatorInterface, + ) { super() } @@ -36,6 +39,20 @@ export class ValetTokenController extends BaseHttpController { ) } + for (const resource of payload.resources) { + if (!this.uuidValitor.validate(resource.remoteIdentifier)) { + return this.json( + { + error: { + tag: ErrorTag.ParametersInvalid, + message: 'Invalid remote resource identifier.', + }, + }, + 400, + ) + } + } + const createValetKeyResponse = await this.createValetKey.execute({ userUuid: response.locals.user.uuid, operation: payload.operation as ValetTokenOperation, diff --git a/packages/common/src/Domain/Validator/UuidValidator.spec.ts b/packages/common/src/Domain/Validator/UuidValidator.spec.ts new file mode 100644 index 000000000..409823ce3 --- /dev/null +++ b/packages/common/src/Domain/Validator/UuidValidator.spec.ts @@ -0,0 +1,34 @@ +import { UuidValidator } from './UuidValidator' + +describe('UuidValidator', () => { + const createValidator = () => new UuidValidator() + + const validUuids = [ + '2221101c-1da9-4d2b-9b32-b8be2a8d1c82', + 'c08f2f29-a74b-42b4-aefd-98af9832391c', + 'b453fa64-1493-443b-b5bb-bca7b9c696c7', + ] + + const invalidUuids = [ + 123, + 'someone@127.0.0.1', + '', + null, + 'b453fa64-1493-443b-b5bb-ca7b9c696c7', + 'c08f*f29-a74b-42b4-aefd-98af9832391c', + 'c08f*f29-a74b-42b4-aefd-98af9832391c', + '../../escaped.sh', + ] + + it('should validate proper uuids', () => { + for (const validUuid of validUuids) { + expect(createValidator().validate(validUuid)).toBeTruthy() + } + }) + + it('should not validate invalid uuids', () => { + for (const invalidUuid of invalidUuids) { + expect(createValidator().validate(invalidUuid as string)).toBeFalsy() + } + }) +}) diff --git a/packages/common/src/Domain/Validator/UuidValidator.ts b/packages/common/src/Domain/Validator/UuidValidator.ts new file mode 100644 index 000000000..ac9eb07d1 --- /dev/null +++ b/packages/common/src/Domain/Validator/UuidValidator.ts @@ -0,0 +1,10 @@ +import { Uuid } from '../DataType/Uuid' +import { ValidatorInterface } from './ValidatorInterface' + +export class UuidValidator implements ValidatorInterface { + private readonly UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-5][0-9a-f]{3}-[089ab][0-9a-f]{3}-[0-9a-f]{12}$/i + + validate(data: Uuid): boolean { + return String(data).toLowerCase().match(this.UUID_REGEX) !== null + } +} diff --git a/packages/common/src/Domain/Validator/ValidatorInterface.ts b/packages/common/src/Domain/Validator/ValidatorInterface.ts new file mode 100644 index 000000000..663ac9777 --- /dev/null +++ b/packages/common/src/Domain/Validator/ValidatorInterface.ts @@ -0,0 +1,3 @@ +export interface ValidatorInterface { + validate(data: T): boolean +} diff --git a/packages/common/src/Domain/index.ts b/packages/common/src/Domain/index.ts index 4345c8a8d..34e1ecdcb 100644 --- a/packages/common/src/Domain/index.ts +++ b/packages/common/src/Domain/index.ts @@ -20,3 +20,5 @@ export * from './Role/RoleName' export * from './Subscription/SubscriptionName' export * from './Type/Either' export * from './Type/Only' +export * from './Validator/UuidValidator' +export * from './Validator/ValidatorInterface' diff --git a/packages/files/src/Bootstrap/Container.ts b/packages/files/src/Bootstrap/Container.ts index 12234eed2..969fe59d8 100644 --- a/packages/files/src/Bootstrap/Container.ts +++ b/packages/files/src/Bootstrap/Container.ts @@ -44,6 +44,7 @@ import { import { MarkFilesToBeRemoved } from '../Domain/UseCase/MarkFilesToBeRemoved/MarkFilesToBeRemoved' import { AccountDeletionRequestedEventHandler } from '../Domain/Handler/AccountDeletionRequestedEventHandler' import { SharedSubscriptionInvitationCanceledEventHandler } from '../Domain/Handler/SharedSubscriptionInvitationCanceledEventHandler' +import { Uuid, UuidValidator, ValidatorInterface } from '@standardnotes/common' export class ContainerConfigLoader { async load(): Promise { @@ -107,6 +108,7 @@ export class ContainerConfigLoader { .toConstantValue(new FSFileUploader(container.get(TYPES.FILE_UPLOAD_PATH), container.get(TYPES.Logger))) container.bind(TYPES.FileRemover).to(FSFileRemover) } + container.bind>(TYPES.UuidValidator).to(UuidValidator) if (env.get('SNS_AWS_REGION', true)) { container.bind(TYPES.SNS).toConstantValue( diff --git a/packages/files/src/Bootstrap/Types.ts b/packages/files/src/Bootstrap/Types.ts index ab087e4b1..bfefb0e99 100644 --- a/packages/files/src/Bootstrap/Types.ts +++ b/packages/files/src/Bootstrap/Types.ts @@ -23,6 +23,7 @@ const TYPES = { FileUploader: Symbol.for('FileUploader'), FileDownloader: Symbol.for('FileDownloader'), FileRemover: Symbol.for('FileRemover'), + UuidValidator: Symbol.for('UuidValidator'), // repositories UploadRepository: Symbol.for('UploadRepository'), diff --git a/packages/files/src/Controller/ValetTokenAuthMiddleware.spec.ts b/packages/files/src/Controller/ValetTokenAuthMiddleware.spec.ts index 984ac4472..d94792f14 100644 --- a/packages/files/src/Controller/ValetTokenAuthMiddleware.spec.ts +++ b/packages/files/src/Controller/ValetTokenAuthMiddleware.spec.ts @@ -4,9 +4,11 @@ import { ValetTokenAuthMiddleware } from './ValetTokenAuthMiddleware' import { NextFunction, Request, Response } from 'express' import { Logger } from 'winston' import { TokenDecoderInterface, ValetTokenData } from '@standardnotes/security' +import { Uuid, ValidatorInterface } from '@standardnotes/common' describe('ValetTokenAuthMiddleware', () => { let tokenDecoder: TokenDecoderInterface + let uuidValidator: ValidatorInterface let request: Request let response: Response let next: NextFunction @@ -15,7 +17,7 @@ describe('ValetTokenAuthMiddleware', () => { debug: jest.fn(), } as unknown as jest.Mocked - const createMiddleware = () => new ValetTokenAuthMiddleware(tokenDecoder, logger) + const createMiddleware = () => new ValetTokenAuthMiddleware(tokenDecoder, uuidValidator, logger) beforeEach(() => { tokenDecoder = {} as jest.Mocked> @@ -32,6 +34,9 @@ describe('ValetTokenAuthMiddleware', () => { uploadBytesUsed: 80, }) + uuidValidator = {} as jest.Mocked> + uuidValidator.validate = jest.fn().mockReturnValue(true) + request = { headers: {}, query: {}, @@ -174,6 +179,30 @@ describe('ValetTokenAuthMiddleware', () => { expect(next).not.toHaveBeenCalled() }) + it('should not authorize if valet token has an invalid remote resource identifier', async () => { + tokenDecoder.decodeToken = jest.fn().mockReturnValue({ + userUuid: '1-2-3', + permittedResources: [ + { + remoteIdentifier: '1-2-3/2-3-4', + unencryptedFileSize: 30, + }, + ], + permittedOperation: 'write', + uploadBytesLimit: -1, + uploadBytesUsed: 80, + }) + + request.headers['x-valet-token'] = 'valet-token' + + uuidValidator.validate = jest.fn().mockReturnValue(false) + + await createMiddleware().handler(request, response, next) + + expect(response.status).toHaveBeenCalledWith(401) + expect(next).not.toHaveBeenCalled() + }) + it('should not authorize if auth valet token is malformed', async () => { request.headers['x-valet-token'] = 'valet-token' diff --git a/packages/files/src/Controller/ValetTokenAuthMiddleware.ts b/packages/files/src/Controller/ValetTokenAuthMiddleware.ts index 85c36ea3a..ae92f4128 100644 --- a/packages/files/src/Controller/ValetTokenAuthMiddleware.ts +++ b/packages/files/src/Controller/ValetTokenAuthMiddleware.ts @@ -1,3 +1,4 @@ +import { Uuid, ValidatorInterface } from '@standardnotes/common' import { TokenDecoderInterface, ValetTokenData } from '@standardnotes/security' import { NextFunction, Request, Response } from 'express' import { inject, injectable } from 'inversify' @@ -9,6 +10,7 @@ import TYPES from '../Bootstrap/Types' export class ValetTokenAuthMiddleware extends BaseMiddleware { constructor( @inject(TYPES.ValetTokenDecoder) private tokenDecoder: TokenDecoderInterface, + @inject(TYPES.UuidValidator) private uuidValidator: ValidatorInterface, @inject(TYPES.Logger) private logger: Logger, ) { super() @@ -45,6 +47,21 @@ export class ValetTokenAuthMiddleware extends BaseMiddleware { return } + for (const resource of valetTokenData.permittedResources) { + if (!this.uuidValidator.validate(resource.remoteIdentifier)) { + this.logger.debug('Invalid remote resource identifier in token.') + + response.status(401).send({ + error: { + tag: 'invalid-auth', + message: 'Invalid valet token.', + }, + }) + + return + } + } + if (this.userHasNoSpaceToUpload(valetTokenData)) { response.status(403).send({ error: {