Skip to content

Commit

Permalink
Fix issues exposed by dotnet 9 sdk (#920)
Browse files Browse the repository at this point in the history
* Add direct reference to new versions of JWTCookieAuthentication's insecure depencencies
* Activate security scan of transitive depencecies on net8
* Warn on IDE0052: Remove unread private member
* Warn on CA1859: Use concrete types when possible for improved performance (but not in test)
* Fix related code
  • Loading branch information
ivarne authored Nov 22, 2024
1 parent f492381 commit 2322191
Show file tree
Hide file tree
Showing 10 changed files with 21 additions and 33 deletions.
6 changes: 6 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ dotnet_diagnostic.IDE0005.severity = warning

dotnet_diagnostic.CA1825.severity = warning

# IDE0052: Remove unread private member
dotnet_diagnostic.IDE0052.severity = warning

# CA1848: Use the LoggerMessage delegates
dotnet_diagnostic.CA1848.severity = none

Expand All @@ -131,6 +134,9 @@ dotnet_diagnostic.CA1822.severity = warning
# IDE0080: Remove unnecessary suppression operator
dotnet_diagnostic.IDE0080.severity = error

# CA1859: Use concrete types when possible for improved performance
dotnet_diagnostic.CA1859.severity = warning

# CA1716: Rename namespace "" so that it no longer conflicts with the reserved language keyword 'Interface'
# TODO: fixing this would be breaking
dotnet_diagnostic.CA1716.severity = suggestion
Expand Down
3 changes: 2 additions & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisMode>Minimum</AnalysisMode>
<Features>strict</Features>
<!-- <CodeAnalysisTreatWarningsAsErrors>false</CodeAnalysisTreatWarningsAsErrors> -->
<NuGetAudit>true</NuGetAudit>
<NuGetAuditMode>all</NuGetAuditMode>
</PropertyGroup>

<ItemGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/Altinn.App.Api/Controllers/ActionsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private async Task<Dictionary<
string,
Dictionary<string, List<ValidationIssueWithSource>>
>?> GetIncrementalValidations(
IInstanceDataAccessor dataAccessor,
InstanceDataUnitOfWork dataAccessor,
DataElementChanges changes,
List<string>? ignoredValidators,
string? language
Expand Down
4 changes: 0 additions & 4 deletions src/Altinn.App.Api/Controllers/PdfController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ public class PdfController : ControllerBase
private readonly IAppResources _resources;
private readonly IAppModel _appModel;
private readonly IDataClient _dataClient;
private readonly IWebHostEnvironment _env;
private readonly IPdfService _pdfService;

/// <summary>
Expand All @@ -41,7 +40,6 @@ public class PdfController : ControllerBase
/// <param name="resources">The app resource service</param>
/// <param name="appModel">The app model service</param>
/// <param name="dataClient">The data client</param>
/// <param name="env">The environment</param>
/// <param name="pdfService">The PDF service</param>
public PdfController(
IInstanceClient instanceClient,
Expand All @@ -50,7 +48,6 @@ public PdfController(
IAppResources resources,
IAppModel appModel,
IDataClient dataClient,
IWebHostEnvironment env,
IPdfService pdfService
)
{
Expand All @@ -59,7 +56,6 @@ IPdfService pdfService
_resources = resources;
_appModel = appModel;
_dataClient = dataClient;
_env = env;
_pdfService = pdfService;
}

Expand Down
4 changes: 1 addition & 3 deletions src/Altinn.App.Api/Controllers/ProfileController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@ namespace Altinn.App.Api.Controllers;
public class ProfileController : Controller
{
private readonly IProfileClient _profileClient;
private readonly ILogger _logger;

/// <summary>
/// Initializes a new instance of the <see cref="ProfileController"/> class
/// </summary>
public ProfileController(IProfileClient profileClient, ILogger<ProfileController> logger)
public ProfileController(IProfileClient profileClient)
{
_profileClient = profileClient;
_logger = logger;
}

/// <summary>
Expand Down
7 changes: 7 additions & 0 deletions src/Altinn.App.Core/Altinn.App.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
<PackageReference Include="Altinn.Platform.Storage.Interface" Version="4.0.3" />
<PackageReference Include="JsonPatch.Net" Version="3.1.1" />
<PackageReference Include="JWTCookieAuthentication" Version="3.0.1" />
<!-- The follwoing are depencencies for JWTCookieAuthentication, but we need newer versions-->
<PackageReference Include="Microsoft.IdentityModel.Protocols.OpenIdConnect" Version="8.2.1"
/>
<PackageReference Include="Microsoft.Rest.ClientRuntime" Version="2.3.24" />
<PackageReference Include="Microsoft.Rest.ClientRuntime.Azure" Version="3.3.19" />
<PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
<!-- End JWTCookieAuthentication deps -->
<PackageReference Include="Microsoft.ApplicationInsights.AspNetCore" Version="2.22.0" />
<PackageReference Include="Microsoft.Extensions.Caching.Hybrid" Version="[9.0.0-preview.9.24556.5]" NoWarn="NU5104" />
<PackageReference Include="Microsoft.FeatureManagement.AspNetCore" Version="3.5.0" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ public readonly record struct ValidationResult
/// <summary>
/// Shorthand: Is the object in a valid state?
/// </summary>
public bool IsValid() => InvalidProperties.IsNullOrEmpty();
public bool IsValid() => InvalidProperties is null || !InvalidProperties.Any();

/// <summary>
/// Helpful summary of the result
Expand Down
11 changes: 1 addition & 10 deletions src/Altinn.App.Core/Internal/Language/ApplicationLanguage.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System.Text.Json;
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Features;
using Altinn.App.Core.Implementation;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace Altinn.App.Core.Internal.Language;
Expand All @@ -18,23 +16,16 @@ public class ApplicationLanguage : IApplicationLanguage
};

private readonly AppSettings _settings;
private readonly ILogger _logger;
private readonly Telemetry? _telemetry;

/// <summary>
/// Initializes a new instance of the <see cref="ApplicationLanguage"/> class.
/// </summary>
/// <param name="settings">The app repository settings.</param>
/// <param name="logger">A logger from the built in logger factory.</param>
/// <param name="telemetry">Telemetry for traces and metrics.</param>
public ApplicationLanguage(
IOptions<AppSettings> settings,
ILogger<AppResourcesSI> logger,
Telemetry? telemetry = null
)
public ApplicationLanguage(IOptions<AppSettings> settings, Telemetry? telemetry = null)
{
_settings = settings.Value;
_logger = logger;
_telemetry = telemetry;
}

Expand Down
12 changes: 0 additions & 12 deletions test/Altinn.App.Api.Tests/Controllers/PdfControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ public PdfControllerTests()
[Fact]
public async Task Request_In_Dev_Should_Generate()
{
var env = new Mock<IWebHostEnvironment>();
env.Setup(a => a.EnvironmentName).Returns("Development");

IOptions<GeneralSettings> generalSettingsOptions = Options.Create<GeneralSettings>(
new() { HostName = "local.altinn.cloud" }
);
Expand Down Expand Up @@ -106,7 +103,6 @@ public async Task Request_In_Dev_Should_Generate()
_appResources.Object,
_appModel.Object,
_dataClient.Object,
env.Object,
pdfService
);

Expand Down Expand Up @@ -146,9 +142,6 @@ public async Task Request_In_Dev_Should_Generate()
[Fact]
public async Task Request_In_Dev_Should_Include_Frontend_Version()
{
var env = new Mock<IWebHostEnvironment>();
env.Setup(a => a.EnvironmentName).Returns("Development");

IOptions<GeneralSettings> generalSettingsOptions = Options.Create<GeneralSettings>(
new() { HostName = "local.altinn.cloud" }
);
Expand Down Expand Up @@ -186,7 +179,6 @@ public async Task Request_In_Dev_Should_Include_Frontend_Version()
_appResources.Object,
_appModel.Object,
_dataClient.Object,
env.Object,
pdfService
);

Expand Down Expand Up @@ -228,9 +220,6 @@ public async Task Request_In_Dev_Should_Include_Frontend_Version()
[Fact]
public async Task Request_In_TT02_Should_Ignore_Frontend_Version()
{
var env = new Mock<IWebHostEnvironment>();
env.Setup(a => a.EnvironmentName).Returns("Staging");

IOptions<GeneralSettings> generalSettingsOptions = Options.Create<GeneralSettings>(
new() { HostName = "org.apps.tt02.altinn.no" }
);
Expand Down Expand Up @@ -268,7 +257,6 @@ public async Task Request_In_TT02_Should_Ignore_Frontend_Version()
_appResources.Object,
_appModel.Object,
_dataClient.Object,
env.Object,
pdfService
);

Expand Down
3 changes: 2 additions & 1 deletion test/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

<PropertyGroup>
<AnalysisMode>Default</AnalysisMode>
<NoWarn>$(NoWarn);CS1591;CS0618;CS7022;CA1707;CA1822;CA1727;CA2201</NoWarn>
<NoWarn>$(NoWarn);CS1591;CS0618;CS7022;CA1707;CA1822;CA1727;CA2201;CA1859</NoWarn>
<!--
CS1591: Don't warn for missing XML doc
CS0618: This is a test project, so we usually continue testing [Obsolete] apis
CS7022: The entry point of the program is global code; ignoring 'Program.Main(string[])' entry point
CA1822: Mark members as static
CA1727: Use PascalCase for named placeholders in the logging message template
CA2201: Do not raise reserved exception types
CA1859: Use concrete types when possible for improved performance
-->
</PropertyGroup>

Expand Down

0 comments on commit 2322191

Please sign in to comment.