microsoft / botbuilder-dotnet

Welcome to the Bot Framework SDK for .NET repository, which is the home for the libraries and packages that enable developers to build sophisticated bot applications using .NET.
https://github.com/Microsoft/botframework
MIT License
878 stars 484 forks source link

TeamsSSOTokenExchangeMiddleware.DeduplicatedTokenExchangeIdAsync fails on BlobStorage ETag validation #6861

Open coderjoe opened 1 month ago

coderjoe commented 1 month ago

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: <?xml version="1.0" encoding="utf-8"?>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 commented 3 weeks ago

We are facing the same issue :(

coderjoe commented 3 weeks ago

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 commented 3 weeks ago

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!