diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index b11d62f..fac1bc3 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -3,14 +3,5 @@ # Provably unreachable today, kept intentionally so the auth endpoint fails # closed (HTTP 403) if a future change introduces a new return shape. # See lib/ex_saml/sp_handler.ex for the inline rationale. - ~r/lib\/ex_saml\/sp_handler\.ex:95:.*pattern_match_cov/, - - # `stale_time/1` step 1: the `if t == :none or secs < t` expression is - # tautologically true here because `t` is provably `:none` at this point - # (variable shadowing inside the `case` body — the new `t` is only bound - # after the case finishes). The structure is preserved verbatim from the - # upstream esaml `stale_time/1` to keep behavioral parity and ease future - # backports. See lib/ex_saml/core/saml.ex around line 798. - ~r/lib\/ex_saml\/core\/saml\.ex:803:28:pattern_match/, - ~r/lib\/ex_saml\/core\/saml\.ex:803:54:pattern_match/ + ~r/lib\/ex_saml\/sp_handler\.ex:95:.*pattern_match_cov/ ] diff --git a/lib/ex_saml/core/saml.ex b/lib/ex_saml/core/saml.ex index 7c99839..f483741 100644 --- a/lib/ex_saml/core/saml.ex +++ b/lib/ex_saml/core/saml.ex @@ -780,55 +780,27 @@ defmodule ExSaml.Core.Saml do and falls back to issue_instant + 5 minutes. """ @spec stale_time(Assertion.t()) :: integer() - # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity def stale_time(%Assertion{} = a) do - t = :none - - t = - case a.subject do - %Subject{notonorafter: ""} -> - t - - %Subject{notonorafter: restrict} -> - secs = - restrict - |> Util.saml_to_datetime() - |> :calendar.datetime_to_gregorian_seconds() - - # Dialyzer flags this `if` as tautological because `t` is provably - # `:none` here (variable shadowing inside the `case` body). The - # structure is preserved verbatim from upstream arekinath/esaml's - # `stale_time/1` to keep behavioral parity and ease future backports. - # The corresponding warnings are suppressed in `.dialyzer_ignore.exs`. - if t == :none or secs < t, do: secs, else: t - end - - t = - case Keyword.get(a.conditions, :not_on_or_after) do - nil -> - t + case Enum.reject([subject_expiry(a), conditions_expiry(a)], &is_nil/1) do + [] -> issue_instant_secs(a) + 5 * 60 + candidates -> Enum.min(candidates) + end + end - restrict -> - secs = - restrict - |> Util.saml_to_datetime() - |> :calendar.datetime_to_gregorian_seconds() + defp subject_expiry(%Assertion{subject: %Subject{notonorafter: ""}}), do: nil + defp subject_expiry(%Assertion{subject: %Subject{notonorafter: stamp}}), do: to_secs(stamp) + defp subject_expiry(_), do: nil - if t == :none or secs < t, do: secs, else: t - end - - case t do - :none -> - ii_secs = - a.issue_instant - |> Util.saml_to_datetime() - |> :calendar.datetime_to_gregorian_seconds() + defp conditions_expiry(%Assertion{conditions: conditions}) do + if stamp = Keyword.get(conditions, :not_on_or_after), do: to_secs(stamp) + end - ii_secs + 5 * 60 + defp issue_instant_secs(%Assertion{issue_instant: stamp}), do: to_secs(stamp) - _ -> - t - end + defp to_secs(stamp) do + stamp + |> Util.saml_to_datetime() + |> :calendar.datetime_to_gregorian_seconds() end @doc """ diff --git a/test/ex_saml/core/saml_test.exs b/test/ex_saml/core/saml_test.exs index b91a259..af4c715 100644 --- a/test/ex_saml/core/saml_test.exs +++ b/test/ex_saml/core/saml_test.exs @@ -525,6 +525,68 @@ defmodule ExSaml.Core.SamlTest do end end + # --------------------------------------------------------------------------- + # stale_time/1 + # --------------------------------------------------------------------------- + + describe "stale_time/1" do + defp gregorian(saml_stamp) do + saml_stamp + |> Util.saml_to_datetime() + |> :calendar.datetime_to_gregorian_seconds() + end + + test "falls back to issue_instant + 5 minutes when neither subject nor conditions carry NotOnOrAfter" do + assertion = %Assertion{ + issue_instant: "2026-01-01T00:00:00Z", + subject: %Subject{notonorafter: ""}, + conditions: [] + } + + assert Saml.stale_time(assertion) == gregorian("2026-01-01T00:00:00Z") + 5 * 60 + end + + test "uses subject NotOnOrAfter when only subject restricts" do + assertion = %Assertion{ + issue_instant: "2026-01-01T00:00:00Z", + subject: %Subject{notonorafter: "2026-01-01T00:10:00Z"}, + conditions: [] + } + + assert Saml.stale_time(assertion) == gregorian("2026-01-01T00:10:00Z") + end + + test "uses conditions NotOnOrAfter when only conditions restrict" do + assertion = %Assertion{ + issue_instant: "2026-01-01T00:00:00Z", + subject: %Subject{notonorafter: ""}, + conditions: [not_on_or_after: "2026-01-01T00:10:00Z"] + } + + assert Saml.stale_time(assertion) == gregorian("2026-01-01T00:10:00Z") + end + + test "returns the minimum when both restrictions are present (conditions earlier)" do + assertion = %Assertion{ + issue_instant: "2026-01-01T00:00:00Z", + subject: %Subject{notonorafter: "2026-01-01T00:15:00Z"}, + conditions: [not_on_or_after: "2026-01-01T00:05:00Z"] + } + + assert Saml.stale_time(assertion) == gregorian("2026-01-01T00:05:00Z") + end + + test "returns the minimum when both restrictions are present (subject earlier)" do + assertion = %Assertion{ + issue_instant: "2026-01-01T00:00:00Z", + subject: %Subject{notonorafter: "2026-01-01T00:05:00Z"}, + conditions: [not_on_or_after: "2026-01-01T00:15:00Z"] + } + + assert Saml.stale_time(assertion) == gregorian("2026-01-01T00:05:00Z") + end + end + # --------------------------------------------------------------------------- # validate_stale_assertion # ---------------------------------------------------------------------------