Allow deleting non-reserved-system-keys that start with underscore (#6496)

Author: pragnagopa

src/WebJobs.Script.WebHost/Controllers/KeysController.cs

@@ -240,12 +240,17 @@ private async Task<Dictionary<string, string>> GetHostSecretsByScope(string secr
 
         private async Task<IHttpActionResult> DeleteFunctionSecretAsync(string keyName, string keyScope, ScriptSecretsType secretsType)
         {
-            if (keyName == null || keyName.StartsWith("_"))
+            if (keyName == null)
             {
-                // System keys cannot be deleted.
                 return BadRequest("Invalid key name.");
             }
 
+            if (IsBuiltInSystemKeyName(keyName))
+            {
+                // System keys cannot be deleted.
+                return BadRequest("Cannot delete System Key.");
+            }
+
             if ((secretsType == ScriptSecretsType.Function && keyScope != null && !IsFunction(keyScope)) || 
                 !await _secretManager.DeleteSecretAsync(keyName, keyScope, secretsType))
             {
@@ -262,6 +267,15 @@ private async Task<IHttpActionResult> DeleteFunctionSecretAsync(string keyName,
             return StatusCode(HttpStatusCode.NoContent);
         }
 
+        private static bool IsBuiltInSystemKeyName(string keyName)
+        {
+            if (keyName.Equals(MasterKeyName, StringComparison.OrdinalIgnoreCase))
+            {
+                return true;
+            }
+            return false;
+        }
+
         private bool IsFunction(string functionName)
         {
             string json = null;

test/WebJobs.Script.Tests/Controllers/Admin/KeysControllerTests.cs

@@ -134,17 +134,38 @@ public async Task PutKey_Succeeds()
             _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Once);
         }
 
-        [Fact]
-        public async Task DeleteKey_Succeeds()
+        [Theory]
+        [InlineData("key1", false)]
+        [InlineData("_key1", false)]
+        [InlineData("_master", true)]
+        [InlineData("_MASter", true)]
+        [InlineData(null, true)]
+        public async Task DeleteKey_Tests(string keyName, bool invalidKey)
         {
             _testController.Request = new HttpRequestMessage(HttpMethod.Get, "https://local/admin/functions/keys/key2");
 
-            _secretsManagerMock.Setup(p => p.DeleteSecretAsync("key2", "TestFunction1", ScriptSecretsType.Function)).ReturnsAsync(true);
-
-            var result = (StatusCodeResult)(await _testController.Delete("TestFunction1", "key2"));
-            Assert.Equal(HttpStatusCode.NoContent, result.StatusCode);
+            _secretsManagerMock.Setup(p => p.DeleteSecretAsync(keyName, "TestFunction1", ScriptSecretsType.Function)).ReturnsAsync(true);
 
-            _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Once);
+            if (invalidKey)
+            {
+                if (string.IsNullOrEmpty(keyName))
+                {
+                    var result = (BadRequestErrorMessageResult)(await _testController.Delete("TestFunction1", keyName));
+                    Assert.Equal("Invalid key name.", result.Message);
+                }
+                else
+                {
+                    var result = (BadRequestErrorMessageResult)(await _testController.Delete("TestFunction1", keyName));
+                    Assert.Equal("Cannot delete System Key.", result.Message);
+                }
+                _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Never);
+            }
+            else
+            {
+                var result = (StatusCodeResult)(await _testController.Delete("TestFunction1", keyName));
+                Assert.Equal(HttpStatusCode.NoContent, result.StatusCode);
+                _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Once);
+            }
         }
 
         [Fact]
@@ -167,15 +188,6 @@ public async Task DeleteKey_NotAKey_ReturnsNotFound()
             _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Never);
         }
 
-        [Fact]
-        public async Task DeleteKey_InvalidKeyName_ReturnsBadRequest()
-        {
-            var result = (BadRequestErrorMessageResult)(await _testController.Delete("TestFunction1", "_test"));
-            Assert.Equal("Invalid key name.", result.Message);
-
-            _functionsSyncManagerMock.Verify(p => p.TrySyncTriggersAsync(false), Times.Never);
-        }
-
         protected virtual void Dispose(bool disposing)
         {
             if (disposing)