From 8558d0b1f1e8425b14b7c9247cecf9ae85ef60e6 Mon Sep 17 00:00:00 2001 From: Shaun Hamilton Date: Mon, 19 May 2025 12:53:24 +0200 Subject: [PATCH] fix(api): catch invalid ms-username url (#60402) Co-authored-by: Tom <20648924+moT01@users.noreply.github.com> --- api/src/routes/protected/user.test.ts | 63 ++++++++++++++++++--------- api/src/routes/protected/user.ts | 53 ++++++++++++++++------ api/src/routes/public/user.test.ts | 29 ------------ 3 files changed, 82 insertions(+), 63 deletions(-) diff --git a/api/src/routes/protected/user.test.ts b/api/src/routes/protected/user.test.ts index 2fe1c792256..be9c5d31766 100644 --- a/api/src/routes/protected/user.test.ts +++ b/api/src/routes/protected/user.test.ts @@ -973,21 +973,15 @@ Thanks and regards, }); it('handles invalid transcript urls', async () => { - mockedFetch.mockImplementationOnce(() => - Promise.resolve({ - ok: false - }) - ); - const response = await superPost('/user/ms-username').send({ msTranscriptUrl: 'https://www.example.com' }); expect(response.body).toStrictEqual({ type: 'error', - message: 'flash.ms.transcript.link-err-2' + message: 'flash.ms.transcript.link-err-1' }); - expect(response.statusCode).toBe(404); + expect(response.statusCode).toBe(400); }); it('handles the case that MS does not return a username', async () => { @@ -999,7 +993,8 @@ Thanks and regards, ); const response = await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/not/transcript/8u6ert43q1p' }); expect(response.body).toStrictEqual({ @@ -1029,7 +1024,8 @@ Thanks and regards, }); const response = await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/mot01/transcript/8wert4' }); expect(response.body).toStrictEqual({ @@ -1052,7 +1048,8 @@ Thanks and regards, }) ); const response = await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/mot01/transcript/8ert43q' }); expect(response.body).toStrictEqual({ @@ -1074,7 +1071,8 @@ Thanks and regards, ); await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/mot01/transcript/12345' }); const linkedAccount = @@ -1122,10 +1120,12 @@ Thanks and regards, }); await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo' }); await superPost('/user/ms-username').send({ - msTranscriptUrl: 'https://www.example.com' + msTranscriptUrl: + 'https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo' }); const linkedAccounts = @@ -1311,18 +1311,41 @@ describe('Microsoft helpers', () => { const urlWithQueryParamsAndSlash = `${urlWithSlash}?foo=bar`; it('should extract the transcript id from the url', () => { - expect(getMsTranscriptApiUrl(urlWithoutSlash)).toBe(expectedUrl); + expect(getMsTranscriptApiUrl(urlWithoutSlash)).toEqual({ + error: null, + data: expectedUrl + }); }); it('should handle trailing slashes', () => { - expect(getMsTranscriptApiUrl(urlWithSlash)).toBe(expectedUrl); + expect(getMsTranscriptApiUrl(urlWithSlash)).toEqual({ + error: null, + data: expectedUrl + }); }); it('should ignore query params', () => { - expect(getMsTranscriptApiUrl(urlWithQueryParams)).toBe(expectedUrl); - expect(getMsTranscriptApiUrl(urlWithQueryParamsAndSlash)).toBe( - expectedUrl - ); + expect(getMsTranscriptApiUrl(urlWithQueryParams)).toEqual({ + error: null, + data: expectedUrl + }); + expect(getMsTranscriptApiUrl(urlWithQueryParamsAndSlash)).toEqual({ + error: null, + data: expectedUrl + }); + }); + + it('should return an error for invalid URLs', () => { + const validBadUrl = 'https://www.example.com/invalid-url'; + expect(getMsTranscriptApiUrl(validBadUrl)).toEqual({ + error: expect.any(String), + data: null + }); + const invalidUrl = ' '; + expect(getMsTranscriptApiUrl(invalidUrl)).toEqual({ + error: expect.any(String), + data: null + }); }); }); }); diff --git a/api/src/routes/protected/user.ts b/api/src/routes/protected/user.ts index 85e1fb9e89d..385e7807a6f 100644 --- a/api/src/routes/protected/user.ts +++ b/api/src/routes/protected/user.ts @@ -29,21 +29,30 @@ import { JWT_SECRET } from '../../utils/env'; /** * Helper function to get the api url from the shared transcript link. + * Example msTranscriptUrl: https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo. * * @param msTranscript Shared transcript link. * @returns Microsoft transcript api url. */ -export const getMsTranscriptApiUrl = (msTranscript: string) => { - // example msTranscriptUrl: https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo - const url = new URL(msTranscript); - - // TODO(Post-MVP): throw if it doesn't match? - const transcriptUrlRegex = /\/transcript\/([^/]+)\/?/; - const id = transcriptUrlRegex.exec(url.pathname)?.[1]; - return `https://learn.microsoft.com/api/profiles/transcript/share/${ - id ?? '' - }`; -}; +export function getMsTranscriptApiUrl(msTranscript: string) { + try { + const url = new URL(msTranscript); + const transcriptUrlRegex = /\/transcript\/([^/]+)\/?/; + const id = transcriptUrlRegex.exec(url.pathname)?.[1]; + if (!id) { + return { error: `Invalid transcript URL: ${msTranscript}`, data: null }; + } + return { + error: null, + data: `https://learn.microsoft.com/api/profiles/transcript/share/${id}` + }; + } catch (e) { + return { + error: `Invalid transcript URL: ${msTranscript}\n${JSON.stringify(e)}`, + data: null + }; + } +} /** * Wrapper for endpoints related to user account management, @@ -272,17 +281,33 @@ export const userRoutes: FastifyPluginCallbackTypebox = ( }, async (req, reply) => { const logger = fastify.log.child({ req, res: reply }); - logger.info(`User ${req.user?.id} requested linking of msUsername`); + logger.info( + `User ${req.user?.id} requested linking of msUsername "${req.body.msTranscriptUrl}"` + ); try { const user = await fastify.prisma.user.findUniqueOrThrow({ where: { id: req.user?.id } }); - const msApiRes = await fetch( - getMsTranscriptApiUrl(req.body.msTranscriptUrl) + const maybeTranscriptUrl = getMsTranscriptApiUrl( + req.body.msTranscriptUrl ); + if (maybeTranscriptUrl.error !== null) { + logger.warn( + { error: maybeTranscriptUrl.error }, + 'Unable to parse Microsoft transcript URL' + ); + return reply + .status(400) + .send({ type: 'error', message: 'flash.ms.transcript.link-err-1' }); + } + + const transcriptUrl = maybeTranscriptUrl.data; + + const msApiRes = await fetch(transcriptUrl); + if (!msApiRes.ok) { logger.warn( { status: msApiRes.status }, diff --git a/api/src/routes/public/user.test.ts b/api/src/routes/public/user.test.ts index f8eb290354a..6487d358dbf 100644 --- a/api/src/routes/public/user.test.ts +++ b/api/src/routes/public/user.test.ts @@ -8,7 +8,6 @@ import { setupServer, createSuperRequest } from '../../../jest.utils'; -import { getMsTranscriptApiUrl } from '../protected/user'; import { replacePrivateData } from './user'; const mockedFetch = jest.fn(); @@ -444,34 +443,6 @@ describe('userRoutes', () => { }); }); -describe('Microsoft helpers', () => { - describe('getMsTranscriptApiUrl', () => { - const expectedUrl = - 'https://learn.microsoft.com/api/profiles/transcript/share/8u6awert43q1plo'; - - const urlWithoutSlash = - 'https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo'; - const urlWithSlash = `${urlWithoutSlash}/`; - const urlWithQueryParams = `${urlWithoutSlash}?foo=bar`; - const urlWithQueryParamsAndSlash = `${urlWithSlash}?foo=bar`; - - it('should extract the transcript id from the url', () => { - expect(getMsTranscriptApiUrl(urlWithoutSlash)).toBe(expectedUrl); - }); - - it('should handle trailing slashes', () => { - expect(getMsTranscriptApiUrl(urlWithSlash)).toBe(expectedUrl); - }); - - it('should ignore query params', () => { - expect(getMsTranscriptApiUrl(urlWithQueryParams)).toBe(expectedUrl); - expect(getMsTranscriptApiUrl(urlWithQueryParamsAndSlash)).toBe( - expectedUrl - ); - }); - }); -}); - describe('get-public-profile helpers', () => { describe('replacePrivateData', () => { const user = {