feat(files): add validating remote identifiers

This commit is contained in:
Karol Sójko 2022-09-19 09:43:46 +02:00
parent 719d8558a3
commit db15457ce4
No known key found for this signature in database
GPG key ID: A50543BF560BDEB0
12 changed files with 145 additions and 5 deletions

View file

@ -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<StatisticsStoreInterface>(TYPES.StatisticsStore)
.toConstantValue(new RedisStatisticsStore(periodKeyGenerator, container.get(TYPES.Redis)))
container.bind<ValidatorInterface<Uuid>>(TYPES.UuidValidator).to(UuidValidator)
if (env.get('SNS_TOPIC_ARN', true)) {
container

View file

@ -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

View file

@ -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<Uuid>
let request: Request
let response: Response
const createController = () => new ValetTokenController(createValetToken)
const createController = () => new ValetTokenController(createValetToken, uuidValidator)
beforeEach(() => {
createValetToken = {} as jest.Mocked<CreateValetToken>
createValetToken.execute = jest.fn().mockReturnValue({ success: true, valetToken: 'foobar' })
uuidValidator = {} as jest.Mocked<ValidatorInterface<Uuid>>
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 = <results.JsonResult>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'

View file

@ -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<Uuid>,
) {
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,

View file

@ -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()
}
})
})

View file

@ -0,0 +1,10 @@
import { Uuid } from '../DataType/Uuid'
import { ValidatorInterface } from './ValidatorInterface'
export class UuidValidator implements ValidatorInterface<Uuid> {
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
}
}

View file

@ -0,0 +1,3 @@
export interface ValidatorInterface<T> {
validate(data: T): boolean
}

View file

@ -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'

View file

@ -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<Container> {
@ -107,6 +108,7 @@ export class ContainerConfigLoader {
.toConstantValue(new FSFileUploader(container.get(TYPES.FILE_UPLOAD_PATH), container.get(TYPES.Logger)))
container.bind<FileRemoverInterface>(TYPES.FileRemover).to(FSFileRemover)
}
container.bind<ValidatorInterface<Uuid>>(TYPES.UuidValidator).to(UuidValidator)
if (env.get('SNS_AWS_REGION', true)) {
container.bind<AWS.SNS>(TYPES.SNS).toConstantValue(

View file

@ -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'),

View file

@ -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<ValetTokenData>
let uuidValidator: ValidatorInterface<Uuid>
let request: Request
let response: Response
let next: NextFunction
@ -15,7 +17,7 @@ describe('ValetTokenAuthMiddleware', () => {
debug: jest.fn(),
} as unknown as jest.Mocked<Logger>
const createMiddleware = () => new ValetTokenAuthMiddleware(tokenDecoder, logger)
const createMiddleware = () => new ValetTokenAuthMiddleware(tokenDecoder, uuidValidator, logger)
beforeEach(() => {
tokenDecoder = {} as jest.Mocked<TokenDecoderInterface<ValetTokenData>>
@ -32,6 +34,9 @@ describe('ValetTokenAuthMiddleware', () => {
uploadBytesUsed: 80,
})
uuidValidator = {} as jest.Mocked<ValidatorInterface<Uuid>>
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'

View file

@ -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<ValetTokenData>,
@inject(TYPES.UuidValidator) private uuidValidator: ValidatorInterface<Uuid>,
@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: {