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.

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

Save google cookies for re-use #176

Closed
wants to merge 9 commits into from
Closed

Save google cookies for re-use #176

wants to merge 9 commits into from

Conversation

madeddie
Copy link
Contributor

This PR adds the ability to store and re-use the google auth cookies after first login.

The reason I've done this is because with our company IP we got the captcha continuously and the SAML assertion is only valid for 10 minutes, so not really useful for repeat use.
We also use SAML for other app access, mostly via the web, which gives us the benefit of having an active Google login session making logging in and switching roles in those app painless. This change allows for a similar workflow on CLI.

Mind you, there are security implications; these cookies can be used to impersonate you without having to log in. I've not addressed those concerns at all.

I've also not added proper tests for these, but will do so ASAP. This PR is mostly POC and to illicit comments about if a change like this would make it into the main codebase.

@coveralls
Copy link

coveralls commented Mar 17, 2020

Coverage Status

Coverage increased (+1.2%) to 48.753% when pulling 04592ed on madeddie:retain-session into d473d67 on cevoaustralia:master.

@stevemac007
Copy link
Contributor

Thanks for this contribution @madeddie - Does capturing the cookies reduce the issues with Captcha?

It would be great to have something that improves the customer experience, but the security concerns you have raised are also things that need to be considered. The risk on a secured machine feels similar to being able to open the browser and extract the cookies from there.

@madeddie
Copy link
Contributor Author

The captcha happens on login with Google. Storing the session cookies means no longer logging in with Google, so the captcha isn't needed until the Google session expires. I think that happens every 30 days or so.

And yes, it's like having access to the cookies in a browser.

@stevemac007
Copy link
Contributor

I'll have to take this for a test drive myself now.

I wonder if we should reverse the flag logic - after this merge people WILL have their cookies cached, it's probably safer to negate by default and then allow a user to cache the login cookies.

@stevemac007
Copy link
Contributor

This is awesome - other than an issue with the Expires value not being set on my cookies, and so the google_cookies property was failing.

adding a c['expires'] is None or to the front of the condition skipped my issue. Still digging to find out why the expires were not set in the first place.

@madeddie
Copy link
Contributor Author

That's a good question, I think Google always sends an expiry with its session cookies, so not sure where it gets lost. Reversing the flag logic would be fine. We are internally using it all the time, but that of course doesn't mean everyone will :)

@madeddie
Copy link
Contributor Author

@stevemac007 haven't been able to figure out why the cookies lack expire attribs, but I started having the same issue.
I made, what I think, is a bit of a dirty hack to re-do authentication when the google cookies don't seem to be valid anymore. The cleaner thing would be to rewrite the authentication logic to allow to try one and fallback to another when it fails, but don't really have the time for that now.
I've also reversed the logic, flag --cache-cookies now makes the Google cookies be stored and used, by default they're not.

@dplusic
Copy link

dplusic commented Jul 1, 2020

I built aws-google-auth from your branch, and it works like a charm.
Thank you for your work.
This is very useful feature for my environment in which the credential duration is an one hour.
I hope that this pr will be merged soon.

@madeddie
Copy link
Contributor Author

@stevemac007 rebased on master and added 2 tests for the new google cookie cache functionality

@madeddie
Copy link
Contributor Author

I've fixed the tests so they finish successfully on travis-ci, locally they fail for me though. I'll see if I can find a reason for it.
I've noticed some of the tests fail when the sts profile already exist. I'll see if I can make those tests ignore an existing aws configuration in another PR.

@madeddie
Copy link
Contributor Author

madeddie commented Dec 9, 2020

I won't be using aws-google-auth on a daily basis anymore, I'm closing the PR since I have no vested interest.

@madeddie madeddie closed this Dec 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants