-
Notifications
You must be signed in to change notification settings - Fork 343
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
[Suggestion] Fix two issues with dSTS authority, one when using WithTenantId, and … #4146
Conversation
…adding dSTS as a supported tenant override. Add a test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generalized a bit the logic and made the unit test handle the other tenant id
PR status seems ok, some CIAM tests are failing due to invalid secret. @gladjohn - should we disable these tests as well? |
I will let you merge the PR when the other tests are fixed. Thanks. |
tests are passing now. Once @neha-bhargava approves we will merge this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks @jennyf19
…adding dSTS as a supported tenant override. Add a test
Fixes #4144, fixes #4145
Just a suggestion. I'm having issues running the MSAL tests, so assuming this would work. have tested E2E with Id Web and seems to be working.