Skip to content

Commit

Permalink
fix(@aws-amplify/api): error handling for signed requests in RestClie…
Browse files Browse the repository at this point in the history 
…nt (#1362)

* fix for #1341

* Added Test for #1341
Removed unused mocks from tests
  • Loading branch information
@cornr @elorzafe
cornr authored and elorzafe committed Aug 7, 2018
1 parent 1d556af commit fbbeffb
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 134 deletions.
155 changes: 27 additions & 128 deletions packages/api/__tests__/RestClient-unit-test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
jest.mock('@aws-amplify/core/lib/Signer', () => {
return {
default: {
sign: () => {
return {
data: 'data',
headers: {}
};
sign: (request: any, access_info: any, service_info?: any) => {
return request;
}
}
}
Expand All @@ -15,12 +12,18 @@ jest.mock('axios', () => {
return {
default: (signed_params) => {
return new Promise((res, rej) => {
res({
data: 'data'
})
if (signed_params.reject) {
rej({
data: 'error'
});
} else {
res({
data: 'data'
});
}
});
}
}
};
});

import { RestClient } from '../src/RestClient';
Expand All @@ -41,17 +44,6 @@ const spyon = jest.spyOn(Credentials, 'get').mockImplementation(() => {
describe('RestClient test', () => {
describe('ajax', () => {
test('fetch with signed request', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const apiOptions = {
headers: {},
endpoints: {},
Expand All @@ -67,18 +59,22 @@ describe('RestClient test', () => {
expect(await restClient.ajax('url', 'method', {})).toEqual('data');
});

test('fetch with signed request', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});
test('fetch with signed failing request', async () => {
const apiOptions = {
headers: {},
endpoints: {},
credentials: {
accessKeyId: 'accessKeyId',
secretAccessKey: 'secretAccessKey',
sessionToken: 'sessionToken'
}
};

const restClient = new RestClient(apiOptions);
expect(restClient.ajax('url', 'method', {reject: true})).rejects.toEqual({data: 'error'});
});

test('fetch with signed request', async () => {
const apiOptions = {
headers: {},
endpoints: {},
Expand All @@ -95,15 +91,6 @@ describe('RestClient test', () => {
});

test('ajax with no credentials', async () => {
window.fetch = jest.fn().mockImplementationOnce(() => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {return 'response JSON'}
});
});
});

const apiOptions = {
headers: {},
endpoints: {}
Expand All @@ -119,17 +106,6 @@ describe('RestClient test', () => {
});

test('ajax with extraParams', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const apiOptions = {
headers: {},
endpoints: {},
Expand All @@ -146,17 +122,6 @@ describe('RestClient test', () => {
});

test('ajax with Authorization header', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const apiOptions = {
headers: {},
endpoints: {},
Expand All @@ -175,17 +140,6 @@ describe('RestClient test', () => {

describe('get test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand All @@ -212,17 +166,6 @@ describe('RestClient test', () => {

describe('put test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand All @@ -249,17 +192,6 @@ describe('RestClient test', () => {

describe('patch test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand All @@ -286,17 +218,6 @@ describe('RestClient test', () => {

describe('post test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand All @@ -323,17 +244,6 @@ describe('RestClient test', () => {

describe('del test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand All @@ -359,17 +269,6 @@ describe('RestClient test', () => {

describe('head test', () => {
test('happy case', async () => {
window.fetch = jest.fn().mockImplementationOnce((signed_params_url, signed_params) => {
return new Promise((res, rej) => {
res({
status: '200',
json: () => {
return signed_params.data;
}
});
});
});

const spyon = jest.spyOn(RestClient.prototype, 'ajax');

const apiOptions = {
Expand Down
14 changes: 8 additions & 6 deletions packages/api/src/RestClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,16 @@ export class RestClient {
return this._request(params, isAllResponse);

}

// Signing the request in case there credentials are available
return Credentials.get()
.then(credentials => this._signed({ ...params, ...extraParams }, credentials, isAllResponse))
.catch(err => {
logger.debug('No credentials available, the request will be unsigned');
return this._request(params, isAllResponse);
});
.then(
credentials => this._signed({ ...params, ...extraParams }, credentials, isAllResponse),
err => {
logger.debug('No credentials available, the request will be unsigned');
return this._request(params, isAllResponse);
}
);
}

/**
Expand Down

0 comments on commit fbbeffb

Please sign in to comment.