From 72c3257d195d03f6ba2907ea63ee7e6d64defa70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Fri, 4 Sep 2020 22:11:34 +0200 Subject: [PATCH] BackendSrv: Uses credentials and defaults to same-origin (#27385) Co-authored-by: Liu Yang yangliuyu@163.com --- .../src/services/backendSrv.ts | 8 +++- public/app/core/utils/fetch.test.ts | 47 +++++++++++++++---- public/app/core/utils/fetch.ts | 29 ++++++++---- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/packages/grafana-runtime/src/services/backendSrv.ts b/packages/grafana-runtime/src/services/backendSrv.ts index 60bd3eed1ee..87f5a7c7c43 100644 --- a/packages/grafana-runtime/src/services/backendSrv.ts +++ b/packages/grafana-runtime/src/services/backendSrv.ts @@ -62,8 +62,12 @@ export type BackendSrvRequest = { params?: Record; /** - * Indicates whether or not cross-site Access-Control requests should be made using credentials such as cookies, authorization headers or TLS client certificates. Setting withCredentials has no effect on same-site requests. - * In addition, this flag is also used to indicate when cookies are to be ignored in the response. + * The credentials read-only property of the Request interface indicates whether the user agent should send cookies from the other domain in the case of cross-origin requests. + */ + credentials?: RequestCredentials; + + /** + * @deprecated withCredentials is deprecated in favor of credentials */ withCredentials?: boolean; }; diff --git a/public/app/core/utils/fetch.test.ts b/public/app/core/utils/fetch.test.ts index 71731d11a9c..31f0048bccd 100644 --- a/public/app/core/utils/fetch.test.ts +++ b/public/app/core/utils/fetch.test.ts @@ -2,6 +2,7 @@ import 'whatwg-fetch'; // fetch polyfill needed for PhantomJs rendering import { isContentTypeApplicationJson, parseBody, + parseCredentials, parseHeaders, parseInitFromOptions, parseUrlFromOptions, @@ -27,17 +28,18 @@ describe('parseUrlFromOptions', () => { describe('parseInitFromOptions', () => { it.each` - method | data | withCredentials | expected - ${undefined} | ${undefined} | ${undefined} | ${{ method: undefined, headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined }} - ${'GET'} | ${undefined} | ${undefined} | ${{ method: 'GET', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined }} - ${'POST'} | ${{ id: '0' }} | ${undefined} | ${{ method: 'POST', headers: { map: { 'content-type': 'application/json', accept: 'application/json, text/plain, */*' } }, body: '{"id":"0"}' }} - ${'PUT'} | ${{ id: '0' }} | ${undefined} | ${{ method: 'PUT', headers: { map: { 'content-type': 'application/json', accept: 'application/json, text/plain, */*' } }, body: '{"id":"0"}' }} - ${'monkey'} | ${undefined} | ${undefined} | ${{ method: 'monkey', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined }} - ${'GET'} | ${undefined} | ${true} | ${{ method: 'GET', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'include' }} + method | data | withCredentials | credentials | expected + ${undefined} | ${undefined} | ${undefined} | ${undefined} | ${{ method: undefined, headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'same-origin' }} + ${'GET'} | ${undefined} | ${undefined} | ${undefined} | ${{ method: 'GET', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'same-origin' }} + ${'POST'} | ${{ id: '0' }} | ${undefined} | ${undefined} | ${{ method: 'POST', headers: { map: { 'content-type': 'application/json', accept: 'application/json, text/plain, */*' } }, body: '{"id":"0"}', credentials: 'same-origin' }} + ${'PUT'} | ${{ id: '0' }} | ${undefined} | ${undefined} | ${{ method: 'PUT', headers: { map: { 'content-type': 'application/json', accept: 'application/json, text/plain, */*' } }, body: '{"id":"0"}', credentials: 'same-origin' }} + ${'monkey'} | ${undefined} | ${undefined} | ${'omit'} | ${{ method: 'monkey', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'omit' }} + ${'GET'} | ${undefined} | ${true} | ${undefined} | ${{ method: 'GET', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'include' }} + ${'GET'} | ${undefined} | ${true} | ${'omit'} | ${{ method: 'GET', headers: { map: { accept: 'application/json, text/plain, */*' } }, body: undefined, credentials: 'omit' }} `( - "when called with method: '$method', data: '$data' and withCredentials: '$withCredentials' then result should be '$expected'", - ({ method, data, withCredentials, expected }) => { - expect(parseInitFromOptions({ method, data, withCredentials, url: '' })).toEqual(expected); + "when called with method: '$method', data: '$data', withCredentials: '$withCredentials' and credentials: '$credentials' then result should be '$expected'", + ({ method, data, withCredentials, credentials, expected }) => { + expect(parseInitFromOptions({ method, data, withCredentials, credentials, url: '' })).toEqual(expected); } ); }); @@ -100,3 +102,28 @@ describe('parseBody', () => { } ); }); + +describe('parseCredentials', () => { + it.each` + options | expected + ${undefined} | ${undefined} + ${{}} | ${'same-origin'} + ${{ credentials: undefined }} | ${'same-origin'} + ${{ credentials: undefined, withCredentials: undefined }} | ${'same-origin'} + ${{ credentials: undefined, withCredentials: false }} | ${'same-origin'} + ${{ credentials: undefined, withCredentials: true }} | ${'include'} + ${{ credentials: 'invalid' }} | ${'invalid'} + ${{ credentials: 'invalid', withCredentials: undefined }} | ${'invalid'} + ${{ credentials: 'invalid', withCredentials: false }} | ${'invalid'} + ${{ credentials: 'invalid', withCredentials: true }} | ${'invalid'} + ${{ credentials: 'omit' }} | ${'omit'} + ${{ credentials: 'omit', withCredentials: undefined }} | ${'omit'} + ${{ credentials: 'omit', withCredentials: false }} | ${'omit'} + ${{ credentials: 'omit', withCredentials: true }} | ${'omit'} + `( + "when called with options: '$options' then the result should be '$expected'", + ({ options, isAppJson, expected }) => { + expect(parseCredentials(options)).toEqual(expected); + } + ); +}); diff --git a/public/app/core/utils/fetch.ts b/public/app/core/utils/fetch.ts index fb706f52ec3..ccf051ca572 100644 --- a/public/app/core/utils/fetch.ts +++ b/public/app/core/utils/fetch.ts @@ -1,25 +1,19 @@ import { BackendSrvRequest } from '@grafana/runtime'; import omitBy from 'lodash/omitBy'; +import { deprecationWarning } from '@grafana/data'; export const parseInitFromOptions = (options: BackendSrvRequest): RequestInit => { const method = options.method; const headers = parseHeaders(options); const isAppJson = isContentTypeApplicationJson(headers); const body = parseBody(options, isAppJson); - - if (options?.withCredentials) { - return { - method, - headers, - body, - credentials: 'include', - }; - } + const credentials = parseCredentials(options); return { method, headers, body, + credentials, }; }; @@ -114,3 +108,20 @@ export const parseUrlFromOptions = (options: BackendSrvRequest): string => { const serializedParams = serializeParams(cleanParams); return options.params && serializedParams.length ? `${options.url}?${serializedParams}` : options.url; }; + +export const parseCredentials = (options: BackendSrvRequest): RequestCredentials => { + if (!options) { + return options; + } + + if (options.credentials) { + return options.credentials; + } + + if (options.withCredentials) { + deprecationWarning('BackendSrvRequest', 'withCredentials', 'credentials'); + return 'include'; + } + + return 'same-origin'; +};