Skip to content

Commit

Permalink
fix: login error before discovery initialized after logout (#173)
Browse files Browse the repository at this point in the history 
* fix: login error before discovery initialized after logout
* chore: add test
* fix: add promise refer
  • Loading branch information
@embbnux
embbnux authored Mar 14, 2022
1 parent 08fb47f commit cf52ee2
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 42 deletions.
130 changes: 118 additions & 12 deletions sdk/src/platform/Platform-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ describe('RingCentral.platform.Platform', () => {
implicit: true,
usePKCE: true,
}),
).to.throw('PKCE only works with Authrization Code Flow');
).to.throw('PKCE only works with Authorization Code Flow');

expect(
platform.loginUrl({
Expand Down Expand Up @@ -984,9 +984,6 @@ describe('RingCentral.platform.Platform', () => {

const sdk = createSdk({enableDiscovery: true, discoveryServer: 'http://whatever', server: ''});
const platform = sdk.platform();
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
Expand Down Expand Up @@ -1024,9 +1021,6 @@ describe('RingCentral.platform.Platform', () => {

const sdk = createSdk({enableDiscovery: true, discoveryServer: 'http://whatever', server: ''});
const platform = sdk.platform();
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
Expand All @@ -1050,9 +1044,6 @@ describe('RingCentral.platform.Platform', () => {

const sdk = createSdk({enableDiscovery: true, discoveryServer: 'http://whatever', server: ''});
const platform = sdk.platform();
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
const clientFetchErrorSpy = spy(() => {});
sdk.client().on(sdk.client().events.requestError, clientFetchErrorSpy);
let hasError = false;
Expand All @@ -1067,7 +1058,6 @@ describe('RingCentral.platform.Platform', () => {
}
expect(hasError).to.equal(true);
expect(clientFetchErrorSpy.calledThrice).to.equal(true);
await platform.discovery().init();
});
});

Expand All @@ -1085,7 +1075,6 @@ describe('RingCentral.platform.Platform', () => {

const sdk = createSdk({enableDiscovery: true, discoveryServer: 'http://whatever', server: ''});
platform = sdk.platform();
await platform.discovery().init();
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
Expand Down Expand Up @@ -1125,9 +1114,51 @@ describe('RingCentral.platform.Platform', () => {
expect(noErrors).to.equal(true);
const loginUrl = await platform.loginUrlWithDiscovery();
const initialData = await platform.discovery().initialData();
expect(initialData.authApi.authorizationUri).to.equal(initialDiscoveryData.authApi.authorizationUri);
expect(loginUrl.indexOf(initialDiscoveryData.authApi.authorizationUri)).to.equal(0);
});

it('should init discovery when logout error', async () => {
cleanFetchMock();
logout(404);
const initialDiscoveryData = getInitialDiscoveryMockData();
initialDiscoveryData.authApi.authorizationUri = 'http://whatever1/restapi/oauth/authorize';
apiCall('GET', '/.well-known/entry-points/initial?clientId=whatever', initialDiscoveryData);
let hasError = false;
try {
await platform.logout();
} catch (e) {
hasError = true;
}
expect(hasError).to.equal(true);
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
const initialData = await platform.discovery().initialData();
expect(initialData.authApi.authorizationUri).to.equal(initialDiscoveryData.authApi.authorizationUri);
});

it('should login successfully after logout', async () => {
cleanFetchMock();
logout();
const initialDiscoveryData = getInitialDiscoveryMockData();
initialDiscoveryData.authApi.authorizationUri = 'http://whatever1/restapi/oauth/authorize';
apiCall('GET', '/.well-known/entry-points/initial?clientId=whatever', initialDiscoveryData);
await platform.logout();
const externalDiscoveryData = getExternalDiscoveryMockData();
apiCall('GET', '/.well-known/entry-points/external', externalDiscoveryData);
authentication();
const noErrors = true;
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
token_uri: 'http://whatever/restapi/oauth/token',
});
expect(noErrors).to.equal(true);
const externalData = await platform.discovery().externalData();
expect(externalData.coreApi.baseUri).to.equal(externalDiscoveryData.coreApi.baseUri);
});

it('should trigger refresh external discovery data when Discovery-Required', async () => {
apiCall('GET', '/restapi/v1.0/foo/1', {increment: 1}, 200, 'OK', {
'Discovery-Required': 1,
Expand Down Expand Up @@ -1184,6 +1215,81 @@ describe('RingCentral.platform.Platform', () => {
});
});

describe('Init discovery with clearCacheOnRefreshError flag ', () => {
let platform;

beforeEach(async () => {
cleanFetchMock();
const initialDiscoveryData = getInitialDiscoveryMockData();
const externalDiscoveryData = getExternalDiscoveryMockData();
// mock
apiCall('GET', '/.well-known/entry-points/initial?clientId=whatever', initialDiscoveryData);
apiCall('GET', '/.well-known/entry-points/external', externalDiscoveryData);
authentication();

const sdk = createSdk({
enableDiscovery: true,
discoveryServer: 'http://whatever',
server: '',
clearCacheOnRefreshError: true,
});
platform = sdk.platform();
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
});

it('should init discovery when login error', async () => {
cleanFetchMock();
authentication(502);
const initialDiscoveryData = getInitialDiscoveryMockData();
initialDiscoveryData.authApi.authorizationUri = 'http://whatever1/restapi/oauth/authorize';
apiCall('GET', '/.well-known/entry-points/initial?clientId=whatever', initialDiscoveryData);
let hasError = false;
try {
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
token_uri: 'http://whatever/restapi/oauth/token',
});
} catch (e) {
hasError = true;
}
expect(hasError).to.equal(true);
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
const initialData = await platform.discovery().initialData();
expect(initialData.authApi.authorizationUri).to.equal(initialDiscoveryData.authApi.authorizationUri);
});

it('should init discovery when refresh error', async () => {
authentication();
await platform.login({
code: 'whatever',
discovery_uri: 'http://whatever/.well-known/entry-points/external',
token_uri: 'http://whatever/restapi/oauth/token',
});
cleanFetchMock();
tokenRefresh(true);
const initialDiscoveryData = getInitialDiscoveryMockData();
initialDiscoveryData.authApi.authorizationUri = 'http://whatever1/restapi/oauth/authorize';
apiCall('GET', '/.well-known/entry-points/initial?clientId=whatever', initialDiscoveryData);
let hasError = false;
try {
await platform.refresh();
} catch (e) {
hasError = true;
}
expect(hasError).to.equal(true);
if (platform.discoveryInitPromise) {
await platform.discoveryInitPromise;
}
const initialData = await platform.discovery().initialData();
expect(initialData.authApi.authorizationUri).to.equal(initialDiscoveryData.authApi.authorizationUri);
});
});

describe('API request with discovery tag', () => {
let platform;
let sdk;
Expand Down
41 changes: 23 additions & 18 deletions sdk/src/platform/Platform.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,6 @@ export default class Platform extends EventEmitter {
this._discovery.on(this._discovery.events.initialized, discoveryData => {
this._authorizeEndpoint = discoveryData.authApi.authorizationUri;
});
this.on(events.logoutSuccess, this._fetchDiscoveryAndUpdateAuthorizeEndpoint);
this.on(events.logoutError, this._fetchDiscoveryAndUpdateAuthorizeEndpoint);
this.on(events.refreshError, this._updateDiscoveryAndAuthorizeEndpointOnRefreshError);
this.on(events.loginError, this._updateDiscoveryAndAuthorizeEndpointOnRefreshError);
this._client.on(this._client.events.requestSuccess, response => {
if (response.headers.get('discovery-required')) {
this._discovery.refreshExternalData();
Expand Down Expand Up @@ -190,17 +186,6 @@ export default class Platform extends EventEmitter {
return this._discovery;
}

private _fetchDiscoveryAndUpdateAuthorizeEndpoint = async () => {
const discoveryData = await this._discovery.fetchInitialData();
this._authorizeEndpoint = discoveryData.authApi.authorizationUri;
};

private _updateDiscoveryAndAuthorizeEndpointOnRefreshError = async () => {
if (this._clearCacheOnRefreshError) {
await this._fetchDiscoveryAndUpdateAuthorizeEndpoint();
}
};

public createUrl(path = '', options: CreateUrlOptions = {}) {
let builtUrl = '';

Expand Down Expand Up @@ -229,7 +214,13 @@ export default class Platform extends EventEmitter {

public async loginUrlWithDiscovery(options: LoginUrlOptions = {}) {
if (this._discovery) {
await this._fetchDiscoveryAndUpdateAuthorizeEndpoint();
// fetch new discovery when generate login url
const discoveryData = await this._discovery.fetchInitialData();
this._authorizeEndpoint = discoveryData.authApi.authorizationUri;
if (this._discoveryInitPromise) {
// await init discovery if it's not initialized
await this._discoveryInitPromise;
}
}
return this.loginUrl(options);
}
Expand Down Expand Up @@ -282,7 +273,7 @@ export default class Platform extends EventEmitter {
query.discovery = true;
}
if (usePKCE && implicit) {
throw new Error('PKCE only works with Authrization Code Flow');
throw new Error('PKCE only works with Authorization Code Flow');
}
this._codeVerifier = '';
if (usePKCE) {
Expand Down Expand Up @@ -495,7 +486,12 @@ export default class Platform extends EventEmitter {

return response;
} catch (e) {
if (this._clearCacheOnRefreshError) await this._cache.clean();
if (this._clearCacheOnRefreshError) {
await this._cache.clean();
if (this._discovery) {
this._discoveryInitPromise = this.initDiscovery(); // request new init data after refresh error and cache cleaned
}
}

this.emit(this.events.loginError, e);

Expand Down Expand Up @@ -571,6 +567,9 @@ export default class Platform extends EventEmitter {
} catch (e) {
if (this._clearCacheOnRefreshError) {
await this._cache.clean();
if (this._discovery) {
this._discoveryInitPromise = this.initDiscovery(); // request new init data after refresh error and cache cleaned
}
}

this.emit(this.events.refreshError, e);
Expand Down Expand Up @@ -619,9 +618,15 @@ export default class Platform extends EventEmitter {
}

await this._cache.clean();
if (this._discovery) {
this._discoveryInitPromise = this.initDiscovery(); // request new init data after logout
}
this.emit(this.events.logoutSuccess, res);
return res;
} catch (e) {
if (this._discovery) {
this._discoveryInitPromise = this.initDiscovery(); // request new init data after logout error
}
this.emit(this.events.logoutError, e);

throw e;
Expand Down
29 changes: 17 additions & 12 deletions sdk/src/test/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,25 @@ export function apiCall(method, path, json, status = 200, statusText = 'OK', hea
});
}

export function authentication() {
apiCall('POST', '/restapi/oauth/token', {
access_token: 'ACCESS_TOKEN',
token_type: 'bearer',
expires_in: 3600,
refresh_token: 'REFRESH_TOKEN',
refresh_token_expires_in: 60480,
scope: 'SMS RCM Foo Boo',
expireTime: new Date().getTime() + 3600000,
});
export function authentication(status = 200) {
apiCall(
'POST',
'/restapi/oauth/token',
{
access_token: 'ACCESS_TOKEN',
token_type: 'bearer',
expires_in: 3600,
refresh_token: 'REFRESH_TOKEN',
refresh_token_expires_in: 60480,
scope: 'SMS RCM Foo Boo',
expireTime: new Date().getTime() + 3600000,
},
status,
);
}

export function logout() {
apiCall('POST', '/restapi/oauth/revoke', {});
export function logout(status = 200) {
apiCall('POST', '/restapi/oauth/revoke', {}, status);
}

export function tokenRefresh(failure = false) {
Expand Down

0 comments on commit cf52ee2

Please sign in to comment.