From cf6618071e7cb4187e9583a0f7f4240a141b4c9f Mon Sep 17 00:00:00 2001 From: Daniel Neighman Date: Wed, 23 Dec 2015 23:24:32 -0800 Subject: [PATCH] Fix a bug where session that have not been loaded on sign_out If more than one location is logged in to the session, but have not yet been loaded, the associated tokens were not being revoked even though the session was cleared. --- CHANGELOG.md | 6 ++++ README.md | 2 +- lib/guardian/plug.ex | 70 ++++++++++++++++++++----------------- mix.exs | 2 +- test/guardian/plug_test.exs | 22 +++++++----- 5 files changed, 59 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527cf8d97..1be79a09b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +# v 0.8.1 + +* Fix a bug with logout where it was not checking the session, only the assigns + This meant that if you had not verified the session the token would not be + revoked. + # v 0.7.1 * Adds basic Phoenix controller helpers diff --git a/README.md b/README.md index e8ac2770d..4c76f937f 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,7 @@ mix.deps defp deps do [ # ... - {:guardian, "~> 0.8.0"} + {:guardian, "~> 0.8.1"} # ... ] end diff --git a/lib/guardian/plug.ex b/lib/guardian/plug.ex index 443229ec6..7a0cb84ff 100644 --- a/lib/guardian/plug.ex +++ b/lib/guardian/plug.ex @@ -198,48 +198,54 @@ defmodule Guardian.Plug do end defp sign_out_via_key(conn, :all) do + keys = session_locations(conn) conn - |> Plug.Conn.clear_session - |> clear_jwt_assign(:all) - |> clear_resource_assign(:all) - |> clear_claims_assign(:all) + |> revoke_from_session(keys) + |> Plug.Conn.clear_session + |> clear_jwt_assign(keys) + |> clear_resource_assign(keys) + |> clear_claims_assign(keys) end defp sign_out_via_key(conn, the_key) do - Plug.Conn.delete_session(conn, claims_key(the_key)) - |> clear_jwt_assign(the_key) - |> clear_resource_assign(the_key) - |> clear_claims_assign(the_key) - end - - defp clear_resource_assign(conn, :all) do - Dict.keys(conn.assigns) - |> Enum.filter(&(String.starts_with?(to_string(&1), "guardian_") and String.ends_with?(to_string(&1), "_resource"))) - |> Enum.reduce(conn, fn(key, c) -> clear_resource_assign(c, key_from_other(key)) end) + conn + |> revoke_from_session(the_key) + |> Plug.Conn.delete_session(base_key(the_key)) + |> clear_jwt_assign(the_key) + |> clear_resource_assign(the_key) + |> clear_claims_assign(the_key) end + defp clear_resource_assign(conn, nil), do: conn + defp clear_resource_assign(conn, []), do: conn + defp clear_resource_assign(conn, [h|t]), do: clear_resource_assign(conn, h) |> clear_resource_assign(t) defp clear_resource_assign(conn, key), do: Plug.Conn.assign(conn, resource_key(key), nil) - defp clear_claims_assign(conn, :all) do - Dict.keys(conn.assigns) - |> Enum.filter(&(String.starts_with?(to_string(&1), "guardian_") and String.ends_with?(to_string(&1), "_claims"))) - |> Enum.reduce(conn, fn(key, c) -> clear_claims_assign(c, key_from_other(key)) end) - end - + defp clear_claims_assign(conn, nil), do: conn + defp clear_claims_assign(conn, []), do: conn + defp clear_claims_assign(conn, [h|t]), do: clear_claims_assign(conn, h) |> clear_claims_assign(t) defp clear_claims_assign(conn, key), do: Plug.Conn.assign(conn, claims_key(key), nil) - defp clear_jwt_assign(conn, :all) do - Dict.keys(conn.assigns) - |> Enum.filter(&(String.starts_with?(to_string(&1), "guardian_") and String.ends_with?(to_string(&1), "_jwt"))) - |> Enum.reduce(conn, fn(key, c) -> clear_jwt_assign(c, key_from_other(key)) end) - end - - defp clear_jwt_assign(conn, key) do - jwt = current_token(conn, key) - case claims(conn, key) do - { :ok, the_claims } -> Guardian.revoke!(jwt, the_claims) - _ -> :ok + defp clear_jwt_assign(conn, nil), do: conn + defp clear_jwt_assign(conn, []), do: conn + defp clear_jwt_assign(conn, [h|t]), do: clear_jwt_assign(conn, h) |> clear_jwt_assign(t) + defp clear_jwt_assign(conn, key), do: Plug.Conn.assign(conn, jwt_key(key), nil) + + defp session_locations(conn) do + conn.private.plug_session + |> Map.keys + |> Enum.map(&Guardian.Keys.key_from_other/1) + |> Enum.filter(&(&1 != nil)) + end + + defp revoke_from_session(conn, []), do: conn + defp revoke_from_session(conn, [h|t]), do: revoke_from_session(conn, h) |> revoke_from_session(t) + defp revoke_from_session(conn, key) do + case Plug.Conn.get_session(conn, base_key(key)) do + nil -> conn + jwt -> + Guardian.revoke!(jwt) + conn end - Plug.Conn.assign(conn, jwt_key(key), nil) end end diff --git a/mix.exs b/mix.exs index 7b30ecab2..bd2f9dfd5 100644 --- a/mix.exs +++ b/mix.exs @@ -1,7 +1,7 @@ defmodule Guardian.Mixfile do use Mix.Project - @version "0.8.0" + @version "0.8.1" @url "https://github.com/ueberauth/guardian" @maintainers ["Daniel Neighman", "Sonny Scroggin", "Sean Callan"] diff --git a/test/guardian/plug_test.exs b/test/guardian/plug_test.exs index b046bbd18..066825846 100644 --- a/test/guardian/plug_test.exs +++ b/test/guardian/plug_test.exs @@ -119,19 +119,23 @@ defmodule Guardian.PlugTest do test "sign_out/1", context do conn = conn_with_fetched_session(context.conn) - |> Guardian.Plug.sign_in(%{ user: "here" }, :token) + |> Guardian.Plug.sign_in(%{ user: "here" }, :token) assert Guardian.Plug.current_resource(conn) == %{ user: "here" } cleared_conn = conn - |> Plug.Conn.assign(Guardian.Keys.claims_key(:default), %{ claims: "yeah" }) - |> Plug.Conn.assign(Guardian.Keys.claims_key(:secret), %{ claims: "yeah" }) - |> Plug.Conn.assign(Guardian.Keys.resource_key(:default), "resource") - |> Plug.Conn.assign(Guardian.Keys.resource_key(:secret), "resource") - |> Plug.Conn.assign(Guardian.Keys.jwt_key(:default), "token") - |> Plug.Conn.assign(Guardian.Keys.jwt_key(:secret), "token") - |> Guardian.Plug.sign_out - + |> Plug.Conn.put_session(Guardian.Keys.base_key(:default), "default jwt") + |> Plug.Conn.put_session(Guardian.Keys.base_key(:secret), "secret jwt") + |> Plug.Conn.assign(Guardian.Keys.claims_key(:default), %{ claims: "yeah" }) + |> Plug.Conn.assign(Guardian.Keys.claims_key(:secret), %{ claims: "yeah" }) + |> Plug.Conn.assign(Guardian.Keys.resource_key(:default), "resource") + |> Plug.Conn.assign(Guardian.Keys.resource_key(:secret), "resource") + |> Plug.Conn.assign(Guardian.Keys.jwt_key(:default), "token") + |> Plug.Conn.assign(Guardian.Keys.jwt_key(:secret), "token") + |> Guardian.Plug.sign_out + + assert Plug.Conn.get_session(cleared_conn, Guardian.Keys.base_key(:default)) == nil + assert Plug.Conn.get_session(cleared_conn, Guardian.Keys.base_key(:secret)) == nil assert cleared_conn.assigns[Guardian.Keys.claims_key(:default)] == nil assert cleared_conn.assigns[Guardian.Keys.claims_key(:secret)] == nil assert cleared_conn.assigns[Guardian.Keys.resource_key(:default)] == nil