diff --git a/src/bot/__tests__/requestReview.test.ts b/src/bot/__tests__/requestReview.test.ts index 8e627df3..86437d90 100644 --- a/src/bot/__tests__/requestReview.test.ts +++ b/src/bot/__tests__/requestReview.test.ts @@ -284,7 +284,7 @@ describe('requestReview', () => { [ActionId.HACKERRANK_URL]: { [ActionId.HACKERRANK_URL]: { type: 'plain_text_input', - value: 'https://www.hackerrank.com/test/example123', + value: 'https://www.hackerrank.com/test/example123?authkey=validkey123', }, }, }; @@ -384,7 +384,86 @@ _Candidate Identifier: some-identifier_ messageTimestamp: '100', }, ], - hackerRankUrl: 'https://www.hackerrank.com/test/example123', + hackerRankUrl: 'https://www.hackerrank.com/test/example123?authkey=validkey123', + }); + }); + + describe('HackerRank URL validation', () => { + it('should reject submission when HackerRank URL is missing authkey parameter', async () => { + const invalidUrlValues = { + ...defaultValues, + [ActionId.HACKERRANK_URL]: { + [ActionId.HACKERRANK_URL]: { + type: 'plain_text_input', + value: + 'https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary', + }, + }, + }; + + const param = buildParam(invalidUrlValues); + await requestReview.callback(param); + + expect(param.ack).toHaveBeenCalledWith({ + response_action: 'errors', + errors: { + [ActionId.HACKERRANK_URL]: + 'Please provide a valid HackerRank URL with an authkey. Use the "Share Report" button to get the correct URL.', + }, + }); + + // Should not proceed with creating the review + expect(activeReviewRepo.create).not.toHaveBeenCalled(); + expect(param.client.chat.postMessage).not.toHaveBeenCalled(); + }); + + it('should accept submission when HackerRank URL contains authkey parameter', async () => { + const validUrlValues = { + ...defaultValues, + [ActionId.HACKERRANK_URL]: { + [ActionId.HACKERRANK_URL]: { + type: 'plain_text_input', + value: + 'https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary?authkey=789', + }, + }, + }; + + const { param } = await callCallback(buildParam(validUrlValues)); + + // Should acknowledge without errors + expect(param.ack).toHaveBeenCalledWith(); + + // Should proceed with creating the review + expect(activeReviewRepo.create).toHaveBeenCalled(); + expect(param.client.chat.postMessage).toHaveBeenCalled(); + }); + + it('should reject submission when HackerRank URL is invalid format', async () => { + const invalidUrlValues = { + ...defaultValues, + [ActionId.HACKERRANK_URL]: { + [ActionId.HACKERRANK_URL]: { + type: 'plain_text_input', + value: 'not-a-valid-url', + }, + }, + }; + + const param = buildParam(invalidUrlValues); + await requestReview.callback(param); + + expect(param.ack).toHaveBeenCalledWith({ + response_action: 'errors', + errors: { + [ActionId.HACKERRANK_URL]: + 'Please provide a valid HackerRank URL with an authkey. Use the "Share Report" button to get the correct URL.', + }, + }); + + // Should not proceed with creating the review + expect(activeReviewRepo.create).not.toHaveBeenCalled(); + expect(param.client.chat.postMessage).not.toHaveBeenCalled(); }); }); }); diff --git a/src/bot/requestReview.ts b/src/bot/requestReview.ts index 9508b803..9a495404 100644 --- a/src/bot/requestReview.ts +++ b/src/bot/requestReview.ts @@ -19,6 +19,7 @@ import { } from './enums'; import { chatService } from '@/services/ChatService'; import { determineExpirationTime } from '@utils/reviewExpirationUtils'; +import { validateHackerRankUrl } from '@utils/urlValidation'; export const requestReview = { app: undefined as unknown as App, @@ -166,13 +167,29 @@ export const requestReview = { async callback(params: CallbackParam): Promise { const { ack, client, body } = params; - await ack(); if (!isViewSubmitActionParam(params)) { // TODO: How should we handle this case(if we need to)? log.d('callback called for non-submit action'); } + // Extract and validate HackerRank URL before acknowledging + const hackerRankUrl = blockUtils.getBlockValue(body, ActionId.HACKERRANK_URL); + const hackerRankUrlValue = hackerRankUrl.value; + + if (!validateHackerRankUrl(hackerRankUrlValue)) { + await ack({ + response_action: 'errors', + errors: { + [ActionId.HACKERRANK_URL]: + 'Please provide a valid HackerRank URL with an authkey. Use the "Share Report" button to get the correct URL.', + }, + }); + return; + } + + await ack(); + const user = body.user; const channel = process.env.INTERVIEWING_CHANNEL_ID; const numberOfInitialReviewers = Number(process.env.NUMBER_OF_INITIAL_REVIEWERS); @@ -181,7 +198,6 @@ export const requestReview = { const numberOfRequestedReviewers = blockUtils.getBlockValue(body, ActionId.NUMBER_OF_REVIEWERS); const candidateIdentifier = blockUtils.getBlockValue(body, ActionId.CANDIDATE_IDENTIFIER); const candidateType = blockUtils.getBlockValue(body, ActionId.CANDIDATE_TYPE); - const hackerRankUrl = blockUtils.getBlockValue(body, ActionId.HACKERRANK_URL); const numberOfReviewersValue = numberOfRequestedReviewers.value; const deadlineValue = deadline.selected_option.value; @@ -189,7 +205,6 @@ export const requestReview = { const candidateIdentifierValue = candidateIdentifier.value; const candidateTypeValue = candidateType.selected_option.value; const candidateTypeDisplay = candidateType.selected_option.text.text; - const hackerRankUrlValue = hackerRankUrl.value; log.d( 'requestReview.callback', 'Parsed values:', diff --git a/src/utils/__tests__/urlValidation.test.ts b/src/utils/__tests__/urlValidation.test.ts new file mode 100644 index 00000000..eeb77d0a --- /dev/null +++ b/src/utils/__tests__/urlValidation.test.ts @@ -0,0 +1,41 @@ +import { validateHackerRankUrl } from '../urlValidation'; + +describe('validateHackerRankUrl', () => { + it('should return true for a valid HackerRank URL with authkey', () => { + const validUrl = + 'https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary?authkey=789'; + expect(validateHackerRankUrl(validUrl)).toBe(true); + }); + + it('should return false for a HackerRank URL without authkey', () => { + const invalidUrl = + 'https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary'; + expect(validateHackerRankUrl(invalidUrl)).toBe(false); + }); + + it('should return true for a URL with authkey and other query parameters', () => { + const validUrl = + 'https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary?foo=bar&authkey=abc123&baz=qux'; + expect(validateHackerRankUrl(validUrl)).toBe(true); + }); + + it('should return false for an invalid URL format', () => { + const invalidUrl = 'not-a-valid-url'; + expect(validateHackerRankUrl(invalidUrl)).toBe(false); + }); + + it('should return false for an empty string', () => { + expect(validateHackerRankUrl('')).toBe(false); + }); + + it('should return false for a URL with authkey in the path but not as a query parameter', () => { + const invalidUrl = 'https://www.hackerrank.com/work/tests/authkey/123/report'; + expect(validateHackerRankUrl(invalidUrl)).toBe(false); + }); + + it('should return true for a URL with authkey as an empty value', () => { + // Even with an empty value, the parameter exists + const validUrl = 'https://www.hackerrank.com/work/tests/123/report?authkey='; + expect(validateHackerRankUrl(validUrl)).toBe(true); + }); +}); diff --git a/src/utils/urlValidation.ts b/src/utils/urlValidation.ts new file mode 100644 index 00000000..75cc9c41 --- /dev/null +++ b/src/utils/urlValidation.ts @@ -0,0 +1,24 @@ +/** + * Validates that a HackerRank URL contains an authkey query parameter. + * + * @param url - The HackerRank URL to validate + * @returns true if the URL contains an authkey parameter, false otherwise + * + * @example + * ```typescript + * validateHackerRankUrl('https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary?authkey=abc123') + * // returns true + * + * validateHackerRankUrl('https://www.hackerrank.com/work/tests/123/candidates/completed/456/report/summary') + * // returns false + * ``` + */ +export function validateHackerRankUrl(url: string): boolean { + try { + const urlObj = new URL(url); + return urlObj.searchParams.has('authkey'); + } catch { + // Invalid URL format + return false; + } +}