fix(425): parse NextJS data for role aliases

joepurdy commented 1 year ago

Description

This PR updates the parsing in gimme_aws_creds/aws.py to support the new NextJS SAML login page that AWS plans to release. The changes from this PR are backwards compatible by first asserting whether the new NextJS metadata is present in the HTML response for the sign-in page. In the event this metadata is present a new approach is used to parse the JSON data for the necessary alias names. If not, the old codepath is used for parsing this data from the HTML content of the legacy page.

Related Issue

Fixes #425 for new AWS SAML login page

Motivation and Context

While AWS did rollback their recent change to the SAML sign-in page, it's likely they will move forward with the new NextJS application at an, at this time, uncertain future date. Applying this fix proactively will ensure when this occurs that the aliases can be parsed from the new metadata.

How Has This Been Tested?

I've empirically tested this locally using a response I saved from the new SAML page while it was deployed earlier today. I plan to update tests/test_aws_resolver.py prior to merging this PR with a new test case using mock data in the new format (I still need to redact my local response of private data).

Screenshots (if appropriate):

N/A

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document. (See note)
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

Note to maintainers:

The link to submit the Individual Contributor License Agreement in this project's CONTRIBUTING document is not working. I'd be happy to submit a CLA prior to this PR merging so long as a maintainer could point me in the right direction for that.

Additionally, as mentioned previously I don't love introducing new functionality without tests so I'm going to spend some time this afternoon mocking the new response HTML that I have locally to turn it into an additional test case prior to merging.

EDIT: I finished adding a new unit test and cleaning up the mock HTML data by creating a fixtures directory in a599592 and 531baa2 and confirmed all new and existing tests pass. This is ready for review/merge.

joepurdy commented 1 year ago

@epierce Wanted to flag this for your review since you're the most active maintainer from Nike. Feel free to checkout the linked issue for more context on the upcoming AWS SAML sign-in page change. I've kept this backwards compatible to allow implementing the fix before AWS rolls out their new page again.

Appreciate your time on this, we're big fans/users of GAC over at @ArcadiaPower so we're happy to help keep it working great when AWS throws a curveball.

epierce commented 1 year ago

This looks good - it may have to be updated when AWS releases the new version of the role selection screen again, but this puts us in a good position to handle that. I have to go through Change Control here before cutting a new release, so it'll probably be early next week, but I'll get it out as soon as possible.

Nike-Inc / gimme-aws-creds