Skip to content

Commit

Permalink
[core] support empty CSRF token (#7)
Browse files Browse the repository at this point in the history
* [core][deps] remove unused deps

* [core][test] add test for response.ok=false

* [core] support csrf_token=''

* [core][tests] move mock to sole function that uses it
  • Loading branch information
williaster authored and zhaoyongjie committed Nov 26, 2021
1 parent eabb10d commit f5d62e3
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
},
"dependencies": {
"babel-runtime": "^6.26.0",
"url-search-params-polyfill": "^4.0.1",
"whatwg-fetch": "^2.0.4"
},
"beemo": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ class SupersetClient {
}

getCSRFToken() {
this.csrfToken = null;

// If we can request this resource successfully, it means that the user has
// authenticated. If not we throw an error prompting to authenticate.
this.csrfPromise = callApi({
Expand All @@ -46,10 +48,10 @@ class SupersetClient {
if (response.json) {
this.csrfToken = response.json.csrf_token;
this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken };
this.didAuthSuccessfully = !!this.csrfToken;
this.didAuthSuccessfully = this.csrfToken !== null && this.csrfPromise !== undefined;
}

if (!this.csrfToken) {
if (!this.didAuthSuccessfully) {
return Promise.reject({ error: 'Failed to fetch CSRF token' });
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import 'whatwg-fetch';
import 'url-search-params-polyfill';

const DEFAULT_HEADERS = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,7 @@ export default function parseResponse(apiPromise, parseMethod = 'json') {
return apiPromise
.then(response => {
if (!response.ok) {
return Promise.reject({
error: response.error || 'An error occurred',
response,
status: response.status,
statusText: response.statusText,
});
return Promise.reject(response);
}

return response;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { LOGIN_GLOB } from './fixtures/constants';

describe('SupersetClient', () => {
beforeAll(() => {
fetchMock.get(LOGIN_GLOB, { csrf_token: '1234' });
fetchMock.get(LOGIN_GLOB, { csrf_token: '' });
});

afterAll(fetchMock.restore);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,12 @@ describe('callApi()', () => {

const mockGetUrl = '/mock/get/url';
const mockPostUrl = '/mock/post/url';
const mockErrorUrl = '/mock/error/url';

const mockGetPayload = { get: 'payload' };
const mockPostPayload = { post: 'payload' };
const mockErrorPayload = { status: 500, statusText: 'Internal error' };

fetchMock.get(mockGetUrl, mockGetPayload);
fetchMock.post(mockPostUrl, mockPostPayload);
fetchMock.get(mockErrorUrl, () => Promise.reject(mockErrorPayload));

afterEach(fetchMock.reset);

Expand Down Expand Up @@ -145,6 +142,10 @@ describe('callApi()', () => {
});

it('rejects if the request throws', () => {
const mockErrorUrl = '/mock/error/url';
const mockErrorPayload = { status: 500, statusText: 'Internal error' };
fetchMock.get(mockErrorUrl, () => Promise.reject(mockErrorPayload));

expect.assertions(3);

return callApi({ url: mockErrorUrl, method: 'GET' })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,20 @@ describe('parseResponse()', () => {
return Promise.resolve();
});
});

it('rejects if request.ok=false', () => {
const mockNotOkayUrl = '/mock/notokay/url';
fetchMock.get(mockNotOkayUrl, 404); // 404s result in not response.ok=false

expect.assertions(3);
const apiPromise = callApi({ url: mockNotOkayUrl, method: 'GET' });

return parseResponse(apiPromise)
.then(throwIfCalled)
.catch(response => {
expect(fetchMock.calls(mockNotOkayUrl)).toHaveLength(1);
expect(response.ok).toBe(false);
expect(response.status).toBe(404);
});
});
});

0 comments on commit f5d62e3

Please sign in to comment.