diff --git a/lib/samly/auth_handler.ex b/lib/samly/auth_handler.ex index 277d358..a9cf580 100644 --- a/lib/samly/auth_handler.ex +++ b/lib/samly/auth_handler.ex @@ -73,6 +73,7 @@ defmodule Samly.AuthHandler do Helper.gen_idp_signin_req(sp, idp_rec, Map.get(idp, :nameid_format)) conn + |> State.delete_assertion(assertion_key) |> configure_session(renew: true) |> put_session("relay_state", relay_state) |> put_session("idp_id", idp_id) diff --git a/lib/samly/helper.ex b/lib/samly/helper.ex index aa1bf7e..32b1374 100644 --- a/lib/samly/helper.ex +++ b/lib/samly/helper.ex @@ -79,14 +79,14 @@ defmodule Samly.Helper do def decode_idp_signout_resp(sp, saml_encoding, saml_response) do resp_ns = [ - {'samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'}, - {'saml', 'urn:oasis:names:tc:SAML:2.0:assertion'}, - {'ds', 'http://www.w3.org/2000/09/xmldsig#'} + {~c"samlp", ~c"urn:oasis:names:tc:SAML:2.0:protocol"}, + {~c"saml", ~c"urn:oasis:names:tc:SAML:2.0:assertion"}, + {~c"ds", ~c"http://www.w3.org/2000/09/xmldsig#"} ] with {:ok, xml_frag} <- decode_saml_payload(saml_encoding, saml_response), nodes when is_list(nodes) and length(nodes) == 1 <- - :xmerl_xpath.string('/samlp:LogoutResponse', xml_frag, [{:namespace, resp_ns}]) do + :xmerl_xpath.string(~c"/samlp:LogoutResponse", xml_frag, [{:namespace, resp_ns}]) do :esaml_sp.validate_logout_response(xml_frag, sp) else _ -> {:error, :invalid_request} @@ -95,13 +95,13 @@ defmodule Samly.Helper do def decode_idp_signout_req(sp, saml_encoding, saml_request) do req_ns = [ - {'samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'}, - {'saml', 'urn:oasis:names:tc:SAML:2.0:assertion'} + {~c"samlp", ~c"urn:oasis:names:tc:SAML:2.0:protocol"}, + {~c"saml", ~c"urn:oasis:names:tc:SAML:2.0:assertion"} ] with {:ok, xml_frag} <- decode_saml_payload(saml_encoding, saml_request), nodes when is_list(nodes) and length(nodes) == 1 <- - :xmerl_xpath.string('/samlp:LogoutRequest', xml_frag, [{:namespace, req_ns}]) do + :xmerl_xpath.string(~c"/samlp:LogoutRequest", xml_frag, [{:namespace, req_ns}]) do :esaml_sp.validate_logout_request(xml_frag, sp) else _ -> {:error, :invalid_request} diff --git a/lib/samly/idp_data.ex b/lib/samly/idp_data.ex index 1868f5c..dbf5c74 100644 --- a/lib/samly/idp_data.ex +++ b/lib/samly/idp_data.ex @@ -177,7 +177,7 @@ defmodule Samly.IdpData do @spec verify_slo_url(%IdpData{}) :: %IdpData{} defp verify_slo_url(%IdpData{} = idp_data) do if idp_data.valid? && idp_data.slo_redirect_url == nil && idp_data.slo_post_url == nil do - Logger.warn("[Samly] SLO Endpoint missing in [#{inspect(idp_data.metadata_file)}]") + Logger.warning("[Samly] SLO Endpoint missing in [#{inspect(idp_data.metadata_file)}]") end idp_data @@ -221,22 +221,22 @@ defmodule Samly.IdpData do to_charlist(format) :email -> - 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress' + ~c"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" :x509 -> - 'urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName' + ~c"urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName" :windows -> - 'urn:oasis:names:tc:SAML:1.1:nameid-format:WindowsDomainQualifiedName' + ~c"urn:oasis:names:tc:SAML:1.1:nameid-format:WindowsDomainQualifiedName" :krb -> - 'urn:oasis:names:tc:SAML:2.0:nameid-format:kerberos' + ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:kerberos" :persistent -> - 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" :transient -> - 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' + ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:transient" invalid_nameid_format -> Logger.error( diff --git a/lib/samly/provider.ex b/lib/samly/provider.ex index 991a044..f31ad14 100644 --- a/lib/samly/provider.ex +++ b/lib/samly/provider.ex @@ -48,7 +48,7 @@ defmodule Samly.Provider do value unknown -> - Logger.warn( + Logger.warning( "[Samly] invalid_data idp_id_from: #{inspect(unknown)}. Using :path_segment" ) diff --git a/lib/samly/state/ets.ex b/lib/samly/state/ets.ex index a04d644..e00b74c 100644 --- a/lib/samly/state/ets.ex +++ b/lib/samly/state/ets.ex @@ -46,7 +46,7 @@ defmodule Samly.State.ETS do @impl Samly.State.Store def get_assertion(_conn, assertion_key, assertions_table) do case :ets.lookup(assertions_table, assertion_key) do - [{^assertion_key, %Assertion{} = assertion}] -> assertion + [{^assertion_key, %Assertion{} = assertion}] -> validate_assertion_expiry(assertion) _ -> nil end end @@ -62,4 +62,18 @@ defmodule Samly.State.ETS do :ets.delete(assertions_table, assertion_key) conn end + + defp validate_assertion_expiry( + %Assertion{subject: %{notonorafter: not_on_or_after}} = assertion + ) do + now = DateTime.utc_now() + + case DateTime.from_iso8601(not_on_or_after) do + {:ok, not_on_or_after, _} -> + if DateTime.compare(now, not_on_or_after) == :lt, do: assertion, else: nil + + _ -> + nil + end + end end diff --git a/lib/samly/state/session.ex b/lib/samly/state/session.ex index 42f2260..2a28959 100644 --- a/lib/samly/state/session.ex +++ b/lib/samly/state/session.ex @@ -34,7 +34,7 @@ defmodule Samly.State.Session do %{key: key} = opts case Conn.get_session(conn, key) do - {^assertion_key, %Assertion{} = assertion} -> assertion + {^assertion_key, %Assertion{} = assertion} -> validate_assertion_expiry(assertion) _ -> nil end end @@ -50,4 +50,18 @@ defmodule Samly.State.Session do %{key: key} = opts Conn.delete_session(conn, key) end + + defp validate_assertion_expiry( + %Assertion{subject: %{notonorafter: not_on_or_after}} = assertion + ) do + now = DateTime.utc_now() + + case DateTime.from_iso8601(not_on_or_after) do + {:ok, not_on_or_after, _} -> + if DateTime.compare(now, not_on_or_after) == :lt, do: assertion, else: nil + + _ -> + nil + end + end end diff --git a/test/samly_idp_data_test.exs b/test/samly_idp_data_test.exs index 3c67bb4..1e58a97 100644 --- a/test/samly_idp_data_test.exs +++ b/test/samly_idp_data_test.exs @@ -262,7 +262,7 @@ defmodule SamlyIdpDataTest do test "nameid-format-in-metadata-but-not-config-should-use-metadata", %{sps: sps} do %IdpData{} = idp_data = IdpData.load_provider(@idp_config1, sps) - assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' + assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:transient" end test "nameid-format-in-config-but-not-metadata-should-use-config", %{sps: sps} do @@ -273,7 +273,7 @@ defmodule SamlyIdpDataTest do }) %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) - assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress' + assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress" end test "nameid-format-in-metadata-and-config-should-use-config", %{sps: sps} do @@ -283,7 +283,7 @@ defmodule SamlyIdpDataTest do }) %IdpData{} = idp_data = IdpData.load_provider(idp_config, sps) - assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent' + assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" end test "nameid-format-in-neither-metadata-nor-config-should-be-unknown", %{sps: sps} do diff --git a/test/samly_state_test.exs b/test/samly_state_test.exs index 8bd3e0e..9daac8e 100644 --- a/test/samly_state_test.exs +++ b/test/samly_state_test.exs @@ -2,46 +2,96 @@ defmodule Samly.StateTest do use ExUnit.Case, async: true use Plug.Test - setup do - opts = - Plug.Session.init( - store: :cookie, - key: "_samly_state_test_session", - encryption_salt: "salty enc", - signing_salt: "salty signing", - key_length: 64 - ) - - Samly.State.init(Samly.State.Session) - - conn = - conn(:get, "/") - |> Plug.Session.call(opts) - |> fetch_session() - - [conn: conn] - end + describe "With Session Cache" do + setup do + opts = + Plug.Session.init( + store: :cookie, + key: "_samly_state_test_session", + encryption_salt: "salty enc", + signing_salt: "salty signing", + key_length: 64 + ) - test "put/get assertion", %{conn: conn} do - assertion = %Samly.Assertion{} - assertion_key = {"idp1", "name1"} - conn = Samly.State.put_assertion(conn, assertion_key, assertion) - assert assertion == Samly.State.get_assertion(conn, assertion_key) - end + Samly.State.init(Samly.State.Session) + + conn = + conn(:get, "/") + |> Plug.Session.call(opts) + |> fetch_session() + + [conn: conn] + end + + test "put/get assertion", %{conn: conn} do + not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601() + assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert assertion == Samly.State.get_assertion(conn, assertion_key) + end + + test "get failure for unknown assertion key", %{conn: conn} do + assertion = %Samly.Assertion{} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name2"})) + end - test "get failure for unknown assertion key", %{conn: conn} do - assertion = %Samly.Assertion{} - assertion_key = {"idp1", "name1"} - conn = Samly.State.put_assertion(conn, assertion_key, assertion) - assert nil == Samly.State.get_assertion(conn, {"idp1", "name2"}) + test "get failure for expired assertion key", %{conn: conn} do + assertion = %Samly.Assertion{} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name1"})) + end + + test "delete assertion", %{conn: conn} do + not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601() + assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert assertion == Samly.State.get_assertion(conn, assertion_key) + conn = Samly.State.delete_assertion(conn, assertion_key) + assert is_nil(Samly.State.get_assertion(conn, assertion_key)) + end end - test "delete assertion", %{conn: conn} do - assertion = %Samly.Assertion{} - assertion_key = {"idp1", "name1"} - conn = Samly.State.put_assertion(conn, assertion_key, assertion) - assert assertion == Samly.State.get_assertion(conn, assertion_key) - conn = Samly.State.delete_assertion(conn, assertion_key) - assert nil == Samly.State.get_assertion(conn, assertion_key) + describe "With ETS Cache" do + setup do + Samly.State.init(Samly.State.ETS) + [conn: conn(:get, "/")] + end + + test "put/get assertion", %{conn: conn} do + not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601() + assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert assertion == Samly.State.get_assertion(conn, assertion_key) + end + + test "get failure for unknown assertion key", %{conn: conn} do + assertion = %Samly.Assertion{} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name2"})) + end + + test "get failure for expired assertion key", %{conn: conn} do + assertion = %Samly.Assertion{} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name1"})) + end + + test "delete assertion", %{conn: conn} do + not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601() + assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}} + assertion_key = {"idp1", "name1"} + conn = Samly.State.put_assertion(conn, assertion_key, assertion) + assert assertion == Samly.State.get_assertion(conn, assertion_key) + conn = Samly.State.delete_assertion(conn, assertion_key) + assert is_nil(Samly.State.get_assertion(conn, assertion_key)) + end end end