From 9baef5b2880ed23d33f0e7c7c0a0ee58e09fc852 Mon Sep 17 00:00:00 2001 From: Stephane ROBINO Date: Wed, 18 Aug 2021 08:51:27 +0200 Subject: [PATCH] fix: try refreshing cookie on multiple cases (#683) * fix: try refreshing cookie on multiple cases * chore: add test cases and update changelog * chore: update changelog with missing feature --- CHANGELOG.md | 13 +++++++++++- lib/guardian/plug/verify_header.ex | 24 ++++++++-------------- lib/guardian/plug/verify_session.ex | 24 ++++++++-------------- test/guardian/plug/verify_header_test.exs | 23 +++++++++++++++++++-- test/guardian/plug/verify_session_test.exs | 24 ++++++++++++++++++++-- 5 files changed, 73 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d9eab0f2..d82fa78bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,15 +1,26 @@ # Changelog +## v2.2.1 + +### Enhancement + +* `Guardian.Plug.VerifyHeader` and `Guardian.Plug.VerifySession` `:refresh_from_cookie` option will try refreshing +when access token not found, invalid or expired if cookie present [#683](https://github.com/ueberauth/guardian/pull/683) + ## v2.2.0 ### Enhancement * Add `:scheme` option to `Guardian.Plug.VerifyHeader` [#680](https://github.com/ueberauth/guardian/pull/680) +* Add `:refresh_from_cookie` option to `Guardian.Plug.VerifyHeader` and `Guardian.Plug.VerifySession` +to replace `Guardian.Plug.VerifyCookie` plug [#675](https://github.com/ueberauth/guardian/pull/675) ### Deprecation -* `:realm` option configuration of `Guardian.Plug.VerifyHeader` is deprecated +* `:realm` option configuration of `Guardian.Plug.VerifyHeader` is deprecated please use `:scheme` instead. +* `Guardian.Plug.VerifyCookie` is deprecated in favor of `:refresh_from_cookie` option in +`Guardian.Plug.VerifyHeader` and `Guardian.Plug.VerifySession` ## v2.1.2 ### Enhancement diff --git a/lib/guardian/plug/verify_header.ex b/lib/guardian/plug/verify_header.ex index 4d599c669..16fc9bf56 100644 --- a/lib/guardian/plug/verify_header.ex +++ b/lib/guardian/plug/verify_header.ex @@ -94,14 +94,8 @@ if Code.ensure_loaded?(Plug) do |> Guardian.Plug.put_current_token(token, key: key) |> Guardian.Plug.put_current_claims(claims, key: key) else - :no_token_found -> - conn - - {:error, reason} -> - handle_error(conn, reason, opts) - - _ -> - conn + error -> + handle_error(conn, error, opts) end end @@ -127,25 +121,25 @@ if Code.ensure_loaded?(Plug) do end end - defp handle_error(conn, :token_expired = reason, opts) do + defp handle_error(conn, error, opts) do if refresh_from_cookie_opts = fetch_refresh_from_cookie_options(opts) do Guardian.Plug.VerifyCookie.refresh_from_cookie(conn, refresh_from_cookie_opts) else - apply_error(conn, reason, opts) + apply_error(conn, error, opts) end end - defp handle_error(conn, reason, opts) do - apply_error(conn, reason, opts) - end - - defp apply_error(conn, reason, opts) do + defp apply_error(conn, {:error, reason}, opts) do conn |> Pipeline.fetch_error_handler!(opts) |> apply(:auth_error, [conn, {:invalid_token, reason}, opts]) |> Guardian.Plug.maybe_halt(opts) end + defp apply_error(conn, _, _) do + conn + end + defp fetch_refresh_from_cookie_options(opts) do case Keyword.get(opts, :refresh_from_cookie) do value when is_list(value) -> value diff --git a/lib/guardian/plug/verify_session.ex b/lib/guardian/plug/verify_session.ex index 3ed832694..1f502724a 100644 --- a/lib/guardian/plug/verify_session.ex +++ b/lib/guardian/plug/verify_session.ex @@ -73,36 +73,30 @@ if Code.ensure_loaded?(Plug) do |> Guardian.Plug.put_current_token(token, key: key) |> Guardian.Plug.put_current_claims(claims, key: key) else - :no_token_found -> - conn - - {:error, reason} -> - handle_error(conn, reason, opts) - - _ -> - conn + error -> + handle_error(conn, error, opts) end end - defp handle_error(conn, :token_expired = reason, opts) do + defp handle_error(conn, error, opts) do if refresh_from_cookie_opts = fetch_refresh_from_cookie_options(opts) do Guardian.Plug.VerifyCookie.refresh_from_cookie(conn, refresh_from_cookie_opts) else - apply_error(conn, reason, opts) + apply_error(conn, error, opts) end end - defp handle_error(conn, reason, opts) do - apply_error(conn, reason, opts) - end - - defp apply_error(conn, reason, opts) do + defp apply_error(conn, {:error, reason}, opts) do conn |> Pipeline.fetch_error_handler!(opts) |> apply(:auth_error, [conn, {:invalid_token, reason}, opts]) |> Guardian.Plug.maybe_halt(opts) end + defp apply_error(conn, _, _) do + conn + end + defp fetch_refresh_from_cookie_options(opts) do case Keyword.get(opts, :refresh_from_cookie) do value when is_list(value) -> value diff --git a/test/guardian/plug/verify_header_test.exs b/test/guardian/plug/verify_header_test.exs index 8fb8fba09..64e0d136e 100644 --- a/test/guardian/plug/verify_header_test.exs +++ b/test/guardian/plug/verify_header_test.exs @@ -305,8 +305,27 @@ defmodule Guardian.Plug.VerifyHeaderTest do |> Pipeline.put_error_handler(ctx.handler) |> VerifyHeader.call(refresh_from_cookie: [module: ctx.impl]) - assert conn.status == 401 - assert conn.halted + refute conn.halted + assert new_access_token = Guardian.Plug.current_token(conn) + assert {:ok, _} = apply(ctx.impl, :decode_and_verify, [new_access_token]) + assert %{"sub" => "User:jane", "typ" => "access"} = Guardian.Plug.current_claims(conn) + end + + test "when no header found", ctx do + {:ok, refresh_token, _} = apply(ctx.impl, :encode_and_sign, [%{id: "jane"}, %{}, [token_type: "refresh"]]) + + conn = + :get + |> conn("/") + |> put_req_cookie("guardian_default_token", refresh_token) + |> Pipeline.put_module(ctx.impl) + |> Pipeline.put_error_handler(ctx.handler) + |> VerifyHeader.call(refresh_from_cookie: [module: ctx.impl]) + + refute conn.halted + assert new_access_token = Guardian.Plug.current_token(conn) + assert {:ok, _} = apply(ctx.impl, :decode_and_verify, [new_access_token]) + assert %{"sub" => "User:jane", "typ" => "access"} = Guardian.Plug.current_claims(conn) end end end diff --git a/test/guardian/plug/verify_session_test.exs b/test/guardian/plug/verify_session_test.exs index 8e676cc3e..a4b2bb70c 100644 --- a/test/guardian/plug/verify_session_test.exs +++ b/test/guardian/plug/verify_session_test.exs @@ -312,8 +312,28 @@ defmodule Guardian.Plug.VerifySessionTest do |> Pipeline.put_error_handler(ctx.handler) |> VerifySession.call(refresh_from_cookie: []) - assert conn.status == 401 - assert conn.halted + refute conn.halted + assert new_access_token = Guardian.Plug.current_token(conn) + assert {:ok, _} = apply(ctx.impl, :decode_and_verify, [new_access_token]) + assert %{"sub" => "User:jane", "typ" => "access"} = Guardian.Plug.current_claims(conn) + end + + test "when no session found", ctx do + {:ok, refresh_token, _} = apply(ctx.impl, :encode_and_sign, [%{id: "jane"}, %{}, [token_type: "refresh"]]) + + conn = + :get + |> conn("/") + |> put_req_cookie("guardian_default_token", refresh_token) + |> init_test_session(%{}) + |> Pipeline.put_module(ctx.impl) + |> Pipeline.put_error_handler(ctx.handler) + |> VerifySession.call(refresh_from_cookie: []) + + refute conn.halted + assert new_access_token = Guardian.Plug.current_token(conn) + assert {:ok, _} = apply(ctx.impl, :decode_and_verify, [new_access_token]) + assert %{"sub" => "User:jane", "typ" => "access"} = Guardian.Plug.current_claims(conn) end end end