-
Notifications
You must be signed in to change notification settings - Fork 242
Onboard Id Web to Threading Analyzers #3041
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
Conversation
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.
LGTM at first review, @westin-m
Make sure you run end to end tests with credentials.
There was a race condition at some point, which is why the credential loaders used GetAwaiter.GetAsync().
I'm surprised there is no change in the public API?
src/Microsoft.Identity.Web.Certificate/DefaultCertificateLoader.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web.OWIN/OpenIdConnectCachingSecurityTokenProvider.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Identity.Web/AzureADB2COpenIDConnectEventHandlers.cs
Outdated
Show resolved
Hide resolved
jennyf19
left a comment
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.
would like to have clarity on which E2E tests were run to verify things are working as expected, and which scenarios were covered.
would like to have no public API changes now.
I've added this to the description |
The following E2E scenarios were tested using the corresponding devapps:
Web app calls web api call graph
B2C web app calls web api
Owin
Blazer server calls web api
Blazer server B2C calls api
Ciam web app calls api
Web app calls graph
Deamon console calling downstream api, graph
All are working as expected.