Skip to content

Commit

Permalink
[SupersetClient] allow csrf token to be passed as configuration (#9)
Browse files Browse the repository at this point in the history
* [SupersetClient] allow csrf token to be passed as configuration

* [SupersetClient] update readme

* [build-config] ^0.0.20

* [SupersetClient] fix test.

* [build-config] ^0.0.21

* [build-config] ^0.0.22

* [build-config] ^0.0.23
  • Loading branch information
williaster authored and zhaoyongjie committed Nov 26, 2021
1 parent 405dda5 commit f0b02cd
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*.map
*.min.js

babel.config.js
build/
coverage/
esm/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ for use within the Superset application, or used to issue `CORS` requests in oth
a high-level it supports:

- `CSRF` token authentication
- queues requests in the case that another request is made before the token is received
- it checks for a token before every request, an external app that uses this can detect this by
catching errors, or explicitly checking `SupersetClient.isAuthorized()`
- a token may be passed at configuration time, else the client will handle fetching and passing
the token in all subsequent requests.
- queues requests in the case that another request is made before the token is received.
- it checks for a token before every request, and will fail if no token was received or if it has
expired. In either case the user should be directed to re-authenticate.
- supports `GET` and `POST` requests (no `PUT` or `DELETE`)
- timeouts
- query aborts through the `AbortController` API
Expand Down Expand Up @@ -46,12 +48,14 @@ SupersetClient.post(...requestConfig)
The following flags can be passed in the client config call
`SupersetClient.configure(...clientConfig);`

- `protocol = 'http'`
- `protocol = 'http:'`
- `host`
- `headers`
- `credentials = 'same-origin'` (set to `include` for non-Superset apps)
- `mode = 'same-origin'` (set to `cors` for non-Superset apps)
- `timeout`
- `csrfToken` you can configure the client with a CSRF token at configuration time, else the client
will attempt to fetch this before any other requests are issued

##### Per-request Configuration

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
"lint:fix": "yarn run prettier --write && yarn run eslint --fix",
"test": "yarn run jest",
"prettier": "beemo prettier \"./{src,test}/**/*.{js,jsx,json,md}\"",
"sync:gitignore": "beemo sync-dotfiles --filter=gitignore",
"prepublish": "yarn run build"
},
"repository": {
Expand All @@ -40,12 +39,12 @@
},
"homepage": "https://github.com/apache-superset/superset-ui#readme",
"devDependencies": {
"@data-ui/build-config": "^0.0.14",
"@data-ui/build-config": "^0.0.23",
"fetch-mock": "^6.5.2",
"node-fetch": "^2.2.0"
},
"dependencies": {
"babel-runtime": "^6.26.0",
"@babel/runtime": "^7.1.2",
"whatwg-fetch": "^2.0.4"
},
"beemo": {
Expand All @@ -65,7 +64,6 @@
"rules": {
"prefer-promise-reject-errors": "off"
}
},
"jest": {}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class SupersetClient {
mode = 'same-origin',
timeout,
credentials,
csrfToken = null,
} = config;

this.headers = headers;
Expand All @@ -17,16 +18,20 @@ class SupersetClient {
this.timeout = timeout;
this.protocol = `${protocol}${protocol.slice(-1) === ':' ? '' : ':'}`;
this.credentials = credentials;
this.csrfToken = null;
this.didAuthSuccessfully = false;
this.csrfPromise = null;
this.csrfToken = csrfToken;
this.csrfPromise = this.isAuthenticated() ? Promise.resolve(this.csrfToken) : null;
}

isAuthenticated() {
return this.didAuthSuccessfully;
// if CSRF protection is disabled in the Superset app, the token may be an empty string
return this.csrfToken !== null && this.csrfToken !== undefined;
}

init() {
init(force = false) {
if (this.isAuthenticated() && !force) {
return this.csrfPromise;
}

return this.getCSRFToken();
}

Expand All @@ -48,14 +53,13 @@ class SupersetClient {
if (response.json) {
this.csrfToken = response.json.csrf_token;
this.headers = { ...this.headers, 'X-CSRFToken': this.csrfToken };
this.didAuthSuccessfully = this.csrfToken !== null && this.csrfPromise !== undefined;
}

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

return response;
return this.csrfToken;
});

return this.csrfPromise;
Expand Down Expand Up @@ -140,10 +144,10 @@ const PublicAPI = {
return singletonClient;
},
get: (...args) => hasInstance() && singletonClient.get(...args),
init: () => hasInstance() && singletonClient.init(),
init: force => hasInstance() && singletonClient.init(force),
isAuthenticated: () => hasInstance() && singletonClient.isAuthenticated(),
post: (...args) => hasInstance() && singletonClient.post(...args),
reAuthenticate: () => hasInstance() && singletonClient.getCSRFToken(),
reAuthenticate: () => hasInstance() && singletonClient.init(/* force = */ true),
reset: () => {
singletonClient = null;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,20 @@ describe('SupersetClient', () => {
const csrfSpy = jest.spyOn(SupersetClient.prototype, 'getCSRFToken');

PublicAPI.configure({});
expect(authenticatedSpy).toHaveBeenCalledTimes(1);

PublicAPI.init();
expect(initSpy).toHaveBeenCalledTimes(1);
expect(csrfSpy).toHaveBeenCalledTimes(1);

PublicAPI.get({ url: mockGetUrl });
PublicAPI.post({ url: mockPostUrl });
PublicAPI.isAuthenticated();
PublicAPI.reAuthenticate({});

expect(initSpy).toHaveBeenCalledTimes(1);
expect(initSpy).toHaveBeenCalledTimes(2);
expect(getSpy).toHaveBeenCalledTimes(1);
expect(postSpy).toHaveBeenCalledTimes(1);
expect(authenticatedSpy).toHaveBeenCalledTimes(1);
expect(csrfSpy).toHaveBeenCalledTimes(2); // from init() + reAuthenticate()

initSpy.mockRestore();
Expand All @@ -79,7 +81,7 @@ describe('SupersetClient', () => {
describe('CSRF', () => {
afterEach(fetchMock.reset);

it('calls superset/csrf_token/ upon initialization', () => {
it('calls superset/csrf_token/ when init() is called if no CSRF token is passed', () => {
expect.assertions(1);
const client = new SupersetClient({});

Expand All @@ -90,6 +92,35 @@ describe('SupersetClient', () => {
});
});

it('does NOT call superset/csrf_token/ when init() is called if a CSRF token is passed', () => {
expect.assertions(1);
const client = new SupersetClient({ csrfToken: 'abc' });

return client.init().then(() => {
expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0);

return Promise.resolve();
});
});

it('calls superset/csrf_token/ when init(force=true) is called even if a CSRF token is passed', () => {
expect.assertions(4);
const initialToken = 'inital_token';
const client = new SupersetClient({ csrfToken: initialToken });

return client.init().then(() => {
expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(0);
expect(client.csrfToken).toBe(initialToken);

return client.init(true).then(() => {
expect(fetchMock.calls(LOGIN_GLOB)).toHaveLength(1);
expect(client.csrfToken).not.toBe(initialToken);

return Promise.resolve();
});
});
});

it('isAuthenticated() returns true if there is a token and false if not', () => {
expect.assertions(2);
const client = new SupersetClient({});
Expand All @@ -102,6 +133,15 @@ describe('SupersetClient', () => {
});
});

it('isAuthenticated() returns true if a token is passed at configuration', () => {
expect.assertions(2);
const clientWithoutToken = new SupersetClient({ csrfToken: null });
const clientWithToken = new SupersetClient({ csrfToken: 'token' });

expect(clientWithoutToken.isAuthenticated()).toBe(false);
expect(clientWithToken.isAuthenticated()).toBe(true);
});

it('init() throws if superset/csrf_token/ returns an error', () => {
expect.assertions(1);

Expand All @@ -120,7 +160,7 @@ describe('SupersetClient', () => {
// reset
fetchMock.get(
LOGIN_GLOB,
{ csrf_token: 1234 },
{ csrf_token: '1234' },
{
overwriteRoutes: true,
},
Expand Down Expand Up @@ -167,7 +207,7 @@ describe('SupersetClient', () => {
.then(throwIfCalled)
.catch(error => {
expect(error).toEqual(expect.objectContaining({ error: expect.any(String) }));
expect(client.didAuthSuccessfully).toBe(false);
expect(client.isAuthenticated()).toBe(false);

return Promise.resolve();
});
Expand All @@ -183,7 +223,7 @@ describe('SupersetClient', () => {
.ensureAuth()
.then(throwIfCalled)
.catch(() => {
expect(client.didAuthSuccessfully).toBe(true);
expect(client.isAuthenticated()).toBe(true);

return Promise.resolve();
}),
Expand Down Expand Up @@ -211,7 +251,7 @@ describe('SupersetClient', () => {
.then(throwIfCalled)
.catch(error2 => {
expect(error2).toEqual(expect.objectContaining(rejectValue));
expect(client.didAuthSuccessfully).toBe(false);
expect(client.isAuthenticated()).toBe(false);

// reset
fetchMock.get(
Expand Down

0 comments on commit f0b02cd

Please sign in to comment.