SAML configurable attr fields (#3011)

* Make saml user profile attribute names configurable

* Add the configuration in runtime execution

* Update tests to avoid concurrency issues

config/config.exs

@@ -88,7 +88,15 @@ config :trento, :pow_assent, user_identities_context: Trento.UserIdentities
 
 config :trento, :oidc, enabled: false
 config :trento, :oauth2, enabled: false
-config :trento, :saml, enabled: false
+
+config :trento, :saml,
+  enabled: false,
+  user_profile_attributes: %{
+    username_field: "username",
+    email_field: "email",
+    first_name_field: "firstName",
+    last_name_field: "lastName"
+  }
 
 # Agent heartbeat interval. Adding one extra second to the agent 5s interval to avoid glitches
 config :trento, Trento.Heartbeats, interval: :timer.seconds(6)

config/runtime.exs

@@ -259,7 +259,13 @@ if config_env() in [:prod, :demo] do
       callback_url: "/auth/saml_callback",
       idp_id:
         System.get_env("SAML_IDP_ID") ||
-          raise("environment variable SAML_IDP_ID is missing")
+          raise("environment variable SAML_IDP_ID is missing"),
+      user_profile_attributes: %{
+        username_field: System.get_env("SAML_USERNAME_ATTR_NAME", "username"),
+        email_field: System.get_env("SAML_EMAIL_ATTR_NAME", "email"),
+        first_name_field: System.get_env("SAML_FIRSTNAME_ATTR_NAME", "firstName"),
+        last_name_field: System.get_env("SAML_LASTNAME_ATTR_NAME", "lastName")
+      }
 
     config :trento, :pow_assent,
       providers: [

lib/trento_web/auth/assent_saml_strategy.ex

@@ -18,7 +18,23 @@ defmodule TrentoWeb.Auth.AssentSamlStrategy do
   end
 
   def callback(_config, %{attributes: attributes}) do
-    case normalize(attributes) do
+    %{
+      username_field: username_field,
+      email_field: email_field,
+      first_name_field: first_name_field,
+      last_name_field: last_name_field
+    } = user_profile_attributes()
+
+    expected_attributes =
+      Map.new(attributes, fn
+        {^username_field, username} -> {:username, username}
+        {^email_field, email} -> {:email, email}
+        {^first_name_field, first_name} -> {:first_name, first_name}
+        {^last_name_field, last_name} -> {:last_name, last_name}
+        other -> other
+      end)
+
+    case normalize(expected_attributes) do
       {:ok, user} ->
         {:ok, %{user: user, token: %{}}}
 
@@ -28,10 +44,10 @@ defmodule TrentoWeb.Auth.AssentSamlStrategy do
   end
 
   defp normalize(%{
-         "username" => username,
-         "email" => email,
-         "firstName" => first_name,
-         "lastName" => last_name
+         username: username,
+         email: email,
+         first_name: first_name,
+         last_name: last_name
        }) do
     {:ok,
      %{
@@ -46,4 +62,7 @@ defmodule TrentoWeb.Auth.AssentSamlStrategy do
   defp normalize(%{}) do
     {:error, :user_attributes_missing}
   end
+
+  defp user_profile_attributes,
+    do: Application.fetch_env!(:trento, :saml)[:user_profile_attributes]
 end

test/trento_web/auth/assent_saml_strategy_test.exs

@@ -13,6 +13,18 @@ defmodule TrentoWeb.Auth.AssentSamlStrategyTest do
   end
 
   describe "callback/2" do
+    setup do
+      Application.put_env(:trento, :saml,
+        enabled: false,
+        user_profile_attributes: %{
+          username_field: "username",
+          email_field: "email",
+          first_name_field: "firstName",
+          last_name_field: "lastName"
+        }
+      )
+    end
+
     test "should return a normalized user" do
       username = Faker.Internet.user_name()
       email = Faker.Internet.email()

test/trento_web/controllers/session_controller_test.exs

@@ -638,6 +638,30 @@ defmodule TrentoWeb.SessionControllerTest do
       |> json_response(401)
       |> assert_schema("Unauthorized", api_spec)
     end
+  end
+
+  describe "saml_callback endpoint" do
+    setup %{conn: conn} = context do
+      Samly.State.init(Samly.State.ETS)
+
+      Application.put_env(:trento, :saml,
+        enabled: false,
+        user_profile_attributes: %{
+          username_field: "username",
+          email_field: "email",
+          first_name_field: "firstName",
+          last_name_field: "lastName"
+        }
+      )
+
+      conn =
+        conn
+        |> Plug.Conn.put_private(:plug_session, %{})
+        |> Plug.Conn.put_private(:plug_session_fetch, :done)
+        |> Pow.Plug.put_config(otp_app: :trento)
+
+      Map.put(context, :conn, conn)
+    end
 
     test "should return the credentials when the saml callback flow is completed without errors and the user does not exist on trento",
          %{conn: conn, api_spec: api_spec} do
@@ -650,8 +674,6 @@ defmodule TrentoWeb.SessionControllerTest do
         end
       )
 
-      Samly.State.init(Samly.State.ETS)
-
       username = Faker.Internet.user_name()
 
       not_on_or_after =
@@ -710,8 +732,6 @@ defmodule TrentoWeb.SessionControllerTest do
 
     test "should return unauthorized in saml callback flow when user attributes are missing",
          %{conn: conn, api_spec: api_spec} do
-      Samly.State.init(Samly.State.ETS)
-
       not_on_or_after =
         DateTime.utc_now()
         |> DateTime.add(8, :hour)