Skip to content

Commit

Permalink
Clear refactoring recommendations
Browse files Browse the repository at this point in the history
  • Loading branch information
AndersAbel committed Mar 26, 2020
1 parent 6ddb550 commit 4a629eb
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 32 deletions.
8 changes: 3 additions & 5 deletions Sustainsys.Saml2/WebSSO/CommandFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,19 +53,17 @@ public static class CommandFactory
/// <returns>A command implementation or notFoundCommand if invalid.</returns>
public static ICommand GetCommand(string commandName)
{
ICommand command;

if(commandName == null)
if (commandName == null)
{
throw new ArgumentNullException(nameof(commandName));
}

if(commandName.StartsWith("/", StringComparison.OrdinalIgnoreCase))
if (commandName.StartsWith("/", StringComparison.OrdinalIgnoreCase))
{
commandName = commandName.Substring(1);
}

if (commands.TryGetValue(commandName, out command))
if (commands.TryGetValue(commandName, out ICommand command))
{
return command;
}
Expand Down
2 changes: 1 addition & 1 deletion Sustainsys.Saml2/WebSSO/Saml2ArtifactBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private static IdentityProvider GetIdp(
.SequenceEqual(sourceId));
}

private static SHA1 sha1 = SHA1.Create();
private static readonly SHA1 sha1 = SHA1.Create();

/// <summary>
/// Create a SAML artifact value.
Expand Down
9 changes: 3 additions & 6 deletions Sustainsys.Saml2/WebSSO/Saml2RedirectBinding.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ private static TrustLevel GetTrustLevel(XmlElement documentElement, HttpRequestD
return TrustLevel.None;
}

IdentityProvider idp;
if (!options.IdentityProviders.TryGetValue(new EntityId(issuer), out idp))
if (!options.IdentityProviders.TryGetValue(new EntityId(issuer), out IdentityProvider idp))
{
throw new InvalidSignatureException(string.Format(CultureInfo.InvariantCulture, "Cannot verify signature of message from unknown sender {0}.", issuer));
}
Expand All @@ -143,8 +142,7 @@ private static void CheckSignature(HttpRequestData request, IdentityProvider idp
var rawQueryStringParams = request.Url.Query.TrimStart('?').Split('&').Select(qp => qp.Split('=')).ToDictionary(kv => kv[0], kv => kv[1]);

var msgParam = "";
string msg;
if (rawQueryStringParams.TryGetValue("SAMLRequest", out msg))
if (rawQueryStringParams.TryGetValue("SAMLRequest", out string msg))
{
msgParam = "SAMLRequest=" + msg;
}
Expand All @@ -154,8 +152,7 @@ private static void CheckSignature(HttpRequestData request, IdentityProvider idp
}

var relayStateParam = "";
string relayState;
if (rawQueryStringParams.TryGetValue("RelayState", out relayState))
if (rawQueryStringParams.TryGetValue("RelayState", out string relayState))
{
relayStateParam = "&RelayState=" + relayState;
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/Owin.Tests/CommandResultExtensionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public void CommandResultExtensions_DoesNotApplyCookieWhenNoNameSet()

var setCookieHeader = context.Response.Headers["Set-Cookie"];

var protectedData = HttpRequestData.ConvertBinaryData(
HttpRequestData.ConvertBinaryData(
StubDataProtector.Protect(cr.GetSerializedRequestState()));

setCookieHeader.Should().Be(null);
Expand Down
4 changes: 2 additions & 2 deletions Tests/Owin.Tests/Saml2AuthenticationMiddlewareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void Saml2AuthenticationMiddleware_CtorSetsDefaultAuthOption()

options.SignInAsAuthenticationType.Should().BeNull();

var middleware = new Saml2AuthenticationMiddleware(new StubOwinMiddleware(0, null),
new Saml2AuthenticationMiddleware(new StubOwinMiddleware(0, null),
CreateAppBuilder(), options);

options.SignInAsAuthenticationType.Should().Be(DefaultSignInAsAuthenticationType);
Expand Down Expand Up @@ -1348,7 +1348,7 @@ public async Task Saml2AuthenticationMiddleware_SignInUrlRedirectsToIdp()
context.Response.StatusCode.Should().Be(303);
context.Response.Headers["Location"].Should().StartWith("https://idp2.example.com/idp?SAMLRequest");

var relayState = ExtractRelayState(context);
ExtractRelayState(context);

var storedAuthnData = ExtractRequestState(options.DataProtector, context);

Expand Down
39 changes: 25 additions & 14 deletions Tests/Tests.Shared/WebSSO/LogoutCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ public void LogoutCommand_Run_ReturnsLogoutRequest()
new Claim(Saml2ClaimTypes.SessionIndex, "SessionId", null, "https://idp.example.com")
}, "Federation"));

var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"));
request.User = user;
var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"))
{
User = user
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand Down Expand Up @@ -139,8 +141,10 @@ public void LogoutCommand_Run_ReturnsLogoutRequest_POST()
new Claim(Saml2ClaimTypes.SessionIndex, "SessionId", null, "https://idp.example.com")
}, "Federation"));

var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"));
request.User = user;
var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"))
{
User = user
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand All @@ -163,8 +167,10 @@ public void LogoutCommand_Run_NoCookieName_WhenLogoutStateDisabled()
new Claim(Saml2ClaimTypes.SessionIndex, "SessionId", null, "https://idp.example.com")
}, "Federation"));

var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"));
request.User = user;
var request = new HttpRequestData("GET", new Uri("http://sp-internal.example.com/Saml2/Logout"))
{
User = user
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand All @@ -190,8 +196,10 @@ public void LogoutCommand_Run_PreservesReturnUrl()
new Claim(Saml2ClaimTypes.SessionIndex, "SessionId", null, "https://idp.example.com")
}, "Federation"));

var request = new HttpRequestData("GET", new Uri("http://sp.example.com/Saml2/Logout?ReturnUrl=%2FLoggedOut"));
request.User = user;
var request = new HttpRequestData("GET", new Uri("http://sp.example.com/Saml2/Logout?ReturnUrl=%2FLoggedOut"))
{
User = user
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand Down Expand Up @@ -283,8 +291,10 @@ public void LogoutCommand_Run_ReturnsLogoutRequest_IgnoresThreadPrincipal()
new Claim(Saml2ClaimTypes.SessionIndex, "SessionId", null, "https://idp.example.com")
}, "Federation"));

var request = new HttpRequestData("GET", new Uri("http://sp.example.com/Saml2/Logout"));
request.User = user;
var request = new HttpRequestData("GET", new Uri("http://sp.example.com/Saml2/Logout"))
{
User = user
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand Down Expand Up @@ -383,7 +393,7 @@ public void LogoutCommand_Run_HandlesLogoutResponse_InPost()
xml.Sign(SignedXmlHelper.TestCert);

var responseData = Convert.ToBase64String(Encoding.UTF8.GetBytes(xml.OuterXml));

var httpRequest = new HttpRequestData(
"POST",
new Uri("http://something"),
Expand All @@ -394,9 +404,10 @@ public void LogoutCommand_Run_HandlesLogoutResponse_InPost()
new KeyValuePair<string, IEnumerable<string>>("RelayState", new[] { relayState })
},
Enumerable.Empty<KeyValuePair<string, string>>(),
null);

httpRequest.StoredRequestState = new StoredRequestState(null, new Uri("http://loggedout.example.com"), null, null);
null)
{
StoredRequestState = new StoredRequestState(null, new Uri("http://loggedout.example.com"), null, null)
};

var options = StubFactory.CreateOptions();
options.SPOptions.ServiceCertificates.Add(SignedXmlHelper.TestCert);
Expand Down
5 changes: 2 additions & 3 deletions Tests/Tests.Shared/WebSSO/Saml2ArtifactBindingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ public void Saml2ArtifactBinding_Unbind_FromGet_SignsArtifactResolve()
Certificate = SignedXmlHelper.TestCert
});

var result = Saml2Binding.Get(Saml2BindingType.Artifact).Unbind(r, options);
Saml2Binding.Get(Saml2BindingType.Artifact).Unbind(r, options);

StubServer.LastArtifactResolutionWasSigned.Should().BeTrue();
}
Expand Down Expand Up @@ -299,8 +299,7 @@ public void Saml2ArtifactBinding_Bind()
var artifact = Convert.FromBase64String(
Uri.UnescapeDataString(query["SAMLart"]));

ISaml2Message storedMessage;
Saml2ArtifactBinding.PendingMessages.TryRemove(artifact, out storedMessage)
Saml2ArtifactBinding.PendingMessages.TryRemove(artifact, out ISaml2Message storedMessage)
.Should().BeTrue();

storedMessage.Should().BeSameAs(message);
Expand Down

0 comments on commit 4a629eb

Please sign in to comment.