Skip to content

Commit

Permalink
sec: 390 - Add validation for potential XSS vuln (#2797)
Browse files Browse the repository at this point in the history
* add tests, and validation for provider

* add back supportServiceless param
  • Loading branch information
ajay-sentry authored Apr 30, 2024
1 parent 2274a57 commit 940744c
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 5 deletions.
8 changes: 4 additions & 4 deletions src/shared/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import config from 'config'
import { camelizeKeys } from 'shared/utils/camelizeKeys'
import { snakeifyKeys } from 'shared/utils/snakeifyKeys'

import { generatePath, getHeaders } from './helpers'
import { generatePath, getHeaders, isProvider } from './helpers'

function _fetch({
path,
Expand Down Expand Up @@ -62,10 +62,10 @@ function graphql({
signal,
supportsServiceless = false,
}) {
let uri = `${config.API_URL}/graphql/${provider}`
let uri = `${config.API_URL}/graphql/`

if (supportsServiceless && !provider) {
uri = `${config.API_URL}/graphql/`
if (provider && isProvider(provider) && !supportsServiceless) {
uri = `${config.API_URL}/graphql/${provider}`
}

const headers = {
Expand Down
48 changes: 48 additions & 0 deletions src/shared/api/api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { setupServer } from 'msw/node'
import config from 'config'

import Api from './api'
import { AllProvidersArray } from './helpers'

const rawUserData = {
profile: {
Expand Down Expand Up @@ -223,6 +224,53 @@ describe('when using a graphql request', () => {
})
})

describe('when different strings entered as provider', () => {
let fetchMock
afterEach(() => {
global.fetch.mockRestore()
})

it('does not have the provider in the url for non-provider', async () => {
fetchMock = jest.fn((url, options) => {
expect(url).toBe(`${config.API_URL}/graphql/`)
return Promise.resolve({
ok: true,
json: async () => ({ data: { example: 'dummy data' } }),
})
})

jest.spyOn(global, 'fetch').mockImplementation(fetchMock)

await Api.graphql({
provider: 'random hacks and stuff',
query: 'query MyInfo { me }',
})

expect(fetchMock).toHaveBeenCalled()
})
test.each(AllProvidersArray)(
'has the provider in the url for %s',
async (provider) => {
fetchMock = jest.fn((url, options) => {
expect(url).toBe(`${config.API_URL}/graphql/${provider}`)
return Promise.resolve({
ok: true,
json: async () => ({ data: { example: 'dummy data' } }),
})
})

jest.spyOn(global, 'fetch').mockImplementation(fetchMock)

await Api.graphql({
provider: provider,
query: 'query MyInfo { me }',
})

expect(fetchMock).toHaveBeenCalled()
}
)
})

describe('the request is unsuccessful', () => {
it('returns the error status code', async () => {
const data = Api.graphql({
Expand Down
2 changes: 1 addition & 1 deletion src/shared/api/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export interface NetworkErrorObject {
dev: `${string} - ${number} ${string}`
}

const AllProvidersArray = [
export const AllProvidersArray = [
'gh',
'gl',
'bb',
Expand Down

0 comments on commit 940744c

Please sign in to comment.