Skip to content

Commit

Permalink
Add expiration buffer to prevent credentials to expire earlier than r…
Browse files Browse the repository at this point in the history
…equest may finish. (#678)

* Add expiration buffer variable and tests for it

Signed-off-by: Ivan <[email protected]>
  • Loading branch information
ozonep authored Dec 26, 2023
1 parent bc55912 commit 97ccbab
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
## [Unreleased]
### Added
- Updated `guides/index_lifecycle.md` to provide example of `ignore_unavailable: true` while deleting indices. ([665](https://github.com/opensearch-project/opensearch-js/pull/665))
- Add expiration buffer to prevent credentials to expire earlier than request may finish in case AWS SDK v3 is used. ([678](https://github.com/opensearch-project/opensearch-js/pull/678))
### Dependencies
- Bumps `@types/node` from 20.9.0 to 20.10.5
- Bumps `eslint` from 8.54.0 to 8.56.0
Expand Down
5 changes: 3 additions & 2 deletions USER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ const client = new Client({
// Must return a Promise that resolve to an AWS.Credentials object.
// This function is used to acquire the credentials when the client start and
// when the credentials are expired.
// The Client will refresh the Credentials only when they are expired.
// With AWS SDK V2, Credentials.refreshPromise is used when available to refresh the credentials.
// The Client will treat the Credentials as expired if within
// `requestTimeout` ms of expiration (default is 30000 ms).

// Example with AWS SDK V3:
getCredentials: () => {
Expand All @@ -109,6 +109,7 @@ const client = new Client({
return credentialsProvider();
},
}),
requestTimeout: 60000, // Also used for refreshing credentials in advance
node: 'https://search-xxx.region.es.amazonaws.com', // OpenSearch domain URL
// node: "https://xxx.region.aoss.amazonaws.com" for OpenSearch Serverless
});
Expand Down
1 change: 1 addition & 0 deletions lib/Transport.js
Original file line number Diff line number Diff line change
Expand Up @@ -729,4 +729,5 @@ module.exports.internals = {
randomSelector,
generateRequestId,
lowerCaseHeaders,
toMs,
};
12 changes: 11 additions & 1 deletion lib/aws/AwsSigv4Signer.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const Transport = require('../Transport');
const aws4 = require('aws4');
const AwsSigv4SignerError = require('./errors');
const crypto = require('crypto');
const { toMs } = Transport.internals;

const getAwsSDKCredentialsProvider = async () => {
// First try V3
Expand Down Expand Up @@ -102,6 +103,12 @@ function AwsSigv4Signer(opts = {}) {
}

const currentCredentials = credentialsState.credentials;
/**
* For AWS SDK V3
* Make sure token will expire no earlier than `expiryBufferMs` milliseconds in the future.
*/
const expiryBufferMs = toMs(options.requestTimeout || this.requestTimeout);

let expired = false;
if (!currentCredentials) {
// Credentials haven't been acquired yet.
Expand All @@ -120,7 +127,10 @@ function AwsSigv4Signer(opts = {}) {
expired = true;
}
// AWS SDK V3, Credentials.expiration is a Date object
else if (currentCredentials.expiration && currentCredentials.expiration < new Date()) {
else if (
currentCredentials.expiration &&
currentCredentials.expiration.getTime() - Date.now() < expiryBufferMs
) {
expired = true;
}

Expand Down
122 changes: 122 additions & 0 deletions test/unit/lib/aws/awssigv4signer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -472,3 +472,125 @@ test('Basic aws failure to refresh credentials', (t) => {
.catch(t.fail);
});
});

test('Basic aws sdk v3 when token expires earlier than `requestTimeout` ms in the future.', (t) => {
t.plan(4);

function handler(req, res) {
res.setHeader('Content-Type', 'application/json;utf=8');
res.end(JSON.stringify({ hello: 'world' }));
}

buildServer(handler, ({ port }, server) => {
const mockRegion = 'us-east-1';

let getCredentialsCalled = 0;

const AwsSigv4SignerOptions = {
getCredentials: () =>
new Promise((resolve) => {
setTimeout(() => {
getCredentialsCalled++;
resolve({
accessKeyId: uuidv4(),
secretAccessKey: uuidv4(),
expiration: new Date(Date.now() + 1000 * 25),
});
}, 100);
}),
region: mockRegion,
};

const auth = AwsSigv4Signer(AwsSigv4SignerOptions);

const client = new Client({
...auth,
node: `http://localhost:${port}`,
requestTimeout: 1000 * 30,
});

client
.search({
index: 'test',
q: 'foo:bar',
})
.then(({ body }) => {
t.same(body, { hello: 'world' });
t.same(getCredentialsCalled, 1);
client
.search({
index: 'test',
q: 'foo:bar',
})
.then(({ body }) => {
t.same(body, { hello: 'world' });
t.same(getCredentialsCalled, 2);

server.stop();
})
.catch(t.fail);
})
.catch(t.fail);
});
});

test('Basic aws sdk v3 when token expires later than `requestTimeout` ms in the future.', (t) => {
t.plan(4);

function handler(req, res) {
res.setHeader('Content-Type', 'application/json;utf=8');
res.end(JSON.stringify({ hello: 'world' }));
}

buildServer(handler, ({ port }, server) => {
const mockRegion = 'us-east-1';

let getCredentialsCalled = 0;

const AwsSigv4SignerOptions = {
getCredentials: () =>
new Promise((resolve) => {
setTimeout(() => {
getCredentialsCalled++;
resolve({
accessKeyId: uuidv4(),
secretAccessKey: uuidv4(),
expiration: new Date(Date.now() + 1000 * 45),
});
}, 100);
}),
region: mockRegion,
};

const auth = AwsSigv4Signer(AwsSigv4SignerOptions);

const client = new Client({
...auth,
node: `http://localhost:${port}`,
requestTimeout: 1000 * 30,
});

client
.search({
index: 'test',
q: 'foo:bar',
})
.then(({ body }) => {
t.same(body, { hello: 'world' });
t.same(getCredentialsCalled, 1);
client
.search({
index: 'test',
q: 'foo:bar',
})
.then(({ body }) => {
t.same(body, { hello: 'world' });
t.same(getCredentialsCalled, 1);

server.stop();
})
.catch(t.fail);
})
.catch(t.fail);
});
});

0 comments on commit 97ccbab

Please sign in to comment.