Draft PR, as this is proposing configuration changes and I'm sure there's gonna be some tweaks requested.

There are also some TODOs left, that I won't address until the general direction of this change is agreed upon.

Background

The Stack Overflow code base uses Attribute Based Routing, and has deviated considerably from the ASP.NET MVC defaults. Some of this is for good reason, and some of it is because our code dates back to the beta release of MVC 1, and we just made different choices than Microsoft over the years.

Some of our deviations are:

• we use a single attribute for all routing configuration (so there are no HttpGet or HttpPost equivalents)
• we allow multiple attributes on a method
• we default to checking CSRF tokens for certain HTTP verbs
• there's an explicit opt-out of CSRF token checking

An example:

[StackRoute("foo", HttpVerbs.Post)]
public ActionResult Foo() { ... } // this is _safe_

[StackRoute("bar", HttpVerbs.Put, EnsureXSRFSafe = false)]
public ActionResult Bar() { ... } // this should be _flagged_, it's not safe and needs a documented justification

This proposal seeks to add the flexibility we need, while keeping the out-of-the-box behavior of Security Code Scan unchanged.

Change

This replaces CsrfProtectionAttributes with CsrfProtection, which exposes many new options that replace the hardcoded type names in the current analyzers.

Namely, CsrfProtection exposes:

• Name (required)
• NameSpace (required)
• ControllerName
• NonActionAttributes (List)
• AllowAnonymousAttributes (List)
• VulnerableAttributes (List)
• AntiCsrfAttributes (List)
• IgnoreAttributes (List)
• ActionAttributes (List)

Each attribute collection is made up of:

• AttributeName (required)
• Condition

Conditions are similar to Conditions on Behaviors, except they apply only to attribute constructors and named arguments (which are actually property setters) and the "then" branch is implicitly "consider this attribute" and thus not specified.

In Main.yml the declarations for ASP.NET MVC and ASP.NET Core MVC have been converted to this new format, and the format has been documented.

As a consequence of moving hardcoded values into Main.yml, this change also merged the two different analyzers into a single CsrfTokenDiagnosticAnalyzer.

A test has been added that demonstrates the Stack Overflow-style of routing, which illustrates the use of conditions.

All tests are passing locally.

Questions

1. Is a proposal in this vein acceptable?

2. How should the configuration section be structured? I don't love what's in this proposal, it's just adequate for my purposes.

3. What names should be used for configuration? I just kind of made them up as I went along, or based them on the hardcoded variable names in the existing analyzers.

4. How should removing/replacing the existing configs be done? It's easy enough to port them, but having two ways to configure CSRF protection may not be desirable.

5. Is there any additional flexibility that should be included as part of this change?