Skip to content

Commit

Permalink
Add redirect URL validation
Browse files Browse the repository at this point in the history 
- Use same solution as for SignInCommand's query param validation
  to prevent an open redirect.
- Allow relative redirect URLs
  • Loading branch information
@AndersAbel
AndersAbel committed Feb 18, 2019
1 parent eb05d22 commit 20aa29e
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 60 deletions.
9 changes: 7 additions & 2 deletions Sustainsys.Saml2/WebSSO/AcsCommand.cs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Sustainsys.Saml2.Configuration;
using Sustainsys.Saml2.Exceptions;
using Sustainsys.Saml2.Internal;
using Sustainsys.Saml2.Metadata;
using Sustainsys.Saml2.Saml2P;
using System;
Expand Down Expand Up @@ -117,10 +118,14 @@ private static Uri GetLocation(StoredRequestState storedRequestState, IdentityPr

if (identityProvider.RelayStateUsedAsReturnUrl)
{
if (Uri.IsWellFormedUriString(relayState, UriKind.Absolute))
if (!PathHelper.IsLocalWebUrl(relayState))
{
return new Uri(relayState);
if (!options.Notifications.ValidateAbsoluteReturnUrl(relayState))
{
throw new InvalidOperationException("Return Url must be a relative Url.");
}
}
return new Uri(relayState, UriKind.RelativeOrAbsolute);
}
}

Expand Down
117 changes: 59 additions & 58 deletions Tests/Tests.Shared/WebSSO/AcsCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Sustainsys.Saml2.Tests.WebSSO;
using Sustainsys.Saml2.TestHelpers;
using Microsoft.IdentityModel.Tokens.Saml2;
using System.Runtime.CompilerServices;

namespace Sustainsys.Saml2.Tests.WebSso
{
Expand Down Expand Up @@ -91,10 +92,10 @@ public void AcsCommand_Run_ErrorOnIncorrectXml()

Action a = () => new AcsCommand().Run(r, Options.FromConfiguration);

a.Should().Throw<BadFormatSamlResponseException>()
.WithMessage("The SAML response contains incorrect XML")
.Where(ex => ex.Data["Saml2Response"] as string == "<foo />")
.WithInnerException<XmlException>();
a.Should().Throw<BadFormatSamlResponseException>()
.WithMessage("The SAML response contains incorrect XML")
.Where(ex => ex.Data["Saml2Response"] as string == "<foo />")
.WithInnerException<XmlException>();
}

[TestMethod]
Expand Down Expand Up @@ -274,7 +275,7 @@ public void AcsCommand_Run_WithReturnUrl_SuccessfulResult()
null)
);

var ids = new ClaimsIdentity[] { new ClaimsIdentity("Federation")};
var ids = new ClaimsIdentity[] { new ClaimsIdentity("Federation") };
ids[0].AddClaim(new Claim(ClaimTypes.NameIdentifier, "SomeUser", null, "https://idp.example.com"));

var expected = new CommandResult()
Expand Down Expand Up @@ -346,8 +347,8 @@ public void AcsCommand_Run_WithReturnUrl_SuccessfulResult_NoConfigReturnUrl()
[TestMethod]
public void AcsCommand_Run_UnsolicitedResponse_ThrowsOnNoConfiguredReturnUrl()
{
var response =
@"<saml2p:Response xmlns:saml2p=""urn:oasis:names:tc:SAML:2.0:protocol""
var response =
@"<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>
Expand Down Expand Up @@ -695,21 +696,25 @@ public void AcsCommand_Run_UsesIdpFromNotification()
actual.Principal.Claims.First().Issuer.Should().Be("https://other.idp.example.com");
}

[TestMethod]
public void AcsCommand_Run_WithRelayStateUsedAsReturnUrl_Success()
private void RelayStateAsReturnUrl(string relayState, IOptions options, [CallerMemberName] string caller = null)
{
if(string.IsNullOrEmpty(caller))
{
throw new ArgumentNullException(nameof(caller));
}

var response =
@"<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"">
ID = """ + caller + @""" Version=""2.0"" IssueInstant=""2013-01-01T00:00:00Z"">
<saml2:Issuer>
https://idp5.example.com
</saml2:Issuer>
<saml2p:Status>
<saml2p:StatusCode Value=""urn:oasis:names:tc:SAML:2.0:status:Success"" />
</saml2p:Status>
<saml2:Assertion
Version=""2.0"" ID=""" + MethodBase.GetCurrentMethod().Name + @"_Assertion2""
Version=""2.0"" ID=""" + caller + @"_Assertion""
IssueInstant=""2013-09-25T00:00:00Z"">
<saml2:Issuer>https://idp5.example.com</saml2:Issuer>
<saml2:Subject>
Expand All @@ -723,19 +728,21 @@ public void AcsCommand_Run_WithRelayStateUsedAsReturnUrl_Success()
var responseFormValue = Convert.ToBase64String
(Encoding.UTF8.GetBytes(SignedXmlHelper.SignXml(response)));

var relayStateFormValue = "https://somedomain.com/restrictedpage";
var formData = new List<KeyValuePair<string, IEnumerable<string>>>
{
new KeyValuePair<string, IEnumerable<string>>("SAMLResponse", new string[] { responseFormValue }),
};
if(relayState != null)
{
formData.Add(new KeyValuePair<string, IEnumerable<string>>("RelayState", new string[] { relayState }));
}

var r = new HttpRequestData(
"POST",
new Uri("http://localhost"),
"/ModulePath",
new KeyValuePair<string, IEnumerable<string>>[]
{
new KeyValuePair<string, IEnumerable<string>>("SAMLResponse", new string[] { responseFormValue }),
new KeyValuePair<string, IEnumerable<string>>("RelayState", new string[] { relayStateFormValue })
},
null
);
formData,
null);

var ids = new ClaimsIdentity[] { new ClaimsIdentity("Federation") };

Expand All @@ -745,55 +752,49 @@ public void AcsCommand_Run_WithRelayStateUsedAsReturnUrl_Success()
{
Principal = new ClaimsPrincipal(ids),
HttpStatusCode = HttpStatusCode.SeeOther,
Location = new Uri(relayStateFormValue),
Location = relayState != null ? new Uri(relayState, UriKind.RelativeOrAbsolute) : null,
};

new AcsCommand().Run(r, StubFactory.CreateOptions())
.Should().BeEquivalentTo(expected, opt => opt.IgnoringCyclicReferences());
new AcsCommand().Run(r, options)
.Location.OriginalString.Should().Be(relayState);
}

[TestMethod]
public void AcsCommand_Run_WithRelayStateUsedAsReturnUrl_Success()
{
RelayStateAsReturnUrl("/someUrl", StubFactory.CreateOptions());
}

[TestMethod]
public void AcsCommand_Run_WithRelayStateUsedAsReturnUrl_Missing()
{
var response =
@"<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://idp5.example.com
</saml2:Issuer>
<saml2p:Status>
<saml2p:StatusCode Value=""urn:oasis:names:tc:SAML:2.0:status:Success"" />
</saml2p:Status>
<saml2:Assertion
Version=""2.0"" ID=""" + MethodBase.GetCurrentMethod().Name + @"_Assertion2""
IssueInstant=""2013-09-25T00:00:00Z"">
<saml2:Issuer>https://idp5.example.com</saml2:Issuer>
<saml2:Subject>
<saml2:NameID>SomeUser</saml2:NameID>
<saml2:SubjectConfirmation Method=""urn:oasis:names:tc:SAML:2.0:cm:bearer"" />
</saml2:Subject>
<saml2:Conditions NotOnOrAfter=""2100-01-01T00:00:00Z"" />
</saml2:Assertion>
</saml2p:Response>";
this.Invoking(t => t.RelayStateAsReturnUrl(null, StubFactory.CreateOptions()))
.Should().Throw<ConfigurationErrorsException>();
}

var responseFormValue = Convert.ToBase64String
(Encoding.UTF8.GetBytes(SignedXmlHelper.SignXml(response)));
[TestMethod]
public void AcsCommand_Run_WithRelayStateUserAsReturnUrl_AbsolutUrlThrows()
{
this.Invoking(t => t.RelayStateAsReturnUrl("https://absolute.example.com/something", StubFactory.CreateOptions()))
.Should().Throw<InvalidOperationException>().WithMessage("*relative*");
}

var r = new HttpRequestData(
"POST",
new Uri("http://localhost"),
"/ModulePath",
new KeyValuePair<string, IEnumerable<string>>[]
{
new KeyValuePair<string, IEnumerable<string>>("SAMLResponse", new string[] { responseFormValue })
},
null
);
[TestMethod]
public void AcsCommand_Run_WithRelayStateUserAsReturnUrl_AbsolutUrlValidatesThroughNotification()
{
var options = StubFactory.CreateOptions();

Action a = () => new AcsCommand().Run(r, Options.FromConfiguration);
bool called = false;
options.Notifications.ValidateAbsoluteReturnUrl = url =>
{
called = true;
return true;
};

a.Should().Throw<ConfigurationErrorsException>();
}
// Should not throw this time.
RelayStateAsReturnUrl("https://absolute.example.com/something", options);

called.Should().BeTrue("Notifaction should have been called");
}
}
}

0 comments on commit 20aa29e

Please sign in to comment.