Skip to content

Fix AzureAd options validation #13480

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

Merged
merged 2 commits into from
Aug 27, 2019
Merged

Fix AzureAd options validation #13480

merged 2 commits into from
Aug 27, 2019

Conversation

Tratcher
Copy link
Member

Ask mode template

Description

#9967 added options validation for AzureAd auth options, but it introduced a regression where it would throw for other cookie or jwt auth providers.

#13327 was a community members fix for cookies in master. I've cherry picked that commit, also fixed jwt, and added tests.

Customer Impact

Customers may not be able to use AzureAd auth combine with other kinds of auth in one app.

Regression?

Yes, identified externally in preview8.

Risk

Low.

@Pilchie

@Tratcher Tratcher added area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. labels Aug 27, 2019
@Tratcher Tratcher added this to the 3.0.0 milestone Aug 27, 2019
@Tratcher Tratcher requested review from blowdart, HaoK and javiercn August 27, 2019 17:55
@Tratcher Tratcher self-assigned this Aug 27, 2019
@HaoK
Copy link
Member

HaoK commented Aug 27, 2019

Should we just consider reverting the validation change instead?

@HaoK
Copy link
Member

HaoK commented Aug 27, 2019

The validation change was supposedly just to make things easier to debug, if its causing regressions, that seems not worth the supposed benefit

@Tratcher
Copy link
Member Author

The validation is useful, it just got applied too broadly.

@Pilchie
Copy link
Member

Pilchie commented Aug 27, 2019

This was approved by tactics, once test failure is understood.

@Tratcher
Copy link
Member Author

Flaky blazor test, retrying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants