local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 72 Pull requests 5 Projects 4 Releases Packages 1 Activity Actions

OIDC handling and linking closes #171 #192

Merged
moritz merged 9 commits from feature/oidc_handling into main 2025-11-13 16:36:04 +01:00
Conversation 1 Commits 9 Files changed 21 +7 -22
2 changed files with 7 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 55fb845855 - Show all commits

3
lib/accounts/user.ex
View file

@ -190,8 +190,9 @@ defmodule Mv.Accounts.User do
changeset changeset
|> Ash.Changeset.change_attribute(:oidc_id, oidc_id) |> Ash.Changeset.change_attribute(:oidc_id, oidc_id)
rafael commented 2025-11-12 10:31:39 +01:00
Outdated
Copy link

Does this do anything? it looks like oidc_id is the same value we're getting out of the changeset a few lines above.

Does this do anything? it looks like `oidc_id` is the same value we're getting out of the changeset a few lines above.
moritz commented 2025-11-13 16:29:44 +01:00
Outdated
Copy link

On line 184, oidc_id is taken out of the changeset as an argument, but on line 191 it is set as an attribute. These are two different things in Ash:
Arguments are temporary values that are passed to the action
Attributes are the persisted fields in the data model So the code transforms the argument into an attribute that is stored in the database.

On line 184, oidc_id is taken out of the changeset as an argument, but on line 191 it is set as an attribute. These are two different things in Ash: Arguments are temporary values that are passed to the action Attributes are the persisted fields in the data model So the code transforms the argument into an attribute that is stored in the database.
# Update email if it differs from OIDC provider # Update email if it differs from OIDC provider
# change_attribute/3 already checks if value matches existing value
|> then(fn cs -> |> then(fn cs ->
moritz marked this conversation as resolved Outdated
rafael commented 2025-11-12 10:34:34 +01:00
Outdated
Copy link

Is the equality check needed? From the ash docs:

Adds a change to the changeset, unless the value matches the existing value.

Is the equality check needed? From the [ash docs](https://hexdocs.pm/ash/Ash.Changeset.html#change_attribute/3): > Adds a change to the changeset, **unless the value matches the existing value.**
if new_email && to_string(cs.data.email) != new_email do if new_email do
Ash.Changeset.change_attribute(cs, :email, new_email) Ash.Changeset.change_attribute(cs, :email, new_email)
else else
cs cs

26
lib/accounts/user/validations/oidc_email_collision.ex
View file

@ -69,13 +69,11 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do
# User exists with this email - check if it's an upsert or registration # User exists with this email - check if it's an upsert or registration
is_upsert = not is_nil(existing_oidc_user) is_upsert = not is_nil(existing_oidc_user)
handle_existing_user( if is_upsert do
user_with_email, handle_upsert_scenario(user_with_email, user_info, existing_oidc_user)
new_oidc_id, else
user_info, handle_create_scenario(user_with_email, new_oidc_id, user_info)
is_upsert, end
existing_oidc_user
)
{:error, error} -> {:error, error} ->
# Database error - log for debugging but don't expose internals to user # Database error - log for debugging but don't expose internals to user
@ -84,20 +82,6 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do
end end
end end
defp handle_existing_user(
user_with_email,
new_oidc_id,
user_info,
is_upsert,
existing_oidc_user
) do
if is_upsert do
handle_upsert_scenario(user_with_email, user_info, existing_oidc_user)
else
handle_create_scenario(user_with_email, new_oidc_id, user_info)
end
end
# Handle email update for existing OIDC user # Handle email update for existing OIDC user
defp handle_upsert_scenario(user_with_email, user_info, existing_oidc_user) do defp handle_upsert_scenario(user_with_email, user_info, existing_oidc_user) do
cond do cond do
moritz marked this conversation as resolved Outdated
rafael commented 2025-11-12 10:46:49 +01:00
Outdated
Copy link

Removing this function and inlining its code instead would be shorter and could be more readable, even at the cost of an extra level of indentation above.

Removing this function and inlining its code instead would be shorter and could be more readable, even at the cost of an extra level of indentation above.