Fixes the tests

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

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{

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: "[email protected]",
-    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=\"[email protected]\""
-      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: "[email protected]"})
-          |> 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

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

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

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