diff --git a/lib/document_viewer_web/controllers/auth_controller.ex b/lib/document_viewer_web/controllers/auth_controller.ex index 9921aaa..17853cd 100644 --- a/lib/document_viewer_web/controllers/auth_controller.ex +++ b/lib/document_viewer_web/controllers/auth_controller.ex @@ -38,6 +38,7 @@ defmodule DocumentViewerWeb.AuthController do roles = extra.raw_info.userinfo["resource_access"][client_id]["roles"] || [] UserActionLogger.log(username, :login) + time_left = expiration - current_time if @document_viewer_role in roles do diff --git a/lib/document_viewer_web/ueberauth/strategy/fake.ex b/lib/document_viewer_web/ueberauth/strategy/fake.ex index 6a4fcb2..d26f8d7 100644 --- a/lib/document_viewer_web/ueberauth/strategy/fake.ex +++ b/lib/document_viewer_web/ueberauth/strategy/fake.ex @@ -6,6 +6,25 @@ defmodule DocumentViewerWeb.Ueberauth.Strategy.Fake do use Ueberauth.Strategy @impl Ueberauth.Strategy + + @doc """ + We want to be able to test different responses from keycloak. In order to do that we need + this fake strategy to return different things in different tests. Unfortunately Ueberauth + doesn't provide a great way to do that so we are hacking our way into that by providing + query params that do different things when detected. You may use the following: + + user_type=no_valid_role - this will return a user without the correct roles. + + See authenticated_no_valid_role/1 in DocumentViewerWeb.ConnCase for more. + """ + def handle_request!(%{params: %{"user_type" => "no_valid_role"}} = conn) do + params = Ueberauth.Strategy.Helpers.with_state_param([], conn) + + conn + |> redirect!(~p"/auth/keycloak/callback?#{params}&user_type=no_valid_role") + |> halt() + end + def handle_request!(conn) do # Ueberauth does a thing to check for CSRF attacks. It essentially adds a state param # that gets checked by ueberauth. This ensures we add it correctly in this fake strategy. @@ -27,11 +46,13 @@ defmodule DocumentViewerWeb.Ueberauth.Strategy.Fake do end @impl Ueberauth.Strategy - def handle_callback!(conn) do - conn - end + def handle_callback!(conn), do: conn @impl Ueberauth.Strategy + def uid(%{params: %{"user_type" => username}}) do + username + end + def uid(_conn) do "fake_uid" end @@ -51,6 +72,20 @@ defmodule DocumentViewerWeb.Ueberauth.Strategy.Fake do %Ueberauth.Auth.Info{} end + @impl Ueberauth.Strategy + def extra(%{params: %{"user_type" => "no_valid_role"}}) do + %Ueberauth.Auth.Extra{ + raw_info: %{ + claims: %{"aud" => "fake_aud"}, + userinfo: %{ + "resource_access" => %{ + "fake_aud" => %{"roles" => ["admin"]} + } + } + } + } + end + @impl Ueberauth.Strategy def extra(_conn) do %Ueberauth.Auth.Extra{ diff --git a/test/document_viewer_web/controllers/auth_controller_test.exs b/test/document_viewer_web/controllers/auth_controller_test.exs index 0350d8d..21d4539 100644 --- a/test/document_viewer_web/controllers/auth_controller_test.exs +++ b/test/document_viewer_web/controllers/auth_controller_test.exs @@ -1,16 +1,10 @@ defmodule DocumentViewerWeb.AuthControllerTest do - use DocumentViewerWeb.ConnCase + # This is not async because we change the logger level in here which could lead to + # flaky tests elsewhere. + use DocumentViewerWeb.ConnCase, async: false use Plug.Test import ExUnit.CaptureLog - @mock_auth %Ueberauth.Auth{ - uid: "test@mbta.com", - credentials: %Ueberauth.Auth.Credentials{ - expires_at: System.system_time(:second) + 1_000, - other: %{roles: ["test-group"]} - } - } - describe "GET /auth/:provider" do test "redirects to the callback", %{conn: conn} do conn = get(conn, ~p"/auth/keycloak") @@ -22,9 +16,7 @@ defmodule DocumentViewerWeb.AuthControllerTest do @tag capture_log: true test "redirects to the index page for an ueberauth auth", %{conn: conn} do conn = conn |> get(~p"/auth/keycloak") - assert "/auth/keycloak/callback?state=" <> _state = path = redirected_to(conn) - conn = conn |> get(path) assert %{ @@ -41,15 +33,17 @@ defmodule DocumentViewerWeb.AuthControllerTest do end test "logs a successful login", %{conn: conn} do + Logger.configure(level: :info) + log = capture_log(fn -> conn = conn |> get(~p"/auth/keycloak") - path = conn |> redirected_to() + assert "/auth/keycloak/callback?state=" <> _state = path = redirected_to(conn) get(conn, path) end) - assert log =~ "username=\"test@mbta.com\"" - assert log =~ "action=:login" + assert log =~ "User action: action=:login, username=\"fake_uid\"" + Logger.configure(level: :warning) end @tag capture_log: true @@ -68,15 +62,10 @@ defmodule DocumentViewerWeb.AuthControllerTest do test "logs ueberauth failures", %{conn: conn} do log = capture_log(fn -> - conn - |> Plug.Test.init_test_session(%{username: "test@mbta.com"}) - |> assign(:ueberauth_failure, %Ueberauth.Failure{ - errors: [%Ueberauth.Failure.Error{message: "failed"}] - }) - |> get(~p"/auth/keycloak/callback") + conn |> get(~p"/auth/keycloak/callback") end) - assert log =~ "Ueberauth error: failed" + assert log =~ "Ueberauth error: Cross-Site Request Forgery attack" end end end diff --git a/test/document_viewer_web/controllers/document_controller_test.exs b/test/document_viewer_web/controllers/document_controller_test.exs index 0b33ee0..a0a57ff 100644 --- a/test/document_viewer_web/controllers/document_controller_test.exs +++ b/test/document_viewer_web/controllers/document_controller_test.exs @@ -1,5 +1,6 @@ defmodule DocumentViewerWeb.DocumentControllerTest do - use DocumentViewerWeb.ConnCase + # We re-assign log level so this has to by synchronous. + use DocumentViewerWeb.ConnCase, async: false import ExUnit.CaptureLog import Test.Support.Helpers @@ -32,17 +33,25 @@ defmodule DocumentViewerWeb.DocumentControllerTest do test "unauthenticated, redirects you to keycloak auth", %{conn: conn} do conn = get(conn, "/documents/BUCKET/FILE.pdf") - assert redirected_to(conn) == "/auth/keycloak" end - @tag :authenticated_no_valid_role test "authenticated not in document-viewer group, redirects you to mbta.com", %{ conn: conn } do - conn = get(conn, "/documents/BUCKET/FILE.pdf") + # If you attempt to sign in without the correct role the sign in fails. Which means + # if you then attempt to go to a route that requires sign in, you will be redirected + # back to the log in route. + log = + capture_log(fn -> + # This is defined in the `__using__` from `use DocumentViewerWeb.ConnCase` + conn = authenticated_no_valid_role(conn) + assert redirected_to(conn) == "https://www.mbta.com" + conn = get(conn, "/documents/BUCKET/FILE.pdf") + assert redirected_to(conn) == "/auth/keycloak" + end) - assert redirected_to(conn) == "https://www.mbta.com" + assert log =~ "[warning] Document viewer role not found in the roles for user: admin" end end @@ -98,13 +107,20 @@ defmodule DocumentViewerWeb.DocumentControllerTest do assert redirected_to(conn) == "/auth/keycloak" end - @tag :authenticated_no_valid_role test "authenticated not in document-viewer group, redirects you to mbta.com", %{ conn: conn } do - conn = get(conn, "/documents/BUCKET/FILE.pdf/pdf") + log = + capture_log(fn -> + # This is defined in the `__using__` from `use DocumentViewerWeb.ConnCase` + conn = authenticated_no_valid_role(conn) + assert redirected_to(conn) == "https://www.mbta.com" + + conn = get(conn, "/documents/BUCKET/FILE.pdf/pdf") + assert redirected_to(conn) == "/auth/keycloak" + end) - assert redirected_to(conn) == "https://www.mbta.com" + assert log =~ "[warning] Document viewer role not found in the roles for user: admin" end end end diff --git a/test/document_viewer_web/controllers/query_controller_test.exs b/test/document_viewer_web/controllers/query_controller_test.exs index 41982db..986f501 100644 --- a/test/document_viewer_web/controllers/query_controller_test.exs +++ b/test/document_viewer_web/controllers/query_controller_test.exs @@ -16,13 +16,20 @@ defmodule DocumentViewerWeb.QueryControllerTest do assert redirected_to(conn) == "/auth/keycloak" end - @tag :authenticated_no_valid_role test "authenticated not in document-viewer group, redirects you to mbta.com", %{ conn: conn } do - conn = get(conn, "/") + log = + capture_log(fn -> + # This is defined in the `__using__` from `use DocumentViewerWeb.ConnCase` + conn = authenticated_no_valid_role(conn) + assert redirected_to(conn) == "https://www.mbta.com" - assert redirected_to(conn) == "https://www.mbta.com" + conn = get(conn, "/") + assert redirected_to(conn) == "/auth/keycloak" + end) + + assert log =~ "[warning] Document viewer role not found in the roles for user: admin" end end @@ -75,10 +82,18 @@ defmodule DocumentViewerWeb.QueryControllerTest do assert redirected_to(conn) == "/auth/keycloak" end - @tag capture_log: true test "authenticated not in document-viewer group, redirects you to mbta.com", %{conn: conn} do - conn = post(conn, "/", %{"query" => %{"last_name" => "Ng"}}) - assert redirected_to(conn) == "https://www.mbta.com" + log = + capture_log(fn -> + # This is defined in the `__using__` from `use DocumentViewerWeb.ConnCase` + conn = authenticated_no_valid_role(conn) + assert redirected_to(conn) == "https://www.mbta.com" + + conn = post(conn, "/", %{"query" => %{"last_name" => "Ng"}}) + assert redirected_to(conn) == "/auth/keycloak" + end) + + assert log =~ "[warning] Document viewer role not found in the roles for user: admin" end end end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index fc841e5..0807900 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -28,6 +28,19 @@ defmodule DocumentViewerWeb.ConnCase do # The default endpoint for testing @endpoint DocumentViewerWeb.Endpoint use DocumentViewerWeb, :verified_routes + + @doc """ + Function that logs in as a user without a correct role. Ideally we could just use a + different fake strategy but ueberauth does not provide a good way of swapping them + out on a per test basis. So instead we are hacking in this. + """ + def authenticated_no_valid_role(conn) do + # This username is detected in our fake ueberauth strategy and returns a user + # without the valid roles. + conn = conn |> Phoenix.ConnTest.get("/auth/keycloak?user_type=no_valid_role") + path = conn |> Phoenix.ConnTest.redirected_to() + conn |> Phoenix.ConnTest.get(path) + end end end @@ -47,18 +60,6 @@ defmodule DocumentViewerWeb.ConnCase do {conn, user} - tags[:authenticated_no_valid_role] -> - user = "test_user" - - conn = - Phoenix.ConnTest.build_conn() - |> Plug.Test.init_test_session(%{}) - |> Guardian.Plug.sign_in(DocumentViewerWeb.AuthManager, user, %{roles: []}) - |> Plug.Conn.put_session(:username, user) - |> IO.inspect(limit: :infinity, label: "???????") - - {conn, user} - true -> {Phoenix.ConnTest.build_conn(), nil} end