Take home assignment Gérard

.gitignore

@@ -35,3 +35,5 @@ docker-compose.yml
 node_modules
 .serena
 .playwright-mcp
+mailgun_logger_accounts_events.sql
+*.pem

lib/mailgun_logger/roles/roles.ex

@@ -7,14 +7,23 @@ defmodule MailgunLogger.Roles do
 
   @superuser_role "superuser"
   @admin_role "admin"
+  @member_role "member"
 
   #########################################################
 
-  @default_actions ~w()
+  @all_roles [@superuser_role, @admin_role, @member_role]
+  @assignable_roles Enum.reject(@all_roles, &(&1 == @superuser_role))
 
-  @admin_actions ~w(do_stuff) ++ @default_actions
+  #########################################################
 
-  @superuser_actions ~w() ++ @admin_actions
+  @default_actions ~w()
+  @member_actions ~w(
+  view_events
+  view_event_details
+  edit_profile
+) ++ @default_actions
+  @admin_actions ~w(do_stuff assign_roles) ++ @member_actions
+  @superuser_actions ~w(manage_admins) ++ @admin_actions
 
   #########################################################
 
@@ -36,6 +45,15 @@ defmodule MailgunLogger.Roles do
     |> Repo.one()
   end
 
+  @spec get_roles_by_names([String.t()]) :: [Role.t()]
+  def get_roles_by_names([]), do: []
+
+  def get_roles_by_names(names) do
+    Role
+    |> where([r], r.name in ^names)
+    |> Repo.all()
+  end
+
   @spec get_by_user(User.t()) :: [Role.t()]
   def get_by_user(%User{} = user) do
     user
@@ -55,6 +73,11 @@ defmodule MailgunLogger.Roles do
     Enum.any?(roles, &can?(&1.name, action))
   end
 
+  for action <- @member_actions do
+    action = String.to_atom(action)
+    def can?(@member_role, unquote(action)), do: true
+  end
+
   for action <- @admin_actions do
     action = String.to_atom(action)
     def can?(@admin_role, unquote(action)), do: true
@@ -69,14 +92,37 @@ defmodule MailgunLogger.Roles do
 
   def is?(%User{roles: roles}, :superuser), do: is(roles, "superuser")
   def is?(%User{roles: roles}, :admin), do: is(roles, "admin")
+  def is?(%User{roles: roles}, :member), do: is(roles, "member")
   def is?(_, _), do: raise("Roles.is/2 requires roles to be preloaded")
 
   defp is(roles, role) when is_binary(role), do: Enum.map(roles, & &1.name) |> Enum.member?(role)
 
   def abilities(%User{roles: []}), do: []
-  def abilities(%User{roles: roles}), do: hd(roles) |> abilities()
+
+  def abilities(%User{roles: roles}),
+    do:
+      roles
+      |> Enum.flat_map(&abilities/1)
+      |> Enum.uniq()
+
   def abilities(%Role{name: "admin"}), do: @admin_actions
   def abilities(%Role{name: "superuser"}), do: @superuser_actions
+  def abilities(%Role{name: "member"}), do: @member_actions
 
   def roles(%User{roles: roles}), do: Enum.map(roles, & &1.name)
+
+  @spec can_modify_roles?(User.t(), User.t()) :: boolean()
+  def can_modify_roles?(user, target) do
+    user.id != target.id and
+      Enum.any?([:manage_admins, :assign_roles], fn action ->
+        can?(user, action)
+        # and not can?(target, action)
+        # NOTE: For when we don't want admins to manage superusers or allow same role
+        # role modification, might be overengineering for now
+      end)
+  end
+
+  def assignable_roles() do
+    @assignable_roles
+  end
 end

lib/mailgun_logger/users/user.ex

@@ -44,6 +44,8 @@ defmodule MailgunLogger.User do
   @doc false
   @spec changeset(User.t(), map()) :: Ecto.Changeset.t()
   def changeset(%User{} = user, attrs \\ %{}) do
+    roles = Map.get(attrs, "roles", []) |> Roles.get_roles_by_names()
+
     user
     |> cast(attrs, [:firstname, :lastname, :email, :password, :theme])
     |> validate_required([:email, :password])
@@ -53,21 +55,25 @@ defmodule MailgunLogger.User do
     |> unique_constraint(:email)
     |> hash_password()
     |> generate_token()
+    |> put_assoc(:roles, roles)
   end
 
   @doc false
   @spec update_changeset(User.t(), map()) :: Ecto.Changeset.t()
   def update_changeset(%User{} = user, attrs \\ %{}) do
+    roles = Map.get(attrs, "roles", []) |> Roles.get_roles_by_names()
+
     user
     |> cast(attrs, [:firstname, :lastname, :email, :theme])
     |> update_change(:email, &String.downcase/1)
     |> validate_format(:email, @email_format)
+    |> put_assoc(:roles, roles)
     |> unique_constraint(:email)
   end
 
   @doc "Used when creating an admin, e.g. from the setup flow"
-  @spec admin_changeset(User.t(), map()) :: Ecto.Changeset.t()
-  def admin_changeset(%User{} = user, attrs) do
+  @spec superuser_changeset(User.t(), map()) :: Ecto.Changeset.t()
+  def superuser_changeset(%User{} = user, attrs) do
     user
     |> cast(attrs, [:email, :password])
     |> validate_required([:email, :password])
@@ -77,7 +83,7 @@ defmodule MailgunLogger.User do
     |> unique_constraint(:email)
     |> hash_password()
     |> generate_token()
-    |> put_assoc(:roles, [Roles.get_role_by_name("admin")])
+    |> put_assoc(:roles, [Roles.get_role_by_name("superuser")])
   end
 
   @doc false

lib/mailgun_logger/users/users.ex

@@ -4,7 +4,6 @@ defmodule MailgunLogger.Users do
   alias MailgunLogger.Repo
   alias MailgunLogger.User
 
-
   @type ecto_user() :: {:ok, User.t()} | {:error, Ecto.Changeset.t()}
   @type maybe_user() :: User.t() | nil
 
@@ -140,7 +139,7 @@ defmodule MailgunLogger.Users do
   @spec create_admin(map()) :: ecto_user()
   def create_admin(attrs) do
     %User{}
-    |> User.admin_changeset(attrs)
+    |> User.superuser_changeset(attrs)
     |> Repo.insert()
   end
 

lib/mailgun_logger_web/controllers/profile_controller.ex

@@ -1,4 +1,5 @@
 defmodule MailgunLoggerWeb.ProfileController do
+  alias MailgunLogger.Roles
   use MailgunLoggerWeb, :controller
 
   alias MailgunLogger.Users
@@ -7,10 +8,11 @@ defmodule MailgunLoggerWeb.ProfileController do
   def edit(conn, _) do
     user = conn.assigns.current_user
     changeset = User.changeset(user)
+    assignable_roles = Roles.assignable_roles()
 
     conn
     |> put_view(MailgunLoggerWeb.UserView)
-    |> render(:profile, changeset: changeset, user: user)
+    |> render(:profile, changeset: changeset, user: user, assignable_roles: assignable_roles)
   end
 
   def update(conn, %{"user" => params}) do

lib/mailgun_logger_web/controllers/setup_controller.ex

@@ -15,7 +15,7 @@ defmodule MailgunLoggerWeb.SetupController do
     end
   end
 
- def create_root(conn, %{"user" => params}) do
+  def create_root(conn, %{"user" => params}) do
     # Only allow create_root if there are no users in the database yet,
     # otherwise it's possible for any unauthenticated request to this
     # endpoint to make an admin user.

lib/mailgun_logger_web/controllers/user_controller.ex

@@ -1,4 +1,5 @@
 defmodule MailgunLoggerWeb.UserController do
+  alias MailgunLogger.Roles
   use MailgunLoggerWeb, :controller
 
   alias MailgunLogger.Users
@@ -11,7 +12,8 @@ defmodule MailgunLoggerWeb.UserController do
 
   def new(conn, _) do
     changeset = User.changeset(%User{})
-    render(conn, :new, changeset: changeset)
+    assignable_roles = Roles.assignable_roles()
+    render(conn, :new, changeset: changeset, assignable_roles: assignable_roles)
   end
 
   def create(conn, %{"user" => params}) do
@@ -24,18 +26,45 @@ defmodule MailgunLoggerWeb.UserController do
   def edit(conn, %{"id" => id}) do
     user = Users.get_user!(id)
     changeset = User.changeset(user)
-    render(conn, :edit, changeset: changeset, user: user)
+
+    render(conn, :edit,
+      changeset: changeset,
+      user: user,
+      assignable_roles: Roles.assignable_roles(),
+      editable_roles: Roles.can_modify_roles?(conn.assigns.current_user, user)
+    )
   end
 
   def update(conn, %{"id" => id, "user" => params}) do
-    user = Users.get_user!(id)
+    target = Users.get_user!(id)
+    actor = conn.assigns.current_user
+    params = Map.put(params, "roles", Map.get(params, "roles", []))
+
+    roles_modified? =
+      MapSet.new(Enum.map(target.roles, & &1.name)) !=
+        MapSet.new(Map.get(params, "roles"))
 
-    case Users.update_user(user, params) do
-      {:ok, _} ->
-        redirect(conn, to: Routes.user_path(conn, :index))
+    if roles_modified? and not Roles.can_modify_roles?(actor, target) do
+      conn
+      |> put_flash(:error, "Not authorized to modify roles")
+      |> redirect(to: Routes.user_path(conn, :edit, target))
+    else
+      case Users.update_user(target, params) do
+        {:ok, _} ->
+          redirect(conn, to: Routes.user_path(conn, :index))
 
-      {:error, changeset} ->
-        render(conn, :edit, changeset: changeset, user: user)
+        {:error, changeset} ->
+          render(conn, :edit,
+            changeset: changeset,
+            user: target,
+            assignable_roles: Roles.assignable_roles(),
+            editable_roles:
+              Roles.can_modify_roles?(
+                actor,
+                target
+              )
+          )
+      end
     end
   end
 

lib/mailgun_logger_web/plugs/authorize.ex

@@ -0,0 +1,21 @@
+defmodule MailgunLoggerWeb.Plugs.Authorize do
+  import Plug.Conn
+  import Phoenix.Controller
+  alias MailgunLoggerWeb.Router.Helpers
+  alias MailgunLogger.Roles
+
+  def init(action), do: action
+
+  def call(conn, action) do
+    user = conn.assigns.current_user
+
+    if Roles.can?(user, action) do
+      conn
+    else
+      conn
+      |> put_status(:forbidden)
+      |> redirect(to: Helpers.event_path(conn, :index))
+      |> halt()
+    end
+  end
+end

lib/mailgun_logger_web/router.ex

@@ -23,6 +23,10 @@ defmodule MailgunLoggerWeb.Router do
     plug(MailgunLoggerWeb.Plugs.Auth)
   end
 
+  pipeline :require_admin do
+    plug MailgunLoggerWeb.Plugs.Authorize, :do_stuff
+  end
+
   # Always except in prod
   if Application.compile_env(:mailgun_logger, :env) == :dev do
     forward("/sent_emails", Bamboo.SentEmailViewerPlug)
@@ -71,14 +75,20 @@ defmodule MailgunLoggerWeb.Router do
 
   scope "/", MailgunLoggerWeb do
     pipe_through([:browser, :auth])
-
     get("/logout", AuthController, :logout)
 
+    resources "/events", EventController, only: [:index, :show]
+    get "/events/:id/stored_message", EventController, :stored_message
+
+    get "/profile", ProfileController, :edit
+    put "/profile", ProfileController, :update
+  end
+
+  scope "/", MailgunLoggerWeb do
+    pipe_through([:browser, :auth, :require_admin])
+
     resources("/events", EventController, only: [:index, :show])
-    get("/events/:id/stored_message", EventController, :stored_message)
     resources("/accounts", AccountController, except: [:show])
-    get("/profile", ProfileController, :edit)
-    put("/profile", ProfileController, :update)
     resources("/users", UserController, except: [:show])
 
     get("/", PageController, :index)

lib/mailgun_logger_web/templates/account/edit.html.heex

@@ -1,4 +1,15 @@
 <h1>Edit account</h1>
-<%= render "form.html", action: Routes.account_path(@conn, :update, @account), changeset: @changeset, flash: @flash %>
+{render("form.html",
+  action: Routes.account_path(@conn, :update, @account),
+  changeset: @changeset,
+  flash: @flash
+)}
 <br />
-<.link href={Routes.account_path(@conn, :delete, @account)} method="delete" data-confirm="Are you sure?" class="btn btn-danger btn-sm">delete</.link>
+<.link
+  href={Routes.account_path(@conn, :delete, @account)}
+  method="delete"
+  data-confirm="Are you sure?"
+  class="btn btn-danger btn-sm"
+>
+  delete
+</.link>

lib/mailgun_logger_web/templates/account/form.html.heex

@@ -3,22 +3,32 @@
     <p>Oops, something went wrong! Please check the errors below.</p>
   </div>
 
-  <p>These settings can be found on your Mailgun dashboard. An account covers a sending domain / API key combo. If you have multiple domains, add them on seperate accounts.</p>
+  <p>
+    These settings can be found on your Mailgun dashboard. An account covers a sending domain / API key combo. If you have multiple domains, add them on seperate accounts.
+  </p>
 
   <div :if={Phoenix.Flash.get(@flash, :info)} class="alert alert-info" role="info">
-    <%= Phoenix.Flash.get(@flash, :info) %>
+    {Phoenix.Flash.get(@flash, :info)}
   </div>
 
   <.input field={f[:domain]} label="Sending domain" />
-  <div class="form-hint"><a href="https://app.mailgun.com/app/sending/domains" target="_blank">Find on Mailgun dashboard</a></div>
+  <div class="form-hint">
+    <a href="https://app.mailgun.com/app/sending/domains" target="_blank">
+      Find on Mailgun dashboard
+    </a>
+  </div>
 
   <.input field={f[:api_key]} label="API key" />
-  <div class="form-hint"><a href="https://app.mailgun.com/app/account/security/api_keys" target="_blank">Find on Mailgun dashboard</a></div>
+  <div class="form-hint">
+    <a href="https://app.mailgun.com/app/account/security/api_keys" target="_blank">
+      Find on Mailgun dashboard
+    </a>
+  </div>
 
   <.input field={f[:is_active]} type="checkbox" label="Is active" />
   <.input field={f[:is_eu]} type="checkbox" label="Is EU" />
 
   <div class="form-group mt-5">
-    <%= submit "submit", class: "btn btn-primary"%>
+    {submit("submit", class: "btn btn-primary")}
   </div>
 </.form>