Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Updated token retrieval to use new API #270

Merged
merged 5 commits into from
Sep 29, 2021
Merged

fix: Updated token retrieval to use new API #270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5e4f686
feat: OIDC provider (with PR comments)
Sep 18, 2021
8d77d75
feat: OIDC provider (with PR comments)
Sep 18, 2021
8c9da02
Merge remote-tracking branch 'origin/master'
Sep 28, 2021
915fd44
updated OIDC based on new GH API
Sep 29, 2021
819c6bb
Merge remote-tracking branch 'upstream/master'
Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ Resources:
Condition: CreateOIDCProvider
Properties:
Url: https://vstoken.actions.githubusercontent.com
ClientIdList: [sigstore]
ClientIdList: ['sts.amazonaws.com']
ThumbprintList: [a031c46782e6e6c662c2c87c76da9aa62ccabd8e]

Outputs:
Expand Down
4,252 changes: 36 additions & 4,216 deletions dist/index.js
View file
Open in desktop

Large diffs are not rendered by default.

18 changes: 1 addition & 17 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ const aws = require('aws-sdk');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const axios = require('axios');

// The max time that a GitHub action is allowed to run is 6 hours.
// That seems like a reasonable default to use if no role duration is defined.
Expand Down Expand Up @@ -185,21 +184,6 @@ async function exportAccountId(maskAccountId, region) {
return accountId;
}

async function getWebIdentityToken() {
const isDefined = i => !!i;
const {ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN} = process.env;

assert(
[ACTIONS_ID_TOKEN_REQUEST_URL, ACTIONS_ID_TOKEN_REQUEST_TOKEN].every(isDefined),
'Missing required environment value. Are you running in GitHub Actions?'
);
const { data } = await axios.get(`${ACTIONS_ID_TOKEN_REQUEST_URL}&audience=sigstore`, {
headers: {"Authorization": `bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}`}
}
);
return data.value;
}

function loadCredentials() {
// Force the SDK to re-resolve credentials with the default provider chain.
//
Expand Down Expand Up @@ -303,7 +287,7 @@ async function run() {
let sourceAccountId;
let webIdentityToken;
if(useGitHubOIDCProvider()) {
webIdentityToken = await getWebIdentityToken();
webIdentityToken = await core.getIDToken('sts.amazonaws.com');
roleDurationSeconds = core.getInput('role-duration-seconds', {required: false}) || DEFAULT_ROLE_DURATION_FOR_OIDC_ROLES;
// We don't validate the credentials here because we don't have them yet when using OIDC.
} else {
Expand Down
10 changes: 6 additions & 4 deletions index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ const core = require('@actions/core');
const assert = require('assert');
const aws = require('aws-sdk');
const run = require('./index.js');
const axios = require('axios');

jest.mock('@actions/core');
jest.mock("axios");

const FAKE_ACCESS_KEY_ID = 'MY-AWS-ACCESS-KEY-ID';
const FAKE_SECRET_ACCESS_KEY = 'MY-AWS-SECRET-ACCESS-KEY';
Expand Down Expand Up @@ -91,6 +89,12 @@ describe('Configure AWS Credentials', () => {
.fn()
.mockImplementation(mockGetInput(DEFAULT_INPUTS));

core.getIDToken = jest
.fn()
.mockImplementation(() => {
return "testtoken"
});

mockStsCallerIdentity.mockReset();
mockStsCallerIdentity
.mockReturnValueOnce({
Expand Down Expand Up @@ -571,7 +575,6 @@ describe('Configure AWS Credentials', () => {
test('only role arn and region provided to use GH OIDC Token', async () => {
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = 'test-token';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these environment variables anymore?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. These being set is how we determine whether or not we are in a self-hosted runner or in a "proper" GitHub Action. If we remove them, the test will behave as if it were in a self-hosted runner (where OIDC isn't supported)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then where are we using it's test-token value in the test?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use it in the test, we use it in the action.

https://github.com/aws-actions/configure-aws-credentials/blob/master/index.js#L284

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're less concerned with the actual value of the ENV variable, we just need it set to something so that the action sees that it has been set and behaves as if the test were running in a 'real' GitHub Action.

process.env.ACTIONS_ID_TOKEN_REQUEST_URL = 'https://www.example.com/token/endpoint';
axios.get.mockImplementation(() => Promise.resolve({ data: {value: "testtoken"} }));
core.getInput = jest
.fn()
.mockImplementation(mockGetInput({'role-to-assume': ROLE_ARN, 'aws-region': FAKE_REGION}));
Expand All @@ -592,7 +595,6 @@ describe('Configure AWS Credentials', () => {
const CUSTOM_ROLE_DURATION = 1234;
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = 'test-token';
process.env.ACTIONS_ID_TOKEN_REQUEST_URL = 'https://www.example.com/token/endpoint';
axios.get.mockImplementation(() => Promise.resolve({ data: {value: "testtoken"} }));
core.getInput = jest
.fn()
.mockImplementation(mockGetInput({'role-to-assume': ROLE_ARN, 'aws-region': FAKE_REGION, 'role-duration-seconds': CUSTOM_ROLE_DURATION}));
Expand Down