chore: improve authentication handling

Using the admin UI was previously plagued by auth errors that would
occur after just a few minutes of idle time and required refreshing the
page, losing any work in progress.

These changes should improve how we handle authentication, allowing an
admin to be idle for up to 30 minutes and signed in for up to 12 hours
before needing to reauthenticate, following the TID Session Management
guidelines and the prior art of mbta/screenplay#520.

Also, when an admin's session has expired and they try to load new data
in the admin UI, they will get a message indicating they need to refresh
the page, instead of the generic "an error occurred".

assets/src/util/admin.tsx

@@ -27,7 +27,7 @@ const gatherSelectOptions = (rows, columnId) => {
 
 const doSubmit = async (path, data) => {
   try {
-    const result = await fetch(path, {
+    const response = await fetch(path, {
       method: "POST",
       headers: {
         "content-type": "application/json",
@@ -36,11 +36,17 @@ const doSubmit = async (path, data) => {
       credentials: "include",
       body: JSON.stringify(data),
     });
-    const json = await result.json();
-    return json;
-  } catch (err) {
-    alert("An error occurred.");
-    throw err;
+
+    if (response.status === 401) {
+      alert("Your session has expired; refresh the page to continue.");
+      throw new Error("unauthenticated");
+    } else {
+      const json = await response.json();
+      return json;
+    }
+  } catch (error) {
+    alert(`An error occurred: ${error}`);
+    throw error;
   }
 };
 

config/config.exs

@@ -44,12 +44,25 @@ config :screens,
   redirect_http?: true,
   keycloak_role: "screens-admin"
 
-config :screens, ScreensWeb.AuthManager, issuer: "screens"
+max_session_time = 12 * 60 * 60
+
+config :screens, ScreensWeb.AuthManager,
+  idle_time: 30 * 60,
+  issuer: "screens",
+  max_session_time: max_session_time
 
 # Placeholder for Keycloak authentication, defined for real in environment configs
 config :ueberauth, Ueberauth,
   providers: [
-    keycloak: nil
+    keycloak: {
+      Ueberauth.Strategy.Oidcc,
+      authorization_params: %{max_age: "#{max_session_time}"},
+      authorization_params_passthrough: ~w(prompt login_hint),
+      issuer: :keycloak_issuer,
+      scopes: ~w(openid email),
+      uid_field: "email",
+      userinfo: true
+    }
   ]
 
 config :ex_cldr,

config/prod.exs

@@ -35,13 +35,6 @@ config :screens, :screens_by_alert,
   screens_last_updated_ttl_seconds: 3600,
   screens_ttl_seconds: 40
 
-# Configure Ueberauth to use Keycloak
-config :ueberauth, Ueberauth,
-  providers: [
-    keycloak:
-      {Ueberauth.Strategy.Oidcc, userinfo: true, uid_field: "email", scopes: ~w(openid email)}
-  ]
-
 # ## SSL Support
 #
 # To get SSL working, you will need to add the `https` key

config/runtime.exs

@@ -45,12 +45,6 @@ if config_env() == :prod do
       coder: Screens.ScreensByAlert.Memcache.SafeErlangCoder
     ]
 
-  keycloak_opts = [
-    issuer: :keycloak_issuer,
-    client_id: System.fetch_env!("KEYCLOAK_CLIENT_ID"),
-    client_secret: System.fetch_env!("KEYCLOAK_CLIENT_SECRET")
-  ]
-
   config :ueberauth_oidcc,
     issuers: [
       %{
@@ -59,7 +53,10 @@ if config_env() == :prod do
       }
     ],
     providers: [
-      keycloak: keycloak_opts
+      keycloak: [
+        client_id: System.fetch_env!("KEYCLOAK_CLIENT_ID"),
+        client_secret: System.fetch_env!("KEYCLOAK_CLIENT_SECRET")
+      ]
     ]
 end
 

lib/screens/ueberauth/strategy/fake.ex

@@ -40,6 +40,9 @@ defmodule Screens.Ueberauth.Strategy.Fake do
   def extra(conn) do
     %Ueberauth.Auth.Extra{
       raw_info: %UeberauthOidcc.RawInfo{
+        claims: %{
+          "iat" => System.system_time(:second)
+        },
         userinfo: %{
           "resource_access" => %{
             "dev-client" => %{"roles" => Ueberauth.Strategy.Helpers.options(conn)[:roles]}

lib/screens_web/auth_manager.ex

@@ -3,6 +3,9 @@ defmodule ScreensWeb.AuthManager do
 
   use Guardian, otp_app: :screens
 
+  @idle_time Application.compile_env!(:screens, [__MODULE__, :idle_time])
+  @max_session_time Application.compile_env!(:screens, [__MODULE__, :max_session_time])
+
   @impl true
   def subject_for_token(resource, _claims) do
     {:ok, resource}
@@ -14,4 +17,17 @@ defmodule ScreensWeb.AuthManager do
   end
 
   def resource_from_claims(_), do: {:error, :invalid_claims}
+
+  @impl true
+  def verify_claims(%{"auth_time" => user_authed_at, "iat" => token_issued_at} = claims, _opts) do
+    auth_expires_at = user_authed_at + @max_session_time
+    token_expires_at = token_issued_at + @idle_time
+
+    # is either expiration time in the past?
+    if min(auth_expires_at, token_expires_at) < System.system_time(:second) do
+      {:error, {:auth_expired, claims["sub"]}}
+    else
+      {:ok, claims}
+    end
+  end
 end

lib/screens_web/auth_manager/error_handler.ex

@@ -3,11 +3,23 @@ defmodule ScreensWeb.AuthManager.ErrorHandler do
 
   @behaviour Guardian.Plug.ErrorHandler
 
-  @impl Guardian.Plug.ErrorHandler
-  def auth_error(conn, {_type, _reason}, _opts) do
-    Phoenix.Controller.redirect(
-      conn,
-      to: ScreensWeb.Router.Helpers.auth_path(conn, :request, "keycloak")
-    )
+  alias Phoenix.Controller
+  alias ScreensWeb.Router.Helpers, as: Routes
+
+  @impl true
+  def auth_error(conn, error, _opts) do
+    case Controller.get_format(conn) do
+      "html" ->
+        Controller.redirect(conn,
+          to: Routes.auth_path(conn, :request, "keycloak", auth_params(error))
+        )
+
+      "json" ->
+        Plug.Conn.send_resp(conn, 401, "unauthenticated")
+    end
   end
+
+  defp auth_params({:invalid_token, {:auth_expired, sub}}), do: [prompt: "login", login_hint: sub]
+  defp auth_params({:unauthenticated, _}), do: []
+  defp auth_params(_error), do: [prompt: "login"]
 end

lib/screens_web/auth_manager/pipeline.ex

@@ -11,6 +11,23 @@ defmodule ScreensWeb.AuthManager.Pipeline do
   plug(Guardian.Plug.VerifySession, claims: %{"typ" => "access"})
   plug(Guardian.Plug.VerifyHeader, claims: %{"typ" => "access"})
   plug(Guardian.Plug.LoadResource, allow_blank: true)
+  plug :refresh_idle_token
+
+  @doc """
+  Refresh the token with each request.
+  This allows us to use the `iat` time in the token as an idle timeout.
+  """
+  def refresh_idle_token(conn, _opts) do
+    old_token = Guardian.Plug.current_token(conn)
+
+    case ScreensWeb.AuthManager.refresh(old_token) do
+      {:ok, _old, {new_token, _new_claims}} ->
+        Guardian.Plug.put_session_token(conn, new_token)
+
+      _ ->
+        conn
+    end
+  end
 
   def save_previous_path(
         %Plug.Conn{query_string: query_string, request_path: request_path} = conn,

lib/screens_web/controllers/auth_controller.ex

@@ -1,18 +1,23 @@
 defmodule ScreensWeb.AuthController do
-  require Logger
-
   use ScreensWeb, :controller
   plug Ueberauth
 
+  alias Screens.Log
+
   # Respond with 404 instead of crashing when the path doesn't match a supported provider
   def request(conn, %{"provider" => provider}) when provider != "keycloak" do
     send_resp(conn, 404, "Not Found")
   end
 
-  def callback(%{assigns: %{ueberauth_auth: auth}} = conn, _params) do
+  def callback(%{assigns: %{ueberauth_auth: %{provider: :keycloak} = auth}} = conn, _params) do
     username = auth.uid
-    expiration = auth.credentials.expires_at
-    current_time = System.system_time(:second)
+
+    auth_time =
+      Map.get(
+        auth.extra.raw_info.claims,
+        "auth_time",
+        auth.extra.raw_info.claims["iat"]
+      )
 
     keycloak_client_id =
       get_in(Application.get_env(:ueberauth_oidcc, :providers), [:keycloak, :client_id])
@@ -23,12 +28,12 @@ defmodule ScreensWeb.AuthController do
     redirect_to = Plug.Conn.get_session(conn, :previous_path, ~p"/admin")
 
     conn
-    |> Plug.Conn.delete_session(:previous_path)
+    |> configure_session(drop: true)
     |> Guardian.Plug.sign_in(
       ScreensWeb.AuthManager,
       username,
-      %{roles: roles},
-      ttl: {expiration - current_time, :seconds}
+      %{auth_time: auth_time, roles: roles},
+      ttl: {30, :minutes}
     )
     |> redirect(to: redirect_to)
   end
@@ -45,7 +50,7 @@ defmodule ScreensWeb.AuthController do
       end)
       |> Enum.join(", ")
 
-    Logger.info("[ueberauth_failure] messages=\"#{error_messages}\"")
+    Log.warning("ueberauth_failure", messages: error_messages)
 
     send_resp(conn, 401, "unauthenticated")
   end

mix.exs

@@ -80,7 +80,7 @@ defmodule Screens.MixProject do
       {:hackney, "== 1.20.1"},
       {:guardian, "~> 2.3.1"},
       {:ueberauth, "~> 0.10"},
-      {:ueberauth_oidcc, "~> 0.3"},
+      {:ueberauth_oidcc, "~> 0.4"},
       {:corsica, "~> 2.1"},
       {:lcov_ex, "~> 0.2", only: [:dev, :test], runtime: false},
       {:sentry, "~> 10.4"},

test/screens_web/auth_manager/error_handler_test.exs

@@ -6,9 +6,10 @@ defmodule ScreensWeb.AuthManager.ErrorHandlerTest do
       conn =
         conn
         |> init_test_session(%{})
+        |> Phoenix.Controller.put_format("html")
         |> ScreensWeb.AuthManager.ErrorHandler.auth_error({:some_type, :reason}, [])
 
-      assert html_response(conn, 302) =~ "\"/auth/keycloak\""
+      assert redirected_to(conn) =~ "/auth/keycloak?prompt=login"
     end
   end
 end

test/screens_web/controllers/auth_controller_test.exs

@@ -1,17 +1,23 @@
 defmodule ScreensWeb.Controllers.AuthControllerTest do
   use ScreensWeb.ConnCase
 
+  import ExUnit.CaptureLog
+
   describe "callback" do
     test "redirects on success and saves refresh token", %{conn: conn} do
       current_time = System.system_time(:second)
 
       auth = %Ueberauth.Auth{
+        provider: :keycloak,
         uid: "[email protected]",
         credentials: %Ueberauth.Auth.Credentials{
           expires_at: current_time + 1_000
         },
-        extra: %{
-          raw_info: %{
+        extra: %Ueberauth.Auth.Extra{
+          raw_info: %UeberauthOidcc.RawInfo{
+            claims: %{
+              "iat" => System.system_time(:second)
+            },
             userinfo: %{
               "resource_access" => %{
                 "test-client" => %{"roles" => ["screens-admin"]}
@@ -33,14 +39,17 @@ defmodule ScreensWeb.Controllers.AuthControllerTest do
     end
 
     test "handles generic failure", %{conn: conn} do
-      conn =
-        conn
-        |> assign(:ueberauth_failure, %Ueberauth.Failure{})
-        |> get(ScreensWeb.Router.Helpers.auth_path(conn, :callback, "keycloak"))
+      logs =
+        capture_log([level: :warning], fn ->
+          conn =
+            conn
+            |> assign(:ueberauth_failure, %Ueberauth.Failure{})
+            |> get(ScreensWeb.Router.Helpers.auth_path(conn, :callback, "keycloak"))
 
-      response = response(conn, 401)
+          assert response(conn, 401) =~ "unauthenticated"
+        end)
 
-      assert response =~ "unauthenticated"
+      assert logs =~ "ueberauth_failure"
     end
   end