diff --git a/api/src/routes/challenge.test.ts b/api/src/routes/challenge.test.ts index 506e0b44371..04221e95321 100644 --- a/api/src/routes/challenge.test.ts +++ b/api/src/routes/challenge.test.ts @@ -112,12 +112,16 @@ describe('challengeRoutes', () => { }); describe('POST /coderoad-challenge-completed', () => { - test('should return 500 if no tutorialId', async () => { + test('should return 400 if no tutorialId', async () => { const response = await superRequest('/coderoad-challenge-completed', { method: 'POST', setCookies }); - expect(response.status).toBe(500); + expect(response.body).toEqual({ + msg: `'tutorialId' not found in request body`, + type: 'error' + }); + expect(response.status).toBe(400); }); test('should return 400 if no user token', async () => { @@ -127,11 +131,11 @@ describe('challengeRoutes', () => { }).send({ tutorialId: 'freeCodeCamp/learn-bash-by-building-a-boilerplate:v1.0.0' }); - expect(response.status).toBe(400); expect(response.body).toEqual({ msg: `'Coderoad-User-Token' not found in request headers`, type: 'error' }); + expect(response.status).toBe(400); }); test('should return 400 if invalid user token', async () => { diff --git a/api/src/routes/challenge.ts b/api/src/routes/challenge.ts index af91ee06829..c0a0bee8c1e 100644 --- a/api/src/routes/challenge.ts +++ b/api/src/routes/challenge.ts @@ -10,7 +10,10 @@ import { type CompletedChallenge } from '../utils/common-challenge-functions'; import { JWT_SECRET } from '../utils/env'; -import { formatValidationError } from '../utils/error-formatting'; +import { + formatCoderoadChallengeCompletedValidation, + formatProjectCompletedValidation +} from '../utils/error-formatting'; import { getChallenges } from '../utils/get-challenges'; import { ProgressTimestamp, getPoints } from '../utils/progress'; import { @@ -45,44 +48,29 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = ( '/coderoad-challenge-completed', { schema: schemas.coderoadChallengeCompleted, - attachValidation: true + errorHandler(error, request, reply) { + if (error.validation) { + void reply.code(400); + return formatCoderoadChallengeCompletedValidation(error.validation); + } else { + fastify.errorHandler(error, request, reply); + } + } }, async (req, reply) => { - let userToken; - const { 'coderoad-user-token': encodedUserToken } = req.headers; const { tutorialId } = req.body; - if (!tutorialId) { - void reply.code(400); - return { - type: 'error', - msg: `'tutorialId' not found in request body` - } as const; - } - - if (!encodedUserToken) { - void reply.code(400); - return { - type: 'error', - msg: `'Coderoad-User-Token' not found in request headers` - } as const; - } - + let userToken; try { - if (typeof encodedUserToken === 'string') { - const payload = jwt.verify( - encodedUserToken, - JWT_SECRET - ) as JwtPayload; - userToken = payload.userToken; - } + const payload = jwt.verify(encodedUserToken, JWT_SECRET) as JwtPayload; + userToken = payload.userToken; } catch { void reply.code(400); return { type: 'error', msg: `invalid user token` } as const; } - const tutorialRepo = tutorialId?.split(':')[0]; + const tutorialRepo = tutorialId.split(':')[0]; const tutorialOrg = tutorialRepo?.split('/')?.[0]; if (tutorialOrg !== 'freeCodeCamp') { @@ -183,7 +171,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = ( errorHandler(error, request, reply) { if (error.validation) { void reply.code(400); - return formatValidationError(error.validation); + return formatProjectCompletedValidation(error.validation); } else { fastify.errorHandler(error, request, reply); } @@ -270,7 +258,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = ( errorHandler(error, request, reply) { if (error.validation) { void reply.code(400); - return formatValidationError(error.validation); + return formatProjectCompletedValidation(error.validation); } else { fastify.errorHandler(error, request, reply); } @@ -322,7 +310,7 @@ export const challengeRoutes: FastifyPluginCallbackTypebox = ( errorHandler(error, request, reply) { if (error.validation) { void reply.code(400); - return formatValidationError(error.validation); + return formatProjectCompletedValidation(error.validation); } else { fastify.errorHandler(error, request, reply); } diff --git a/api/src/schemas.ts b/api/src/schemas.ts index 82ead55db11..52229aeb9c9 100644 --- a/api/src/schemas.ts +++ b/api/src/schemas.ts @@ -415,6 +415,7 @@ export const schemas = { body: Type.Object({ tutorialId: Type.String() }), + headers: Type.Object({ 'coderoad-user-token': Type.String() }), response: { 200: Type.Object({ type: Type.Literal('success'), diff --git a/api/src/utils/error-formatting.test.ts b/api/src/utils/error-formatting.test.ts deleted file mode 100644 index 51531c1c57f..00000000000 --- a/api/src/utils/error-formatting.test.ts +++ /dev/null @@ -1,72 +0,0 @@ -import { ErrorObject } from 'ajv'; -import { formatValidationError } from './error-formatting'; - -const missingSolutionError = { - instancePath: '', - schemaPath: '#/required', - keyword: 'required', - params: { missingProperty: 'solution' }, - message: "must have required property 'solution'" -}; - -const missingIdError = { - instancePath: '', - schemaPath: '#/required', - keyword: 'required', - params: { missingProperty: 'id' }, - message: "must have required property 'id'" -}; - -describe('Error formatting', () => { - describe('formatValidationError', () => { - it('should handle missing solutions', () => { - const formattedError = formatValidationError([missingSolutionError]); - - expect(formattedError).toStrictEqual({ - type: 'error', - message: - 'You have not provided the valid links for us to inspect your work.' - }); - }); - - it('should handle missing ids', () => { - const formattedError = formatValidationError([missingIdError]); - - expect(formattedError).toStrictEqual({ - type: 'error', - - message: 'That does not appear to be a valid challenge submission.' - }); - }); - - it('should return a generic error message for other errors', () => { - const formattedError = formatValidationError([ - { - ...missingSolutionError, - params: { missingProperty: 'notSolution' } - } - ]); - - expect(formattedError).toStrictEqual({ - type: 'error', - message: 'That does not appear to be a valid challenge submission.' - }); - }); - - it('should throw if passed zero errors', () => { - expect(() => formatValidationError([] as ErrorObject[])).toThrow( - Error( - 'Bad Argument: the array of errors must have exactly one element.' - ) - ); - }); - - it('should throw if passed more than one error', () => { - expect(() => formatValidationError([{}, {}] as ErrorObject[])).toThrow( - Error( - 'Bad Argument: the array of errors must have exactly one element.' - ) - ); - }); - }); -}); diff --git a/api/src/utils/error-formatting.ts b/api/src/utils/error-formatting.ts index 0a2c0afeebc..222024d824e 100644 --- a/api/src/utils/error-formatting.ts +++ b/api/src/utils/error-formatting.ts @@ -1,30 +1,41 @@ import { ErrorObject } from 'ajv'; -export type FormattedError = { +type FormattedError = { type: 'error'; - message: - | 'You have not provided the valid links for us to inspect your work.' - | 'That does not appear to be a valid challenge submission.' - // the next isn't generated here, but the type is more general. - | 'You have to complete the project before you can submit a URL.'; + message: string; }; -/** - * Format invalid challenge submission errors. - * - * @param errors An array of validation errors. - * @returns Formatted errors that can be used in the response. - */ -export const formatValidationError = ( - errors: ErrorObject[] -): FormattedError => { - if (errors.length !== 1) { +// TODO(Post-MVP): Normalize error responses (either msg or messge, not both) +type CodeRoadError = { + type: 'error'; + msg: string; +}; + +const getError = (errors: ErrorObject[]): ErrorObject => { + // This is a guard against accidentally enabling allErrors in ajv and making + // the server more vulnerable to DOS. + const error = errors[0]; + if (!error || errors.length !== 1) { throw new Error( 'Bad Argument: the array of errors must have exactly one element.' ); } + return error; +}; - return errors[0]?.params.missingProperty === 'solution' +/** + * Format validation errors for /project-completed. + * + * @param errors An array of validation errors. + * @returns Formatted errors that can be used in the response. + */ +export const formatProjectCompletedValidation = ( + errors: ErrorObject[] +): FormattedError => { + const error = getError(errors); + + return error.instancePath === '' && + error.params.missingProperty === 'solution' ? { type: 'error', message: @@ -35,3 +46,32 @@ export const formatValidationError = ( message: 'That does not appear to be a valid challenge submission.' }; }; + +/** + * Format validation errors for /coderoad-challenge-completed. + * + * @param errors An array of validation errors. + * @returns Formatted errors that can be used in the response. + */ +export const formatCoderoadChallengeCompletedValidation = ( + errors: ErrorObject[] +): CodeRoadError => { + const error = getError(errors); + + // TODO(Post-MVP): Return error saying that the body is not an object. + if (error.instancePath === '' && error.message === 'must be object') + return { type: 'error', msg: `'tutorialId' not found in request body` }; + + if ( + error.instancePath === '' && + error.params.missingProperty === 'coderoad-user-token' + ) { + return { + type: 'error', + msg: `'Coderoad-User-Token' not found in request headers` + }; + } else { + // by process of elimination: + return { type: 'error', msg: `'tutorialId' not found in request body` }; + } +};