Skip to content

PYTHON-3501 Ensure Auth Environment Variables are Always Dynamic #1107

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

Merged
merged 8 commits into from
Nov 4, 2022
Merged

PYTHON-3501 Ensure Auth Environment Variables are Always Dynamic #1107

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6b9a3af
PYTHON-3501 Ensure Auth Environment Variables are Always Dynamic
blink1073 Nov 3, 2022
3f422a8
try again
blink1073 Nov 3, 2022
75ee7b8
try again
blink1073 Nov 3, 2022
328dc95
try again
blink1073 Nov 3, 2022
eb64ee2
use mock os.environ
blink1073 Nov 4, 2022
17ca4c5
try again
blink1073 Nov 4, 2022
8a07332
try again
blink1073 Nov 4, 2022
e2fa50c
try again
blink1073 Nov 4, 2022
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
58 changes: 58 additions & 0 deletions test/auth_aws/test_auth_aws.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os
import sys
import unittest
from unittest.mock import patch

sys.path[0:0] = [""]

Expand Down Expand Up @@ -111,6 +112,63 @@ def test_poisoned_cache(self):
client.get_database().test.find_one()
self.assertNotEqual(auth.get_cached_credentials(), None)

def test_environment_variables_ignored(self):
Copy link
Member

@ShaneHarvey ShaneHarvey Nov 3, 2022

Choose a reason for hiding this comment

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

Is this behavior outlined in the spec? Shouldn't the env vars take precedence even if we already have a previous cached credential?

Copy link
Member

@ShaneHarvey ShaneHarvey Nov 3, 2022

Choose a reason for hiding this comment

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

Discussed in person, this behavior happens because we rely on aws for the credential lookup. The code is like:

if cached_creds:
    return cached_creds
creds = get_aws_creds()
if should_cache(creds):
    cached_creds = creds
return creds

So if the app is always using env vars then there will never be a cached credential and dynamic env var changes will propagate immediately. However, if the app is using temp creds then later tries to switch to dynamic env var creds, then changes won't propagate until the cached creds are cleared.

creds = self.setup_cache()
self.assertIsNotNone(creds)
prev = os.environ.copy()

client = MongoClient(self.uri)
self.addCleanup(client.close)

client.get_database().test.find_one()

self.assertIsNotNone(auth.get_cached_credentials())

mock_env = dict(
AWS_ACCESS_KEY_ID="foo", AWS_SECRET_ACCESS_KEY="bar", AWS_SESSION_TOKEN="baz"
)

with patch.dict("os.environ", mock_env):
self.assertEqual(os.environ["AWS_ACCESS_KEY_ID"], "foo")
client.get_database().test.find_one()

auth.set_cached_credentials(None)

client2 = MongoClient(self.uri)
self.addCleanup(client2.close)

with patch.dict("os.environ", mock_env):
self.assertEqual(os.environ["AWS_ACCESS_KEY_ID"], "foo")
with self.assertRaises(OperationFailure):
client2.get_database().test.find_one()

def test_no_cache_environment_variables(self):
creds = self.setup_cache()
self.assertIsNotNone(creds)
auth.set_cached_credentials(None)

mock_env = dict(AWS_ACCESS_KEY_ID=creds.username, AWS_SECRET_ACCESS_KEY=creds.password)
if creds.token:
mock_env["AWS_SESSION_TOKEN"] = creds.token

client = MongoClient(self.uri)
self.addCleanup(client.close)

with patch.dict(os.environ, mock_env):
self.assertEqual(os.environ["AWS_ACCESS_KEY_ID"], creds.username)
client.get_database().test.find_one()

self.assertIsNone(auth.get_cached_credentials())

mock_env["AWS_ACCESS_KEY_ID"] = "foo"

client2 = MongoClient(self.uri)
self.addCleanup(client2.close)

with patch.dict("os.environ", mock_env), self.assertRaises(OperationFailure):
self.assertEqual(os.environ["AWS_ACCESS_KEY_ID"], "foo")
client2.get_database().test.find_one()


class TestAWSLambdaExamples(unittest.TestCase):
def test_shared_client(self):
Expand Down