fix(organisations): don't fetch an organisation with a non-numeric id

frontend/common/stores/organisation-store.js

@@ -7,8 +7,8 @@ import { getSubscriptionMetadata } from 'common/services/useSubscriptionMetadata
 import Dispatcher from 'common/dispatcher/dispatcher'
 import BaseStore from './base/_store'
 import data from 'common/data/base/_data'
+import { isNumericId } from 'common/utils/isNumericId'
 import filter from 'lodash/filter'
-import find from 'lodash/find'
 import findIndex from 'lodash/findIndex'
 import keyBy from 'lodash/keyBy'
 
@@ -75,10 +75,7 @@ const controller = {
     const idInt = parseInt(id)
     store.saving()
     if (store.model) {
-      store.model.projects = filter(
-        store.model.projects,
-        (p) => p.id !== idInt,
-      )
+      store.model.projects = filter(store.model.projects, (p) => p.id !== idInt)
       store.model.keyedProjects = keyBy(store.model.projects, 'id')
     }
     API.trackEvent(Constants.events.REMOVE_PROJECT)
@@ -118,6 +115,11 @@ const controller = {
       })
   },
   getOrganisation: (id, force) => {
+    // store.id seeds to the string 'account' and the dispatcher falls back to
+    // it, so a non-numeric id reaches here. The API can only reject it.
+    if (!isNumericId(id)) {
+      return
+    }
     if (`${id}` !== `${store.id}` || force) {
       store.id = id
       store.loading()
@@ -211,7 +213,9 @@ const controller = {
             projects.sort((a, b) => {
               const textA = a.name.toLowerCase()
               const textB = b.name.toLowerCase()
-              return textA < textB ? -1 : textA > textB ? 1 : 0
+              // Not localeCompare: it orders accents differently.
+              if (textA < textB) return -1
+              return textA > textB ? 1 : 0
             })
             store.model.projects = projects
             store.model.keyedProjects = keyBy(store.model.projects, 'id')

frontend/common/utils/__tests__/isNumericId.test.ts

@@ -0,0 +1,36 @@
+import { isNumericId } from 'common/utils/isNumericId'
+
+describe('isNumericId', () => {
+  // '007' is deliberate: DRF coerces a leading-zero string to the key 7, so the
+  // request works and rejecting it would be stricter than the API.
+  it.each([1, 42, '7', '007'])('accepts %p', (id) => {
+    expect(isNumericId(id)).toBe(true)
+  })
+
+  // 'account' prompted this: it reached projects/?organisation=account, which
+  // the API rejects as not an integer.
+  it.each(['account', 'undefined', 'NaN', '1.5', 'getting-started'])(
+    'refuses %p',
+    (id) => {
+      expect(isNumericId(id)).toBe(false)
+    },
+  )
+
+  it.each([undefined, null, '', 0, NaN])('refuses %p', (id) => {
+    expect(isNumericId(id)).toBe(false)
+  })
+
+  // Number() turns each of these into an integer, so truthiness alone lets
+  // them through.
+  it.each([true, [], ['5'], {}])('refuses the coercible %p', (id) => {
+    expect(isNumericId(id)).toBe(false)
+  })
+
+  // 0 and '0' disagreed under the truthiness check. Neither is a primary key.
+  it.each(['0', 0, '-1', -1])(
+    'refuses %p, which is not a primary key',
+    (id) => {
+      expect(isNumericId(id)).toBe(false)
+    },
+  )
+})

frontend/common/utils/isNumericId.ts

@@ -0,0 +1,7 @@
+// Ids reach API calls unvalidated, since route params and store values arrive
+// as strings. Number() also coerces non-ids ([] and true become numbers), so the
+// input is narrowed first, and an id must be positive: these are serial keys.
+export const isNumericId = (id: unknown): boolean =>
+  (typeof id === 'number' || typeof id === 'string') &&
+  Number.isInteger(Number(id)) &&
+  Number(id) > 0