Skip to content

Commit

Permalink
Clear build refactoring suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
AndersAbel committed Mar 22, 2020
1 parent 5d38ece commit 2ad5e26
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 48 deletions.
6 changes: 1 addition & 5 deletions Sustainsys.Saml2.AspNetCore2/Saml2Handler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,8 @@ public class Saml2Handler : IAuthenticationRequestHandler, IAuthenticationSignOu
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Maintainability", "CA1500:VariableNamesShouldNotMatchFieldNames", MessageId = "context")]
public Task InitializeAsync(AuthenticationScheme scheme, HttpContext context)
{
if(context == null)
{
throw new ArgumentNullException(nameof(context));
}
this.context = context ?? throw new ArgumentNullException(nameof(context));

this.context = context;
options = optionsCache.GetOrAdd(scheme.Name, () => optionsFactory.Create(scheme.Name));

emitSameSiteNone = options.Notifications.EmitSameSiteNone(context.Request.GetUserAgent());
Expand Down
17 changes: 8 additions & 9 deletions Sustainsys.Saml2.Owin/Saml2AuthenticationHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ protected async override Task<AuthenticationTicket> AuthenticateCoreAsync()
var identities = result.Principal.Identities.Select(i =>
new ClaimsIdentity(i, null, Options.SignInAsAuthenticationType, i.NameClaimType, i.RoleClaimType));

var authProperties = new AuthenticationProperties(result.RelayData);
authProperties.RedirectUri = result.Location.OriginalString;
var authProperties = new AuthenticationProperties(result.RelayData)
{
RedirectUri = result.Location.OriginalString
};
if (result.SessionNotOnOrAfter.HasValue)
{
authProperties.AllowRefresh = false;
Expand Down Expand Up @@ -111,15 +113,13 @@ protected override async Task ApplyResponseChallengeAsync()
if (challenge != null)
{
EntityId idp;
string strIdp;
if (challenge.Properties.Dictionary.TryGetValue("idp", out strIdp))
if (challenge.Properties.Dictionary.TryGetValue("idp", out string strIdp))
{
idp = new EntityId(strIdp);
}
else
{
object objIdp = null;
Context.Environment.TryGetValue("saml2.idp", out objIdp);
Context.Environment.TryGetValue("saml2.idp", out object objIdp);
idp = objIdp as EntityId;
}
var redirectUri = challenge.Properties.RedirectUri;
Expand Down Expand Up @@ -196,9 +196,8 @@ protected async override Task ApplyResponseGrantAsync()
public override async Task<bool> InvokeAsync()
{
var Saml2Path = new PathString(Options.SPOptions.ModulePath);
PathString remainingPath;

if (Request.Path.StartsWithSegments(Saml2Path, out remainingPath))
if (Request.Path.StartsWithSegments(Saml2Path, out PathString remainingPath))
{
if (remainingPath == new PathString("/" + CommandFactory.AcsCommandName))
{
Expand Down Expand Up @@ -231,7 +230,7 @@ public override async Task<bool> InvokeAsync()

return true;
}
catch(Exception ex)
catch (Exception ex)
{
Options.SPOptions.Logger.WriteError("Error in Saml2 for " + Request.Path, ex);
throw;
Expand Down
18 changes: 7 additions & 11 deletions Sustainsys.Saml2/IdentityProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@ public class IdentityProvider
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly", MessageId = "sp")]
public IdentityProvider(EntityId entityId, SPOptions spOptions)
{
if (spOptions == null)
{
throw new ArgumentNullException(nameof(spOptions));
}

EntityId = entityId;
this.spOptions = spOptions;

this.spOptions = spOptions ?? throw new ArgumentNullException(nameof(spOptions));

OutboundSigningAlgorithm = spOptions.OutboundSigningAlgorithm;
}

Expand Down Expand Up @@ -170,7 +167,7 @@ public Uri SingleSignOnServiceUrl
}
}

private IDictionary<int, Uri> artifactResolutionServiceUrls
private readonly IDictionary<int, Uri> artifactResolutionServiceUrls
= new ConcurrentDictionary<int, Uri>();

/// <summary>
Expand Down Expand Up @@ -347,7 +344,7 @@ public CommandResult Bind(ISaml2Message request)
return Saml2Binding.Get(Binding).Bind(request);
}

private ConfiguredAndLoadedSigningKeysCollection signingKeys =
private readonly ConfiguredAndLoadedSigningKeysCollection signingKeys =
new ConfiguredAndLoadedSigningKeysCollection();

/// <summary>
Expand All @@ -362,7 +359,7 @@ public ConfiguredAndLoadedSigningKeysCollection SigningKeys
}
}

object metadataLoadLock = new object();
readonly object metadataLoadLock = new object();

private void DoLoadMetadata()
{
Expand Down Expand Up @@ -505,8 +502,7 @@ private void ScheduleMetadataRefresh()
[ExcludeFromCodeCoverage]
private static void DoLoadMetadataIfTargetAlive(WeakReference<IdentityProvider> target)
{
IdentityProvider idp;
if(target.TryGetTarget(out idp))
if (target.TryGetTarget(out IdentityProvider idp))
{
idp.DoLoadMetadata();
}
Expand Down
6 changes: 3 additions & 3 deletions Sustainsys.Saml2/Tokens/TokenReplayCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ namespace Sustainsys.Saml2.Tokens
class TokenReplayCache : ITokenReplayCache
{
#if NETSTANDARD2_0
MemoryCache cache = new MemoryCache(new MemoryCacheOptions());
readonly MemoryCache cache = new MemoryCache(new MemoryCacheOptions());
#else
MemoryCache cache = new MemoryCache(nameof(TokenReplayCache));
readonly MemoryCache cache = new MemoryCache(nameof(TokenReplayCache));
#endif

// Dummy object to store in cache.
private static object cacheObject = new object();
private static readonly object cacheObject = new object();

public bool TryAdd(string securityToken, DateTime expiresOn)
{
Expand Down
3 changes: 1 addition & 2 deletions Sustainsys.Saml2/WebSSO/LogOutCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ public static CommandResult InitiateLogout(HttpRequestData request, Uri returnUr
sessionIndexClaim = request.User.FindFirst(Saml2ClaimTypes.SessionIndex);
}

IdentityProvider idp;
var knownIdp = options.IdentityProviders.TryGetValue(new EntityId(idpEntityId), out idp);
var knownIdp = options.IdentityProviders.TryGetValue(new EntityId(idpEntityId), out IdentityProvider idp);

options.SPOptions.Logger.WriteVerbose("Initiating logout, checking requirements for federated logout"
+ "\n Issuer of LogoutNameIdentifier claim (should be Idp entity id): " + idpEntityId
Expand Down
29 changes: 14 additions & 15 deletions Tests/Tests.Shared/Helpers/StubServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ public class StubServer
#endif
static IDictionary<string, string> GetContent()
{
var content = new Dictionary<string, string>();

content["/idpMetadata"] = string.Format(
var content = new Dictionary<string, string>
{
["/idpMetadata"] = string.Format(
@"<EntityDescriptor xmlns=""urn:oasis:names:tc:SAML:2.0:metadata""
entityID=""http://localhost:13428/idpMetadata"" validUntil=""2100-01-02T14:42:43Z"">
<IDPSSODescriptor
Expand All @@ -57,9 +57,9 @@ public class StubServer
ResponseLocation=""http://localhost:{1}/logoutResponse""/>
</IDPSSODescriptor>
</EntityDescriptor>
", SignedXmlHelper.KeyInfoXml, IdpMetadataSsoPort);
", SignedXmlHelper.KeyInfoXml, IdpMetadataSsoPort),

content["/idpMetadataNoCertificate"] =
["/idpMetadataNoCertificate"] =
@"<EntityDescriptor xmlns=""urn:oasis:names:tc:SAML:2.0:metadata""
entityID=""http://localhost:13428/idpMetadataNoCertificate"" cacheDuration=""PT15M"">
<IDPSSODescriptor
Expand All @@ -71,9 +71,9 @@ public class StubServer
Binding=""urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect""
Location=""http://localhost:13428/logout""/>
</IDPSSODescriptor>
</EntityDescriptor>";
</EntityDescriptor>",

content["/idpMetadataOtherEntityId"] = string.Format(
["/idpMetadataOtherEntityId"] = string.Format(
@"<EntityDescriptor xmlns=""urn:oasis:names:tc:SAML:2.0:metadata""
entityID=""http://other.entityid.example.com"">
<IDPSSODescriptor
Expand All @@ -86,9 +86,9 @@ public class StubServer
Binding=""urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect""
Location=""http://wrong.entityid.example.com/acs""/>
</IDPSSODescriptor>
</EntityDescriptor>", SignedXmlHelper.KeyInfoXml);
</EntityDescriptor>", SignedXmlHelper.KeyInfoXml),

content["/federationMetadata"] = string.Format(
["/federationMetadata"] = string.Format(
@"<EntitiesDescriptor xmlns=""urn:oasis:names:tc:SAML:2.0:metadata"" validUntil=""2100-01-01T14:43:15Z"">
<EntityDescriptor entityID=""http://idp.federation.example.com/metadata"">
<IDPSSODescriptor
Expand All @@ -110,7 +110,8 @@ public class StubServer
</SPSSODescriptor>
</EntityDescriptor>
</EntitiesDescriptor>
", SignedXmlHelper.KeyInfoXml);
", SignedXmlHelper.KeyInfoXml)
};

var federationMetadataSigned = string.Format(
@"<EntitiesDescriptor ID=""federationMetadataSigned"" xmlns=""urn:oasis:names:tc:SAML:2.0:metadata"" validUntil=""2100-01-01T14:43:15Z"">
Expand Down Expand Up @@ -366,16 +367,14 @@ public class StubServer

static async Task HandleRequestAsync(HttpContext ctx, Func<Task> next)
{
string data;

switch (ctx.Request.Path.ToString())
{
case "/ars":
await ArtifactResolutionService(ctx);
return;
default:
var content = GetContent();
if (content.TryGetValue(ctx.Request.Path.ToString(), out data))
if (content.TryGetValue(ctx.Request.Path.ToString(), out string data))
{
await ctx.Response.WriteAsync(data);
return;
Expand All @@ -386,7 +385,7 @@ static async Task HandleRequestAsync(HttpContext ctx, Func<Task> next)
}

#if NETCOREAPP2_1
public static void Start(TestContext testContext)
public static void Start()
{
host = new WebHostBuilder()
.UseUrls("http://localhost:13428")
Expand All @@ -396,7 +395,7 @@ public static void Start(TestContext testContext)
host.Start();
}
#else
public static void Start(TestContext testContext)
public static void Start()
{
host = WebApp.Start("http://localhost:13428", app =>
{
Expand Down
4 changes: 3 additions & 1 deletion Tests/Tests.Shared/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ namespace Tests
public class Startup
{
[AssemblyInitialize]
#pragma warning disable IDE0060 // Remove unused parameter
public static void Initialize(TestContext testContext)
#pragma warning restore IDE0060 // Remove unused parameter
{
// This is needed because testhost.exe uses its own location for the config file
// There appears to be no way to set it in .NET Core. See:
Expand All @@ -22,7 +24,7 @@ public static void Initialize(TestContext testContext)
// https://github.com/dotnet/corefx/issues/26626
string asmPath = Assembly.GetExecutingAssembly().Location;
SustainsysSaml2Section.Configuration = ConfigurationManager.OpenExeConfiguration(asmPath);
StubServer.Start(testContext);
StubServer.Start();
}

[AssemblyCleanup]
Expand Down
2 changes: 0 additions & 2 deletions Tests/Tests.Shared/WebSSO/SignInCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ public void SignInCommand_Run_ReturnsAuthnRequestForDefaultIdp()
[TestMethod]
public void SignInCommand_Run_MapsReturnUrl()
{
var defaultDestination = Options.FromConfiguration.IdentityProviders.Default.SingleSignOnServiceUrl;

var httpRequest = new HttpRequestData("GET", new Uri("http://localhost/signin?ReturnUrl=%2FReturn.aspx"));

var actual = new SignInCommand().Run(httpRequest, Options.FromConfiguration);
Expand Down

0 comments on commit 2ad5e26

Please sign in to comment.