From f86388095713e0050fae00b7e91e599d9d253c6a Mon Sep 17 00:00:00 2001
From: Paul Ming <paulming@microsoft.com>
Date: Thu, 1 Aug 2019 15:52:50 -0700
Subject: [PATCH] CA3147 handle non-async Task<ActionResult-derived> ASP.NET
 MVC controller action methods

---
 ...ersWithValidateAntiforgeryTokenAnalyzer.cs |  7 +-
 ...ndlersWithValidateAntiforgeryTokenTests.cs | 67 +++++++++++++++++++
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/src/Microsoft.NetFramework.Analyzers/Core/MarkVerbHandlersWithValidateAntiforgeryTokenAnalyzer.cs b/src/Microsoft.NetFramework.Analyzers/Core/MarkVerbHandlersWithValidateAntiforgeryTokenAnalyzer.cs
index 74d35bf135..fa5d21b656 100644
--- a/src/Microsoft.NetFramework.Analyzers/Core/MarkVerbHandlersWithValidateAntiforgeryTokenAnalyzer.cs
+++ b/src/Microsoft.NetFramework.Analyzers/Core/MarkVerbHandlersWithValidateAntiforgeryTokenAnalyzer.cs
@@ -122,10 +122,9 @@ public override void Initialize(AnalysisContext analysisContext)
                                 || methodSymbol.IsStatic
                                 || !methodSymbol.IsPublic()
                                 || !(methodSymbol.ReturnType.Inherits(actionResultSymbol)  // FxCop implementation only looked at ActionResult-derived return types.
-                                     || (methodSymbol.IsAsync
-                                         && wellKnownTypeProvider.IsTaskOfType(
-                                             methodSymbol.ReturnType,
-                                             (ITypeSymbol typeArgument) => typeArgument.Inherits(actionResultSymbol))))
+                                     || wellKnownTypeProvider.IsTaskOfType(
+                                            methodSymbol.ReturnType,
+                                            (ITypeSymbol typeArgument) => typeArgument.Inherits(actionResultSymbol)))
                                 || (!methodSymbol.ContainingType.Inherits(mvcControllerSymbol)
                                     && !methodSymbol.ContainingType.Inherits(mvcControllerBaseSymbol)))
                             {
diff --git a/src/Microsoft.NetFramework.Analyzers/UnitTests/Security/MarkVerbHandlersWithValidateAntiforgeryTokenTests.cs b/src/Microsoft.NetFramework.Analyzers/UnitTests/Security/MarkVerbHandlersWithValidateAntiforgeryTokenTests.cs
index ed43a39ccb..9cb61003bc 100644
--- a/src/Microsoft.NetFramework.Analyzers/UnitTests/Security/MarkVerbHandlersWithValidateAntiforgeryTokenTests.cs
+++ b/src/Microsoft.NetFramework.Analyzers/UnitTests/Security/MarkVerbHandlersWithValidateAntiforgeryTokenTests.cs
@@ -849,6 +849,73 @@ public async Task<ActionResult> DoSomethingAsync(string input)
             return null;
         }
     }
+}"
+            );
+        }
+
+        [Fact]
+        public async Task MissingVerbsAndTokenTaskButNotAsync_CSharp_Diagnostic()
+        {
+            await VerifyCS.VerifyAnalyzerAsync(SystemWebMvcNamespaceCSharp + @"
+namespace Blah
+{
+    using System.Threading.Tasks;
+    using System.Web.Mvc;
+
+    public class ApiController : Controller
+    {
+        public Task<ActionResult> DoSomethingAsync(string input)
+        {
+            return null;
+        }
+    }
+}
+",
+                GetCA3147CSharpNoVerbsNoToken(SystemWebMvcNamespaceCSharpLineCount + 9, 35, "DoSomethingAsync"));
+        }
+
+        [Fact]
+        public async Task AcceptVerbsNoTokenTaskButNotAsync_CSharp_Diagnostic()
+        {
+            await VerifyCS.VerifyAnalyzerAsync(SystemWebMvcNamespaceCSharp + @"
+namespace Blah
+{
+    using System.Threading.Tasks;
+    using System.Web.Mvc;
+
+    public class AcceptVerbsNoTokenController : Controller
+    {
+        private const HttpVerbs AllowedVerbs = HttpVerbs.Post | HttpVerbs.Put;
+
+        [AcceptVerbs(AllowedVerbs)]
+        public Task<ActionResult> DoSomethingAsync()
+        {
+            return null;
+        }
+    }
+}
+",
+            GetCA3147CSharpVerbsAndNoToken(SystemWebMvcNamespaceCSharpLineCount + 12, 35, "DoSomethingAsync"));
+        }
+
+        [Fact]
+        public async Task HaveAcceptStringPutAndTokenTaskButNotAsync_CSharp_NoDiagnostic()
+        {
+            await VerifyCS.VerifyAnalyzerAsync(SystemWebMvcNamespaceCSharp + @"
+namespace Blah
+{
+    using System.Threading.Tasks;
+    using System.Web.Mvc;
+
+    public class ApiController : Controller
+    {
+        [AcceptVerbs(""Put"")]
+        [ValidateAntiForgeryToken]
+        public Task<ActionResult> DoSomethingAsync(string input)
+        {
+            return null;
+        }
+    }
 }"
             );
         }