Skip to content
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

[Bug] [C#]: Bot Authentication logic not working as expected #1933

Closed
Tracked by #1102
singhk97 opened this issue Aug 14, 2024 · 0 comments
Closed
Tracked by #1102

[Bug] [C#]: Bot Authentication logic not working as expected #1933

singhk97 opened this issue Aug 14, 2024 · 0 comments
Assignees
Labels
bug Something isn't working dotnet Change/fix applies to dotnet. If all three, use the 'JS & dotnet & Python' label .NET Pull requests that update .net code

Comments

@singhk97
Copy link
Collaborator

singhk97 commented Aug 14, 2024

Bugs (Verified for .NET SDK)

  1. Incorrect setting name is provided to FilteredTeamsSSOTokenExchangeMiddleware.
  2. OnUserSignInSuccess is not triggered when token exchange is done successfully.
    • This is because FilteredTeamsSSOTokenExchangeMiddleware intercepts the incoming activity and so the auth logic in the OnTurnAsync method is not invoked.
@singhk97 singhk97 self-assigned this Aug 14, 2024
@singhk97 singhk97 added bug Something isn't working .NET Pull requests that update .net code dotnet Change/fix applies to dotnet. If all three, use the 'JS & dotnet & Python' label labels Aug 14, 2024
singhk97 added a commit that referenced this issue Sep 4, 2024
## Linked issues

closes: #1744 #1933

## Details

Fixed bug in both #1744 and #1933. 

The issue does not persist in either JS, or PY so the fix is only for
C#.

#### Change details
* Refactored `FilteredTeamsSSOTokenExchangeMiddleware` as the underlying
middleware was not being invoked correctly.
* The `app.adapter.Use` method was being called for every incomming
request (since the `Application` object is a transient in the Asp.NET
service collection).
* As for the issue in #1744 - The code is updated to use
`DateTime.UtcNow` by default.

## Attestation Checklist

- [x] My code follows the style guidelines of this project

- I have checked for/fixed spelling, linting, and other errors
- I have commented my code for clarity
- I have made corresponding changes to the documentation (updating the
doc strings in the code is sufficient)
- My changes generate no new warnings
- I have added tests that validates my changes, and provides sufficient
test coverage. I have tested with:
  - Local testing
  - E2E testing in Teams
- New and existing unit tests pass locally with my changes

### Additional information

> Feel free to add other relevant information below
@singhk97 singhk97 closed this as completed Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dotnet Change/fix applies to dotnet. If all three, use the 'JS & dotnet & Python' label .NET Pull requests that update .net code
Projects
None yet
Development

No branches or pull requests

1 participant