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

Change Load Order of AntiforgeryToken #3993

Merged

Conversation

deanmarcussen
Copy link
Member

As requested :)

@deanmarcussen
Copy link
Member Author

deanmarcussen commented Aug 2, 2019

Maybe there is still potential for there to be a conflict between the orders of AutoValidateAntiforgeryTokenAttribute and the IgnoreAntiforgeryToken, though I was able to test it and the IgnoreAntiforgeryToken does work with the ImportRemoteInstanceController as I can hit it with the debugger.

However it might be working more by blind luck

Both the Attributes have a default order of 1000, but I think by adding a specific order to the Startup for the AVFT we have circumvented the issue.

See aspnet/Mvc#7795 and dotnet/aspnetcore#10384 and aspnet/Mvc#5863

The simple thing maybe to change the AVFT order to 950 for clarity, but that is again a magic number, so may have an impact somewhere else.

I'll update this pr to do that, having tested it at 950 and found no issues so far.

(Maybe this is a good idea / maybe bad idea)

@deanmarcussen deanmarcussen changed the title Add requested comment to Mvc Startup for AntiforgeryTokenAttribute Change Load Order of AntiforgeryToken Aug 2, 2019
@jtkech
Copy link
Member

jtkech commented Aug 3, 2019

Hmm, maybe just use 999 and a link to the related mvc issue saying that AutoValidateAntiforgeryTokenAttribute needs a lower order than IgnoreAntiforgeryToken whose default order is also 1000.

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.

3 participants