Skip to content

Commit

Permalink
Added new compatibility option IgnoreMissingInResponseTo.
Browse files Browse the repository at this point in the history
- This will not throw an exception in the Saml response if the InResponseTo
   attribute is missing.
- Closes #747
- Closes #588
  • Loading branch information
TalalTayyab authored and AndersAbel committed Mar 29, 2018
1 parent 782ac71 commit 445b2ab
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 14 deletions.
9 changes: 9 additions & 0 deletions Sustainsys.Saml2/Configuration/Compatibility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public Compatibility(CompatibilityElement configElement)
UnpackEntitiesDescriptorInIdentityProviderMetadata =
configElement.UnpackEntitiesDescriptorInIdentityProviderMetadata;
DisableLogoutStateCookie = configElement.DisableLogoutStateCookie;
IgnoreMissingInResponseTo = configElement.IgnoreMissingInResponseTo;
}

/// <summary>
Expand Down Expand Up @@ -62,5 +63,13 @@ public Compatibility(CompatibilityElement configElement)
/// e.g. when value cannot parse as absolute URI
/// </summary>
public bool IgnoreAuthenticationContextInResponse { get; set; }

/// <summary>
/// Ignore the check for the missing InResponseTo attribute in the Saml response.
/// This is different to setting the allowUnsolicitedAuthnResponse as it will only
/// ignore the InResponseTo attribute if there is no relayState. Setting
/// IgnoreMissingInResponseTo to true will always skip the check.
/// </summary>
public bool IgnoreMissingInResponseTo { get; set; }
}
}
32 changes: 26 additions & 6 deletions Sustainsys.Saml2/Configuration/CompatibilityElement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace Sustainsys.Saml2.Configuration
{
/// <summary>
/// Compatibility settings. Can be used to make Saml2 accept
/// certain non-standard behaviour.
/// certain non-standard behaviour.
/// </summary>
public class CompatibilityElement : ConfigurationElement
{
Expand All @@ -32,7 +32,7 @@ public override bool IsReadOnly()
/// for an IdentityProvider, automatically check inside it if there
/// is a single EntityDescriptor and in that case use it.
/// </summary>
[ConfigurationProperty(nameof(unpackEntitiesDescriptorInIdentityProviderMetadata), IsRequired = false)]
[ConfigurationProperty(unpackEntitiesDescriptorInIdentityProviderMetadata, IsRequired = false)]
public bool UnpackEntitiesDescriptorInIdentityProviderMetadata
{
get
Expand All @@ -45,15 +45,14 @@ public bool UnpackEntitiesDescriptorInIdentityProviderMetadata
}
}

const string disableLogoutStateCookie
= nameof(disableLogoutStateCookie);
const string disableLogoutStateCookie = nameof(disableLogoutStateCookie);

/// <summary>
/// Do not send logout state cookie, e.g. if you are not using ReturnUrl
/// or if you know the cookie will be lost due to cross-domain redirects
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage( "Microsoft.Naming", "CA1726:UsePreferredTerms", MessageId = "Logout" )]
[ConfigurationProperty(nameof(disableLogoutStateCookie), IsRequired = false)]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1726:UsePreferredTerms", MessageId = "Logout")]
[ConfigurationProperty(disableLogoutStateCookie, IsRequired = false)]
public bool DisableLogoutStateCookie
{
get
Expand All @@ -65,5 +64,26 @@ public bool DisableLogoutStateCookie
base[disableLogoutStateCookie] = value;
}
}

const string ignoreMissingInResponseTo = nameof(ignoreMissingInResponseTo);

/// <summary>
/// Ignore the check for the missing InResponseTo attribute in the Saml response.
/// This is different to setting the allowUnsolicitedAuthnResponse as it will only
/// ignore the InResponseTo attribute if there is no relayState. Setting
/// IgnoreMissingInResponseTo to true will always skip the check.
/// </summary>
[ConfigurationProperty(ignoreMissingInResponseTo, IsRequired = false)]
public bool IgnoreMissingInResponseTo
{
get
{
return (bool)base[ignoreMissingInResponseTo];
}
set
{
base[ignoreMissingInResponseTo] = value;
}
}
}
}
45 changes: 39 additions & 6 deletions Sustainsys.Saml2/SAML2P/Saml2Response.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class Saml2Response : ISaml2Message
/// <exception cref="XmlException">On xml errors or unexpected xml structure.</exception>
public static Saml2Response Read(string xml)
{
return Read(xml, null);
return Read(xml, null, null);
}

/// <summary>
Expand All @@ -46,10 +46,24 @@ public static Saml2Response Read(string xml)
/// <returns>Saml2Response</returns>
/// <exception cref="XmlException">On xml errors or unexpected xml structure.</exception>
public static Saml2Response Read(string xml, Saml2Id expectedInResponseTo)
{
return Read(xml, expectedInResponseTo, null);
}

/// <summary>
/// Read the supplied Xml and parse it into a response.
/// </summary>
/// <param name="xml">xml data.</param>
/// <param name="expectedInResponseTo">The expected value of the
/// InReplyTo parameter in the message.</param>
/// <param name="options">Service provider settings used when validating Saml response</param>
/// <returns>Saml2Response</returns>
/// <exception cref="XmlException">On xml errors or unexpected xml structure.</exception>
public static Saml2Response Read(string xml, Saml2Id expectedInResponseTo, IOptions options)
{
var x = XmlHelpers.XmlDocumentFromString(xml);

return new Saml2Response(x.DocumentElement, expectedInResponseTo);
return new Saml2Response(x.DocumentElement, expectedInResponseTo, options);
}

/// <summary>
Expand All @@ -58,7 +72,18 @@ public static Saml2Response Read(string xml, Saml2Id expectedInResponseTo)
/// <param name="xml">Root xml element.</param>
/// <param name="expectedInResponseTo">The expected value of the
/// InReplyTo parameter in the message.</param>
public Saml2Response(XmlElement xml, Saml2Id expectedInResponseTo)
public Saml2Response(XmlElement xml, Saml2Id expectedInResponseTo): this(xml, expectedInResponseTo, null)
{
}

/// <summary>
/// Ctor
/// </summary>
/// <param name="xml">Root xml element.</param>
/// <param name="expectedInResponseTo">The expected value of the
/// InReplyTo parameter in the message.</param>
/// <param name="options">Service provider settings used when validating Saml response</param>
public Saml2Response(XmlElement xml, Saml2Id expectedInResponseTo, IOptions options)
{
if (xml == null)
{
Expand All @@ -80,7 +105,7 @@ public Saml2Response(XmlElement xml, Saml2Id expectedInResponseTo)

id = new Saml2Id(xml.Attributes["ID"].Value);

ReadAndValidateInResponseTo(xml, expectedInResponseTo);
ReadAndValidateInResponseTo(xml, expectedInResponseTo, options);

issueInstant = DateTime.Parse(xml.Attributes["IssueInstant"].Value,
CultureInfo.InvariantCulture, DateTimeStyles.AdjustToUniversal);
Expand Down Expand Up @@ -112,9 +137,10 @@ public Saml2Response(XmlElement xml, Saml2Id expectedInResponseTo)
}
}

[SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "IgnoreMissingInResponseTo")]
[SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "InResponseTo")]
[SuppressMessage("Microsoft.Naming", "CA2204:Literals should be spelled correctly", MessageId = "RelayState")]
private void ReadAndValidateInResponseTo(XmlElement xml, Saml2Id expectedInResponseTo)
private void ReadAndValidateInResponseTo(XmlElement xml, Saml2Id expectedInResponseTo, IOptions options)
{
var parsedInResponseTo = xml.Attributes["InResponseTo"].GetValueIfNotNull();
if (parsedInResponseTo != null)
Expand All @@ -139,11 +165,18 @@ private void ReadAndValidateInResponseTo(XmlElement xml, Saml2Id expectedInRespo
}
else
{
if (options?.SPOptions.Compatibility.IgnoreMissingInResponseTo ?? false)
{
return;
};

if (expectedInResponseTo != null)
{
throw new Saml2ResponseFailedValidationException(
string.Format(CultureInfo.InvariantCulture,
"Expected message to contain InResponseTo \"{0}\", but found none.",
"Expected message to contain InResponseTo \"{0}\", but found none. If this error occurs " +
"due to the Idp not setting InResponseTo according to the SAML2 specification, this check " +
"can be disabled by setting the IgnoreMissingInResponseTo compatibility flag to true.",
expectedInResponseTo));
}
}
Expand Down
2 changes: 1 addition & 1 deletion Sustainsys.Saml2/WebSSO/AcsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public CommandResult Run(HttpRequestData request, IOptions options)
unbindResult = binding.Unbind(request, options);
options.Notifications.MessageUnbound(unbindResult);

var samlResponse = new Saml2Response(unbindResult.Data, request.StoredRequestState?.MessageId);
var samlResponse = new Saml2Response(unbindResult.Data, request.StoredRequestState?.MessageId, options);

var result = ProcessResponse(options, samlResponse, request.StoredRequestState);
if(unbindResult.RelayState != null)
Expand Down
3 changes: 3 additions & 0 deletions Tests/Tests/Configuration/SPOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public void TestCleanup()
SustainsysSaml2Section.Current.Metadata.AllowChange = false;
SustainsysSaml2Section.Current.Compatibility.UnpackEntitiesDescriptorInIdentityProviderMetadata = false;
SustainsysSaml2Section.Current.Compatibility.DisableLogoutStateCookie = false;
SustainsysSaml2Section.Current.Compatibility.IgnoreMissingInResponseTo = false;
SustainsysSaml2Section.Current.Compatibility.AllowChange = false;
}
}
Expand Down Expand Up @@ -65,6 +66,7 @@ public void SPOptions_Constructor_LoadsConfig()
config.Compatibility.AllowChange = true;
config.Compatibility.UnpackEntitiesDescriptorInIdentityProviderMetadata = true;
config.Compatibility.DisableLogoutStateCookie = true;
config.Compatibility.IgnoreMissingInResponseTo = true;

SPOptions subject = new SPOptions(SustainsysSaml2Section.Current);
subject.ReturnUrl.Should().Be(config.ReturnUrl);
Expand All @@ -85,6 +87,7 @@ public void SPOptions_Constructor_LoadsConfig()
subject.RequestedAuthnContext.Comparison.Should().Be(AuthnContextComparisonType.Minimum);
subject.Compatibility.UnpackEntitiesDescriptorInIdentityProviderMetadata.Should().BeTrue();
subject.Compatibility.DisableLogoutStateCookie.Should().BeTrue();
subject.Compatibility.IgnoreMissingInResponseTo.Should().BeTrue();
}

[TestMethod]
Expand Down
60 changes: 59 additions & 1 deletion Tests/Tests/Saml2P/Saml2ResponseTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1542,7 +1542,56 @@ public void Saml2Response_Read_ThrowsOnNoInResponseTo_When_OneWasExpected()
Action a = () => Saml2Response.Read(responseXML, new Saml2Id("ExpectedId"));

a.ShouldThrow<Saml2ResponseFailedValidationException>()
.WithMessage("Expected message to contain InResponseTo \"ExpectedId\", but found none.");
.WithMessage("Expected message to contain InResponseTo \"ExpectedId\", but found none*");
}

[TestMethod]
public void Saml2Response_Read_CorrectResponse_When_MissingInResponseTo_And_IgnoreMissingEnabled()
{
var options = Options.FromConfiguration;
options.SPOptions.Compatibility.IgnoreMissingInResponseTo = true;

var responseXML =
@"<?xml version=""1.0"" encoding=""UTF-8""?>
<saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
xmlns:saml2=""urn:oasis:names:tc:SAML:2.0:assertion""
ID = """ + MethodBase.GetCurrentMethod().Name + @""" Version=""2.0"" IssueInstant=""2013-01-01T00:00:00Z"">
<saml2:Issuer>https://idp.example.com</saml2:Issuer>
<saml2p:Status>
<saml2p:StatusCode Value=""urn:oasis:names:tc:SAML:2.0:status:Requester"" />
</saml2p:Status>
</saml2p:Response>";

responseXML = SignedXmlHelper.SignXml(responseXML);

Action a = () => Saml2Response.Read(responseXML, new Saml2Id("ExpectedId"), options);

a.ShouldNotThrow();
}

[TestMethod]
public void Saml2Response_Read_ThrowsOnNoInResponseTo_When_MissingInResponseTo_AndIgnoreMissingDisabled()
{
var options = Options.FromConfiguration;
options.SPOptions.Compatibility.IgnoreMissingInResponseTo = false;

var responseXML =
@"<?xml version=""1.0"" encoding=""UTF-8""?>
<saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
xmlns:saml2=""urn:oasis:names:tc:SAML:2.0:assertion""
ID = """ + MethodBase.GetCurrentMethod().Name + @""" Version=""2.0"" IssueInstant=""2013-01-01T00:00:00Z"">
<saml2:Issuer>https://idp.example.com</saml2:Issuer>
<saml2p:Status>
<saml2p:StatusCode Value=""urn:oasis:names:tc:SAML:2.0:status:Requester"" />
</saml2p:Status>
</saml2p:Response>";

responseXML = SignedXmlHelper.SignXml(responseXML);

Action a = () => Saml2Response.Read(responseXML, new Saml2Id("ExpectedId"), options);

a.ShouldThrow<Saml2ResponseFailedValidationException>()
.WithMessage("Expected message to contain InResponseTo \"ExpectedId\", but found none*");
}

[TestMethod]
Expand Down Expand Up @@ -1882,6 +1931,15 @@ public void Saml2Response_Ctor_Nullcheck()
.And.ParamName.Should().Be("xml");
}

[TestMethod]
public void Saml2Response_Ctor_Options_Nullcheck()
{
Action a = () => new Saml2Response(null, new Saml2Id("foo"), null);

a.ShouldThrow<ArgumentNullException>()
.And.ParamName.Should().Be("xml");
}

[TestMethod]
public void Saml2Response_Xml_FromData_ContainsBasicData()
{
Expand Down

0 comments on commit 445b2ab

Please sign in to comment.