Add middleware for optional strict CLI version checking (#3564)

* Add a middleware for strict version checking

* Fill in doc comments

* Move header names to constants

* Make version middleware more easily testable and add first unit test

* Add the rest of the version check unit tests

* Fix build errors for dropped FluentAssertions return values in other tests

* dotnet format

* Fix import order

* fix str.lower on optional str

* Rename TestCliVersion to CheckCliVersion

* Use OneFuzzResultVoid instead of nullabe Error

* Change version parser to Semver

* Restore projects that use ApiService

* Add a prerelease version test case

src/ApiService/ApiService/ApiService.csproj

@@ -13,7 +13,7 @@
     <PackageReference Include="Microsoft.Azure.Functions.Worker.ApplicationInsights" Version="1.0.0-preview4" />
     <PackageReference Include="Microsoft.Rest.ClientRuntime" Version="2.3.24" />
 
-    <PackageReference Include="Semver" Version="2.1.0" />
+    <PackageReference Include="Semver" Version="2.3.0" />
     <PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.3.0" />
     <PackageReference Include="Microsoft.Azure.AppConfiguration.Functions.Worker" Version="6.0.0" />
     <PackageReference Include="Microsoft.FeatureManagement" Version="2.5.1" />

src/ApiService/ApiService/OneFuzzTypes/Enums.cs

@@ -51,6 +51,7 @@ public enum ErrorCode {
     ADO_VALIDATION_INVALID_PATH = 495,
     ADO_VALIDATION_INVALID_PROJECT = 496,
     INVALID_RETENTION_PERIOD = 497,
+    INVALID_CLI_VERSION = 498,
     // NB: if you update this enum, also update enums.py
 }
 

src/ApiService/ApiService/Program.cs

@@ -18,6 +18,7 @@
 using Microsoft.FeatureManagement;
 using Microsoft.Graph;
 using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
+using Semver;
 namespace Microsoft.OneFuzz.Service;
 
 public class Program {
@@ -58,6 +59,67 @@ public async Async.Task Invoke(FunctionContext context, FunctionExecutionDelegat
         }
     }
 
+    /// <summary>
+    /// Represents a middleware that can optionally perform strict version checking based on data sent in request headers.
+    /// </summary>
+    public class VersionCheckingMiddleware : IFunctionsWorkerMiddleware {
+        private const string CliVersionHeader = "Cli-Version";
+        private const string StrictVersionHeader = "Strict-Version";
+        private readonly SemVersion _oneFuzzServiceVersion;
+        private readonly IRequestHandling _requestHandling;
+
+        /// <summary>
+        /// Initializes an instance of <see cref="VersionCheckingMiddleware"/> with the provided config and request handling objects.
+        /// </summary>
+        /// <param name="config">The service config containing the service version.</param>
+        /// <param name="requestHandling">The request handling object to create HTTP responses with.</param>
+        public VersionCheckingMiddleware(IServiceConfig config, IRequestHandling requestHandling) {
+            _oneFuzzServiceVersion = SemVersion.Parse(config.OneFuzzVersion, SemVersionStyles.Strict);
+            _requestHandling = requestHandling;
+        }
+
+        public OneFuzzResultVoid CheckCliVersion(Azure.Functions.Worker.Http.HttpHeadersCollection headers) {
+            var doStrictVersionCheck =
+                headers.TryGetValues(StrictVersionHeader, out var strictVersion)
+                && strictVersion?.FirstOrDefault()?.Equals("true", StringComparison.InvariantCultureIgnoreCase) == true; // "== true" necessary here to avoid implicit null -> bool casting
+
+            if (doStrictVersionCheck) {
+                if (!headers.TryGetValues(CliVersionHeader, out var cliVersion)) {
+                    return Error.Create(ErrorCode.INVALID_REQUEST, $"'{StrictVersionHeader}' is set to true without a corresponding '{CliVersionHeader}' header");
+                }
+                if (!SemVersion.TryParse(cliVersion?.FirstOrDefault() ?? "", SemVersionStyles.Strict, out var version)) {
+                    return Error.Create(ErrorCode.INVALID_CLI_VERSION, $"'{CliVersionHeader}' header value is not a valid sematic version");
+                }
+                if (version.ComparePrecedenceTo(_oneFuzzServiceVersion) < 0) {
+                    return Error.Create(ErrorCode.INVALID_CLI_VERSION, "cli is out of date");
+                }
+            }
+
+            return OneFuzzResultVoid.Ok;
+        }
+
+        /// <summary>
+        /// Checks the request for two headers, cli version and one indicating whether to do strict version checking.
+        /// When both are present and the cli is out of date, a descriptive response is sent back.
+        /// </summary>
+        /// <param name="context">The function context.</param>
+        /// <param name="next">The function execution delegate.</param>
+        /// <returns>A <seealso cref="Task"/> </returns>
+        public async Async.Task Invoke(FunctionContext context, FunctionExecutionDelegate next) {
+            var requestData = await context.GetHttpRequestDataAsync();
+            if (requestData is not null) {
+                var error = CheckCliVersion(requestData.Headers);
+                if (!error.IsOk) {
+                    var response = await _requestHandling.NotOk(requestData, error.ErrorV, "version middleware");
+                    context.GetInvocationResult().Value = response;
+                    return;
+                }
+            }
+
+            await next(context);
+        }
+    }
+
 
     //Move out expensive resources into separate class, and add those as Singleton
     // ArmClient, Table Client(s), Queue Client(s), HttpClient, etc.
@@ -161,6 +223,7 @@ public static async Async.Task Main() {
                 builder.UseMiddleware<LoggingMiddleware>();
                 builder.UseMiddleware<Auth.AuthenticationMiddleware>();
                 builder.UseMiddleware<Auth.AuthorizationMiddleware>();
+                builder.UseMiddleware<VersionCheckingMiddleware>();
 
                 //this is a must, to tell the host that worker logging is done by us
                 builder.Services.Configure<WorkerOptions>(workerOptions => workerOptions.Capabilities["WorkerApplicationInsightsLoggingEnabled"] = bool.TrueString);

src/ApiService/ApiService/onefuzzlib/Versions.cs

@@ -2,11 +2,11 @@
 
 namespace Microsoft.OneFuzz.Service;
 
-public class versions {
+public class Versions {
     public static bool IsMinimumVersion(string versionStr, string minimumStr) {
         var version = SemVersion.Parse(versionStr, SemVersionStyles.Any);
         var minimum = SemVersion.Parse(minimumStr, SemVersionStyles.Any);
 
-        return version >= minimum;
+        return version.ComparePrecedenceTo(minimum) >= 0;
     }
 }

src/ApiService/ApiService/packages.lock.json

@@ -385,9 +385,9 @@
       },
       "Semver": {
         "type": "Direct",
-        "requested": "[2.1.0, )",
-        "resolved": "2.1.0",
-        "contentHash": "1jUT0PwgKO9d9F/X2n762qLp7v/30OpMtJPFRtmjPXUX2/J0lnqiGiSJNNsW3yYTj5StF0Z1yE36TrvtGpcbrg=="
+        "requested": "[2.3.0, )",
+        "resolved": "2.3.0",
+        "contentHash": "4vYo1zqn6pJ1YrhjuhuOSbIIm0CpM47grbpTJ5ABjOlfGt/EhMEM9ed4MRK5Jr6gVnntWDqOUzGeUJp68PZGjw=="
       },
       "SmartAnalyzers.CSharpExtensions.Annotations": {
         "type": "Direct",

src/ApiService/FunctionalTests/Auth.cs

@@ -47,7 +47,7 @@ public async Task<AuthenticationResult> Auth(CancellationToken cancelationToken)
             _token = await _app.AcquireTokenForClient(_authConfig.Scopes).ExecuteAsync(cancelationToken);
             return _token;
         } finally {
-            _lockObj.Release();
+            _ = _lockObj.Release();
         }
     }
 

src/ApiService/FunctionalTests/FunctionalTests.csproj

@@ -9,7 +9,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Compile Include="..\ApiService\HttpClient.cs" Link="1f-api\HttpClient.cs" />
+    <ProjectReference Include="..\ApiService\ApiService.csproj" />
   </ItemGroup>
 
   <ItemGroup>
@@ -18,6 +18,7 @@
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
       <PrivateAssets>all</PrivateAssets>
     </PackageReference>
+    <PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http" Version="3.0.13" />
     <PackageReference Include="Microsoft.Identity.Client" Version="4.52.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.2" />
     <PackageReference Include="System.Net.Http" Version="4.3.4" />
@@ -34,4 +35,4 @@
     </PackageReference>
   </ItemGroup>
 
-</Project>
+</Project>

src/ApiService/FunctionalTests/TestContainer.cs

@@ -19,27 +19,27 @@ public TestContainer(ITestOutputHelper output) {
         [Fact]
         public async Task DownloadNonExistentContainer() {
             var r1 = await _downloadApi.Get();
-            r1.IsOk.Should().BeFalse();
-            r1.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
+            _ = r1.IsOk.Should().BeFalse();
+            _ = r1.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
 
             var r2 = await _downloadApi.Get(container: Guid.NewGuid().ToString());
-            r2.IsOk.Should().BeFalse();
-            r2.ErrorV!.Item2!.ShouldBeProvided("filename").Should().BeTrue();
+            _ = r2.IsOk.Should().BeFalse();
+            _ = r2.ErrorV!.Item2!.ShouldBeProvided("filename").Should().BeTrue();
 
             var r3 = await _downloadApi.Get(filename: Guid.NewGuid().ToString());
-            r3.IsOk.Should().BeFalse();
-            r3.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
+            _ = r3.IsOk.Should().BeFalse();
+            _ = r3.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
 
             var r4 = await _downloadApi.Get(container: Guid.NewGuid().ToString(), filename: Guid.NewGuid().ToString());
-            r4.IsOk.Should().BeFalse();
-            r4.ErrorV!.Item1.Should().Be(System.Net.HttpStatusCode.NotFound);
+            _ = r4.IsOk.Should().BeFalse();
+            _ = r4.ErrorV!.Item1.Should().Be(System.Net.HttpStatusCode.NotFound);
         }
 
         [Fact]
         public async Task CreateGetDeleteContainer() {
             var containerName = Guid.NewGuid().ToString();
             var container = await _containerApi.Post(containerName);
-            container.IsOk.Should().BeTrue($"failed to create container due to {container.ErrorV}");
+            _ = container.IsOk.Should().BeTrue($"failed to create container due to {container.ErrorV}");
 
             var c = await _containerApi.Get(containerName);
 

src/ApiService/FunctionalTests/TestInfo.cs

@@ -16,8 +16,8 @@ public TestInfo(ITestOutputHelper output) {
         [Fact]
         async Task GetInfo() {
             var info = await _infoApi.Get();
-            info.IsOk.Should().BeTrue();
-            info.OkV!.Versions.ContainsKey("onefuzz").Should().BeTrue();
+            _ = info.IsOk.Should().BeTrue();
+            _ = info.OkV!.Versions.ContainsKey("onefuzz").Should().BeTrue();
         }
     }
 }

src/ApiService/FunctionalTests/TestNode.cs

@@ -24,14 +24,14 @@ public TestNode(ITestOutputHelper output) {
         async Task GetNonExistentNode() {
             var n = await _nodeApi.Get(Guid.NewGuid());
 
-            n.IsOk.Should().BeFalse();
-            n.ErrorV!.UnableToFindNode.Should().BeTrue();
+            _ = n.IsOk.Should().BeFalse();
+            _ = n.ErrorV!.UnableToFindNode.Should().BeTrue();
         }
 
         [Fact]
         async Task GetAllNodes() {
             var ns = await _nodeApi.Get();
-            ns.IsOk.Should().BeTrue("failed to get all nodes due to {0}", ns.ErrorV);
+            _ = ns.IsOk.Should().BeTrue("failed to get all nodes due to {0}", ns.ErrorV);
             foreach (var n in ns.OkV!) {
                 _output.WriteLine($"node machine id: {n.MachineId}, scaleset id: {n.ScalesetId}, poolName: {n.PoolName}, poolId: {n.PoolId} state: {n.State}, version: {n.Version}");
             }
@@ -41,8 +41,8 @@ async Task GetAllNodes() {
         [Fact]
         async Task DeleteNonExistentNode() {
             var n = await _nodeApi.Delete(Guid.NewGuid());
-            n.IsError.Should().BeTrue();
-            n.Error!.UnableToFindNode.Should().BeTrue();
+            _ = n.IsError.Should().BeTrue();
+            _ = n.Error!.UnableToFindNode.Should().BeTrue();
         }
 
         [Fact]
@@ -51,25 +51,25 @@ async Task GetPatchPostDelete() {
             var (pool, scaleset) = await Helpers.CreatePoolAndScaleset(_poolApi, _scalesetApi, "linux");
 
             scaleset = await _scalesetApi.WaitWhile(scaleset.ScalesetId, sc => sc.State == "init" || sc.State == "setup");
-            scaleset.Nodes!.Should().NotBeEmpty();
+            _ = scaleset.Nodes!.Should().NotBeEmpty();
 
             var nodeState = scaleset.Nodes!.First();
             var nodeResult = await _nodeApi.Get(nodeState.MachineId);
 
-            nodeResult.IsOk.Should().BeTrue("failed to get node due to {0}", nodeResult.ErrorV);
+            _ = nodeResult.IsOk.Should().BeTrue("failed to get node due to {0}", nodeResult.ErrorV);
             var node = nodeResult.OkV!.First();
             node = await _nodeApi.WaitWhile(node.MachineId, n => n.State == "init" || n.State == "setup");
 
             var r = await _nodeApi.Patch(node.MachineId);
-            r.Result.Should().BeTrue();
+            _ = r.Result.Should().BeTrue();
 
             var rr = await _nodeApi.Update(node.MachineId, false);
 
             var d = await _nodeApi.Delete(node.MachineId);
-            d.Result.Should().BeTrue();
+            _ = d.Result.Should().BeTrue();
 
             var deletePool = await _poolApi.Delete(pool.Name);
-            deletePool.Result.Should().BeTrue();
+            _ = deletePool.Result.Should().BeTrue();
         }
     }
 }

src/ApiService/FunctionalTests/TestPool.cs

@@ -21,7 +21,7 @@ public TestPool(ITestOutputHelper output) {
         [Fact]
         public async Task GetNonExistentPool() {
             var p = await _poolApi.Get(name: Guid.NewGuid().ToString());
-            p.ErrorV!.UnableToFindPoolError.Should().BeTrue("{0}", p.ErrorV!);
+            _ = p.ErrorV!.UnableToFindPoolError.Should().BeTrue("{0}", p.ErrorV!);
         }
 
 
@@ -34,7 +34,7 @@ public async Task DeleteFunctionalTestPools() {
         [Fact]
         public async Task GetPools() {
             var pools = await _poolApi.Get();
-            pools.IsOk.Should().BeTrue();
+            _ = pools.IsOk.Should().BeTrue();
 
             if (!pools.OkV!.Any()) {
                 _output.WriteLine("Got empty pools");
@@ -54,16 +54,16 @@ public async Task CreateAndDelete() {
             _output.WriteLine($"creating pool {newPoolName}");
             var newPool = await _poolApi.Create(newPoolName, "linux");
 
-            newPool.IsOk.Should().BeTrue("failed to create new pool: {0}", newPool.ErrorV);
+            _ = newPool.IsOk.Should().BeTrue("failed to create new pool: {0}", newPool.ErrorV);
 
             var poolsCreated = await _poolApi.Get();
-            poolsCreated.IsOk.Should().BeTrue("failed to get pools: {0}", poolsCreated.ErrorV);
+            _ = poolsCreated.IsOk.Should().BeTrue("failed to get pools: {0}", poolsCreated.ErrorV);
 
             var newPools = poolsCreated.OkV!.Where(p => p.Name == newPoolName);
-            newPools.Count().Should().Be(1);
+            _ = newPools.Count().Should().Be(1);
 
             var deletedPoolResult = await _poolApi.Delete(newPoolName);
-            deletedPoolResult.Result.Should().BeTrue();
+            _ = deletedPoolResult.Result.Should().BeTrue();
         }
     }
 }

src/ApiService/FunctionalTests/TestProxy.cs

@@ -26,7 +26,7 @@ public TestProxy(ITestOutputHelper output) {
         public async Task GetProxies() {
             var allProxiesResult = await _proxyApi.Get();
 
-            allProxiesResult.IsOk.Should().BeTrue("failed to get proxies due to {0}", allProxiesResult.ErrorV);
+            _ = allProxiesResult.IsOk.Should().BeTrue("failed to get proxies due to {0}", allProxiesResult.ErrorV);
 
             if (!allProxiesResult.OkV!.Any()) {
                 _output.WriteLine("Got empty list of proxies");
@@ -42,12 +42,12 @@ public async Task CreateResetDelete() {
             var (newPool, newScaleset) = await Helpers.CreatePoolAndScaleset(_poolApi, _scalesetApi, "linux");
 
             newScaleset = await _scalesetApi.WaitWhile(newScaleset.ScalesetId, sc => sc.State == "init" || sc.State == "setup");
-            newScaleset.Nodes!.Should().NotBeEmpty();
+            _ = newScaleset.Nodes!.Should().NotBeEmpty();
 
             var firstNode = newScaleset.Nodes!.First();
 
             var nodeResult = await _nodeApi.Get(machineId: firstNode.MachineId);
-            nodeResult.IsOk.Should().BeTrue();
+            _ = nodeResult.IsOk.Should().BeTrue();
             var node = nodeResult.OkV!.First();
 
             node = await _nodeApi.WaitWhile(node.MachineId, n => n.State == "init" || n.State == "setup");
@@ -56,23 +56,23 @@ public async Task CreateResetDelete() {
 
             var proxyAgain = await _proxyApi.Create(newScaleset.ScalesetId, node.MachineId, 2223, 1);
 
-            proxy.IsOk.Should().BeTrue("failed to create proxy due to {0}", proxy.ErrorV);
-            proxyAgain.IsOk.Should().BeTrue("failed to create proxy with same config due to {0}", proxyAgain.ErrorV);
+            _ = proxy.IsOk.Should().BeTrue("failed to create proxy due to {0}", proxy.ErrorV);
+            _ = proxyAgain.IsOk.Should().BeTrue("failed to create proxy with same config due to {0}", proxyAgain.ErrorV);
 
-            proxy.OkV!.Should().BeEquivalentTo(proxyAgain.OkV!);
+            _ = proxy.OkV!.Should().BeEquivalentTo(proxyAgain.OkV!);
             _output.WriteLine($"created proxy dst ip: {proxy.OkV!.Forward.DstIp}, srcPort: {proxy.OkV.Forward.SrcPort} dstport: {proxy.OkV!.Forward.DstPort}, ip: {proxy.OkV!.Ip}");
 
 
             var proxyReset = await _proxyApi.Reset(newScaleset.Region);
-            proxyReset.Result.Should().BeTrue();
+            _ = proxyReset.Result.Should().BeTrue();
 
             var deleteProxy = await _proxyApi.Delete(newScaleset.ScalesetId, node.MachineId);
-            deleteProxy.Result.Should().BeTrue();
+            _ = deleteProxy.Result.Should().BeTrue();
 
             _output.WriteLine($"deleted proxy");
 
             var deletePool = await _poolApi.Delete(newPool.Name);
-            deletePool.Result.Should().BeTrue();
+            _ = deletePool.Result.Should().BeTrue();
             _output.WriteLine($"deleted pool {newPool.Name}");
         }
     }