Add optional cookiejar support to reduce password/MFA prompts #342
Conversation
There was a problem hiding this comment.
This is a really great feature! much safer than just storing the password in the keyring, and it aligns to the organization policies regarding caching of credentials.
Regarding storing the cookiejar in clear, I agree with most of your analysis, and also I think it is good to add the feature ASAP add encryption on the cookiejar later in a future PR.
Just a pair of comments re security:
- The AWS tokens in disk likely have a lower TTL than the okta cookie. Also in some orgs, those tokens are IP restricted.
- I am not sure about this: But can the okta cookie be used for other stuff?
- The user does not always store the creds in
~/.aws/credentials. Myself, I dump to a json I encrypt, to decrypt and use adhoc later as env-vars. But the usual use-case is in clear.
And the PR is of great quality btw.
| super().set_cookie(cookie) | ||
|
|
||
| def save(self): | ||
| """Mimics LWPCookieJar.save but with multi-process safety.""" |
There was a problem hiding this comment.
There was a problem hiding this comment.
Just a suggestion, I wonder if you can instead create a jar = LWPCookieJar() and copy the cookies:
jar = LWPCookieJar()
for cookie in self:
if cookie.is_expired(now):
continue
jar.set_cookie(cookie)
jar.save(tmp.name)
Although that opens the file again and the code is rather simple
| tmp.flush() | ||
| # rename is an atomic operation (within the same fs) | ||
| Path(tmp.name).replace(self._path) | ||
| except FileNotFoundError: |
There was a problem hiding this comment.
I guess the error happens because the deletion call executed when the file is closed, then deleted.
I wonder if we can do any of these alternatives:
- Copy the file instead of deleting it. Copying will be also atomic, and the original file will be deleted anyway.
- Pass
Delete=Falseto NamedTemporaryFile
docs https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile
|
@bwynsm Can you take a look at this? Do I need to open an Issue tied to the PR to get some 👀 on it? |
Description
Introduces an optional persistent cookiejar to store/re-use Okta session cookies between executions of
gimme-aws-creds.This will allow users who have multiple AWS tiles in the same Okta organization, or complex MFA polices in Okta to experience fewer MFA/password prompts.
Related Issue
I didn't find any closely related issues, let me know if you'd like me to open one and focus the discussion on adding this functionality there instead of within a PR.
Motivation and Context
Depending on the organization/application policy configured in Okta a user may only be required to authenticate every <x> minutes/hours to access okta and/or a specific application tile.
Despite this policy, because
gimme-aws-credscreates a new Okta session on each invocation (albeit with the same device token), the user will experience password/MFA prompts on each invocation of the CLI which does not match the behavior they would experience in a web browser accessing AWS.Implementation Details
To implement the change a custom cookiejar class is added that extends from the existing RequestsCookieJar. This cookie jar uses the stdlib http.cookiejar.LWPCookieJar to serialize/deserialize the cookies (instead of the less secure pickle option).
To avoid potential parallel invocations of
gimme-aws-credsoverwriting the jar file and corrupting it any writes to the jar are staged in a temporary file in the same directory, then renamed in place (replacing the previous jar) which is an atomic operation within the same filesystem.The jar is not very well optimized writing a file each time a cookie is created/deleted instead of batching changes but given the limited number of cookies/frequency of changes this seems like a reasonable trade-off for simplicity.
Alternatively atexit could be used to flush the jar to disk when the CLI exits instead.
When
auth_sessionis called and a cookie jar exists that contains asidcookie, a call to /api/v1/users/me is made to check if the session is still valid. If it is, the function returns early otherwise it will fall-through to the previous implementation generating a new session.I also cleaned up/unified some of the request logic as part of this change:
verifyandheadersto be set on the requests.Session instead of every method call.I will also note there are potential security implications of storing okta sessions on disk without encryption (ex: Chrome uses the system keychain to store an encryption key). However this has little value for a Python CLI since the entire python binary will be allowed access to the key, not a specific script. Additionally this is a similar risk to writing AWS credentials to disk which the tool already does. Given this is a non-default option I think this is a reasonable trade-off.
How Has This Been Tested?
Screenshots (if appropriate):
N/A
Types of changes
Checklist: