Add support for account slugs

This commit is contained in:
Andrew Dryga
2023-08-07 12:16:22 -05:00
parent e591b92ec9
commit 108097882d
12 changed files with 191 additions and 63 deletions

View File

@@ -15,6 +15,31 @@ defmodule Domain.Accounts do
end
end
def fetch_account_by_id_or_slug(id_or_slug, %Auth.Subject{} = subject) do
with :ok <- Auth.ensure_has_permissions(subject, Authorizer.view_accounts_permission()),
true <- not is_nil(id_or_slug) do
if Validator.valid_uuid?(id_or_slug) do
Account.Query.by_id(id_or_slug)
else
Account.Query.by_slug(id_or_slug)
end
|> Authorizer.for_subject(subject)
|> Repo.fetch()
else
false -> {:error, :not_found}
other -> other
end
end
def fetch_account_by_id_or_slug(id_or_slug) do
if Validator.valid_uuid?(id_or_slug) do
Account.Query.by_id(id_or_slug)
else
Account.Query.by_slug(id_or_slug)
end
|> Repo.fetch()
end
def fetch_account_by_id(id) do
if Validator.valid_uuid?(id) do
Account.Query.by_id(id)

View File

@@ -3,6 +3,7 @@ defmodule Domain.Accounts.Account do
schema "accounts" do
field :name, :string
field :slug, :string
timestamps()
end

View File

@@ -4,7 +4,20 @@ defmodule Domain.Accounts.Account.Changeset do
def create_changeset(attrs) do
%Account{}
|> cast(attrs, [:name])
|> cast(attrs, [:name, :slug])
|> validate_required([:name])
|> validate_length(:name, min: 1, max: 255)
|> validate_slug()
|> validate_length(:slug, min: 3, max: 100)
end
defp validate_slug(changeset) do
validate_change(changeset, :slug, fn field, slug ->
if valid_uuid?(slug) do
[{field, "must can not be a valid UUID"}]
else
[]
end
end)
end
end

View File

@@ -9,4 +9,8 @@ defmodule Domain.Accounts.Account.Query do
def by_id(queryable \\ all(), id) do
where(queryable, [account: account], account.id == ^id)
end
def by_slug(queryable \\ all(), slug) do
where(queryable, [account: account], account.slug == ^slug)
end
end

View File

@@ -0,0 +1,11 @@
defmodule Domain.Repo.Migrations.AddAccountSlugs do
use Ecto.Migration
def change do
alter table(:accounts) do
add(:slug, :string)
end
create(unique_index(:accounts, [:slug], where: "deleted_at IS NULL")
end
end

View File

@@ -42,6 +42,61 @@ defmodule Domain.AccountsTest do
end
end
describe "fetch_account_by_id_or_slug/2" do
setup do
account =
AccountsFixtures.create_account(slug: "follow_the_#{System.unique_integer([:positive])}")
actor = ActorsFixtures.create_actor(type: :account_admin_user, account: account)
identity = AuthFixtures.create_identity(account: account, actor: actor)
subject = AuthFixtures.create_subject(identity)
%{
account: account,
actor: actor,
identity: identity,
subject: subject
}
end
test "returns error when account does not exist", %{subject: subject} do
assert fetch_account_by_id_or_slug(Ecto.UUID.generate(), subject) == {:error, :not_found}
assert fetch_account_by_id_or_slug("foo", subject) == {:error, :not_found}
end
test "returns account when account exists", %{account: account, subject: subject} do
assert {:ok, fetched_account} = fetch_account_by_id_or_slug(account.id, subject)
assert fetched_account.id == account.id
end
test "returns error when subject has no permission to view accounts", %{subject: subject} do
subject = AuthFixtures.remove_permissions(subject)
assert fetch_account_by_id_or_slug(Ecto.UUID.generate(), subject) ==
{:error,
{:unauthorized,
[missing_permissions: [Accounts.Authorizer.view_accounts_permission()]]}}
end
end
describe "fetch_account_by_id_or_slug/1" do
test "returns error when account does not exist" do
assert fetch_account_by_id_or_slug(Ecto.UUID.generate()) == {:error, :not_found}
assert fetch_account_by_id_or_slug("foo") == {:error, :not_found}
end
test "returns account when account exists" do
account =
AccountsFixtures.create_account(slug: "follow_the_#{System.unique_integer([:positive])}")
assert {:ok, fetched_account} = fetch_account_by_id_or_slug(account.id)
assert fetched_account.id == account.id
assert {:ok, fetched_account} = fetch_account_by_id_or_slug(account.slug)
assert fetched_account.id == account.id
end
end
describe "fetch_account_by_id/1" do
test "returns error when account is not found" do
assert fetch_account_by_id(Ecto.UUID.generate()) == {:error, :not_found}

View File

@@ -97,12 +97,18 @@ defmodule Web.Auth do
@doc """
Fetches the session token from the session and assigns the subject to the connection.
"""
def fetch_subject(%Plug.Conn{} = conn, _opts) do
def fetch_subject_and_account(%Plug.Conn{} = conn, _opts) do
with token when not is_nil(token) <- Plug.Conn.get_session(conn, :session_token),
{:ok, subject} <-
Domain.Auth.sign_in(token, conn.assigns.user_agent, conn.remote_ip),
true <- conn.path_params["account_id"] == subject.account.id do
Plug.Conn.assign(conn, :subject, subject)
{:ok, account} <-
Domain.Accounts.fetch_account_by_id_or_slug(
conn.path_params["account_id_or_slug"],
subject
) do
conn
|> Plug.Conn.assign(:account, account)
|> Plug.Conn.assign(:subject, subject)
else
_ -> conn
end
@@ -133,7 +139,7 @@ defmodule Web.Auth do
conn
|> Phoenix.Controller.put_flash(:error, "You must log in to access this page.")
|> maybe_store_return_to()
|> Phoenix.Controller.redirect(to: ~p"/#{conn.path_params["account_id"]}/sign_in")
|> Phoenix.Controller.redirect(to: ~p"/#{conn.path_params["account_id_or_slug"]}/sign_in")
|> Plug.Conn.halt()
end
end
@@ -246,14 +252,13 @@ defmodule Web.Auth do
end
end
defp mount_subject(socket, params, session) do
defp mount_subject(socket, _params, session) do
Phoenix.Component.assign_new(socket, :subject, fn ->
user_agent = Phoenix.LiveView.get_connect_info(socket, :user_agent)
remote_ip = Phoenix.LiveView.get_connect_info(socket, :peer_data).address
with token when not is_nil(token) <- session["session_token"],
{:ok, subject} <- Domain.Auth.sign_in(token, user_agent, remote_ip),
true <- params["account_id"] == subject.account.id do
{:ok, subject} <- Domain.Auth.sign_in(token, user_agent, remote_ip) do
subject
else
_ -> nil
@@ -263,11 +268,12 @@ defmodule Web.Auth do
defp mount_account(
%{assigns: %{subject: subject}} = socket,
%{"account_id" => account_id},
%{"account_id_or_slug" => account_id_or_slug},
_session
) do
Phoenix.Component.assign_new(socket, :account, fn ->
with {:ok, account} <- Domain.Accounts.fetch_account_by_id(account_id, subject) do
with {:ok, account} <-
Domain.Accounts.fetch_account_by_id_or_slug(account_id_or_slug, subject) do
account
else
_ -> nil

View File

@@ -20,7 +20,7 @@ defmodule Web.AuthController do
This is a callback for the UserPass provider which checks login and password to authenticate the user.
"""
def verify_credentials(conn, %{
"account_id" => account_id,
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id,
"userpass" => %{
"provider_identifier" => provider_identifier,
@@ -41,18 +41,18 @@ defmodule Web.AuthController do
conn
|> put_flash(:userpass_provider_identifier, String.slice(provider_identifier, 0, 160))
|> put_flash(:error, "You can not use this method to sign in.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
{:error, :invalid_actor_type} ->
conn
|> put_flash(:info, "Please use client application to access Firezone.")
|> redirect(to: ~p"/#{account_id}")
|> redirect(to: ~p"/#{account_id_or_slug}")
{:error, _reason} ->
conn
|> put_flash(:userpass_provider_identifier, String.slice(provider_identifier, 0, 160))
|> put_flash(:error, "Invalid username or password.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
end
end
@@ -60,7 +60,7 @@ defmodule Web.AuthController do
This is a callback for the Email provider which sends login link.
"""
def request_magic_link(conn, %{
"account_id" => account_id,
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id,
"email" => %{
"provider_identifier" => provider_identifier
@@ -75,7 +75,7 @@ defmodule Web.AuthController do
|> Web.Mailer.deliver()
end
redirect(conn, to: "/#{account_id}/sign_in/providers/email/#{provider_id}")
redirect(conn, to: "/#{account_id_or_slug}/sign_in/providers/email/#{provider_id}")
end
@doc """
@@ -83,7 +83,7 @@ defmodule Web.AuthController do
to authenticate a user.
"""
def verify_sign_in_token(conn, %{
"account_id" => account_id,
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id,
"identity_id" => identity_id,
"secret" => secret
@@ -101,17 +101,17 @@ defmodule Web.AuthController do
{:error, :not_found} ->
conn
|> put_flash(:error, "You can not use this method to sign in.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
{:error, :invalid_actor_type} ->
conn
|> put_flash(:info, "Please use client application to access Firezone.")
|> redirect(to: ~p"/#{account_id}")
|> redirect(to: ~p"/#{account_id_or_slug}")
{:error, _reason} ->
conn
|> put_flash(:error, "The sign in link is invalid or expired.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
end
end
@@ -119,7 +119,10 @@ defmodule Web.AuthController do
This controller redirects user to IdP during sign in for authentication while persisting
verification state to prevent various attacks on OpenID Connect.
"""
def redirect_to_idp(conn, %{"account_id" => account_id, "provider_id" => provider_id}) do
def redirect_to_idp(conn, %{
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id
}) do
with {:ok, provider} <- Domain.Auth.fetch_active_provider_by_id(provider_id) do
redirect_url =
url(~p"/#{provider.account_id}/sign_in/providers/#{provider.id}/handle_callback")
@@ -129,7 +132,7 @@ defmodule Web.AuthController do
{:error, :not_found} ->
conn
|> put_flash(:error, "You can not use this method to sign in.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
end
end
@@ -149,14 +152,14 @@ defmodule Web.AuthController do
This controller handles IdP redirect back to the Firezone when user signs in.
"""
def handle_idp_callback(conn, %{
"account_id" => account_id,
"account_id_or_slug" => account_id_or_slug,
"provider_id" => provider_id,
"state" => state,
"code" => code
}) do
with {:ok, code_verifier, conn} <- verify_state_and_fetch_verifier(conn, provider_id, state) do
payload = {
url(~p"/#{account_id}/sign_in/providers/#{provider_id}/handle_callback"),
url(~p"/#{account_id_or_slug}/sign_in/providers/#{provider_id}/handle_callback"),
code_verifier,
code
}
@@ -174,23 +177,23 @@ defmodule Web.AuthController do
{:error, :not_found} ->
conn
|> put_flash(:error, "You can not use this method to sign in.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
{:error, :invalid_actor_type} ->
conn
|> put_flash(:info, "Please use client application to access Firezone.")
|> redirect(to: ~p"/#{account_id}")
|> redirect(to: ~p"/#{account_id_or_slug}")
{:error, _reason} ->
conn
|> put_flash(:error, "You can not authenticate to this account.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
end
else
{:error, :invalid_state, conn} ->
conn
|> put_flash(:error, "Your session has expired, please try again.")
|> redirect(to: "/#{account_id}/sign_in")
|> redirect(to: "/#{account_id_or_slug}/sign_in")
end
end
@@ -211,9 +214,9 @@ defmodule Web.AuthController do
@state_cookie_key_prefix <> provider_id
end
def sign_out(conn, %{"account_id" => account_id}) do
def sign_out(conn, %{"account_id_or_slug" => account_id_or_slug}) do
conn
|> Auth.sign_out()
|> redirect(to: ~p"/#{account_id}/sign_in")
|> redirect(to: ~p"/#{account_id_or_slug}/sign_in")
end
end

View File

@@ -2,8 +2,8 @@ defmodule Web.Auth.SignIn do
use Web, {:live_view, layout: {Web.Layouts, :public}}
alias Domain.{Auth, Accounts}
def mount(%{"account_id" => account_id}, _session, socket) do
with {:ok, account} <- Accounts.fetch_account_by_id(account_id),
def mount(%{"account_id_or_slug" => account_id_or_slug}, _session, socket) do
with {:ok, account} <- Accounts.fetch_account_by_id_or_slug(account_id_or_slug),
{:ok, [_ | _] = providers} <- Auth.list_active_providers_for_account(account) do
providers_by_adapter =
providers

View File

@@ -163,8 +163,8 @@ defmodule Web.Settings.IdentityProviders.SAML.Components do
>
<.icon name="hero-document-duplicate" class="w-5 h-5 mr-1" />
</button>
<code id="endpoint-value" data-copy={url(~p"/#{@account}/scim/v2")}>
<%= url(~p"/#{@account}/scim/v2") %>
<code id="endpoint-value" data-copy={"/#{@account}/scim/v2"}>
<%= "/#{@account}/scim/v2" %>
</code>
</div>
</td>

View File

@@ -9,7 +9,7 @@ defmodule Web.Router do
plug :fetch_live_flash
plug :put_root_layout, {Web.Layouts, :root}
plug :fetch_user_agent
plug :fetch_subject
plug :fetch_subject_and_account
end
pipeline :api do
@@ -46,7 +46,7 @@ defmodule Web.Router do
end
end
scope "/:account_id/sign_in", Web do
scope "/:account_id_or_slug/sign_in", Web do
pipe_through [:browser, :redirect_if_user_is_authenticated]
live_session :redirect_if_user_is_authenticated,
@@ -75,20 +75,13 @@ defmodule Web.Router do
end
end
scope "/:account_id/scim/v2", Web do
pipe_through [:api]
get "/", ScimController, :index
# TODO: SCIM endpoints
end
scope "/:account_id", Web do
scope "/:account_id_or_slug", Web do
pipe_through [:browser]
get "/sign_out", AuthController, :sign_out
end
scope "/:account_id", Web do
scope "/:account_id_or_slug", Web do
pipe_through [:browser, :ensure_authenticated_admin]
live_session :ensure_authenticated,
@@ -193,7 +186,7 @@ defmodule Web.Router do
live_session :landing,
on_mount: [Web.Sandbox] do
live "/:account_id/", Landing
live "/:account_id_or_slug/", Landing
live "/", Landing
end
end

View File

@@ -105,7 +105,7 @@ defmodule Web.AuthTest do
end
end
describe "fetch_subject/2" do
describe "fetch_subject_and_account/2" do
setup context do
%{conn: assign(context.conn, :user_agent, context.admin_subject.context.user_agent)}
end
@@ -116,14 +116,15 @@ defmodule Web.AuthTest do
conn =
%{
conn
| path_params: %{"account_id" => subject.account.id},
| path_params: %{"account_id_or_slug" => subject.account.id},
remote_ip: {100, 64, 100, 58}
}
|> put_session(:session_token, session_token)
|> fetch_subject([])
|> fetch_subject_and_account([])
assert conn.assigns.subject.identity.id == subject.identity.id
assert conn.assigns.subject.actor.id == subject.actor.id
assert conn.assigns.account.id == subject.account.id
end
test "does not authenticate to an incorrect account", %{conn: conn, admin_subject: subject} do
@@ -132,19 +133,21 @@ defmodule Web.AuthTest do
conn =
%{
conn
| path_params: %{"account_id" => Ecto.UUID.generate()},
| path_params: %{"account_id_or_slug" => Ecto.UUID.generate()},
remote_ip: {100, 64, 100, 58}
}
|> put_session(:session_token, session_token)
|> fetch_subject([])
|> fetch_subject_and_account([])
refute Map.has_key?(conn.assigns, :subject)
refute Map.has_key?(conn.assigns, :account)
end
test "does not authenticate if data is missing", %{conn: conn} do
conn = fetch_subject(conn, [])
conn = fetch_subject_and_account(conn, [])
refute get_session(conn, :session_token)
refute Map.has_key?(conn.assigns, :subject)
refute Map.has_key?(conn.assigns, :account)
end
end
@@ -168,7 +171,7 @@ defmodule Web.AuthTest do
describe "ensure_authenticated/2" do
setup context do
%{conn: %{context.conn | path_params: %{"account_id" => context.account.id}}}
%{conn: %{context.conn | path_params: %{"account_id_or_slug" => context.account.id}}}
end
test "redirects if user is not authenticated", %{account: account, conn: conn} do
@@ -242,7 +245,7 @@ defmodule Web.AuthTest do
} do
{:ok, session_token} = Domain.Auth.create_session_token_from_subject(subject)
session = conn |> put_session(:session_token, session_token) |> get_session()
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:cont, updated_socket} = on_mount(:mount_subject, params, session, socket)
assert updated_socket.assigns.subject.identity.id == subject.identity.id
@@ -255,7 +258,7 @@ defmodule Web.AuthTest do
} do
session_token = "invalid_token"
session = conn |> put_session(:session_token, session_token) |> get_session()
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:cont, updated_socket} = on_mount(:mount_subject, params, session, socket)
assert is_nil(updated_socket.assigns.subject)
@@ -267,23 +270,37 @@ defmodule Web.AuthTest do
admin_subject: subject
} do
session = conn |> get_session()
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:cont, updated_socket} = on_mount(:mount_subject, params, session, socket)
assert is_nil(updated_socket.assigns.subject)
end
end
describe "on_mount: assign_account" do
test "assigns nil to subject assign if account_id doesn't match token", %{
conn: conn,
socket: socket,
admin_subject: subject
} do
socket = %LiveView.Socket{
assigns: %{
__changed__: %{},
subject: subject
},
private: %{
connect_info: %{
user_agent: subject.context.user_agent,
peer_data: %{address: {100, 64, 100, 58}}
}
}
}
{:ok, session_token} = Domain.Auth.create_session_token_from_subject(subject)
session = conn |> put_session(:session_token, session_token) |> get_session()
params = %{"account_id" => Ecto.UUID.generate()}
params = %{"account_id_or_slug" => Ecto.UUID.generate()}
assert {:cont, updated_socket} = on_mount(:mount_subject, params, session, socket)
assert is_nil(updated_socket.assigns.subject)
assert {:cont, updated_socket} = on_mount(:mount_account, params, session, socket)
assert is_nil(updated_socket.assigns.account)
end
end
@@ -311,7 +328,7 @@ defmodule Web.AuthTest do
} do
{:ok, session_token} = Domain.Auth.create_session_token_from_subject(subject)
session = conn |> put_session(:session_token, session_token) |> get_session()
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:cont, updated_socket} =
on_mount(:ensure_authenticated, params, session, socket)
@@ -383,7 +400,7 @@ defmodule Web.AuthTest do
|> put_session(:session_token, session_token)
|> get_session()
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:halt, updated_socket} =
on_mount(:redirect_if_user_is_authenticated, params, session, socket)
@@ -398,7 +415,7 @@ defmodule Web.AuthTest do
} do
session = get_session(conn)
params = %{"account_id" => subject.account.id}
params = %{"account_id_or_slug" => subject.account.id}
assert {:cont, updated_socket} =
on_mount(:redirect_if_user_is_authenticated, params, session, socket)