TeamsSSOTokenExchangeMiddleware.DeduplicatedTokenExchangeIdAsync fails on BlobStorage ETag validation

[coderjoe]

Version

4.22.9

Describe the bug

The TeamsSSOTokenExchangeMiddleware fails in DeduplicatedTokenExchangeIdAsync when Microsoft.Bot.Builder.Azure.Blobs is used as the IStorage provider if/when the WriteAsync method returns a concurrency/etag exception.

As a result the system throws Azure.RequestFailedException

To Reproduce

Steps to reproduce the behavior:

1. Configure the BlobStorage adapter
2. Run multiple reduncant/high availability versions of the bot
3. Use TeamsSSOTokenExchangeMiddleware
4. Intermittently see the error

Expected behavior

It appears as though the adapter already detects similar errors for MemoryAdapter and the CosmosDb adapter but it is not detecting the error from Azure BlobStorage. I would expect that blob storage would be similarly handled.

Screenshots

No screenshots at this time

Additional context

This seems to already be handled by the middleware for MemoryAdapter and CosmosDB.
Does this just need to be extended to look for the Azure BlobStorage version of the exception?

Example failure:

The condition specified using HTTP conditional header(s) is not met.
RequestId:aabbccdd-abcd-1234-1766-24f46a000000
Time:2024-10-22T09:38:33.5806918Z
Status: 412 (The condition specified using HTTP conditional header(s) is not met.)
ErrorCode: ConditionNotMet

Content:
ConditionNotMetThe condition specified using HTTP conditional header(s) is not met.
RequestId:aabbccdd-abcd-1234-1766-24f46a000000
Time:2024-10-22T09:38:33.5806918Z

Headers:
Server: Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0
x-ms-request-id: aabbccdd-abcd-1234-1766-24f46a000000
x-ms-client-request-id: aabbccdd-abcd-1234-855c-57994512770d
x-ms-version: 2024-11-04
x-ms-error-code: ConditionNotMet
Date: Tue, 22 Oct 2024 09:38:32 GMT
Content-Length: 252
Content-Type: application/xml

Example Stack Trace:

at Azure.Storage.Blobs.BlockBlobRestClient.UploadAsync(Int64 contentLength, Stream body, Nullable`1 timeout, Byte[] transactionalContentMD5, String blobContentType, String blobContentEncoding, String blobContentLanguage, Byte[] blobContentMD5, String blobCacheControl, IDictionary`2 metadata, String leaseId, String blobContentDisposition, String encryptionKey, String encryptionKeySha256, Nullable`1 encryptionAlgorithm, String encryptionScope, Nullable`1 tier, Nullable`1 ifModifiedSince, Nullable`1 ifUnmodifiedSince, String ifMatch, String ifNoneMatch, String ifTags, String blobTagsString, Nullable`1 immutabilityPolicyExpiry, Nullable`1 immutabilityPolicyMode, Nullable`1 legalHold, Byte[] transactionalContentCrc64, CancellationToken cancellationToken)
   at Azure.Storage.Blobs.Specialized.BlockBlobClient.UploadInternal(Stream content, BlobHttpHeaders blobHttpHeaders, IDictionary`2 metadata, IDictionary`2 tags, BlobRequestConditions conditions, Nullable`1 accessTier, BlobImmutabilityPolicy immutabilityPolicy, Nullable`1 legalHold, IProgress`1 progressHandler, UploadTransferValidationOptions transferValidationOverride, String operationName, Boolean async, CancellationToken cancellationToken)
   at Azure.Storage.Blobs.Specialized.BlockBlobClient.<>c__DisplayClass65_0.<<GetPartitionedUploaderBehaviors>b__0>d.MoveNext()
--- End of stack trace from previous location ---
   at Azure.Storage.PartitionedUploader`2.UploadInternal(Stream content, Nullable`1 expectedContentLength, TServiceSpecificData args, IProgress`1 progressHandler, Boolean async, CancellationToken cancellationToken)
   at Azure.Storage.Blobs.BlobClient.StagedUploadInternal(Stream content, BlobUploadOptions options, Boolean async, CancellationToken cancellationToken)
   at Microsoft.Bot.Builder.Azure.Blobs.BlobsStorage.WriteAsync(IDictionary`2 changes, CancellationToken cancellationToken)
   at Microsoft.Bot.Builder.Teams.TeamsSSOTokenExchangeMiddleware.DeduplicatedTokenExchangeIdAsync(ITurnContext turnContext, CancellationToken cancellationToken)
   at Microsoft.Bot.Builder.Teams.TeamsSSOTokenExchangeMiddleware.OnTurnAsync(ITurnContext turnContext, NextDelegate next, CancellationToken cancellationToken)
   at Microsoft.Bot.Builder.MiddlewareSet.ReceiveActivityWithStatusAsync(ITurnContext turnContext, BotCallbackHandler callback, CancellationToken cancellationToken)
   at Microsoft.Bot.Builder.BotAdapter.RunPipelineAsync(ITurnContext turnContext, BotCallbackHandler callback, CancellationToken cancellationToken)

[SureshBabu-GV]

We are facing the same issue :(

[coderjoe]

For some further information - we're doing client login/logout via Teams following this example:
https://github.com/OfficeDev/Microsoft-Teams-Samples using the bot-teams-authentication example.

If we use MemoryStorage as our IStorage implementation it works as expected with no errors.
If we use BlobStorage as our IStorage implementation we get errors after every logout.

We have not tested CosmosDb.

[coderjoe]

To try and make things a little easier to debug, I've created a reproduction case using the above example.
https://github.com/coderjoe/Microsoft-Teams-Samples/tree/repro/blob.storage.logout.errors

Clone the above branch of my fork and you will get a c# project with our reproduction case.

Note that when run with no blob configuration it uses MemoryStorage and it works fine.
Run again with blob configuration provided and it will use BlobStorage but exhibit http precondition failed errors.

At this point it is not entirely clear to me if this is actually a bug in the TeamsSSOTokenExchangeMiddleware or whether the bug actually exists in the implementation of the Microsoft.Bot.Builder.Azure.Blobs package.

Hopefully this reproduction will assist. Cheers!

[shaktikumar-singh]

You have created bigger problem now with this fix.
You have just suppressed the exception due to which below code of class "TeamsSSOTokenExchangeMiddleware" has started returning false and breaking middlewere flow. and next middlewere is not getting invoked.

Could you please fix it asap and return true in place of false or fix it in better way. Your this fix started breaking my code.

[ceciliaavila]

The original issue was a bug with BlobsStorage, as it behaved differently than the rest of the storage options (MemoryStorage and CosmosDb).
Returning false and sending an invoke response is the default behavior when there's a problem with the token exchange or there's an Etag conflict.
The ETag conflict case happens when the request was processed by another thread, therefore the next middleware shouldn't be executed.
Now all storages have the same behavior.