Skip to content

Commit 7637ebe

Browse files
k-crossk-cross
authored
Merge pull request #13 from idyll/cached-saml-session-expiry
Get attestation from ETS or Session now checks for expiry.
2 parents 9caa51f + 812b5c3 commit 7637ebe

File tree

8 files changed

+136
-57
lines changed

8 files changed

+136
-57
lines changed

‎lib/samly/auth_handler.ex

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ defmodule Samly.AuthHandler do
7373
Helper.gen_idp_signin_req(sp, idp_rec, Map.get(idp, :nameid_format))
7474

7575
conn
76+
|> State.delete_assertion(assertion_key)
7677
|> configure_session(renew: true)
7778
|> put_session("relay_state", relay_state)
7879
|> put_session("idp_id", idp_id)

‎lib/samly/helper.ex

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,14 @@ defmodule Samly.Helper do
7979

8080
def decode_idp_signout_resp(sp, saml_encoding, saml_response) do
8181
resp_ns = [
82-
{'samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'},
83-
{'saml', 'urn:oasis:names:tc:SAML:2.0:assertion'},
84-
{'ds', 'http://www.w3.org/2000/09/xmldsig#'}
82+
{~c"samlp", ~c"urn:oasis:names:tc:SAML:2.0:protocol"},
83+
{~c"saml", ~c"urn:oasis:names:tc:SAML:2.0:assertion"},
84+
{~c"ds", ~c"http://www.w3.org/2000/09/xmldsig#"}
8585
]
8686

8787
with {:ok, xml_frag} <- decode_saml_payload(saml_encoding, saml_response),
8888
nodes when is_list(nodes) and length(nodes) == 1 <-
89-
:xmerl_xpath.string('/samlp:LogoutResponse', xml_frag, [{:namespace, resp_ns}]) do
89+
:xmerl_xpath.string(~c"/samlp:LogoutResponse", xml_frag, [{:namespace, resp_ns}]) do
9090
:esaml_sp.validate_logout_response(xml_frag, sp)
9191
else
9292
_ -> {:error, :invalid_request}
@@ -95,13 +95,13 @@ defmodule Samly.Helper do
9595

9696
def decode_idp_signout_req(sp, saml_encoding, saml_request) do
9797
req_ns = [
98-
{'samlp', 'urn:oasis:names:tc:SAML:2.0:protocol'},
99-
{'saml', 'urn:oasis:names:tc:SAML:2.0:assertion'}
98+
{~c"samlp", ~c"urn:oasis:names:tc:SAML:2.0:protocol"},
99+
{~c"saml", ~c"urn:oasis:names:tc:SAML:2.0:assertion"}
100100
]
101101

102102
with {:ok, xml_frag} <- decode_saml_payload(saml_encoding, saml_request),
103103
nodes when is_list(nodes) and length(nodes) == 1 <-
104-
:xmerl_xpath.string('/samlp:LogoutRequest', xml_frag, [{:namespace, req_ns}]) do
104+
:xmerl_xpath.string(~c"/samlp:LogoutRequest", xml_frag, [{:namespace, req_ns}]) do
105105
:esaml_sp.validate_logout_request(xml_frag, sp)
106106
else
107107
_ -> {:error, :invalid_request}

‎lib/samly/idp_data.ex

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ defmodule Samly.IdpData do
177177
@spec verify_slo_url(%IdpData{}) :: %IdpData{}
178178
defp verify_slo_url(%IdpData{} = idp_data) do
179179
if idp_data.valid? && idp_data.slo_redirect_url == nil && idp_data.slo_post_url == nil do
180-
Logger.warn("[Samly] SLO Endpoint missing in [#{inspect(idp_data.metadata_file)}]")
180+
Logger.warning("[Samly] SLO Endpoint missing in [#{inspect(idp_data.metadata_file)}]")
181181
end
182182

183183
idp_data
@@ -221,22 +221,22 @@ defmodule Samly.IdpData do
221221
to_charlist(format)
222222

223223
:email ->
224-
'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'
224+
~c"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
225225

226226
:x509 ->
227-
'urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName'
227+
~c"urn:oasis:names:tc:SAML:1.1:nameid-format:X509SubjectName"
228228

229229
:windows ->
230-
'urn:oasis:names:tc:SAML:1.1:nameid-format:WindowsDomainQualifiedName'
230+
~c"urn:oasis:names:tc:SAML:1.1:nameid-format:WindowsDomainQualifiedName"
231231

232232
:krb ->
233-
'urn:oasis:names:tc:SAML:2.0:nameid-format:kerberos'
233+
~c"urn:oasis:names:tc:SAML:2.0:nameid-format:kerberos"
234234

235235
:persistent ->
236-
'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
236+
~c"urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
237237

238238
:transient ->
239-
'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
239+
~c"urn:oasis:names:tc:SAML:2.0:nameid-format:transient"
240240

241241
invalid_nameid_format ->
242242
Logger.error(

‎lib/samly/provider.ex

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ defmodule Samly.Provider do
4848
value
4949

5050
unknown ->
51-
Logger.warn(
51+
Logger.warning(
5252
"[Samly] invalid_data idp_id_from: #{inspect(unknown)}. Using :path_segment"
5353
)
5454

‎lib/samly/state/ets.ex

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ defmodule Samly.State.ETS do
4646
@impl Samly.State.Store
4747
def get_assertion(_conn, assertion_key, assertions_table) do
4848
case :ets.lookup(assertions_table, assertion_key) do
49-
[{^assertion_key, %Assertion{} = assertion}] -> assertion
49+
[{^assertion_key, %Assertion{} = assertion}] -> validate_assertion_expiry(assertion)
5050
_ -> nil
5151
end
5252
end
@@ -62,4 +62,18 @@ defmodule Samly.State.ETS do
6262
:ets.delete(assertions_table, assertion_key)
6363
conn
6464
end
65+
66+
defp validate_assertion_expiry(
67+
%Assertion{subject: %{notonorafter: not_on_or_after}} = assertion
68+
) do
69+
now = DateTime.utc_now()
70+
71+
case DateTime.from_iso8601(not_on_or_after) do
72+
{:ok, not_on_or_after, _} ->
73+
if DateTime.compare(now, not_on_or_after) == :lt, do: assertion, else: nil
74+
75+
_ ->
76+
nil
77+
end
78+
end
6579
end

‎lib/samly/state/session.ex

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ defmodule Samly.State.Session do
3434
%{key: key} = opts
3535

3636
case Conn.get_session(conn, key) do
37-
{^assertion_key, %Assertion{} = assertion} -> assertion
37+
{^assertion_key, %Assertion{} = assertion} -> validate_assertion_expiry(assertion)
3838
_ -> nil
3939
end
4040
end
@@ -50,4 +50,18 @@ defmodule Samly.State.Session do
5050
%{key: key} = opts
5151
Conn.delete_session(conn, key)
5252
end
53+
54+
defp validate_assertion_expiry(
55+
%Assertion{subject: %{notonorafter: not_on_or_after}} = assertion
56+
) do
57+
now = DateTime.utc_now()
58+
59+
case DateTime.from_iso8601(not_on_or_after) do
60+
{:ok, not_on_or_after, _} ->
61+
if DateTime.compare(now, not_on_or_after) == :lt, do: assertion, else: nil
62+
63+
_ ->
64+
nil
65+
end
66+
end
5367
end

‎test/samly_idp_data_test.exs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ defmodule SamlyIdpDataTest do
262262

263263
test "nameid-format-in-metadata-but-not-config-should-use-metadata", %{sps: sps} do
264264
%IdpData{} = idp_data = IdpData.load_provider(@idp_config1, sps)
265-
assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient'
265+
assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:transient"
266266
end
267267

268268
test "nameid-format-in-config-but-not-metadata-should-use-config", %{sps: sps} do
@@ -273,7 +273,7 @@ defmodule SamlyIdpDataTest do
273273
})
274274

275275
%IdpData{} = idp_data = IdpData.load_provider(idp_config, sps)
276-
assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress'
276+
assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"
277277
end
278278

279279
test "nameid-format-in-metadata-and-config-should-use-config", %{sps: sps} do
@@ -283,7 +283,7 @@ defmodule SamlyIdpDataTest do
283283
})
284284

285285
%IdpData{} = idp_data = IdpData.load_provider(idp_config, sps)
286-
assert idp_data.nameid_format == 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent'
286+
assert idp_data.nameid_format == ~c"urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
287287
end
288288

289289
test "nameid-format-in-neither-metadata-nor-config-should-be-unknown", %{sps: sps} do

‎test/samly_state_test.exs

Lines changed: 87 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2,46 +2,96 @@ defmodule Samly.StateTest do
22
use ExUnit.Case, async: true
33
use Plug.Test
44

5-
setup do
6-
opts =
7-
Plug.Session.init(
8-
store: :cookie,
9-
key: "_samly_state_test_session",
10-
encryption_salt: "salty enc",
11-
signing_salt: "salty signing",
12-
key_length: 64
13-
)
14-
15-
Samly.State.init(Samly.State.Session)
16-
17-
conn =
18-
conn(:get, "/")
19-
|> Plug.Session.call(opts)
20-
|> fetch_session()
21-
22-
[conn: conn]
23-
end
5+
describe "With Session Cache" do
6+
setup do
7+
opts =
8+
Plug.Session.init(
9+
store: :cookie,
10+
key: "_samly_state_test_session",
11+
encryption_salt: "salty enc",
12+
signing_salt: "salty signing",
13+
key_length: 64
14+
)
2415

25-
test "put/get assertion", %{conn: conn} do
26-
assertion = %Samly.Assertion{}
27-
assertion_key = {"idp1", "name1"}
28-
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
29-
assert assertion == Samly.State.get_assertion(conn, assertion_key)
30-
end
16+
Samly.State.init(Samly.State.Session)
17+
18+
conn =
19+
conn(:get, "/")
20+
|> Plug.Session.call(opts)
21+
|> fetch_session()
22+
23+
[conn: conn]
24+
end
25+
26+
test "put/get assertion", %{conn: conn} do
27+
not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601()
28+
assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}}
29+
assertion_key = {"idp1", "name1"}
30+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
31+
assert assertion == Samly.State.get_assertion(conn, assertion_key)
32+
end
33+
34+
test "get failure for unknown assertion key", %{conn: conn} do
35+
assertion = %Samly.Assertion{}
36+
assertion_key = {"idp1", "name1"}
37+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
38+
assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name2"}))
39+
end
3140

32-
test "get failure for unknown assertion key", %{conn: conn} do
33-
assertion = %Samly.Assertion{}
34-
assertion_key = {"idp1", "name1"}
35-
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
36-
assert nil == Samly.State.get_assertion(conn, {"idp1", "name2"})
41+
test "get failure for expired assertion key", %{conn: conn} do
42+
assertion = %Samly.Assertion{}
43+
assertion_key = {"idp1", "name1"}
44+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
45+
assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name1"}))
46+
end
47+
48+
test "delete assertion", %{conn: conn} do
49+
not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601()
50+
assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}}
51+
assertion_key = {"idp1", "name1"}
52+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
53+
assert assertion == Samly.State.get_assertion(conn, assertion_key)
54+
conn = Samly.State.delete_assertion(conn, assertion_key)
55+
assert is_nil(Samly.State.get_assertion(conn, assertion_key))
56+
end
3757
end
3858

39-
test "delete assertion", %{conn: conn} do
40-
assertion = %Samly.Assertion{}
41-
assertion_key = {"idp1", "name1"}
42-
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
43-
assert assertion == Samly.State.get_assertion(conn, assertion_key)
44-
conn = Samly.State.delete_assertion(conn, assertion_key)
45-
assert nil == Samly.State.get_assertion(conn, assertion_key)
59+
describe "With ETS Cache" do
60+
setup do
61+
Samly.State.init(Samly.State.ETS)
62+
[conn: conn(:get, "/")]
63+
end
64+
65+
test "put/get assertion", %{conn: conn} do
66+
not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601()
67+
assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}}
68+
assertion_key = {"idp1", "name1"}
69+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
70+
assert assertion == Samly.State.get_assertion(conn, assertion_key)
71+
end
72+
73+
test "get failure for unknown assertion key", %{conn: conn} do
74+
assertion = %Samly.Assertion{}
75+
assertion_key = {"idp1", "name1"}
76+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
77+
assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name2"}))
78+
end
79+
80+
test "get failure for expired assertion key", %{conn: conn} do
81+
assertion = %Samly.Assertion{}
82+
assertion_key = {"idp1", "name1"}
83+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
84+
assert is_nil(Samly.State.get_assertion(conn, {"idp1", "name1"}))
85+
end
86+
87+
test "delete assertion", %{conn: conn} do
88+
not_on_or_after = DateTime.utc_now() |> DateTime.add(8, :hour) |> DateTime.to_iso8601()
89+
assertion = %Samly.Assertion{subject: %{notonorafter: not_on_or_after}}
90+
assertion_key = {"idp1", "name1"}
91+
conn = Samly.State.put_assertion(conn, assertion_key, assertion)
92+
assert assertion == Samly.State.get_assertion(conn, assertion_key)
93+
conn = Samly.State.delete_assertion(conn, assertion_key)
94+
assert is_nil(Samly.State.get_assertion(conn, assertion_key))
95+
end
4696
end
4797
end

0 commit comments

Comments
 (0)