diff --git a/frontend/common/stores/organisation-store.js b/frontend/common/stores/organisation-store.js index ff50e0ba316a..3ad6718ae364 100644 --- a/frontend/common/stores/organisation-store.js +++ b/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') diff --git a/frontend/common/utils/__tests__/isNumericId.test.ts b/frontend/common/utils/__tests__/isNumericId.test.ts new file mode 100644 index 000000000000..2b295e5139c5 --- /dev/null +++ b/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) + }, + ) +}) diff --git a/frontend/common/utils/isNumericId.ts b/frontend/common/utils/isNumericId.ts new file mode 100644 index 000000000000..b139d787cdba --- /dev/null +++ b/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