feat(portal): Add created_by_subject (#9176)

Why:

* We have decided to change the way we will do audit logging. Instead of
soft deleting data and keeping it in the table it was created in, we
will be moving to an audit trail table where various actions will be
recorded in a table/DB specifically for auditing purposes. Due to this
change we need to make sure that we don't have stale/dangling
references. One set of references we keep everywhere is
`created_by_identity_id` and `created_by_actor_id`. Those foreign key
references won't be able to be used after moving to the new audit
system. This commit will allow us to keep that info by pulling the
values and storing the data in a created_by_subject field on the record.
This commit is contained in:
Brian Manifold
2025-05-20 13:03:46 -07:00
committed by GitHub
parent 042d03af2a
commit 12b4a12f26
32 changed files with 292 additions and 47 deletions

View File

@@ -146,30 +146,6 @@ defmodule API.IdentityControllerTest do
}
}
end
test "returns a single identity with empty email field", %{
conn: conn,
account: account,
actor: actor
} do
identity = Fixtures.Auth.create_identity(%{account: account, actor: actor})
conn =
conn
|> authorize_conn(actor)
|> put_req_header("content-type", "application/json")
|> get("/actors/#{actor.id}/identities/#{identity.id}")
assert json_response(conn, 200) == %{
"data" => %{
"id" => identity.id,
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier,
"email" => nil
}
}
end
end
describe "create/2" do
@@ -326,7 +302,7 @@ defmodule API.IdentityControllerTest do
"actor_id" => actor.id,
"provider_id" => identity.provider_id,
"provider_identifier" => identity.provider_identifier,
"email" => nil
"email" => identity.email
}
}

View File

@@ -21,6 +21,7 @@ defmodule Domain.Actors.Group do
has_many :actors, through: [:memberships, :actor]
field :created_by, Ecto.Enum, values: ~w[actor identity provider system]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor

View File

@@ -30,7 +30,7 @@ defmodule Domain.Actors.Group.Changeset do
|> changeset()
|> put_change(:account_id, account.id)
|> cast_membership_assocs(account.id)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
end
def create(%Auth.Provider{} = provider, attrs) do
@@ -43,7 +43,7 @@ defmodule Domain.Actors.Group.Changeset do
|> put_change(:account_id, provider.account_id)
# resurrect synced groups
|> put_change(:deleted_at, nil)
|> put_change(:created_by, :provider)
|> put_subject_trail(:created_by, :provider)
end
def update(%Actors.Group{} = group, attrs) do

View File

@@ -21,7 +21,9 @@ defmodule Domain.Auth.Identity do
belongs_to :account, Domain.Accounts.Account
field :created_by, Ecto.Enum, values: ~w[system provider identity]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor
has_many :clients, Domain.Clients.Client, where: [deleted_at: nil]

View File

@@ -11,8 +11,7 @@ defmodule Domain.Auth.Identity.Changeset do
) do
actor
|> create_identity(provider, attrs)
|> put_change(:created_by, :identity)
|> put_change(:created_by_identity_id, subject.identity.id)
|> put_subject_trail(:created_by, subject)
end
def create_identity(
@@ -21,13 +20,13 @@ defmodule Domain.Auth.Identity.Changeset do
attrs
) do
%Identity{}
|> cast(attrs, ~w[provider_identifier provider_virtual_state]a)
|> cast(attrs, ~w[email provider_identifier provider_virtual_state]a)
|> validate_required(~w[provider_identifier]a)
|> maybe_put_email_from_identifier()
|> put_change(:actor_id, actor.id)
|> put_change(:provider_id, provider.id)
|> put_change(:account_id, account_id)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
|> changeset()
end
@@ -36,7 +35,7 @@ defmodule Domain.Auth.Identity.Changeset do
attrs
) do
%Identity{}
|> cast(attrs, ~w[provider_identifier provider_state provider_virtual_state]a)
|> cast(attrs, ~w[email provider_identifier provider_state provider_virtual_state]a)
|> validate_required(~w[provider_identifier]a)
|> maybe_put_email_from_state()
|> cast_assoc(:actor,
@@ -47,13 +46,13 @@ defmodule Domain.Auth.Identity.Changeset do
)
|> put_change(:provider_id, provider.id)
|> put_change(:account_id, account_id)
|> put_change(:created_by, :provider)
|> put_subject_trail(:created_by, :provider)
|> changeset()
end
def update_identity_and_actor(%Identity{} = identity, attrs) do
identity
|> cast(attrs, ~w[provider_state]a)
|> cast(attrs, ~w[email provider_state]a)
|> maybe_put_email_from_state()
|> cast_assoc(:actor,
with: fn actor, attrs ->

View File

@@ -17,8 +17,10 @@ defmodule Domain.Auth.Provider do
has_many :actor_groups, Domain.Actors.Group, where: [deleted_at: nil]
has_many :identities, Domain.Auth.Identity, where: [deleted_at: nil]
field :created_by, Ecto.Enum, values: ~w[system identity]a
field :created_by, Ecto.Enum, values: ~w[system identity actor]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor
field :last_syncs_failed, :integer
field :last_sync_error, :string

View File

@@ -12,8 +12,8 @@ defmodule Domain.Auth.Provider.Changeset do
def create(account, attrs, %Subject{} = subject) do
account
|> create(attrs)
|> put_change(:created_by, :identity)
|> put_change(:created_by_identity_id, subject.identity.id)
|> reset_created_by()
|> put_subject_trail(:created_by, subject)
end
def create(%Accounts.Account{} = account, attrs) do
@@ -34,7 +34,7 @@ defmodule Domain.Auth.Provider.Changeset do
|> put_change(:account_id, account.id)
|> changeset()
|> validate_inclusion(:adapter, allowed_adapters)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
end
def update(%Provider{} = provider, attrs) do
@@ -135,4 +135,12 @@ defmodule Domain.Auth.Provider.Changeset do
|> change()
|> put_default_value(:deleted_at, DateTime.utc_now())
end
defp reset_created_by(changeset) do
changeset
|> put_change(:created_by, nil)
|> put_change(:created_by_identity_id, nil)
|> put_change(:created_by_actor_id, nil)
|> put_change(:created_by_subject, nil)
end
end

View File

@@ -36,6 +36,7 @@ defmodule Domain.Clients.Client do
# Verification
field :verified_at, :utc_datetime_usec
field :verified_by, Ecto.Enum, values: [:system, :actor, :identity]
field :verified_by_subject, :map
belongs_to :verified_by_actor, Domain.Actors.Actor
belongs_to :verified_by_identity, Domain.Auth.Identity

View File

@@ -183,6 +183,7 @@ defmodule Domain.Clients.Client.Changeset do
|> put_change(:verified_by, nil)
|> put_change(:verified_by_actor_id, nil)
|> put_change(:verified_by_identity_id, nil)
|> put_change(:verified_by_subject, nil)
end
def update(%Clients.Client{} = client, attrs) do

View File

@@ -16,6 +16,7 @@ defmodule Domain.Gateways.Group do
has_many :connections, Domain.Resources.Connection, foreign_key: :gateway_group_id
field :created_by, Ecto.Enum, values: ~w[actor identity system]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor

View File

@@ -13,6 +13,7 @@ defmodule Domain.Policies.Policy do
belongs_to :account, Domain.Accounts.Account
field :created_by, Ecto.Enum, values: ~w[actor identity]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor

View File

@@ -9,7 +9,9 @@ defmodule Domain.Relays.Group do
has_many :tokens, Domain.Tokens.Token, foreign_key: :relay_group_id, where: [deleted_at: nil]
field :created_by, Ecto.Enum, values: ~w[system identity]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor
field :deleted_at, :utc_datetime_usec
timestamps()

View File

@@ -8,15 +8,14 @@ defmodule Domain.Relays.Group.Changeset do
def create(attrs) do
%Relays.Group{}
|> changeset(attrs)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
end
def create(%Accounts.Account{} = account, attrs, %Auth.Subject{} = subject) do
%Relays.Group{account: account}
|> changeset(attrs)
|> put_change(:account_id, account.id)
|> put_change(:created_by, :identity)
|> put_change(:created_by_identity_id, subject.identity.id)
|> put_subject_trail(:created_by, subject)
end
def update(%Relays.Group{} = group, attrs, %Auth.Subject{}) do

View File

@@ -150,17 +150,29 @@ defmodule Domain.Repo.Changeset do
def put_subject_trail(changeset, field, :system) do
changeset
|> put_default_value(field, :system)
|> put_default_value(:"#{field}_subject", %{"name" => "System", "email" => nil})
end
def put_subject_trail(changeset, field, :provider) do
changeset
|> put_default_value(field, :provider)
|> put_default_value(:"#{field}_subject", %{"name" => "Provider", "email" => nil})
end
def put_subject_trail(changeset, field, %Domain.Auth.Subject{identity: nil} = subject) do
changeset
|> put_default_value(field, :actor)
|> put_default_value(:"#{field}_subject", %{"name" => subject.actor.name, "email" => nil})
|> put_default_value(:"#{field}_actor_id", subject.actor.id)
end
def put_subject_trail(changeset, field, %Domain.Auth.Subject{} = subject) do
changeset
|> put_default_value(field, :identity)
|> put_default_value(:"#{field}_subject", %{
"name" => subject.actor.name,
"email" => subject.identity.email
})
|> put_default_value(:"#{field}_actor_id", subject.actor.id)
|> put_default_value(:"#{field}_identity_id", subject.identity.id)
end

View File

@@ -7,6 +7,7 @@ defmodule Domain.Resources.Connection do
belongs_to :gateway_group, Domain.Gateways.Group, primary_key: true
field :created_by, Ecto.Enum, values: ~w[actor identity system]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor

View File

@@ -12,7 +12,7 @@ defmodule Domain.Resources.Connection.Changeset do
def changeset(account_id, connection, attrs) do
base_changeset(account_id, connection, attrs)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
end
defp base_changeset(account_id, connection, attrs) do

View File

@@ -29,6 +29,7 @@ defmodule Domain.Resources.Resource do
has_many :authorized_by_policies, Domain.Policies.Policy, where: [id: {:fragment, "FALSE"}]
field :created_by, Ecto.Enum, values: ~w[identity actor system]a
field :created_by_subject, :map
belongs_to :created_by_actor, Domain.Actors.Actor
belongs_to :created_by_identity, Domain.Auth.Identity

View File

@@ -52,7 +52,7 @@ defmodule Domain.Resources.Resource.Changeset do
|> validate_address(account)
|> put_change(:persistent_id, Ecto.UUID.generate())
|> put_change(:account_id, account.id)
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
|> cast_assoc(:connections,
with: &Connection.Changeset.changeset(account.id, &1, &2)
)

View File

@@ -44,6 +44,7 @@ defmodule Domain.Tokens.Token do
# Maybe this is not needed and they should be in the join tables (eg. relay_group_tokens)
field :created_by, Ecto.Enum, values: ~w[actor identity system]a
field :created_by_subject, :map
belongs_to :created_by_identity, Domain.Auth.Identity
belongs_to :created_by_actor, Domain.Actors.Actor
field :created_by_user_agent, :string

View File

@@ -27,7 +27,7 @@ defmodule Domain.Tokens.Token.Changeset do
|> validate_required(@required_attrs)
|> validate_inclusion(:type, [:email, :browser, :client, :relay_group, :api_client])
|> changeset()
|> put_change(:created_by, :system)
|> put_subject_trail(:created_by, :system)
end
def create(attrs, %Auth.Subject{} = subject) do

View File

@@ -0,0 +1,41 @@
defmodule Domain.Repo.Migrations.AddCreatedBySubject do
use Ecto.Migration
@tables [
:actor_groups,
:auth_identities,
:auth_providers,
:gateway_groups,
:policies,
:relay_groups,
:resource_connections,
:resources,
:tokens
]
def up do
for table <- @tables do
alter table(table) do
add_if_not_exists(:created_by_subject, :jsonb)
end
end
# Clients table is slightly different case
alter table(:clients) do
add_if_not_exists(:verified_by_subject, :jsonb)
end
end
def down do
for table <- @tables do
alter table(table) do
remove_if_exists(:created_by_subject)
end
end
# Clients table is slightly different case
alter table(:clients) do
remove_if_exists(:verified_by_subject)
end
end
end

View File

@@ -0,0 +1,120 @@
defmodule Domain.Repo.Migrations.BackfillCreatedBySubject do
use Ecto.Migration
@tables [
"actor_groups",
"auth_identities",
"auth_providers",
"gateway_groups",
"policies",
"relay_groups",
"resources",
"tokens"
]
def up do
# Backfill tables w/ created_by_subject
for table <- @tables do
execute("""
UPDATE #{table} t
SET created_by_subject =
CASE
WHEN t.created_by = 'system' THEN jsonb_build_object('name', 'System', 'email', NULL)
WHEN t.created_by = 'provider' THEN jsonb_build_object('name', 'Provider', 'email', NULL)
ELSE jsonb_build_object(
'name', COALESCE(data.actor_name, 'Unknown'),
'email', data.identity_email
)
END
FROM (
SELECT
t_inner.id AS tid,
a.name AS actor_name,
i.email AS identity_email
FROM #{table} t_inner
LEFT JOIN actors a ON t_inner.created_by_actor_id = a.id
LEFT JOIN auth_identities i ON t_inner.created_by_identity_id = i.id
WHERE t_inner.created_by_subject IS NULL
) AS data
WHERE t.id = data.tid
""")
end
# Backfill Resource Connections
execute("""
UPDATE resource_connections rc
SET created_by_subject =
CASE
WHEN rc.created_by = 'system' THEN jsonb_build_object('name', 'System', 'email', NULL)
WHEN rc.created_by = 'provider' THEN jsonb_build_object('name', 'Provider', 'email', NULL)
ELSE jsonb_build_object(
'name', COALESCE(data.actor_name, 'Unknown'),
'email', data.identity_email
)
END
FROM (
SELECT
rc_inner.resource_id,
rc_inner.gateway_group_id,
rc_inner.account_id,
a.name AS actor_name,
i.email AS identity_email
FROM resource_connections rc_inner
LEFT JOIN actors a ON rc_inner.created_by_actor_id = a.id
LEFT JOIN auth_identities i ON rc_inner.created_by_identity_id = i.id
WHERE rc_inner.created_by_subject IS NULL
) AS data
WHERE rc.resource_id = data.resource_id
AND rc.gateway_group_id = data.gateway_group_id
AND rc.account_id = data.account_id
""")
# Backfill Clients verified_by_subject
execute("""
UPDATE clients c
SET verified_by_subject =
CASE
WHEN c.verified_at IS NULL THEN NULL
WHEN c.verified_by = 'system' THEN jsonb_build_object('name', 'System', 'email', NULL)
WHEN c.verified_by = 'provider' THEN jsonb_build_object('name', 'Provider', 'email', NULL)
ELSE jsonb_build_object(
'name', COALESCE(data.actor_name, 'Unknown'),
'email', data.identity_email
)
END
FROM (
SELECT
c_inner.id AS cid,
a.name AS actor_name,
i.email AS identity_email
FROM clients c_inner
LEFT JOIN actors a ON c_inner.verified_by_actor_id = a.id
LEFT JOIN auth_identities i ON c_inner.verified_by_identity_id = i.id
WHERE c_inner.verified_by_subject IS NULL
) AS data
WHERE c.id = data.cid
""")
end
def down do
# Remove data from tables w/ created_by_subject
for table <- @tables do
execute("""
UPDATE #{table}
SET created_by_subject = NULL
""")
end
# Remove created_by_subject data from resource_connections table
execute("""
UPDATE resource_connections
SET created_by_subject = NULL
""")
# Remove verified_by_subject data from clients table
execute("""
UPDATE clients
SET verified_by_subject = NULL
""")
end
end

View File

@@ -532,6 +532,7 @@ defmodule Domain.Repo.Seeds do
provider_id: oidc_provider.id,
provider_identifier: Ecto.UUID.generate(),
created_by: :provider,
created_by_subject: %{"name" => "Provider", "email" => nil},
account_id: admin_subject.account.id,
inserted_at: DateTime.utc_now(),
updated_at: DateTime.utc_now()

View File

@@ -757,6 +757,7 @@ defmodule Domain.ActorsTest do
assert group.created_by == :provider
assert group.provider_id == provider.id
assert group.created_by_subject == %{"email" => nil, "name" => "Provider"}
assert group.name in group_names
@@ -1486,6 +1487,11 @@ defmodule Domain.ActorsTest do
assert group.id
assert group.name == attrs.name
assert group.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
group = Repo.preload(group, :memberships)
assert group.memberships == []
end

View File

@@ -637,7 +637,12 @@ defmodule Domain.AuthTest do
} do
assert changeset = new_provider(account)
assert %Ecto.Changeset{data: %Domain.Auth.Provider{}} = changeset
assert changeset.changes == %{account_id: account.id, created_by: :system}
assert changeset.changes == %{
account_id: account.id,
created_by: :system,
created_by_subject: %{"email" => nil, "name" => "System"}
}
provider_attrs =
Fixtures.Auth.provider_attrs(
@@ -774,6 +779,7 @@ defmodule Domain.AuthTest do
assert provider.account_id == account.id
assert provider.created_by == :system
assert provider.created_by_subject == %{"email" => nil, "name" => "System"}
assert is_nil(provider.created_by_identity_id)
assert is_nil(provider.disabled_at)
@@ -837,6 +843,11 @@ defmodule Domain.AuthTest do
assert provider.created_by == :identity
assert provider.created_by_identity_id == subject.identity.id
assert provider.created_by_subject == %{
"email" => subject.identity.email,
"name" => subject.actor.name
}
end
end
@@ -1685,6 +1696,7 @@ defmodule Domain.AuthTest do
assert identity.provider_identifier in provider_identifiers
assert identity.actor.name in actor_names
assert identity.actor.last_synced_at
assert identity.created_by_subject == %{"email" => nil, "name" => "Provider"}
assert Map.get(actor_ids_by_provider_identifier, identity.provider_identifier) ==
identity.actor_id

View File

@@ -215,6 +215,11 @@ defmodule Domain.GatewaysTest do
assert group.created_by == :identity
assert group.created_by_identity_id == subject.identity.id
assert group.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
end
test "returns error when subject has no permission to manage groups", %{
@@ -481,6 +486,12 @@ defmodule Domain.GatewaysTest do
assert token.created_by_identity_id == subject.identity.id
assert token.created_by_user_agent == context.user_agent
assert token.created_by_remote_ip.address == context.remote_ip
assert token.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
refute token.expires_at
end

View File

@@ -311,6 +311,11 @@ defmodule Domain.PoliciesTest do
assert {:ok, policy} = create_policy(attrs, subject)
assert policy.actor_group_id == actor_group.id
assert policy.resource_id == resource.id
assert policy.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
end
test "creates a policy with conditions", %{
@@ -353,6 +358,11 @@ defmodule Domain.PoliciesTest do
assert {:ok, policy} = create_policy(attrs, subject)
assert policy.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
assert policy.conditions == [
%Policies.Condition{
property: :remote_ip,

View File

@@ -174,6 +174,11 @@ defmodule Domain.RelaysTest do
assert group.created_by == :identity
assert group.created_by_identity_id == subject.identity.id
assert group.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
end
test "returns error when subject has no permission to manage groups", %{
@@ -221,6 +226,7 @@ defmodule Domain.RelaysTest do
assert group.created_by == :system
assert is_nil(group.created_by_identity_id)
assert group.created_by_subject == %{"name" => "System", "email" => nil}
end
end

View File

@@ -1237,6 +1237,11 @@ defmodule Domain.ResourcesTest do
assert resource.created_by == :identity
assert resource.created_by_identity_id == subject.identity.id
assert resource.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
assert [%Domain.Resources.Connection{} = connection] = resource.connections
assert connection.resource_id == resource.id
assert connection.gateway_group_id == gateway.group_id
@@ -1244,6 +1249,11 @@ defmodule Domain.ResourcesTest do
assert connection.created_by == :identity
assert connection.created_by_identity_id == subject.identity.id
assert resource.created_by_subject == %{
"name" => subject.actor.name,
"email" => subject.identity.email
}
assert [
%Domain.Resources.Resource.Filter{ports: ["80", "433"], protocol: :tcp},
%Domain.Resources.Resource.Filter{ports: ["100 - 200"], protocol: :udp},

View File

@@ -185,6 +185,7 @@ defmodule Domain.TokensTest do
assert token.account_id == account.id
assert token.actor_id == actor.id
assert token.identity_id == identity.id
assert token.created_by_subject == %{"name" => "System", "email" => nil}
end
end
@@ -263,6 +264,11 @@ defmodule Domain.TokensTest do
assert token.account_id == account.id
assert token.actor_id == actor.id
assert token.identity_id == identity.id
assert token.created_by_subject == %{
"name" => actor.name,
"email" => identity.email
}
end
end

View File

@@ -424,6 +424,15 @@ defmodule Domain.Fixtures.Auth do
random_provider_identifier(provider)
end)
{email, attrs} =
Map.pop_lazy(attrs, :email, fn ->
if to_string(provider_identifier) =~ Domain.Auth.email_regex() do
provider_identifier
else
email()
end
end)
{actor, attrs} =
pop_assoc_fixture(attrs, :actor, fn assoc_attrs ->
assoc_attrs
@@ -435,8 +444,11 @@ defmodule Domain.Fixtures.Auth do
|> Fixtures.Actors.create_actor()
end)
attrs = Map.put(attrs, :provider_identifier, provider_identifier)
attrs = Map.put(attrs, :provider_identifier_confirmation, provider_identifier)
attrs =
attrs
|> Map.put(:provider_identifier, provider_identifier)
|> Map.put(:provider_identifier_confirmation, provider_identifier)
|> Map.put(:email, email)
{:ok, identity} = Auth.upsert_identity(actor, provider, attrs)

View File

@@ -111,6 +111,7 @@ defmodule Web.Live.Groups.NewTest do
test "creates a new group on valid attrs", %{
account: account,
actor: actor,
identity: identity,
conn: conn
} do
@@ -135,6 +136,7 @@ defmodule Web.Live.Groups.NewTest do
assert group.created_by == :identity
assert group.created_by_identity_id == identity.id
assert group.created_by_subject == %{"email" => identity.email, "name" => actor.name}
assert group.account_id == account.id
end