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

@@ -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(urlWithQueryParams)).toEqual({
-      expect(getMsTranscriptApiUrl(urlWithQueryParamsAndSlash)).toBe(
+        error: null,
-        expectedUrl
+        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
+      });
     });
   });
 });

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) => {
+export function getMsTranscriptApiUrl(msTranscript: string) {
-  // example msTranscriptUrl: https://learn.microsoft.com/en-us/users/mot01/transcript/8u6awert43q1plo
+  try {
     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/${
+    if (!id) {
-    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(
+        const maybeTranscriptUrl = getMsTranscriptApiUrl(
-          getMsTranscriptApiUrl(req.body.msTranscriptUrl)
+          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 },

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 = {