From 19239a8d947717fba5807407db7c56ff3fb57a06 Mon Sep 17 00:00:00 2001 From: Anders Abel Date: Mon, 6 Nov 2023 23:24:41 +0100 Subject: [PATCH] Clean build with .NET8 --- Sustainsys.Saml2.sln | 1 + src/.editorconfig | 3 + .../Saml2Handler.cs | 24 ++++- .../Sustainsys.Saml2.AspNetCore.csproj | 5 + .../Serialization/ISamlXmlReader.cs | 4 +- src/Sustainsys.Saml2/Sustainsys.Saml2.csproj | 5 + src/Sustainsys.Saml2/Xml/SignedXmlHelper.cs | 24 +++-- src/Sustainsys.Saml2/Xml/XmlHelpers.cs | 2 +- src/Sustainsys.Saml2/Xml/XmlTraverser.cs | 3 +- .../Saml2HandlerTests.cs | 5 +- .../Sustainsys.Saml2.AspNetCore.Tests.csproj | 5 + .../Sustainsys.Saml2.Tests.Helpers.csproj | 9 ++ .../TestData.cs | 93 +++++++++---------- .../Sustainsys.Saml2.Tests.csproj | 8 +- .../Xml/SignedXmlHelperTests.cs | 13 +-- .../VerifySignature_NoReference.xml | 1 + src/exclusion.dic | 2 + 17 files changed, 127 insertions(+), 80 deletions(-) create mode 100644 src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests/VerifySignature_NoReference.xml create mode 100644 src/exclusion.dic diff --git a/Sustainsys.Saml2.sln b/Sustainsys.Saml2.sln index fa0ddb980..541e175fd 100644 --- a/Sustainsys.Saml2.sln +++ b/Sustainsys.Saml2.sln @@ -12,6 +12,7 @@ EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{67939779-582A-4972-9393-280BB0A6678B}" ProjectSection(SolutionItems) = preProject CONTRIBUTING.md = CONTRIBUTING.md + src\exclusion.dic = src\exclusion.dic LICENSE = LICENSE README.md = README.md SECURITY.md = SECURITY.md diff --git a/src/.editorconfig b/src/.editorconfig index e16f2f810..8685d03cf 100644 --- a/src/.editorconfig +++ b/src/.editorconfig @@ -217,3 +217,6 @@ dotnet_naming_style.begins_with_i.required_prefix = I dotnet_naming_style.begins_with_i.required_suffix = dotnet_naming_style.begins_with_i.word_separator = dotnet_naming_style.begins_with_i.capitalization = pascal_case + +# Spelling +spelling_exclusion_path = .\exclusion.dic \ No newline at end of file diff --git a/src/Sustainsys.Saml2.AspNetCore/Saml2Handler.cs b/src/Sustainsys.Saml2.AspNetCore/Saml2Handler.cs index bcc134b70..ada94f81c 100644 --- a/src/Sustainsys.Saml2.AspNetCore/Saml2Handler.cs +++ b/src/Sustainsys.Saml2.AspNetCore/Saml2Handler.cs @@ -16,27 +16,37 @@ namespace Sustainsys.Saml2.AspNetCore; /// public class Saml2Handler : RemoteAuthenticationHandler { +#if NET8_0_OR_GREATER /// /// Constructor /// /// Options /// Logger factory /// Url encoder - /// System Clock public Saml2Handler( IOptionsMonitor options, ILoggerFactory logger, UrlEncoder encoder -#if NET8_0_OR_GREATER ) : base(options, logger, encoder) + {} #else - , + + /// + /// Constructor + /// + /// Options + /// Logger factory + /// Url encoder + /// System Clock + public Saml2Handler( + IOptionsMonitor options, + ILoggerFactory logger, + UrlEncoder encoder, ISystemClock clock) : base(options, logger, encoder, clock) + { } #endif - { - } /// /// Create events by invoking Options.ServiceResolver.CreateEventsAsync() @@ -95,7 +105,11 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop var authnRequest = new AuthnRequest() { Issuer = Options.EntityId, +#if NET8_0_OR_GREATER + IssueInstant = TimeProvider.GetUtcNow().DateTime, +#else IssueInstant = Clock.UtcNow.DateTime, +#endif AssertionConsumerServiceUrl = BuildRedirectUri(Options.CallbackPath) }; diff --git a/src/Sustainsys.Saml2.AspNetCore/Sustainsys.Saml2.AspNetCore.csproj b/src/Sustainsys.Saml2.AspNetCore/Sustainsys.Saml2.AspNetCore.csproj index 0747ca5a1..16c2e133c 100644 --- a/src/Sustainsys.Saml2.AspNetCore/Sustainsys.Saml2.AspNetCore.csproj +++ b/src/Sustainsys.Saml2.AspNetCore/Sustainsys.Saml2.AspNetCore.csproj @@ -16,4 +16,9 @@ + + + 6 + + diff --git a/src/Sustainsys.Saml2/Serialization/ISamlXmlReader.cs b/src/Sustainsys.Saml2/Serialization/ISamlXmlReader.cs index ba58365b2..b251ecf1c 100644 --- a/src/Sustainsys.Saml2/Serialization/ISamlXmlReader.cs +++ b/src/Sustainsys.Saml2/Serialization/ISamlXmlReader.cs @@ -42,9 +42,9 @@ public interface ISamlXmlReader SamlResponse ReadSamlResponse(XmlTraverser source); /// - /// Read an AuthnReqeust + /// Read an /// /// Xml Traverser to read from - /// AutnnRequest + /// AuthnRequest ReadAuthnRequest(XmlTraverser source); } diff --git a/src/Sustainsys.Saml2/Sustainsys.Saml2.csproj b/src/Sustainsys.Saml2/Sustainsys.Saml2.csproj index 89e759c9c..49927ea9f 100644 --- a/src/Sustainsys.Saml2/Sustainsys.Saml2.csproj +++ b/src/Sustainsys.Saml2/Sustainsys.Saml2.csproj @@ -18,4 +18,9 @@ + + + 6 + + diff --git a/src/Sustainsys.Saml2/Xml/SignedXmlHelper.cs b/src/Sustainsys.Saml2/Xml/SignedXmlHelper.cs index a4c1a6702..94cf8ca42 100644 --- a/src/Sustainsys.Saml2/Xml/SignedXmlHelper.cs +++ b/src/Sustainsys.Saml2/Xml/SignedXmlHelper.cs @@ -17,7 +17,7 @@ namespace Sustainsys.Saml2.Xml; public static class SignedXmlHelper { /// - /// Adds an envoleped signature to the node. + /// Adds an enveloped signature to the node. /// /// Element to sign /// Certificate to use to sign @@ -31,7 +31,7 @@ public static void Sign( } /// - /// Adds an envoleped signature to the node. + /// Adds an enveloped signature to the node. /// /// Element to sign /// Certificate to use to sign @@ -41,10 +41,7 @@ public static void Sign( X509Certificate2 certificate, XmlNode insertAfter) { - if(insertAfter == null) - { - throw new ArgumentNullException(nameof(insertAfter)); - } + ArgumentNullException.ThrowIfNull(insertAfter); var signedXml = CreateSignedXml(element, certificate); @@ -96,8 +93,10 @@ internal SignedXmlWithStrictIdResolution(XmlDocument xmlDocument) /// Id value to find /// XmlElement /// If not exactly one match - public override XmlElement GetIdElement(XmlDocument document, string idValue) + public override XmlElement GetIdElement(XmlDocument? document, string idValue) { + ArgumentNullException.ThrowIfNull(document); + XmlConvert.VerifyNCName(idValue); var possibleNodes = document.SelectNodes($"//*[@ID=\"{idValue}\" or @Id=\"{idValue}\" or @id=\"{idValue}\"]")!; @@ -138,7 +137,7 @@ public static (string? Error, SigningKey? WorkingKey) VerifySignature( string? error = null; SigningKey? workingKey = null; - if (signedXml.SignedInfo.References.Count != 1) + if (signedXml.SignedInfo!.References.Count != 1) { error += "The Signature should contain exactly one reference. "; } @@ -146,6 +145,11 @@ public static (string? Error, SigningKey? WorkingKey) VerifySignature( { foreach (var key in keys) { + if (key.Certificate == null) + { + throw new InvalidOperationException("Signing key certificate cannot be null"); + } + if (signedXml.CheckSignature(key.Certificate, true)) { workingKey = key; @@ -172,7 +176,7 @@ public static (string? Error, SigningKey? WorkingKey) VerifySignature( } else { - var id = reference.Uri[1..]; // Drop off the # + var id = reference.Uri![1..]; // Drop off the # var signedElement = signedXml.GetIdElement(signatureElement.OwnerDocument, id); @@ -205,7 +209,7 @@ public static (string? Error, SigningKey? WorkingKey) VerifySignature( } // The algorithm names has the form http://foo/bar/xyz#rsa-sha256 - var signingHash = signedXml.SignatureMethod[(signedXml.SignatureMethod.LastIndexOf('-') + 1)..]; + var signingHash = signedXml.SignatureMethod![(signedXml.SignatureMethod!.LastIndexOf('-') + 1)..]; if (!allowedHashAlgorithms.Contains(signingHash)) { var allowed = string.Join(", ", allowedHashAlgorithms); diff --git a/src/Sustainsys.Saml2/Xml/XmlHelpers.cs b/src/Sustainsys.Saml2/Xml/XmlHelpers.cs index 4f4c72826..bc8be80e9 100644 --- a/src/Sustainsys.Saml2/Xml/XmlHelpers.cs +++ b/src/Sustainsys.Saml2/Xml/XmlHelpers.cs @@ -32,7 +32,7 @@ internal static string FormatId(byte[] bytes) /// Get an Xml traverser for an XmlDocument /// /// Source XmlElement. Typically the document element - /// XmlTraverser locatet at DocumentElement + /// XmlTraverser located at DocumentElement public static XmlTraverser GetXmlTraverser(this XmlElement xmlElement) => new(xmlElement ?? throw new ArgumentException("DocumentElement cannot be null")); diff --git a/src/Sustainsys.Saml2/Xml/XmlTraverser.cs b/src/Sustainsys.Saml2/Xml/XmlTraverser.cs index ef4ff1486..a552a0497 100644 --- a/src/Sustainsys.Saml2/Xml/XmlTraverser.cs +++ b/src/Sustainsys.Saml2/Xml/XmlTraverser.cs @@ -250,7 +250,7 @@ public bool EnsureNamespace(string namespaceUri) return true; } - // TODO: Reorder params to follow XmlNode convention with localname, namespaceUri + // TODO: Reorder params to follow XmlNode convention with localName, namespaceUri /// /// Ensure that the current node has a specific localName and namespace. /// @@ -329,7 +329,6 @@ public bool HasName(string namespaceUri, string localName) /// /// Local name of attribute /// Attribute value - /// If no such attribute is found. public string GetRequiredAttribute(string localName) { if (CurrentNode == null) diff --git a/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Saml2HandlerTests.cs b/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Saml2HandlerTests.cs index 9692f8d73..f80c1a11d 100644 --- a/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Saml2HandlerTests.cs +++ b/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Saml2HandlerTests.cs @@ -38,8 +38,7 @@ public class Saml2HandlerTests #if NET8_0_OR_GREATER ); #else - , - systemClock); + ,systemClock); #endif var scheme = new AuthenticationScheme("Saml2", "Saml2", typeof(Saml2Handler)); @@ -192,6 +191,6 @@ public async Task HandleRemoteAuthenticate() result.Should().BeTrue(); } - // TODO: Use event to resolve IdentityProvider - presense of EntityId indicates if challenge or response processing + // TODO: Use event to resolve IdentityProvider - presence of EntityId indicates if challenge or response processing // TODO: Event when Xml was created } \ No newline at end of file diff --git a/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Sustainsys.Saml2.AspNetCore.Tests.csproj b/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Sustainsys.Saml2.AspNetCore.Tests.csproj index 5b018184f..adcbd0039 100644 --- a/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Sustainsys.Saml2.AspNetCore.Tests.csproj +++ b/src/Tests/Sustainsys.Saml2.AspNetCore.Tests/Sustainsys.Saml2.AspNetCore.Tests.csproj @@ -32,4 +32,9 @@ + + + 6 + + diff --git a/src/Tests/Sustainsys.Saml2.Tests.Helpers/Sustainsys.Saml2.Tests.Helpers.csproj b/src/Tests/Sustainsys.Saml2.Tests.Helpers/Sustainsys.Saml2.Tests.Helpers.csproj index cfedd615d..da4c1ce89 100644 --- a/src/Tests/Sustainsys.Saml2.Tests.Helpers/Sustainsys.Saml2.Tests.Helpers.csproj +++ b/src/Tests/Sustainsys.Saml2.Tests.Helpers/Sustainsys.Saml2.Tests.Helpers.csproj @@ -8,6 +8,10 @@ false + + 1701;1702;IDE0039;IDE0290;IDE0300 + + @@ -25,4 +29,9 @@ + + + 6 + + diff --git a/src/Tests/Sustainsys.Saml2.Tests.Helpers/TestData.cs b/src/Tests/Sustainsys.Saml2.Tests.Helpers/TestData.cs index f91d41b49..dc32dc3db 100644 --- a/src/Tests/Sustainsys.Saml2.Tests.Helpers/TestData.cs +++ b/src/Tests/Sustainsys.Saml2.Tests.Helpers/TestData.cs @@ -3,54 +3,53 @@ using System.Security.Cryptography.X509Certificates; using System.Xml; -namespace Sustainsys.Saml2.Tests.Helpers +namespace Sustainsys.Saml2.Tests.Helpers; + +public static class TestData { - public static class TestData + public static XmlTraverser GetXmlTraverser([CallerMemberName] string? testName = null) { - public static XmlTraverser GetXmlTraverser([CallerMemberName] string? testName = null) - { - var document = GetXmlDocument(testName); - - return new XmlTraverser(document.DocumentElement ?? throw new InvalidOperationException("XmlDoc contained no DocumentElement")); - } - - public static XmlDocument GetXmlDocument([CallerMemberName] string? testName = null) - { - ArgumentNullException.ThrowIfNull(testName); - - var assemblyName = typeof(TDirectory).Assembly.GetName().Name!; - - var fileName = "../../.." - + typeof(TDirectory).FullName![assemblyName.Length..].Replace('.', '/') - + "/" + testName + ".xml"; - - var document = new XmlDocument(); - document.Load(fileName); - return document; - } - - public static X509Certificate2 Certificate { get; } = new X509Certificate2("Sustainsys.Saml2.Tests.pfx"); - - public static SigningKey SigningKey { get; } = new() - { - Certificate = Certificate, - TrustLevel = TrustLevel.ConfiguredKey - }; - - public static SigningKey[] SingleSigningKey { get; } = - { - SigningKey - }; - - public static SigningKey SigningKey2 { get; } = new() - { - Certificate = new X509Certificate2("Sustainsys.Saml2.Tests2.pfx"), - TrustLevel = TrustLevel.TLS - }; - - public static SigningKey[] SingleSigningKey2 { get; } = - { - SigningKey2 - }; + var document = GetXmlDocument(testName); + + return new XmlTraverser(document.DocumentElement ?? throw new InvalidOperationException("XmlDoc contained no DocumentElement")); } + + public static XmlDocument GetXmlDocument([CallerMemberName] string? testName = null) + { + ArgumentNullException.ThrowIfNull(testName); + + var assemblyName = typeof(TDirectory).Assembly.GetName().Name!; + + var fileName = "../../.." + + typeof(TDirectory).FullName![assemblyName.Length..].Replace('.', '/') + + "/" + testName + ".xml"; + + var document = new XmlDocument(); + document.Load(fileName); + return document; + } + + public static X509Certificate2 Certificate { get; } = new X509Certificate2("Sustainsys.Saml2.Tests.pfx"); + + public static SigningKey SigningKey { get; } = new() + { + Certificate = Certificate, + TrustLevel = TrustLevel.ConfiguredKey + }; + + public static SigningKey[] SingleSigningKey { get; } = + { + SigningKey + }; + + public static SigningKey SigningKey2 { get; } = new() + { + Certificate = new X509Certificate2("Sustainsys.Saml2.Tests2.pfx"), + TrustLevel = TrustLevel.TLS + }; + + public static SigningKey[] SingleSigningKey2 { get; } = new[] + { + SigningKey2 + }; } \ No newline at end of file diff --git a/src/Tests/Sustainsys.Saml2.Tests/Sustainsys.Saml2.Tests.csproj b/src/Tests/Sustainsys.Saml2.Tests/Sustainsys.Saml2.Tests.csproj index df63c0987..4e6537a17 100644 --- a/src/Tests/Sustainsys.Saml2.Tests/Sustainsys.Saml2.Tests.csproj +++ b/src/Tests/Sustainsys.Saml2.Tests/Sustainsys.Saml2.Tests.csproj @@ -10,7 +10,7 @@ - 1701;1702;IDE0039 + 1701;1702;IDE0039;IDE0290;IDE0300 @@ -37,9 +37,9 @@ - - PreserveNewest - + + PreserveNewest + diff --git a/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests.cs b/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests.cs index 4882b1684..81f328478 100644 --- a/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests.cs +++ b/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests.cs @@ -163,7 +163,7 @@ public void VerifySignature_SignatureWrapping() [Fact] public void VerifySignature_NoReference() { - var signedWithoutReference = @"https://idp.example.comtYFIoYmrzmp3H7TXm9IS8DW3buBZIb6sI2ycrn+AOnVcdYnPTJpk3ntHlqQKXNEyXgXZNdqEuFpgI1I0P0TlhM+C3rBJnflkApkxZkak5RwnJzDWTHpsSDjYcm+/XgBy3JVZJuMWb2YPaV8GB6cjBMDrENUEaoKRg+FpzPUZO1EOMcqbocXp5cHie1CkPnD1OtT/cuzMBUMpBGZMxjZwdFpOO7R3CUXh/McxKfoGUQGC3DVpt5T8uGkpj4KqZVPS/qTCRhbPRDjg73BdWbdkFpFWge8G/FgkYxr9LBE1TsrxptppO9xoA5jXwJVZaWndSMvo6TuOjUgqY2w5RTkqhA=="; + var signedWithoutReference = File.ReadAllText("../../../Xml/SignedXmlHelperTests/VerifySignature_NoReference.xml"); var xmlDoc = new XmlDocument(); xmlDoc.LoadXml(signedWithoutReference); @@ -330,7 +330,7 @@ public void VerifySignature_SigningAlgorithmStrength() reference.AddTransform(new XmlDsigExcC14NTransform()); signedXml.AddReference(reference); - signedXml.SignedInfo.SignatureMethod = SignedXml.XmlDsigRSASHA1Url; + signedXml.SignedInfo!.SignatureMethod = SignedXml.XmlDsigRSASHA1Url; signedXml.ComputeSignature(); xd.DocumentElement!.AppendChild(signedXml.GetXml()); @@ -398,7 +398,7 @@ public void IsSignedByAny_NoKeyMatches_ButEmbeddedKeyValidatesContent() } [Fact] - public void VerifySignaure_RejectsDuplicateIdsEvenIfCaseDiffer() + public void VerifySignature_RejectsDuplicateIdsEvenIfCaseDiffer() { // Ensure that a document where referenced id exists in multiple // locations but ID is spelled Id or id is rejected. Allowing this is a @@ -465,12 +465,13 @@ public void VerifySignature_RejectXmlLowerCaseIDs() private class SignRootSignedXml : SignedXml { - public SignRootSignedXml(XmlDocument xd) : base(xd) { } + public SignRootSignedXml(XmlDocument xd) : base(xd) + { } // This allows us to create a signature with invalid reference for the // VerifySignature_RequiresReferenceNCName test. - public override XmlElement GetIdElement(XmlDocument document, string idValue) - => document.DocumentElement!; + public override XmlElement GetIdElement(XmlDocument? document, string idValue) + => document!.DocumentElement!; } [Fact] diff --git a/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests/VerifySignature_NoReference.xml b/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests/VerifySignature_NoReference.xml new file mode 100644 index 000000000..ef1e2fd73 --- /dev/null +++ b/src/Tests/Sustainsys.Saml2.Tests/Xml/SignedXmlHelperTests/VerifySignature_NoReference.xml @@ -0,0 +1 @@ +https://idp.example.comtYFIoYmrzmp3H7TXm9IS8DW3buBZIb6sI2ycrn+AOnVcdYnPTJpk3ntHlqQKXNEyXgXZNdqEuFpgI1I0P0TlhM+C3rBJnflkApkxZkak5RwnJzDWTHpsSDjYcm+/XgBy3JVZJuMWb2YPaV8GB6cjBMDrENUEaoKRg+FpzPUZO1EOMcqbocXp5cHie1CkPnD1OtT/cuzMBUMpBGZMxjZwdFpOO7R3CUXh/McxKfoGUQGC3DVpt5T8uGkpj4KqZVPS/qTCRhbPRDjg73BdWbdkFpFWge8G/FgkYxr9LBE1TsrxptppO9xoA5jXwJVZaWndSMvo6TuOjUgqY2w5RTkqhA== \ No newline at end of file diff --git a/src/exclusion.dic b/src/exclusion.dic new file mode 100644 index 000000000..e746dd66f --- /dev/null +++ b/src/exclusion.dic @@ -0,0 +1,2 @@ +Saml +Sustainsys