Skip to content

Refactor Teams SSO middleware #6884

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.

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sw-joelmut
Copy link
Collaborator

@sw-joelmut sw-joelmut commented Feb 24, 2025

Related issue #6861
#minor

Description

This PR refactors the Teams SSO middleware to fix an issue with the usage of BlobsStorage.
When using BlobsStorage in the middleware, by providing an ETag to an item that will be saved for the first time, it throws an HTTP 412 error, then being captured as an ETag Exception and the conversation gets stuck.
The main reason of why this middleware was saving stuff in the storage was to avoid duplicated requests (when having more than one MS Teams client sign-in), process the first and discard the rest.

We ended up refactoring the middleware to use the storage ETag instead of custom one, and track the deduplication process per user per conversation instead of the token exchange request id that comes in the Activity.Value.

Specific Changes

  • Changed middleware documentation to understand better what this middleware does.
  • Changed the Storage key format to <channelId>/<conversation id>/<user id> to avoid colliding with the same user in other conversations.
  • Added ETagException to handle HTTP 412 errors better, it also includes the "ETag conflict" text at the start to maintain compatibility.
  • Adjusted one unit test to use concurrency.

Testing

The following image shows an example of the new implementation.
image

@sw-joelmut sw-joelmut added the Automation: Parity with js The PR needs to be ported to JS label Feb 24, 2025
@sw-joelmut sw-joelmut requested a review from a team as a code owner February 24, 2025 10:28
@sw-joelmut sw-joelmut marked this pull request as draft February 24, 2025 13:50
@sw-joelmut sw-joelmut marked this pull request as ready for review March 11, 2025 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automation: Parity with js The PR needs to be ported to JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant