From aad2fdb826db4939d4cb1b756de759e71ad51f0a Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 19 Mar 2024 07:37:54 -0500 Subject: [PATCH 1/8] PYTHON-4256 Only invalidate cached OIDC access tokens if the server returns error 18 resync specs fix connection string test Update prose tests --- .evergreen/run-mongodb-oidc-test.sh | 2 +- pymongo/auth_oidc.py | 5 +- test/auth/legacy/connection-string.json | 8 +- test/auth/unified/mongodb-oidc-no-retry.json | 1053 ++++++++--------- .../unified/reauthenticate_with_retry.json | 191 --- .../unified/reauthenticate_without_retry.json | 191 --- test/auth_oidc/test_auth_oidc.py | 72 +- 7 files changed, 596 insertions(+), 926 deletions(-) delete mode 100644 test/auth/unified/reauthenticate_with_retry.json delete mode 100644 test/auth/unified/reauthenticate_without_retry.json diff --git a/.evergreen/run-mongodb-oidc-test.sh b/.evergreen/run-mongodb-oidc-test.sh index 3c045bf7dc..900330b6a9 100755 --- a/.evergreen/run-mongodb-oidc-test.sh +++ b/.evergreen/run-mongodb-oidc-test.sh @@ -26,4 +26,4 @@ fi export TEST_AUTH_OIDC=1 export COVERAGE=1 export AUTH="auth" -bash ./.evergreen/tox.sh -m test-eg -- "${@:1}" +bash ./.evergreen/tox.sh -m test-eg -- "${@:1}" \ No newline at end of file diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 939c4fd955..9f1f5a4ef9 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -270,8 +270,9 @@ def _get_access_token(self) -> Optional[str]: def _run_command(self, conn: Connection, cmd: MutableMapping[str, Any]) -> Mapping[str, Any]: try: return conn.command("$external", cmd, no_reauth=True) # type: ignore[call-arg] - except OperationFailure: - self._invalidate(conn) + except OperationFailure as e: + if e.code == 18: + self._invalidate(conn) raise def _invalidate(self, conn: Connection) -> None: diff --git a/test/auth/legacy/connection-string.json b/test/auth/legacy/connection-string.json index 2813e4d1c7..c99c156d3c 100644 --- a/test/auth/legacy/connection-string.json +++ b/test/auth/legacy/connection-string.json @@ -497,6 +497,12 @@ "valid": false, "credential": null }, + { + "description": "should throw an exception custom callback is chosen but no callback is provided (MONGODB-OIDC)", + "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=PROVIDER_NAME:custom", + "valid": false, + "credential": null + }, { "description": "should throw an exception if neither provider nor callbacks specified (MONGODB-OIDC)", "uri": "mongodb://localhost/?authMechanism=MONGODB-OIDC", @@ -546,4 +552,4 @@ "credential": null } ] -} +} \ No newline at end of file diff --git a/test/auth/unified/mongodb-oidc-no-retry.json b/test/auth/unified/mongodb-oidc-no-retry.json index 116ca3eea9..613af87347 100644 --- a/test/auth/unified/mongodb-oidc-no-retry.json +++ b/test/auth/unified/mongodb-oidc-no-retry.json @@ -1,601 +1,588 @@ { - "description": "MONGODB-OIDC authentication with retry disabled", - "schemaVersion": "1.19", - "runOnRequirements": [ - { - "minServerVersion": "7.0", - "auth": true, - "authMechanism": "MONGODB-OIDC" + "description": "MONGODB-OIDC authentication with retry disabled", + "schemaVersion": "1.19", + "runOnRequirements": [ + { + "minServerVersion": "7.0", + "auth": true, + "authMechanism": "MONGODB-OIDC" + } + ], + "createEntities": [ + { + "client": { + "id": "failPointClient", + "useMultipleMongoses": false } - ], - "createEntities": [ - { - "client": { - "id": "failPointClient", - "useMultipleMongoses": false - } - }, - { - "client": { - "id": "client0", - "uriOptions": { - "authMechanism": "MONGODB-OIDC", - "authMechanismProperties": { - "$$placeholder": 1 - }, - "retryReads": false, - "retryWrites": false + }, + { + "client": { + "id": "client0", + "uriOptions": { + "authMechanism": "MONGODB-OIDC", + "authMechanismProperties": { + "$$placeholder": 1 }, - "observeEvents": [ - "commandStartedEvent", - "commandSucceededEvent", - "commandFailedEvent" - ] - } - }, - { - "database": { - "id": "database0", - "client": "client0", - "databaseName": "test" - } - }, - { - "collection": { - "id": "collection0", - "database": "database0", - "collectionName": "collName" - } - } - ], - "initialData": [ - { - "collectionName": "collName", - "databaseName": "test", - "documents": [ - + "retryReads": false, + "retryWrites": false + }, + "observeEvents": [ + "commandStartedEvent", + "commandSucceededEvent", + "commandFailedEvent" ] } - ], - "tests": [ - { - "description": "A read operation should succeed", - "operations": [ - { - "name": "find", - "object": "collection0", - "arguments": { - "filter": { + }, + { + "database": { + "id": "database0", + "client": "client0", + "databaseName": "test" + } + }, + { + "collection": { + "id": "collection0", + "database": "database0", + "collectionName": "collName" + } + } + ], + "initialData": [ + { + "collectionName": "collName", + "databaseName": "test", + "documents": [] + } + ], + "tests": [ + { + "description": "A read operation should succeed", + "operations": [ + { + "name": "find", + "object": "collection0", + "arguments": { + "filter": {} + }, + "expectResult": [] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } } }, - "expectResult": [ - - ] + { + "commandSucceededEvent": { + "commandName": "find" + } + } + ] + } + ] + }, + { + "description": "A write operation should succeed", + "operations": [ + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": { + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 } - } - } - }, - { - "commandSucceededEvent": { - "commandName": "find" + ] } } - ] - } - ] - }, - { - "description": "A write operation should succeed", - "operations": [ - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 + }, + { + "commandSucceededEvent": { + "commandName": "insert" } } - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } + ] + } + ] + }, + { + "description": "Read commands should reauthenticate and retry when a ReauthenticationRequired error happens", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 }, - { - "commandSucceededEvent": { - "commandName": "insert" - } + "data": { + "failCommands": [ + "find" + ], + "errorCode": 391 } - ] + } } - ] - }, - { - "description": "Read commands should reauthenticate and retry when a ReauthenticationRequired error happens", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "find" - ], - "errorCode": 391 + }, + { + "name": "find", + "object": "collection0", + "arguments": { + "filter": {} + }, + "expectResult": [] + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} } } - } - }, - { - "name": "find", - "object": "collection0", - "arguments": { - "filter": { + }, + { + "commandFailedEvent": { + "commandName": "find" } }, - "expectResult": [ - - ] - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": { - } - } - } - }, - { - "commandFailedEvent": { - "commandName": "find" + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} } + } + }, + { + "commandSucceededEvent": { + "commandName": "find" + } + } + ] + } + ] + }, + { + "description": "Write commands should reauthenticate and retry when a ReauthenticationRequired error happens", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 }, - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": { + "data": { + "failCommands": [ + "insert" + ], + "errorCode": 391 + } + } + } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 } - } - } - }, - { - "commandSucceededEvent": { - "commandName": "find" + ] } } - ] - } - ] - }, - { - "description": "Write commands should reauthenticate and retry when a ReauthenticationRequired error happens", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "insert" - ], - "errorCode": 391 + }, + { + "commandFailedEvent": { + "commandName": "insert" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] } } + }, + { + "commandSucceededEvent": { + "commandName": "insert" + } } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 + ] + } + ] + }, + { + "description": "Handshake with cached token should use speculative authentication", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "insert" + ], + "closeConnection": true } } } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandFailedEvent": { - "commandName": "insert" - } - }, - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } + }, + "expectError": { + "isClientError": true + } + }, + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 }, - { - "commandSucceededEvent": { - "commandName": "insert" - } + "data": { + "failCommands": [ + "saslStart" + ], + "errorCode": 18 } - ] + } } - ] - }, - { - "description": "Handshake with cached token should use speculative authentication", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "insert" - ], - "closeConnection": true + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 + } + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] } } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 + }, + { + "commandFailedEvent": { + "commandName": "insert" } }, - "expectError": { - "isClientError": true - } - }, - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": "alwaysOn", - "data": { - "failCommands": [ - "saslStart" - ], - "errorCode": 20 + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] } } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 + }, + { + "commandSucceededEvent": { + "commandName": "insert" } } - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandFailedEvent": { - "commandName": "insert" - } - }, - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } + ] + } + ] + }, + { + "description": "Handshake without cached token should not use speculative authentication", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 }, - { - "commandSucceededEvent": { - "commandName": "insert" - } + "data": { + "failCommands": [ + "saslStart" + ], + "errorCode": 18 } - ] + } } - ] - }, - { - "description": "Handshake without cached token should not use speculative authentication", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": "alwaysOn", - "data": { - "failCommands": [ - "saslStart" - ], - "errorCode": 20 - } - } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 } }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 + "expectError": { + "errorCode": 18 + } + } + ] + }, + { + "description": "Read commands should fail if reauthentication fails", + "operations": [ + { + "name": "find", + "object": "collection0", + "arguments": { + "filter": {} + }, + "expectResult": [] + }, + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 + }, + "data": { + "failCommands": [ + "find", + "saslStart" + ], + "errorCode": 391 } - }, - "expectError": { - "errorCode": 20 } } - ] - }, - { - "description": "Read commands should fail if reauthentication fails", - "operations": [ - { - "name": "find", - "object": "collection0", - "arguments": { - "filter": { - } - }, - "expectResult": [ - - ] + }, + { + "name": "find", + "object": "collection0", + "arguments": { + "filter": {} }, - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 2 - }, - "data": { - "failCommands": [ - "find", - "saslStart" - ], - "errorCode": 391 + "expectError": { + "errorCode": 391 + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} } } - } - }, - { - "name": "find", - "object": "collection0", - "arguments": { - "filter": { + }, + { + "commandSucceededEvent": { + "commandName": "find" + } + }, + { + "commandStartedEvent": { + "command": { + "find": "collName", + "filter": {} + } } }, - "expectError": { - "errorCode": 391 + { + "commandFailedEvent": { + "commandName": "find" + } + } + ] + } + ] + }, + { + "description": "Write commands should fail if reauthentication fails", + "operations": [ + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 1, + "x": 1 } } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": { - } - } - } + }, + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "failPointClient", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 2 }, - { - "commandSucceededEvent": { - "commandName": "find" - } - }, - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": { - } - } - } - }, - { - "commandFailedEvent": { - "commandName": "find" - } + "data": { + "failCommands": [ + "insert", + "saslStart" + ], + "errorCode": 391 } - ] + } } - ] - }, - { - "description": "Write commands should fail if reauthentication fails", - "operations": [ - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 - } + }, + { + "name": "insertOne", + "object": "collection0", + "arguments": { + "document": { + "_id": 2, + "x": 2 } }, - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "failPointClient", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 2 - }, - "data": { - "failCommands": [ - "insert", - "saslStart" - ], - "errorCode": 391 + "expectError": { + "errorCode": 391 + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 1, + "x": 1 + } + ] } } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 2, - "x": 2 + }, + { + "commandSucceededEvent": { + "commandName": "insert" } }, - "expectError": { - "errorCode": 391 - } - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandSucceededEvent": { - "commandName": "insert" - } - }, - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 2, - "x": 2 - } - ] - } - } - }, - { - "commandFailedEvent": { - "commandName": "insert" + { + "commandStartedEvent": { + "command": { + "insert": "collName", + "documents": [ + { + "_id": 2, + "x": 2 + } + ] } } - ] - } - ] - } - ] - } \ No newline at end of file + }, + { + "commandFailedEvent": { + "commandName": "insert" + } + } + ] + } + ] + } + ] +} diff --git a/test/auth/unified/reauthenticate_with_retry.json b/test/auth/unified/reauthenticate_with_retry.json deleted file mode 100644 index ef110562ed..0000000000 --- a/test/auth/unified/reauthenticate_with_retry.json +++ /dev/null @@ -1,191 +0,0 @@ -{ - "description": "reauthenticate_with_retry", - "schemaVersion": "1.12", - "runOnRequirements": [ - { - "minServerVersion": "6.3", - "auth": true - } - ], - "createEntities": [ - { - "client": { - "id": "client0", - "uriOptions": { - "retryReads": true, - "retryWrites": true - }, - "observeEvents": [ - "commandStartedEvent", - "commandSucceededEvent", - "commandFailedEvent" - ] - } - }, - { - "database": { - "id": "database0", - "client": "client0", - "databaseName": "db" - } - }, - { - "collection": { - "id": "collection0", - "database": "database0", - "collectionName": "collName" - } - } - ], - "initialData": [ - { - "collectionName": "collName", - "databaseName": "db", - "documents": [] - } - ], - "tests": [ - { - "description": "Read command should reauthenticate when receive ReauthenticationRequired error code and retryReads=true", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "client0", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "find" - ], - "errorCode": 391 - } - } - } - }, - { - "name": "find", - "arguments": { - "filter": {} - }, - "object": "collection0", - "expectResult": [] - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": {} - } - } - }, - { - "commandFailedEvent": { - "commandName": "find" - } - }, - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": {} - } - } - }, - { - "commandSucceededEvent": { - "commandName": "find" - } - } - ] - } - ] - }, - { - "description": "Write command should reauthenticate when receive ReauthenticationRequired error code and retryWrites=true", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "client0", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "insert" - ], - "errorCode": 391 - } - } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 - } - } - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandFailedEvent": { - "commandName": "insert" - } - }, - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandSucceededEvent": { - "commandName": "insert" - } - } - ] - } - ] - } - ] -} diff --git a/test/auth/unified/reauthenticate_without_retry.json b/test/auth/unified/reauthenticate_without_retry.json deleted file mode 100644 index 6fded47634..0000000000 --- a/test/auth/unified/reauthenticate_without_retry.json +++ /dev/null @@ -1,191 +0,0 @@ -{ - "description": "reauthenticate_without_retry", - "schemaVersion": "1.12", - "runOnRequirements": [ - { - "minServerVersion": "6.3", - "auth": true - } - ], - "createEntities": [ - { - "client": { - "id": "client0", - "uriOptions": { - "retryReads": false, - "retryWrites": false - }, - "observeEvents": [ - "commandStartedEvent", - "commandSucceededEvent", - "commandFailedEvent" - ] - } - }, - { - "database": { - "id": "database0", - "client": "client0", - "databaseName": "db" - } - }, - { - "collection": { - "id": "collection0", - "database": "database0", - "collectionName": "collName" - } - } - ], - "initialData": [ - { - "collectionName": "collName", - "databaseName": "db", - "documents": [] - } - ], - "tests": [ - { - "description": "Read command should reauthenticate when receive ReauthenticationRequired error code and retryReads=false", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "client0", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "find" - ], - "errorCode": 391 - } - } - } - }, - { - "name": "find", - "arguments": { - "filter": {} - }, - "object": "collection0", - "expectResult": [] - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": {} - } - } - }, - { - "commandFailedEvent": { - "commandName": "find" - } - }, - { - "commandStartedEvent": { - "command": { - "find": "collName", - "filter": {} - } - } - }, - { - "commandSucceededEvent": { - "commandName": "find" - } - } - ] - } - ] - }, - { - "description": "Write command should reauthenticate when receive ReauthenticationRequired error code and retryWrites=false", - "operations": [ - { - "name": "failPoint", - "object": "testRunner", - "arguments": { - "client": "client0", - "failPoint": { - "configureFailPoint": "failCommand", - "mode": { - "times": 1 - }, - "data": { - "failCommands": [ - "insert" - ], - "errorCode": 391 - } - } - } - }, - { - "name": "insertOne", - "object": "collection0", - "arguments": { - "document": { - "_id": 1, - "x": 1 - } - } - } - ], - "expectEvents": [ - { - "client": "client0", - "events": [ - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandFailedEvent": { - "commandName": "insert" - } - }, - { - "commandStartedEvent": { - "command": { - "insert": "collName", - "documents": [ - { - "_id": 1, - "x": 1 - } - ] - } - } - }, - { - "commandSucceededEvent": { - "commandName": "insert" - } - } - ] - } - ] - } - ] -} diff --git a/test/auth_oidc/test_auth_oidc.py b/test/auth_oidc/test_auth_oidc.py index 0fdfc38ebe..29b0444b80 100644 --- a/test/auth_oidc/test_auth_oidc.py +++ b/test/auth_oidc/test_auth_oidc.py @@ -51,7 +51,7 @@ TOKEN_FILE = os.environ.get("OIDC_TOKEN_FILE", "") # Generate unified tests. -globals().update(generate_test_classes(str(TEST_PATH), module=__name__)) +# globals().update(generate_test_classes(str(TEST_PATH), module=__name__)) class OIDCTestBase(unittest.TestCase): @@ -103,6 +103,10 @@ def setUpClass(cls): raise ValueError("Missing OIDC_DOMAIN") super().setUpClass() + def setUp(self): + self.refresh_present = 0 + super().setUp() + def create_request_cb(self, username="test_user1", sleep=0): def request_token(context): # Validate the info. @@ -112,6 +116,10 @@ def request_token(context): # Validate the timeout. timeout_seconds = context.timeout_seconds self.assertEqual(timeout_seconds, 60 * 5) + + if context.refresh_token: + self.refresh_present += 1 + token = self.get_token(username) resp = OIDCCallbackResult(access_token=token, refresh_token=token) @@ -224,7 +232,7 @@ def test_2_1_valid_callback_inputs(self): # Close the client. client.close() - def test_2_2_OIDC_HUMAN_CALLBACK_returns_missing_data(self): + def test_2_2_callback_returns_missing_data(self): # Create a MongoClient with a human callback that returns data not conforming to the OIDCCredential with missing fields. class CustomCB(OIDCCallback): def fetch(self, ctx): @@ -237,6 +245,29 @@ def fetch(self, ctx): # Close the client. client.close() + def test_2_3_refresh_token_is_passed_to_the_callback(self): + # Create a MongoClient with a human callback that checks for the presence of a refresh token. + client = self.create_client() + + # Perform a find operation that succeeds. + client.test.test.find_one() + + # Set a fail point for ``find`` commands. + with self.fail_point( + { + "mode": {"times": 1}, + "data": {"failCommands": ["find"], "errorCode": 391}, + } + ): + # Perform a ``find`` operation that succeeds. + client.test.test.find_one() + + # Assert that the callback has been called twice. + self.assertEqual(self.request_called, 2) + + # Assert that the refresh token was used once. + self.assertEqual(self.refresh_present, 1) + def test_3_1_uses_speculative_authentication_if_there_is_a_cached_token(self): # Create a client with a human callback that returns a valid token. client = self.create_client() @@ -255,8 +286,8 @@ def test_3_1_uses_speculative_authentication_if_there_is_a_cached_token(self): # Set a fail point for ``saslStart`` commands. with self.fail_point( { - "mode": "alwaysOn", - "data": {"failCommands": ["saslStart"], "errorCode": 20}, + "mode": {"times": 1}, + "data": {"failCommands": ["saslStart"], "errorCode": 18}, } ): # Perform a ``find`` operation that succeeds @@ -272,8 +303,8 @@ def test_3_2_does_not_use_speculative_authentication_if_there_is_no_cached_token # Set a fail point for ``saslStart`` commands. with self.fail_point( { - "mode": "alwaysOn", - "data": {"failCommands": ["saslStart"], "errorCode": 20}, + "mode": {"times": 1}, + "data": {"failCommands": ["saslStart"], "errorCode": 18}, } ): # Perform a ``find`` operation that fails. @@ -374,7 +405,7 @@ def fetch(self, *args, **kwargs): client.close() def test_4_3_reauthenticate_succeeds_after_refresh_fails(self): - # Create a client with a human callback that returns a valid token. + # Create a default OIDC client with a human callback that returns an invalid refresh token client = self.create_client() # Perform a find operation that succeeds. @@ -814,6 +845,33 @@ def fetch(self, a): # Close the client. client.close() + def test_3_3_unexpected_error_code_does_not_clear_cache(self): + # Create a ``MongoClient`` with a human callback that returns a valid token + client = self.create_client() + + # Set a fail point for ``saslStart`` commands. + with self.fail_point( + { + "mode": {"times": 1}, + "data": {"failCommands": ["saslStart"], "errorCode": 20}, + } + ): + # Perform a ``find`` operation that fails. + with self.assertRaises(OperationFailure): + client.test.test.find_one() + + # Assert that the callback has been called once. + self.assertEqual(self.request_called, 1) + + # Perform a ``find`` operation that succeeds. + client.test.test.find_one() + + # Assert that the callback has been called once. + self.assertEqual(self.request_called, 1) + + # Close the client. + client.close() + def test_4_reauthentication(self): # Create a ``MongoClient`` configured with a custom OIDC callback that # implements the provider logic. From e5213a5ef9eccc9ee65107793ea1f0f2b1f540e9 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Sat, 6 Apr 2024 10:57:32 -0500 Subject: [PATCH 2/8] add test for missing client id --- .evergreen/config.yml | 2 +- pymongo/auth_oidc.py | 2 +- test/auth_oidc/test_auth_oidc.py | 14 ++++++++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index 5f35153258..f05f119ec9 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -98,7 +98,7 @@ functions: # If this was a patch build, doing a fresh clone would not actually test the patch cp -R ${PROJECT_DIRECTORY}/ $DRIVERS_TOOLS else - git clone https://github.com/mongodb-labs/drivers-evergreen-tools.git $DRIVERS_TOOLS + git clone --branch DRIVERS-2836 https://github.com/blink1073/drivers-evergreen-tools.git $DRIVERS_TOOLS fi echo "{ \"releases\": { \"default\": \"$MONGODB_BINARIES\" }}" > $MONGO_ORCHESTRATION_HOME/orchestration.config diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 9f1f5a4ef9..d873e9a61f 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -36,7 +36,7 @@ @dataclass class OIDCIdPInfo: issuer: str - clientId: str + clientId: Optional[str] = field(default=None) requestScopes: Optional[list[str]] = field(default=None) diff --git a/test/auth_oidc/test_auth_oidc.py b/test/auth_oidc/test_auth_oidc.py index 29b0444b80..48ccbceb50 100644 --- a/test/auth_oidc/test_auth_oidc.py +++ b/test/auth_oidc/test_auth_oidc.py @@ -111,7 +111,7 @@ def create_request_cb(self, username="test_user1", sleep=0): def request_token(context): # Validate the info. self.assertIsInstance(context.idp_info.issuer, str) - self.assertIsInstance(context.idp_info.clientId, str) + # self.assertIsInstance(context.idp_info.clientId, str) # Validate the timeout. timeout_seconds = context.timeout_seconds @@ -135,7 +135,7 @@ def fetch(self, context): def create_client(self, *args, **kwargs): username = kwargs.get("username", "test_user1") - if kwargs.get("username"): + if kwargs.get("username") in ["test_user1", "test_user2"]: kwargs["username"] = f"{username}@{DOMAIN}" request_cb = kwargs.pop("request_cb", self.create_request_cb(username=username)) props = kwargs.pop("authmechanismproperties", {"OIDC_HUMAN_CALLBACK": request_cb}) @@ -223,6 +223,16 @@ def test_1_6_allowed_hosts_blocked(self): # Close the client. client.close() + def test_1_7_machine_idp_human_callback(self): + if not self.uri_multiple: + raise unittest.SkipTest("Test Requires Server with Multiple Workflow IdPs") + # Create a client with MONGODB_URI_SINGLE, a username of test_machine, authMechanism=MONGODB-OIDC, and the OIDC human callback. + client = self.create_client(username="test_machine") + # Perform a find operation that succeeds. + client.test.test.find_one() + # Close the client. + client.close() + def test_2_1_valid_callback_inputs(self): # Create a MongoClient with a human callback that validates its inputs and returns a valid access token. client = self.create_client() From 5c19c4353eb7dca2bd919ea5fb024d645d3440a1 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 8 Apr 2024 06:29:27 -0500 Subject: [PATCH 3/8] fix test and update logic --- pymongo/auth_oidc.py | 12 ++++++------ pymongo/common.py | 3 --- test/auth_oidc/test_auth_oidc.py | 15 ++++++++++++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 2b31ff90fe..d4ddcd3299 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -195,8 +195,8 @@ def _authenticate_machine(self, conn: Connection) -> Mapping[str, Any]: if self.access_token: try: return self._sasl_start_jwt(conn) - except Exception: # noqa: S110 - pass + except Exception: + return self._authenticate_machine(conn) return self._sasl_start_jwt(conn) def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: @@ -204,15 +204,15 @@ def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: if self.access_token: try: return self._sasl_start_jwt(conn) - except Exception: # noqa: S110 - pass + except Exception: + return self._authenticate_human(conn) # If we have a cached refresh token, try a JwtStepRequest with that. if self.refresh_token: try: return self._sasl_start_jwt(conn) - except Exception: # noqa: S110 - pass + except Exception: + return self._authenticate_human(conn) # Start a new Two-Step SASL conversation. # Run a PrincipalStepRequest to get the IdpInfo. diff --git a/pymongo/common.py b/pymongo/common.py index 830874bc8a..7cd48f5e99 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -426,7 +426,6 @@ def validate_read_preference_tags(name: str, value: Any) -> list[dict[str, str]] "AWS_SESSION_TOKEN", "ENVIRONMENT", "TOKEN_RESOURCE", - "ALLOWED_HOSTS", ] ) @@ -442,8 +441,6 @@ def validate_auth_mechanism_properties(option: str, value: Any) -> dict[str, Uni props[key] = value elif isinstance(value, bool): props[key] = str(value).lower() - elif key in ["ALLOWED_HOSTS"] and isinstance(value, list): - props[key] = value elif key in ["OIDC_CALLBACK", "OIDC_HUMAN_CALLBACK"]: if not isinstance(value, OIDCCallback): raise ValueError("callback must be an OIDCCallback object") diff --git a/test/auth_oidc/test_auth_oidc.py b/test/auth_oidc/test_auth_oidc.py index accdf3d3cf..7107dca6b6 100644 --- a/test/auth_oidc/test_auth_oidc.py +++ b/test/auth_oidc/test_auth_oidc.py @@ -227,9 +227,18 @@ def test_1_6_allowed_hosts_blocked(self): # Close the client. client.close() - def test_1_7_machine_idp_human_callback(self): - if not self.uri_multiple: - raise unittest.SkipTest("Test Requires Server with Multiple Workflow IdPs") + def test_1_7_allowed_hosts_in_connection_string_ignored(self): + # Create an OIDC configured client with the connection string: `mongodb+srv://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:%5B%22example.com%22%5D` and a Human Callback. + # Assert that the creation of the client raises a configuration error. + uri = "mongodb+srv://example.com?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:%5B%22example.com%22%5D" + with self.assertRaises(ConfigurationError): + _ = MongoClient( + uri, authmechanismproperties=dict(OIDC_HUMAN_CALLBACK=self.create_request_cb()) + ) + + def test_1_8_machine_idp_human_callback(self): + if not os.environ.get("OIDC_IS_LOCAL"): + raise unittest.SkipTest("Test Requires Local OIDC server") # Create a client with MONGODB_URI_SINGLE, a username of test_machine, authMechanism=MONGODB-OIDC, and the OIDC human callback. client = self.create_client(username="test_machine") # Perform a find operation that succeeds. From 6653e4c12dc7ba29c723710739308536328d33ca Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 8 Apr 2024 06:32:15 -0500 Subject: [PATCH 4/8] end of file fix --- .evergreen/run-mongodb-oidc-test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.evergreen/run-mongodb-oidc-test.sh b/.evergreen/run-mongodb-oidc-test.sh index aa6a5b9615..89a2119308 100755 --- a/.evergreen/run-mongodb-oidc-test.sh +++ b/.evergreen/run-mongodb-oidc-test.sh @@ -29,4 +29,4 @@ fi export TEST_AUTH_OIDC=1 export COVERAGE=1 export AUTH="auth" -bash ./.evergreen/tox.sh -m test-eg -- "${@:1}" \ No newline at end of file +bash ./.evergreen/tox.sh -m test-eg -- "${@:1}" From 0b078994e2890098f9a795868346074ea7af3867 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 9 Apr 2024 07:42:57 -0500 Subject: [PATCH 5/8] finish updating --- pymongo/auth_oidc.py | 4 +++ pymongo/common.py | 2 ++ pymongo/helpers.py | 3 ++ test/auth_oidc/test_auth_oidc.py | 51 ++++++++++++++++++++++++-------- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index d4ddcd3299..891911c96c 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -211,6 +211,10 @@ def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: if self.refresh_token: try: return self._sasl_start_jwt(conn) + except OperationFailure as e: + if e.code == 18: + self.refresh_token = None + return self._authenticate_human(conn) except Exception: return self._authenticate_human(conn) diff --git a/pymongo/common.py b/pymongo/common.py index 7cd48f5e99..e990fddf89 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -441,6 +441,8 @@ def validate_auth_mechanism_properties(option: str, value: Any) -> dict[str, Uni props[key] = value elif isinstance(value, bool): props[key] = str(value).lower() + elif key in ["ALLOWED_HOSTS"] and isinstance(value, list): + props[key] = value elif key in ["OIDC_CALLBACK", "OIDC_HUMAN_CALLBACK"]: if not isinstance(value, OIDCCallback): raise ValueError("callback must be an OIDCCallback object") diff --git a/pymongo/helpers.py b/pymongo/helpers.py index e1769e0df9..916d78a33b 100644 --- a/pymongo/helpers.py +++ b/pymongo/helpers.py @@ -90,6 +90,9 @@ # Server code raised when re-authentication is required _REAUTHENTICATION_REQUIRED_CODE: int = 391 +# Server code raised when authentication fails. +_AUTHENTICATION_FAILURE_CODE: int = 18 + def _gen_index_name(keys: _IndexList) -> str: """Generate an index name from the set of fields it is over.""" diff --git a/test/auth_oidc/test_auth_oidc.py b/test/auth_oidc/test_auth_oidc.py index 7107dca6b6..0f46cefe86 100644 --- a/test/auth_oidc/test_auth_oidc.py +++ b/test/auth_oidc/test_auth_oidc.py @@ -231,7 +231,8 @@ def test_1_7_allowed_hosts_in_connection_string_ignored(self): # Create an OIDC configured client with the connection string: `mongodb+srv://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:%5B%22example.com%22%5D` and a Human Callback. # Assert that the creation of the client raises a configuration error. uri = "mongodb+srv://example.com?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:%5B%22example.com%22%5D" - with self.assertRaises(ConfigurationError): + with self.assertRaises(ConfigurationError), warnings.catch_warnings(): + warnings.simplefilter("ignore") _ = MongoClient( uri, authmechanismproperties=dict(OIDC_HUMAN_CALLBACK=self.create_request_cb()) ) @@ -429,7 +430,15 @@ def fetch(self, *args, **kwargs): def test_4_3_reauthenticate_succeeds_after_refresh_fails(self): # Create a default OIDC client with a human callback that returns an invalid refresh token - client = self.create_client() + cb = self.create_request_cb() + + class CustomRequest(OIDCCallback): + def fetch(self, *args, **kwargs): + result = cb.fetch(*args, **kwargs) + result.refresh_token = "bad" + return result + + client = self.create_client(request_cb=CustomRequest()) # Perform a find operation that succeeds. client.test.test.find_one() @@ -440,38 +449,56 @@ def test_4_3_reauthenticate_succeeds_after_refresh_fails(self): # Force a reauthenication using a fail point. with self.fail_point( { - "mode": {"times": 2}, - "data": {"failCommands": ["find", "saslStart"], "errorCode": 391}, + "mode": {"times": 1}, + "data": {"failCommands": ["find"], "errorCode": 391}, } ): # Perform a find operation that succeeds. client.test.test.find_one() - # Assert that the human callback has been called 3 times. - self.assertEqual(self.request_called, 3) + # Assert that the human callback has been called 2 times. + self.assertEqual(self.request_called, 2) # Close the client. client.close() def test_4_4_reauthenticate_fails(self): - # Create a client with a human callback that returns a valid token. - client = self.create_client() + # Create a default OIDC client with a human callback that returns invalid refresh tokens and + # Returns invalid access tokens after the first access. + cb = self.create_request_cb() + + class CustomRequest(OIDCCallback): + fetch_called = 0 + + def fetch(self, *args, **kwargs): + self.fetch_called += 1 + result = cb.fetch(*args, **kwargs) + result.refresh_token = "bad" + if self.fetch_called > 1: + result.access_token = "bad" + return result + + client = self.create_client(request_cb=CustomRequest()) + # Perform a find operation that succeeds (to force a speculative auth). client.test.test.find_one() # Assert that the human callback has been called once. self.assertEqual(self.request_called, 1) + # Force a reauthentication using a failCommand. with self.fail_point( { - "mode": {"times": 3}, - "data": {"failCommands": ["find", "saslStart"], "errorCode": 391}, + "mode": {"times": 1}, + "data": {"failCommands": ["find"], "errorCode": 391}, } ): # Perform a find operation that fails. with self.assertRaises(OperationFailure): client.test.test.find_one() - # Assert that the human callback has been called two times. - self.assertEqual(self.request_called, 2) + + # Assert that the human callback has been called three times. + self.assertEqual(self.request_called, 3) + # Close the client. client.close() From a5dc4a1be6aa84612ffec37178dbabed4dc2feed Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 9 Apr 2024 10:42:53 -0500 Subject: [PATCH 6/8] update error handling --- pymongo/auth_oidc.py | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 891911c96c..3c58af7a30 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -189,34 +189,43 @@ def get_spec_auth_cmd(self) -> Optional[MutableMapping[str, Any]]: def _authenticate_machine(self, conn: Connection) -> Mapping[str, Any]: # If there is a cached access token, try to authenticate with it. If - # authentication fails, it's possible the cached access token is expired. In - # that case, invalidate the access token, fetch a new access token, and try - # to authenticate again. + # authentication fails with error code 18, invalidate the access token, + # fetch a new access token, and try to authenticate again. If authentication + # fails for any other reason, raise the error to the user. if self.access_token: try: return self._sasl_start_jwt(conn) - except Exception: - return self._authenticate_machine(conn) + except OperationFailure as e: + if e.code == 18: + return self._authenticate_machine(conn) + raise return self._sasl_start_jwt(conn) def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: # If we have a cached access token, try a JwtStepRequest. + # authentication fails with error code 18, invalidate the access token, + # and try to authenticate again. If authentication fails for any other + # reason, raise the error to the user. if self.access_token: try: return self._sasl_start_jwt(conn) - except Exception: - return self._authenticate_human(conn) + except OperationFailure as e: + if e.code == 18: + return self._authenticate_human(conn) + raise # If we have a cached refresh token, try a JwtStepRequest with that. + # If authentication fails with error code 18, invalidate the access and + # refresh tokens, and try to authenticate again. If authentication fails for + # any other reason, raise the error to the user. if self.refresh_token: try: return self._sasl_start_jwt(conn) except OperationFailure as e: if e.code == 18: self.refresh_token = None - return self._authenticate_human(conn) - except Exception: - return self._authenticate_human(conn) + return self._authenticate_human(conn) + raise # Start a new Two-Step SASL conversation. # Run a PrincipalStepRequest to get the IdpInfo. From 96fc02b3129530359511957db057c0f9bb933c76 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Tue, 9 Apr 2024 11:39:00 -0500 Subject: [PATCH 7/8] switch to master branch --- .evergreen/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.evergreen/config.yml b/.evergreen/config.yml index ca08955dc1..a84b842148 100644 --- a/.evergreen/config.yml +++ b/.evergreen/config.yml @@ -98,7 +98,7 @@ functions: # If this was a patch build, doing a fresh clone would not actually test the patch cp -R ${PROJECT_DIRECTORY}/ $DRIVERS_TOOLS else - git clone --branch DRIVERS-2836 https://github.com/blink1073/drivers-evergreen-tools.git $DRIVERS_TOOLS + git clone https://github.com/mongodb-labs/drivers-evergreen-tools.git $DRIVERS_TOOLS fi echo "{ \"releases\": { \"default\": \"$MONGODB_BINARIES\" }}" > $MONGO_ORCHESTRATION_HOME/orchestration.config From c69c3fdab1c18f29dcaf7dee727e11772a7b89d4 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Wed, 10 Apr 2024 08:06:00 -0500 Subject: [PATCH 8/8] address review --- pymongo/auth_oidc.py | 14 ++++++++++---- test/auth_oidc/test_auth_oidc.py | 12 +++++------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pymongo/auth_oidc.py b/pymongo/auth_oidc.py index 3c58af7a30..43b935be7a 100644 --- a/pymongo/auth_oidc.py +++ b/pymongo/auth_oidc.py @@ -28,6 +28,7 @@ from pymongo._csot import remaining from pymongo._gcp_helpers import _get_gcp_response from pymongo.errors import ConfigurationError, OperationFailure +from pymongo.helpers import _AUTHENTICATION_FAILURE_CODE if TYPE_CHECKING: from pymongo.auth import MongoCredential @@ -196,7 +197,7 @@ def _authenticate_machine(self, conn: Connection) -> Mapping[str, Any]: try: return self._sasl_start_jwt(conn) except OperationFailure as e: - if e.code == 18: + if self._is_auth_error(e): return self._authenticate_machine(conn) raise return self._sasl_start_jwt(conn) @@ -210,7 +211,7 @@ def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: try: return self._sasl_start_jwt(conn) except OperationFailure as e: - if e.code == 18: + if self._is_auth_error(e): return self._authenticate_human(conn) raise @@ -222,7 +223,7 @@ def _authenticate_human(self, conn: Connection) -> Optional[Mapping[str, Any]]: try: return self._sasl_start_jwt(conn) except OperationFailure as e: - if e.code == 18: + if self._is_auth_error(e): self.refresh_token = None return self._authenticate_human(conn) raise @@ -294,10 +295,15 @@ def _run_command(self, conn: Connection, cmd: MutableMapping[str, Any]) -> Mappi try: return conn.command("$external", cmd, no_reauth=True) # type: ignore[call-arg] except OperationFailure as e: - if e.code == 18: + if self._is_auth_error(e): self._invalidate(conn) raise + def _is_auth_error(self, err: Exception) -> bool: + if not isinstance(err, OperationFailure): + return False + return err.code == _AUTHENTICATION_FAILURE_CODE + def _invalidate(self, conn: Connection) -> None: # Ignore the invalidation if a token gen id is given and is less than our # current token gen id. diff --git a/test/auth_oidc/test_auth_oidc.py b/test/auth_oidc/test_auth_oidc.py index 0f46cefe86..bb0a950286 100644 --- a/test/auth_oidc/test_auth_oidc.py +++ b/test/auth_oidc/test_auth_oidc.py @@ -35,10 +35,7 @@ from pymongo import MongoClient from pymongo._azure_helpers import _get_azure_response from pymongo._gcp_helpers import _get_gcp_response -from pymongo.auth_oidc import ( - OIDCCallback, - OIDCCallbackResult, -) +from pymongo.auth_oidc import OIDCCallback, OIDCCallbackContext, OIDCCallbackResult from pymongo.cursor import CursorType from pymongo.errors import AutoReconnect, ConfigurationError, OperationFailure from pymongo.hello import HelloCompat @@ -53,7 +50,7 @@ TOKEN_FILE = os.environ.get("OIDC_TOKEN_FILE", "") # Generate unified tests. -# globals().update(generate_test_classes(str(TEST_PATH), module=__name__)) +globals().update(generate_test_classes(str(TEST_PATH), module=__name__)) class OIDCTestBase(unittest.TestCase): @@ -112,10 +109,11 @@ def setUp(self): super().setUp() def create_request_cb(self, username="test_user1", sleep=0): - def request_token(context): + def request_token(context: OIDCCallbackContext): # Validate the info. self.assertIsInstance(context.idp_info.issuer, str) - # self.assertIsInstance(context.idp_info.clientId, str) + if context.idp_info.clientId is not None: + self.assertIsInstance(context.idp_info.clientId, str) # Validate the timeout. timeout_seconds = context.timeout_seconds