jitbit / AspNetSaml

Very simple SAML 2.0 consumer module for ASP.NET/C#
https://www.jitbit.com
Apache License 2.0
361 stars 118 forks source link

Assume expired as default #55

Closed mnbeer closed 2 years ago

mnbeer commented 2 years ago

In IsExpired(), it might be preferable to replace

DateTime expirationDate = DateTime.MaxValue;

with

DateTime expirationDate = DateTime.MinValue;

to assume expiration if a valid NotOnOrAfter is not located.

alex-jitbit commented 2 years ago

Please elaborate why would this be beneficial?

mnbeer commented 2 years ago

I realize that NotOnOrAfter is an optional element in the SAML 2 spec. However, I suspect that most implementations consider it mandatory (which is the case for in my situation). If mandatory, IsExpired should always return true if the DateTime.TryParse line is skipped because the "if" condition fails. Flipping from MaxValue to MinValue takes care of that.

Anyway, thanks for the code. It has been very useful.

alex-jitbit commented 2 years ago

This is a very powerful assumption and a breaking change.

I think that if the XML file does not specify whether it should be expired, then the app should not make decisions for it.