Skip to content
This repository was archived by the owner on Nov 1, 2023. It is now read-only.

Commit f251282

Browse files
authored
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
1 parent 8c315af commit f251282

File tree

20 files changed

+1712
-128
lines changed

20 files changed

+1712
-128
lines changed

src/ApiService/ApiService/ApiService.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
<PackageReference Include="Microsoft.Azure.Functions.Worker.ApplicationInsights" Version="1.0.0-preview4" />
1414
<PackageReference Include="Microsoft.Rest.ClientRuntime" Version="2.3.24" />
1515

16-
<PackageReference Include="Semver" Version="2.1.0" />
16+
<PackageReference Include="Semver" Version="2.3.0" />
1717
<PackageReference Include="Azure.Security.KeyVault.Secrets" Version="4.3.0" />
1818
<PackageReference Include="Microsoft.Azure.AppConfiguration.Functions.Worker" Version="6.0.0" />
1919
<PackageReference Include="Microsoft.FeatureManagement" Version="2.5.1" />

src/ApiService/ApiService/OneFuzzTypes/Enums.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public enum ErrorCode {
5151
ADO_VALIDATION_INVALID_PATH = 495,
5252
ADO_VALIDATION_INVALID_PROJECT = 496,
5353
INVALID_RETENTION_PERIOD = 497,
54+
INVALID_CLI_VERSION = 498,
5455
// NB: if you update this enum, also update enums.py
5556
}
5657

src/ApiService/ApiService/Program.cs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using Microsoft.FeatureManagement;
1919
using Microsoft.Graph;
2020
using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
21+
using Semver;
2122
namespace Microsoft.OneFuzz.Service;
2223

2324
public class Program {
@@ -58,6 +59,67 @@ public async Async.Task Invoke(FunctionContext context, FunctionExecutionDelegat
5859
}
5960
}
6061

62+
/// <summary>
63+
/// Represents a middleware that can optionally perform strict version checking based on data sent in request headers.
64+
/// </summary>
65+
public class VersionCheckingMiddleware : IFunctionsWorkerMiddleware {
66+
private const string CliVersionHeader = "Cli-Version";
67+
private const string StrictVersionHeader = "Strict-Version";
68+
private readonly SemVersion _oneFuzzServiceVersion;
69+
private readonly IRequestHandling _requestHandling;
70+
71+
/// <summary>
72+
/// Initializes an instance of <see cref="VersionCheckingMiddleware"/> with the provided config and request handling objects.
73+
/// </summary>
74+
/// <param name="config">The service config containing the service version.</param>
75+
/// <param name="requestHandling">The request handling object to create HTTP responses with.</param>
76+
public VersionCheckingMiddleware(IServiceConfig config, IRequestHandling requestHandling) {
77+
_oneFuzzServiceVersion = SemVersion.Parse(config.OneFuzzVersion, SemVersionStyles.Strict);
78+
_requestHandling = requestHandling;
79+
}
80+
81+
public OneFuzzResultVoid CheckCliVersion(Azure.Functions.Worker.Http.HttpHeadersCollection headers) {
82+
var doStrictVersionCheck =
83+
headers.TryGetValues(StrictVersionHeader, out var strictVersion)
84+
&& strictVersion?.FirstOrDefault()?.Equals("true", StringComparison.InvariantCultureIgnoreCase) == true; // "== true" necessary here to avoid implicit null -> bool casting
85+
86+
if (doStrictVersionCheck) {
87+
if (!headers.TryGetValues(CliVersionHeader, out var cliVersion)) {
88+
return Error.Create(ErrorCode.INVALID_REQUEST, $"'{StrictVersionHeader}' is set to true without a corresponding '{CliVersionHeader}' header");
89+
}
90+
if (!SemVersion.TryParse(cliVersion?.FirstOrDefault() ?? "", SemVersionStyles.Strict, out var version)) {
91+
return Error.Create(ErrorCode.INVALID_CLI_VERSION, $"'{CliVersionHeader}' header value is not a valid sematic version");
92+
}
93+
if (version.ComparePrecedenceTo(_oneFuzzServiceVersion) < 0) {
94+
return Error.Create(ErrorCode.INVALID_CLI_VERSION, "cli is out of date");
95+
}
96+
}
97+
98+
return OneFuzzResultVoid.Ok;
99+
}
100+
101+
/// <summary>
102+
/// Checks the request for two headers, cli version and one indicating whether to do strict version checking.
103+
/// When both are present and the cli is out of date, a descriptive response is sent back.
104+
/// </summary>
105+
/// <param name="context">The function context.</param>
106+
/// <param name="next">The function execution delegate.</param>
107+
/// <returns>A <seealso cref="Task"/> </returns>
108+
public async Async.Task Invoke(FunctionContext context, FunctionExecutionDelegate next) {
109+
var requestData = await context.GetHttpRequestDataAsync();
110+
if (requestData is not null) {
111+
var error = CheckCliVersion(requestData.Headers);
112+
if (!error.IsOk) {
113+
var response = await _requestHandling.NotOk(requestData, error.ErrorV, "version middleware");
114+
context.GetInvocationResult().Value = response;
115+
return;
116+
}
117+
}
118+
119+
await next(context);
120+
}
121+
}
122+
61123

62124
//Move out expensive resources into separate class, and add those as Singleton
63125
// ArmClient, Table Client(s), Queue Client(s), HttpClient, etc.
@@ -161,6 +223,7 @@ public static async Async.Task Main() {
161223
builder.UseMiddleware<LoggingMiddleware>();
162224
builder.UseMiddleware<Auth.AuthenticationMiddleware>();
163225
builder.UseMiddleware<Auth.AuthorizationMiddleware>();
226+
builder.UseMiddleware<VersionCheckingMiddleware>();
164227

165228
//this is a must, to tell the host that worker logging is done by us
166229
builder.Services.Configure<WorkerOptions>(workerOptions => workerOptions.Capabilities["WorkerApplicationInsightsLoggingEnabled"] = bool.TrueString);

src/ApiService/ApiService/onefuzzlib/Versions.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22

33
namespace Microsoft.OneFuzz.Service;
44

5-
public class versions {
5+
public class Versions {
66
public static bool IsMinimumVersion(string versionStr, string minimumStr) {
77
var version = SemVersion.Parse(versionStr, SemVersionStyles.Any);
88
var minimum = SemVersion.Parse(minimumStr, SemVersionStyles.Any);
99

10-
return version >= minimum;
10+
return version.ComparePrecedenceTo(minimum) >= 0;
1111
}
1212
}

src/ApiService/ApiService/packages.lock.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -385,9 +385,9 @@
385385
},
386386
"Semver": {
387387
"type": "Direct",
388-
"requested": "[2.1.0, )",
389-
"resolved": "2.1.0",
390-
"contentHash": "1jUT0PwgKO9d9F/X2n762qLp7v/30OpMtJPFRtmjPXUX2/J0lnqiGiSJNNsW3yYTj5StF0Z1yE36TrvtGpcbrg=="
388+
"requested": "[2.3.0, )",
389+
"resolved": "2.3.0",
390+
"contentHash": "4vYo1zqn6pJ1YrhjuhuOSbIIm0CpM47grbpTJ5ABjOlfGt/EhMEM9ed4MRK5Jr6gVnntWDqOUzGeUJp68PZGjw=="
391391
},
392392
"SmartAnalyzers.CSharpExtensions.Annotations": {
393393
"type": "Direct",

src/ApiService/FunctionalTests/Auth.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public async Task<AuthenticationResult> Auth(CancellationToken cancelationToken)
4747
_token = await _app.AcquireTokenForClient(_authConfig.Scopes).ExecuteAsync(cancelationToken);
4848
return _token;
4949
} finally {
50-
_lockObj.Release();
50+
_ = _lockObj.Release();
5151
}
5252
}
5353

src/ApiService/FunctionalTests/FunctionalTests.csproj

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
</PropertyGroup>
1010

1111
<ItemGroup>
12-
<Compile Include="..\ApiService\HttpClient.cs" Link="1f-api\HttpClient.cs" />
12+
<ProjectReference Include="..\ApiService\ApiService.csproj" />
1313
</ItemGroup>
1414

1515
<ItemGroup>
@@ -18,6 +18,7 @@
1818
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
1919
<PrivateAssets>all</PrivateAssets>
2020
</PackageReference>
21+
<PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http" Version="3.0.13" />
2122
<PackageReference Include="Microsoft.Identity.Client" Version="4.52.0" />
2223
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.2" />
2324
<PackageReference Include="System.Net.Http" Version="4.3.4" />
@@ -34,4 +35,4 @@
3435
</PackageReference>
3536
</ItemGroup>
3637

37-
</Project>
38+
</Project>

src/ApiService/FunctionalTests/TestContainer.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,27 +19,27 @@ public TestContainer(ITestOutputHelper output) {
1919
[Fact]
2020
public async Task DownloadNonExistentContainer() {
2121
var r1 = await _downloadApi.Get();
22-
r1.IsOk.Should().BeFalse();
23-
r1.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
22+
_ = r1.IsOk.Should().BeFalse();
23+
_ = r1.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
2424

2525
var r2 = await _downloadApi.Get(container: Guid.NewGuid().ToString());
26-
r2.IsOk.Should().BeFalse();
27-
r2.ErrorV!.Item2!.ShouldBeProvided("filename").Should().BeTrue();
26+
_ = r2.IsOk.Should().BeFalse();
27+
_ = r2.ErrorV!.Item2!.ShouldBeProvided("filename").Should().BeTrue();
2828

2929
var r3 = await _downloadApi.Get(filename: Guid.NewGuid().ToString());
30-
r3.IsOk.Should().BeFalse();
31-
r3.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
30+
_ = r3.IsOk.Should().BeFalse();
31+
_ = r3.ErrorV!.Item2!.ShouldBeProvided("container").Should().BeTrue();
3232

3333
var r4 = await _downloadApi.Get(container: Guid.NewGuid().ToString(), filename: Guid.NewGuid().ToString());
34-
r4.IsOk.Should().BeFalse();
35-
r4.ErrorV!.Item1.Should().Be(System.Net.HttpStatusCode.NotFound);
34+
_ = r4.IsOk.Should().BeFalse();
35+
_ = r4.ErrorV!.Item1.Should().Be(System.Net.HttpStatusCode.NotFound);
3636
}
3737

3838
[Fact]
3939
public async Task CreateGetDeleteContainer() {
4040
var containerName = Guid.NewGuid().ToString();
4141
var container = await _containerApi.Post(containerName);
42-
container.IsOk.Should().BeTrue($"failed to create container due to {container.ErrorV}");
42+
_ = container.IsOk.Should().BeTrue($"failed to create container due to {container.ErrorV}");
4343

4444
var c = await _containerApi.Get(containerName);
4545

src/ApiService/FunctionalTests/TestInfo.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ public TestInfo(ITestOutputHelper output) {
1616
[Fact]
1717
async Task GetInfo() {
1818
var info = await _infoApi.Get();
19-
info.IsOk.Should().BeTrue();
20-
info.OkV!.Versions.ContainsKey("onefuzz").Should().BeTrue();
19+
_ = info.IsOk.Should().BeTrue();
20+
_ = info.OkV!.Versions.ContainsKey("onefuzz").Should().BeTrue();
2121
}
2222
}
2323
}

src/ApiService/FunctionalTests/TestNode.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,14 @@ public TestNode(ITestOutputHelper output) {
2424
async Task GetNonExistentNode() {
2525
var n = await _nodeApi.Get(Guid.NewGuid());
2626

27-
n.IsOk.Should().BeFalse();
28-
n.ErrorV!.UnableToFindNode.Should().BeTrue();
27+
_ = n.IsOk.Should().BeFalse();
28+
_ = n.ErrorV!.UnableToFindNode.Should().BeTrue();
2929
}
3030

3131
[Fact]
3232
async Task GetAllNodes() {
3333
var ns = await _nodeApi.Get();
34-
ns.IsOk.Should().BeTrue("failed to get all nodes due to {0}", ns.ErrorV);
34+
_ = ns.IsOk.Should().BeTrue("failed to get all nodes due to {0}", ns.ErrorV);
3535
foreach (var n in ns.OkV!) {
3636
_output.WriteLine($"node machine id: {n.MachineId}, scaleset id: {n.ScalesetId}, poolName: {n.PoolName}, poolId: {n.PoolId} state: {n.State}, version: {n.Version}");
3737
}
@@ -41,8 +41,8 @@ async Task GetAllNodes() {
4141
[Fact]
4242
async Task DeleteNonExistentNode() {
4343
var n = await _nodeApi.Delete(Guid.NewGuid());
44-
n.IsError.Should().BeTrue();
45-
n.Error!.UnableToFindNode.Should().BeTrue();
44+
_ = n.IsError.Should().BeTrue();
45+
_ = n.Error!.UnableToFindNode.Should().BeTrue();
4646
}
4747

4848
[Fact]
@@ -51,25 +51,25 @@ async Task GetPatchPostDelete() {
5151
var (pool, scaleset) = await Helpers.CreatePoolAndScaleset(_poolApi, _scalesetApi, "linux");
5252

5353
scaleset = await _scalesetApi.WaitWhile(scaleset.ScalesetId, sc => sc.State == "init" || sc.State == "setup");
54-
scaleset.Nodes!.Should().NotBeEmpty();
54+
_ = scaleset.Nodes!.Should().NotBeEmpty();
5555

5656
var nodeState = scaleset.Nodes!.First();
5757
var nodeResult = await _nodeApi.Get(nodeState.MachineId);
5858

59-
nodeResult.IsOk.Should().BeTrue("failed to get node due to {0}", nodeResult.ErrorV);
59+
_ = nodeResult.IsOk.Should().BeTrue("failed to get node due to {0}", nodeResult.ErrorV);
6060
var node = nodeResult.OkV!.First();
6161
node = await _nodeApi.WaitWhile(node.MachineId, n => n.State == "init" || n.State == "setup");
6262

6363
var r = await _nodeApi.Patch(node.MachineId);
64-
r.Result.Should().BeTrue();
64+
_ = r.Result.Should().BeTrue();
6565

6666
var rr = await _nodeApi.Update(node.MachineId, false);
6767

6868
var d = await _nodeApi.Delete(node.MachineId);
69-
d.Result.Should().BeTrue();
69+
_ = d.Result.Should().BeTrue();
7070

7171
var deletePool = await _poolApi.Delete(pool.Name);
72-
deletePool.Result.Should().BeTrue();
72+
_ = deletePool.Result.Should().BeTrue();
7373
}
7474
}
7575
}

0 commit comments

Comments
 (0)