PYTHON-3716 OIDC-SASL Follow-Up #1365
Conversation
blink1073
commented
Aug 30, 2023
•
blink1073
commented
- Use updated secrets handling from DRIVERS-2585 Migrate OIDC Secrets Handling mongodb-labs/drivers-evergreen-tools#345
- Use dedicated Atlas hosts for testing
- Move global cache to the mongo client level and update tests
- Clean up reauth logic in preparation for DRIVERS-2672
- Test on macos and Windows - requires DRIVERS-2616 Fix path handling for Windows mongodb-labs/drivers-evergreen-tools#349
- Enforce timeout on callback - we decided against this.
- Update specifications
|
Working locally with the Atlas hosts: |
|
cc @durran |
blink1073
changed the title
.evergreen/run-tests.sh
Outdated
| TEST_ARGS="test/auth_aws/test_auth_oidc.py" | ||
|
|
||
| # Work around for root certifi not being installed. | ||
| # TODO: Remove after PYTHON-3952 |
There was a problem hiding this comment.
Can this be removed now? If not can we open a new ticket so we don't forget?
There was a problem hiding this comment.
Updated to say "is deployed", I still need to coordinate that.
test/auth_aws/test_auth_oidc.py
Outdated
| @@ -1,828 +0,0 @@ | |||
| # Copyright 2023-present MongoDB, Inc. | |||
There was a problem hiding this comment.
Would you be able to rename this file in a separate commit to make this review easier?
blink1073
changed the title
| if [ ! -f "./secrets-export.sh" ]; then | ||
| bash .evergreen/tox.sh -m aws-secrets -- drivers/oidc | ||
| fi | ||
| source ./secrets-export.sh |
There was a problem hiding this comment.
Shouldn't this source be under a set +x for safety?
.evergreen/run-tests.sh
Outdated
| @@ -255,6 +269,9 @@ fi | |||
| # Show the installed packages | |||
| PIP_QUIET=0 python -m pip list | |||
|
|
|||
| python -c "import urllib.request;urllib.request.urlopen('https://www.google.com')" | |||
There was a problem hiding this comment.
What's the purpose of this line?
There was a problem hiding this comment.
That was for debugging, fixed
pymongo/auth_oidc.py
Outdated
| timeout = CALLBACK_TIMEOUT_SECONDS | ||
| if not use_callbacks and not current_valid_token: | ||
| # TODO: DRIVERS-2672, handle machine callback here as well. | ||
| cb = properties.request_token_callback |
There was a problem hiding this comment.
Minor nitpick, but this would be cleaner as a one-liner: cb = properties.request_token_callback if use_callback else None
pymongo/auth_oidc.py
Outdated
| def reauthenticate(self, conn: Connection) -> Optional[Mapping[str, Any]]: | ||
| """Handle a reauthenticate from the server.""" | ||
| # First see if we have the a newer token on the authenticator. | ||
| prev_id = getattr(conn, "oidc_token_gen_id", 0) |
There was a problem hiding this comment.
Let's make oidc_token_gen_id a well defined property on Connection and avoid getattr.
There was a problem hiding this comment.
Ah, it was already there as an optional int, updated
