From 74799cd837b02ec003b68bff0c14d784e76a6de0 Mon Sep 17 00:00:00 2001 From: tudor <7089284+tudddorrr@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:11:07 +0100 Subject: [PATCH 1/3] service and identifier cannot be empty --- src/services/api/player-api.service.ts | 22 ++++++++++-- .../services/_api/player-api/identify.test.ts | 36 +++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/services/api/player-api.service.ts b/src/services/api/player-api.service.ts index 84eb8f84..3c7ac250 100644 --- a/src/services/api/player-api.service.ts +++ b/src/services/api/player-api.service.ts @@ -1,5 +1,5 @@ import { EntityManager } from '@mikro-orm/mysql' -import { Request, Response, Routes, Validate, HasPermission, ForwardTo, forwardRequest } from 'koa-clay' +import { Request, Response, Routes, Validate, HasPermission, ForwardTo, forwardRequest, ValidationCondition } from 'koa-clay' import APIKey, { APIKeyScope } from '../../entities/api-key' import Player from '../../entities/player' import GameSave from '../../entities/game-save' @@ -70,6 +70,15 @@ export async function createPlayerFromIdentifyRequest( } } +function validateIdentifyQueryParam(param: 'service' | 'identifier') { + return async (val?: string): Promise => [ + { + check: (val ?? '').trim().length > 0, + error: `Invalid ${param}, must be a non-empty string` + } + ] +} + @Routes([ { method: 'GET', @@ -90,7 +99,16 @@ export async function createPlayerFromIdentifyRequest( ]) export default class PlayerAPIService extends APIService { @Validate({ - query: ['service', 'identifier'] + query: { + service: { + required: true, + validation: validateIdentifyQueryParam('service') + }, + identifier: { + required: true, + validation: validateIdentifyQueryParam('identifier') + } + } }) @HasPermission(PlayerAPIPolicy, 'identify') @ForwardTo('games.players', 'post') diff --git a/tests/services/_api/player-api/identify.test.ts b/tests/services/_api/player-api/identify.test.ts index 13a7dda7..5cfa5714 100644 --- a/tests/services/_api/player-api/identify.test.ts +++ b/tests/services/_api/player-api/identify.test.ts @@ -124,4 +124,40 @@ describe('Player API service - identify', () => { expect(res.body).toStrictEqual({ message: 'Player not found: Talo aliases must be created using the /v1/players/auth API' }) }) + + it('should require the service to be a non-empty string', async () => { + const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) + const player = await new PlayerFactory([apiKey.game]).one() + await (global.em).persistAndFlush(player) + + const res = await request(global.app) + .get('/v1/players/identify') + .query({ service: '', identifier: player.aliases[0].identifier }) + .auth(token, { type: 'bearer' }) + .expect(400) + + expect(res.body).toStrictEqual({ + errors: { + service: ['Invalid service, must be a non-empty string'] + } + }) + }) + + it('should require the identifier to be a non-empty string', async () => { + const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) + const player = await new PlayerFactory([apiKey.game]).one() + await (global.em).persistAndFlush(player) + + const res = await request(global.app) + .get('/v1/players/identify') + .query({ service: player.aliases[0].service, identifier: '' }) + .auth(token, { type: 'bearer' }) + .expect(400) + + expect(res.body).toStrictEqual({ + errors: { + identifier: ['Invalid identifier, must be a non-empty string'] + } + }) + }) }) From ee592f7cf29393c2c0ed97d8d0af2b25cfab4f73 Mon Sep 17 00:00:00 2001 From: tudor <7089284+tudddorrr@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:28:01 +0100 Subject: [PATCH 2/3] additional tests for missing query params --- .../services/_api/player-api/identify.test.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/services/_api/player-api/identify.test.ts b/tests/services/_api/player-api/identify.test.ts index 5cfa5714..f72aeda5 100644 --- a/tests/services/_api/player-api/identify.test.ts +++ b/tests/services/_api/player-api/identify.test.ts @@ -125,6 +125,24 @@ describe('Player API service - identify', () => { expect(res.body).toStrictEqual({ message: 'Player not found: Talo aliases must be created using the /v1/players/auth API' }) }) + it('should require the service to be set', async () => { + const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) + const player = await new PlayerFactory([apiKey.game]).one() + await (global.em).persistAndFlush(player) + + const res = await request(global.app) + .get('/v1/players/identify') + .query({ identifier: player.aliases[0].identifier }) + .auth(token, { type: 'bearer' }) + .expect(400) + + expect(res.body).toStrictEqual({ + errors: { + service: ['service is missing from the request query'] + } + }) + }) + it('should require the service to be a non-empty string', async () => { const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) const player = await new PlayerFactory([apiKey.game]).one() @@ -143,6 +161,24 @@ describe('Player API service - identify', () => { }) }) + it('should require the identifier to be set', async () => { + const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) + const player = await new PlayerFactory([apiKey.game]).one() + await (global.em).persistAndFlush(player) + + const res = await request(global.app) + .get('/v1/players/identify') + .query({ service: player.aliases[0].service }) + .auth(token, { type: 'bearer' }) + .expect(400) + + expect(res.body).toStrictEqual({ + errors: { + identifier: ['identifier is missing from the request query'] + } + }) + }) + it('should require the identifier to be a non-empty string', async () => { const [apiKey, token] = await createAPIKeyAndToken([APIKeyScope.READ_PLAYERS]) const player = await new PlayerFactory([apiKey.game]).one() From bb2a7c4e6c410c800f48ded70b0e450d01cc3769 Mon Sep 17 00:00:00 2001 From: tudor <7089284+tudddorrr@users.noreply.github.com> Date: Sat, 19 Oct 2024 00:34:55 +0100 Subject: [PATCH 3/3] val can never be empty because required runs first --- src/services/api/player-api.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/api/player-api.service.ts b/src/services/api/player-api.service.ts index 3c7ac250..c84fd55c 100644 --- a/src/services/api/player-api.service.ts +++ b/src/services/api/player-api.service.ts @@ -73,7 +73,7 @@ export async function createPlayerFromIdentifyRequest( function validateIdentifyQueryParam(param: 'service' | 'identifier') { return async (val?: string): Promise => [ { - check: (val ?? '').trim().length > 0, + check: val.trim().length > 0, error: `Invalid ${param}, must be a non-empty string` } ]