Reuse storageState between login attempts with `Browser` provider

[timharris777]

This updates the Browser provider to load/save storageState so that on next login attempt the browser popup will use storageState data from previous login. This prevents having to retype and reselect all login options (username, password, mfa type, etc) every time we get a saml2aws browser popup to login.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 29.03226% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 41.90%. Comparing base (16a69e6) to head (b13e5e7). Report is 3 commits behind head on master.

Files	Patch %	Lines
pkg/provider/browser/browser.go	29.03%	18 Missing and 4 partials :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #1267 +/- ## ========================================== - Coverage 41.93% 41.90% -0.04% ========================================== Files 54 54 Lines 6386 6410 +24 ========================================== + Hits 2678 2686 +8 - Misses 3271 3283 +12 - Partials 437 441 +4 ``` | [Flag](https://app.codecov.io/gh/Versent/saml2aws/pull/1267/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Versent) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/Versent/saml2aws/pull/1267/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Versent) | `41.90% <29.03%> (-0.04%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Versent#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[tinaboyce]

hi @timharris777 . Thank you for spending time to work on this PR and it looks good on my end.

The test code coverage has reported there will be a slight drop in the test code coverage, do you have any capacity to write tests for your changes?

[timharris777]

I just looked at the current tests vs the lines that I added. I don't see much that I could add that would be useful. Certainly I can add code that will bump coverage percent up, but not sure it will actually contribute anything to the integrity of the code. If you still want me to make another commit to bump up the coverage I can do that, but I won't have time in the next couple of days. Maybe friday or next week. Let me know...

[mapkon]

I just looked at the current tests vs the lines that I added. I don't see much that I could add that would be useful. Certainly I can add code that will bump coverage percent up, but not sure it will actually contribute anything to the integrity of the code. If you still want me to make another commit to bump up the coverage I can do that, but I won't have time in the next couple of days. Maybe friday or next week. Let me know...

@timharris777 I will defer to you and @tinaboyce - we can:

• Merge this and get the tests in later
• Wait for the tests (not sure how urgent the issue is)

@tinaboyce thoughts?

[tinaboyce]

@mapkon I am happy to let this through.

Thanks @timharris777 for get back to me, totally agree with you there.

[timharris777]

Just curious... when do you think this change will make it into a stable release?

[mapkon]

We can schedule one for next release. I would ideally want to merge in your tests before testing the whole app, readying for release.

[timharris777]

Hey @mapkon and @tinaboyce , I was looking at writing the test cases for this... but it might be beyond my ability. If you look at the coverage half of my changes are covered in the Authenticate() function and the other half are not covered in the GetSamlResponse() function, which currently has 0% coverage. It looks like someone started to write tests for it (https://github.com/timharris777/saml2aws/blob/f259e30730b939f8570e383cebb254d9a7be0107/pkg/provider/browser/browser_test.go#L126), but never finished because it was too complicated. It involves mocking a bunch of playwright requests which get's pretty dicey. Most of my uncovered code inside of GetSamlResponse() is also playwright commands which I can't really extract into a separate function to unit test. Not sure how to proceed here... Thoughts?

[dephiros]

@timharris777 , saw this PR and super excited. I am testing this in the latest release but unfortunately still require to log in to Google every time the browser open and it shows incognito. Do you have any pointer?

[nmfon]

@timharris777 , great work on this. Big improvement! Just a heads up that saml2aws 2.36.16 throws the following error message on the first attempt to save the storageState.json:

INFO[0032] saving storage state                          provider=browser
INFO[0032] Error saving storage stateopen /Users/nickfone/.aws/saml2aws/storageState.json: no such file or directory  provider=browser

Workaround: manually create the ~/.aws/saml2aws directory.

[mjhoffman65]

Is this secure enough? If someone gets access to the ~/.aws/saml2aws/storageState.json file they would have relatively long lived access to the provider account. In my case, they would have access to my Google account