feat: require a read-only role

This allows us to ensure Keycloak users from non-MBTA accounts do not have access.

config/config.exs

@@ -22,6 +22,7 @@ config :arrow,
   },
   ueberauth_provider: :cognito,
   required_roles: %{
+    view_disruption: ["read-only", "admin"],
     create_disruption: ["admin"],
     update_disruption: ["admin"],
     delete_disruption: ["admin"],
@@ -63,7 +64,11 @@ config :ueberauth, Ueberauth,
     keycloak: {
       Ueberauth.Strategy.OIDC,
       # userinfo is needed for roles
-      fetch_userinfo: true, uid_field: "email", response_type: "code", scope: "openid email"
+      fetch_userinfo: true,
+      userinfo_uid_field: "email",
+      uid_field: "email",
+      response_type: "code",
+      scope: "openid email"
     }
   ]
 

lib/arrow/permissions.ex

@@ -7,7 +7,8 @@ defmodule Arrow.Permissions do
   @required_roles Application.compile_env!(:arrow, :required_roles)
 
   @type action() ::
-          :create_disruption
+          :view_disruption
+          | :create_disruption
           | :update_disruption
           | :delete_disruption
           | :use_api

lib/arrow_web/controllers/auth_controller.ex

@@ -37,7 +37,8 @@ defmodule ArrowWeb.AuthController do
       ArrowWeb.AuthManager,
       username,
       %{
-        roles: roles
+        # all cognito users have read-only access
+        roles: roles ++ ["read-only"]
       },
       ttl: {expiration - current_time, :seconds}
     )
@@ -50,7 +51,7 @@ defmodule ArrowWeb.AuthController do
     expiration = auth.credentials.expires_at
     current_time = System.system_time(:second)
 
-    roles = auth.extra.raw_info.userinfo["roles"]
+    roles = auth.extra.raw_info.userinfo["roles"] || []
     logout_url = logout_url(auth)
 
     conn

lib/arrow_web/controllers/disruption_controller.ex

@@ -8,6 +8,7 @@ defmodule ArrowWeb.DisruptionController do
   alias Ecto.Changeset
   alias Plug.Conn
 
+  plug(Authorize, :view_disruption when action in [:index, :show])
   plug(Authorize, :create_disruption when action in [:new, :create])
   plug(Authorize, :update_disruption when action in [:edit, :update, :update_row_status])
   plug(Authorize, :delete_disruption when action in [:delete])

test/arrow_web/controllers/auth_controller_test.exs

@@ -22,7 +22,7 @@ defmodule ArrowWeb.Controllers.AuthControllerTest do
       response = html_response(conn, 302)
 
       assert response =~ Routes.disruption_path(conn, :index)
-      assert Guardian.Plug.current_claims(conn)["roles"] == ["admin"]
+      assert Enum.sort(Guardian.Plug.current_claims(conn)["roles"]) == ["admin", "read-only"]
       assert get_session(conn, :arrow_username) == "[email protected]"
     end
 
@@ -48,7 +48,7 @@ defmodule ArrowWeb.Controllers.AuthControllerTest do
       conn =
         conn
         |> assign(:ueberauth_auth, auth)
-        |> get(Routes.auth_path(conn, :callback, "cognito"))
+        |> get(Routes.auth_path(conn, :callback, "keycloak"))
 
       response = html_response(conn, 302)
 
@@ -57,6 +57,35 @@ defmodule ArrowWeb.Controllers.AuthControllerTest do
       assert get_session(conn, :arrow_username) == "[email protected]"
     end
 
+    test "handles missing roles (keycloak)", %{conn: conn} do
+      current_time = System.system_time(:second)
+
+      auth = %Ueberauth.Auth{
+        uid: "[email protected]",
+        provider: :keycloak,
+        credentials: %Ueberauth.Auth.Credentials{
+          expires_at: current_time + 1_000,
+          other: %{id_token: "id_token"}
+        },
+        extra: %{
+          raw_info: %{
+            userinfo: %{}
+          }
+        }
+      }
+
+      conn =
+        conn
+        |> assign(:ueberauth_auth, auth)
+        |> get(Routes.auth_path(conn, :callback, "keycloak"))
+
+      response = html_response(conn, 302)
+
+      assert response =~ Routes.disruption_path(conn, :index)
+      assert Guardian.Plug.current_claims(conn)["roles"] == []
+      assert Guardian.Plug.current_resource(conn) == "[email protected]"
+    end
+
     test "handles generic failure", %{conn: conn} do
       conn =
         conn

test/arrow_web/plug/assign_user_test.exs

@@ -18,9 +18,9 @@ defmodule ArrowWeb.Plug.AssignUserTest do
     end
 
     @tag :authenticated
-    test "loads a non-admin into the connection when user has no roles", %{conn: conn} do
+    test "loads a non-admin into the connection when user is not an admin", %{conn: conn} do
       assert AssignUser.call(conn, []).assigns == %{
-               current_user: %User{id: "test_user", roles: MapSet.new()}
+               current_user: %User{id: "test_user", roles: MapSet.new(["read-only"])}
              }
     end
   end

test/support/conn_case.ex

@@ -39,7 +39,7 @@ defmodule ArrowWeb.ConnCase do
 
     cond do
       tags[:authenticated] ->
-        {:ok, conn: build_conn("test_user")}
+        {:ok, conn: build_conn("test_user", ["read-only"])}
 
       tags[:authenticated_admin] ->
         {:ok, conn: build_conn("test_user", ["admin"])}
@@ -53,7 +53,7 @@ defmodule ArrowWeb.ConnCase do
   end
 
   @spec build_conn(String.t(), [String.t()] | []) :: Plug.Conn.t()
-  defp build_conn(user, roles \\ []) do
+  defp build_conn(user, roles) do
     Phoenix.ConnTest.build_conn()
     |> Plug.Conn.put_req_header("x-forwarded-proto", "https")
     |> init_test_session(%{arrow_username: user})