Skip to content

Commit

Permalink
feat: Refresh and validate credentials after setting env var creds (#71)
Browse files Browse the repository at this point in the history 
* feat: Refresh and validate credentials after setting env var creds

* Positive test case

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
@clareliguori @mergify
clareliguori and mergify[bot] committed Jun 3, 2020
1 parent 187737a commit 472e549
Show file tree
Hide file tree
Showing 3 changed files with 193 additions and 2 deletions.
52 changes: 52 additions & 0 deletions dist/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,50 @@ async function exportAccountId(maskAccountId, region) {
return accountId;
}

function loadCredentials() {
// Force the SDK to re-resolve credentials with the default provider chain.
//
// This action typically sets credentials in the environment via environment variables.
// The SDK never refreshes those env-var-based credentials after initial load.
// In case there were already env-var creds set in the actions environment when this action
// loaded, this action needs to refresh the SDK creds after overwriting those environment variables.
//
// The credentials object needs to be entirely recreated (instead of simply refreshed),
// because the credential object type could change when this action writes env var creds.
// For example, the first load could return EC2 instance metadata credentials
// in a self-hosted runner, and the second load could return environment credentials
// from an assume-role call in this action.
aws.config.credentials = null;

return new Promise((resolve, reject) => {
aws.config.getCredentials((err) => {
if (err) {
reject(err);
}
resolve(aws.config.credentials);
})
});
}

async function validateCredentials(expectedAccessKeyId) {
let credentials;
try {
credentials = await loadCredentials();

if (!credentials.accessKeyId) {
throw new Error('Access key ID empty after loading credentials');
}
} catch (error) {
throw new Error(`Credentials could not be loaded, please check your action inputs: ${error.message}`);
}

const actualAccessKeyId = credentials.accessKeyId;

if (expectedAccessKeyId && expectedAccessKeyId != actualAccessKeyId) {
throw new Error('Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action');
}
}

function getStsClient(region) {
return new aws.STS({
region,
Expand Down Expand Up @@ -305,6 +349,13 @@ async function run() {
exportCredentials({accessKeyId, secretAccessKey, sessionToken});
}

// Regardless of whether any source credentials were provided as inputs,
// validate that the SDK can actually pick up credentials. This validates
// cases where this action is on a self-hosted runner that doesn't have credentials
// configured correctly, and cases where the user intended to provide input
// credentials but the secrets inputs resolved to empty strings.
await validateCredentials(accessKeyId);

const sourceAccountId = await exportAccountId(maskAccountId, region);

// Get role credentials if configured to do so
Expand All @@ -318,6 +369,7 @@ async function run() {
roleSessionName
});
exportCredentials(roleCredentials);
await validateCredentials(roleCredentials.accessKeyId);
await exportAccountId(maskAccountId, region);
}
}
Expand Down
52 changes: 52 additions & 0 deletions index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,50 @@ async function exportAccountId(maskAccountId, region) {
return accountId;
}

function loadCredentials() {
// Force the SDK to re-resolve credentials with the default provider chain.
//
// This action typically sets credentials in the environment via environment variables.
// The SDK never refreshes those env-var-based credentials after initial load.
// In case there were already env-var creds set in the actions environment when this action
// loaded, this action needs to refresh the SDK creds after overwriting those environment variables.
//
// The credentials object needs to be entirely recreated (instead of simply refreshed),
// because the credential object type could change when this action writes env var creds.
// For example, the first load could return EC2 instance metadata credentials
// in a self-hosted runner, and the second load could return environment credentials
// from an assume-role call in this action.
aws.config.credentials = null;

return new Promise((resolve, reject) => {
aws.config.getCredentials((err) => {
if (err) {
reject(err);
}
resolve(aws.config.credentials);
})
});
}

async function validateCredentials(expectedAccessKeyId) {
let credentials;
try {
credentials = await loadCredentials();

if (!credentials.accessKeyId) {
throw new Error('Access key ID empty after loading credentials');
}
} catch (error) {
throw new Error(`Credentials could not be loaded, please check your action inputs: ${error.message}`);
}

const actualAccessKeyId = credentials.accessKeyId;

if (expectedAccessKeyId && expectedAccessKeyId != actualAccessKeyId) {
throw new Error('Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action');
}
}

function getStsClient(region) {
return new aws.STS({
region,
Expand Down Expand Up @@ -172,6 +216,13 @@ async function run() {
exportCredentials({accessKeyId, secretAccessKey, sessionToken});
}

// Regardless of whether any source credentials were provided as inputs,
// validate that the SDK can actually pick up credentials. This validates
// cases where this action is on a self-hosted runner that doesn't have credentials
// configured correctly, and cases where the user intended to provide input
// credentials but the secrets inputs resolved to empty strings.
await validateCredentials(accessKeyId);

const sourceAccountId = await exportAccountId(maskAccountId, region);

// Get role credentials if configured to do so
Expand All @@ -185,6 +236,7 @@ async function run() {
roleSessionName
});
exportCredentials(roleCredentials);
await validateCredentials(roleCredentials.accessKeyId);
await exportAccountId(maskAccountId, region);
}
}
Expand Down
91 changes: 89 additions & 2 deletions index.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const core = require('@actions/core');
const assert = require('assert');

const aws = require('aws-sdk');
const run = require('.');

jest.mock('@actions/core');
Expand Down Expand Up @@ -49,6 +49,9 @@ const mockStsAssumeRole = jest.fn();

jest.mock('aws-sdk', () => {
return {
config: {
getCredentials: jest.fn()
},
STS: jest.fn(() => ({
getCallerIdentity: mockStsCallerIdentity,
assumeRole: mockStsAssumeRole,
Expand Down Expand Up @@ -82,6 +85,27 @@ describe('Configure AWS Credentials', () => {
}
});

aws.config.getCredentials.mockReset();
aws.config.getCredentials
.mockImplementationOnce(callback => {
if (!aws.config.credentials) {
aws.config.credentials = {
accessKeyId: FAKE_ACCESS_KEY_ID,
secretAccessKey: FAKE_SECRET_ACCESS_KEY
}
}
callback(null);
})
.mockImplementationOnce(callback => {
if (!aws.config.credentials) {
aws.config.credentials = {
accessKeyId: FAKE_STS_ACCESS_KEY_ID,
secretAccessKey: FAKE_STS_SECRET_ACCESS_KEY
}
}
callback(null);
});

mockStsAssumeRole.mockImplementation(() => {
return {
promise() {
Expand Down Expand Up @@ -134,6 +158,59 @@ describe('Configure AWS Credentials', () => {
expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID);
});

test('action with no accessible credentials fails', async () => {
process.env.SHOW_STACK_TRACE = 'false';
const mockInputs = {'aws-region': FAKE_REGION};
core.getInput = jest
.fn()
.mockImplementation(mockGetInput(mockInputs));
aws.config.getCredentials.mockReset();
aws.config.getCredentials.mockImplementation(callback => {
callback(new Error('No credentials to load'));
});

await run();

expect(core.setFailed).toHaveBeenCalledWith("Credentials could not be loaded, please check your action inputs: No credentials to load");
});

test('action with empty credentials fails', async () => {
process.env.SHOW_STACK_TRACE = 'false';
const mockInputs = {'aws-region': FAKE_REGION};
core.getInput = jest
.fn()
.mockImplementation(mockGetInput(mockInputs));
aws.config.getCredentials.mockReset();
aws.config.getCredentials.mockImplementation(callback => {
aws.config.credentials = {
accessKeyId: ''
}
callback(null);
});

await run();

expect(core.setFailed).toHaveBeenCalledWith("Credentials could not be loaded, please check your action inputs: Access key ID empty after loading credentials");
});

test('action fails when credentials are not set in the SDK correctly', async () => {
process.env.SHOW_STACK_TRACE = 'false';
core.getInput = jest
.fn()
.mockImplementation(mockGetInput(ASSUME_ROLE_INPUTS));
aws.config.getCredentials.mockReset();
aws.config.getCredentials.mockImplementation(callback => {
aws.config.credentials = {
accessKeyId: FAKE_ACCESS_KEY_ID
}
callback(null);
});

await run();

expect(core.setFailed).toHaveBeenCalledWith("Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action");
});

test('session token is optional', async () => {
const mockInputs = {...CREDS_INPUTS, 'aws-region': 'eu-west-1'};
core.getInput = jest
Expand All @@ -154,12 +231,19 @@ describe('Configure AWS Credentials', () => {
expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID);
});

test('session token is cleared if necessary', async () => {
test('existing env var creds are cleared', async () => {
const mockInputs = {...CREDS_INPUTS, 'aws-region': 'eu-west-1'};
core.getInput = jest
.fn()
.mockImplementation(mockGetInput(mockInputs));
process.env.AWS_ACCESS_KEY_ID = 'foo';
process.env.AWS_SECRET_ACCESS_KEY = 'bar';
process.env.AWS_SESSION_TOKEN = 'helloworld';
aws.config.credentials = {
accessKeyId: 'foo',
secretAccessKey: 'bar',
sessionToken: 'helloworld'
};

await run();
expect(mockStsAssumeRole).toHaveBeenCalledTimes(0);
Expand All @@ -174,6 +258,9 @@ describe('Configure AWS Credentials', () => {
expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', 'eu-west-1');
expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);
expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID);
expect(aws.config.credentials.accessKeyId).toBe(FAKE_ACCESS_KEY_ID);
expect(aws.config.credentials.secretAccessKey).toBe(FAKE_SECRET_ACCESS_KEY);
expect(aws.config.credentials.sessionToken).toBeUndefined();
});

test('validates region name', async () => {
Expand Down

0 comments on commit 472e549

Please sign in to comment.