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

remove AuthorizeFolder in secure user data #5451

Merged
merged 2 commits into from
Feb 13, 2018
Merged

Conversation

Rick-Anderson
Copy link
Contributor

@Rick-Anderson Rick-Anderson commented Feb 13, 2018

@Rick-Anderson
Copy link
Contributor Author

Here is the big security change:
image

@Rick-Anderson Rick-Anderson requested a review from HaoK February 13, 2018 00:57
@HaoK
Copy link
Member

HaoK commented Feb 13, 2018

Looks good to me, does this look ok @blowdart @javiercn ?

// debug mode. This is useful when not using Visual Studio.
if (Environment.IsDevelopment() && !skipSSL)
if (Environment.IsDevelopment() && !skipHTTPS)
{
options.Filters.Add(new RequireHttpsAttribute());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't recommend using RequireHttpsAttribute and I'm going to suggest deprecating it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javiercn Should I replace it with AddRedirectToHttps? Don't we need to enforce HTTPS?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if that middleware is wired up.

@Rick-Anderson
Copy link
Contributor Author

@javiercn see #5393 which tracks 2.1 Improvements to HTTPS

@Rick-Anderson Rick-Anderson merged commit 4d69b62 into master Feb 13, 2018
@Rick-Anderson Rick-Anderson deleted the user-data/ra branch February 13, 2018 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants