All checks were successful
continuous-integration/drone/push Build is passing
## Description of the implemented changes The changes were: - [x] Bugfixing - [x] New Feature - [ ] Breaking Change - [x] Refactoring **OIDC-only mode improvements and UX tweaks (success toasts, unauthenticated redirect).** ## What has been changed? ### OIDC-only mode (new feature) - **Admin settings:** "Only OIDC sign-in" is an immediate toggle at the top of the OIDC section (no save button). Enabling it also turns off "Allow direct registration". When OIDC-only is on, the registration checkbox is disabled and shows a tooltip (DaisyUI `<.tooltip>`). - **Backend:** Password sign-in is forbidden via Ash policy (`OidcOnlyActive` check). Password registration is blocked via validation `OidcOnlyBlocksPasswordRegistration`. New plug `OidcOnlySignInRedirect`: when OIDC-only and OIDC are configured, GET `/sign-in` redirects to the OIDC flow; GET `/auth/user/password/sign_in_with_token` is rejected with redirect + flash. `AuthController.success/4` also rejects password sign-in when OIDC-only. - **Tests:** GlobalSettingsLive (OIDC-only UI), AuthController (redirect and password sign-in rejection), User authentication (register_with_password blocked when OIDC-only). ### UX / behaviour (no new feature flag) - **Success toasts:** Success flash messages auto-dismiss after 5 seconds via JS hook `FlashAutoDismiss` and optional `auto_clear_ms` on `<.flash>` (used for success in root layout and `flash_group`). - **Unauthenticated users:** Redirect to sign-in without the "You don't have permission to access this page" flash; that message is only shown to logged-in users who lack access. Logic in `LiveHelpers` and `CheckPagePermission` plug; test updated accordingly. ### Other - Layouts: comment about unprocessed join-request count no longer uses "TODO" (Credo). - Gettext: German translation for "Home" (Startseite); POT/PO kept in sync. - CHANGELOG: Unreleased section updated with the above. ## Definition of Done ### Code Quality - [x] No new technical depths - [x] Linting passed - [x] Documentation is added where needed (module docs, comments where non-obvious) ### Accessibility - [x] New elements are properly defined with html-tags (labels, aria-label on checkboxes) - [x] Colour contrast follows WCAG criteria (unchanged) - [x] Aria labels are added when needed (e.g. oidc-only and registration checkboxes) - [x] Everything is accessible by keyboard (toggles and buttons unchanged) - [x] Tab-Order is comprehensible - [x] All interactive elements have a visible focus (existing patterns) ### Testing - [x] Tests for new code are written (OIDC-only UI, auth controller, user auth; SMTP config builder and mailer) - [x] All tests pass - [ ] axe-core dev tools show no critical or major issues (not re-run for this PR; suggest spot-check on settings and sign-in) ## Additional Notes - **OIDC-only:** When the `OIDC_ONLY` env var is set, the toggle is read-only and shows "(From OIDC_ONLY)". When OIDC is not configured, the toggle is disabled. - **Invalidation:** Enabling OIDC-only sets `registration_enabled: false` in one update; disabling OIDC-only only updates `oidc_only` (registration left as-is). - **Review focus:** Plug order in router (OidcOnlySignInRedirect), policy/validation order in User, and that all OIDC-only paths (form, plug, controller) stay consistent. Reviewed-on: #474 Co-authored-by: Simon <s.thiessen@local-it.org> Co-committed-by: Simon <s.thiessen@local-it.org>
318 lines
8.8 KiB
Elixir
318 lines
8.8 KiB
Elixir
defmodule Mv.Accounts.UserAuthenticationTest do
|
|
@moduledoc """
|
|
Tests for user authentication and identification mechanisms.
|
|
|
|
This test suite verifies that:
|
|
- Password login correctly identifies users via email
|
|
- OIDC login correctly identifies users via oidc_id
|
|
- Session identifiers work as expected for both authentication methods
|
|
"""
|
|
use MvWeb.ConnCase, async: true
|
|
require Ash.Query
|
|
|
|
setup do
|
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
%{actor: system_actor}
|
|
end
|
|
|
|
describe "Password authentication user identification" do
|
|
@tag :test_proposal
|
|
test "password login uses email as identifier" do
|
|
# Create a user with password authentication (no oidc_id)
|
|
user =
|
|
create_test_user(%{
|
|
email: "password.user@example.com",
|
|
password: "securepassword123",
|
|
oidc_id: nil
|
|
})
|
|
|
|
# Verify that the user can be found by email
|
|
email_to_find = to_string(user.email)
|
|
|
|
{:ok, users} =
|
|
Mv.Accounts.User
|
|
|> Ash.Query.filter(email == ^email_to_find)
|
|
|> Ash.read(actor: user)
|
|
|
|
assert length(users) == 1
|
|
found_user = List.first(users)
|
|
assert found_user.id == user.id
|
|
assert to_string(found_user.email) == "password.user@example.com"
|
|
assert is_nil(found_user.oidc_id)
|
|
end
|
|
|
|
@tag :test_proposal
|
|
test "multiple users can exist with different emails" do
|
|
user1 =
|
|
create_test_user(%{
|
|
email: "user1@example.com",
|
|
password: "password123",
|
|
oidc_id: nil
|
|
})
|
|
|
|
user2 =
|
|
create_test_user(%{
|
|
email: "user2@example.com",
|
|
password: "password456",
|
|
oidc_id: nil
|
|
})
|
|
|
|
assert user1.id != user2.id
|
|
assert to_string(user1.email) != to_string(user2.email)
|
|
end
|
|
|
|
@tag :test_proposal
|
|
test "users with same password but different emails are separate accounts" do
|
|
same_password = "shared_password_123"
|
|
|
|
user1 =
|
|
create_test_user(%{
|
|
email: "alice@example.com",
|
|
password: same_password,
|
|
oidc_id: nil
|
|
})
|
|
|
|
user2 =
|
|
create_test_user(%{
|
|
email: "bob@example.com",
|
|
password: same_password,
|
|
oidc_id: nil
|
|
})
|
|
|
|
# Different users despite same password
|
|
assert user1.id != user2.id
|
|
|
|
# Both passwords should hash to different values (bcrypt uses salt)
|
|
assert user1.hashed_password != user2.hashed_password
|
|
end
|
|
end
|
|
|
|
describe "OIDC authentication user identification" do
|
|
@tag :test_proposal
|
|
test "OIDC login with matching oidc_id finds correct user" do
|
|
# Create user with OIDC authentication
|
|
user =
|
|
create_test_user(%{
|
|
email: "oidc.user@example.com",
|
|
oidc_id: "oidc_identifier_12345"
|
|
})
|
|
|
|
# Simulate OIDC callback
|
|
user_info = %{
|
|
"sub" => "oidc_identifier_12345",
|
|
"preferred_username" => "oidc.user@example.com"
|
|
}
|
|
|
|
# Use sign_in_with_oidc to find user by oidc_id
|
|
# Note: This test will FAIL until we implement the security fix
|
|
# that changes the filter from email to oidc_id
|
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
|
|
result =
|
|
Mv.Accounts.read_sign_in_with_oidc(
|
|
%{
|
|
user_info: user_info,
|
|
oauth_tokens: %{}
|
|
},
|
|
actor: system_actor
|
|
)
|
|
|
|
case result do
|
|
{:ok, found_user} when is_struct(found_user) ->
|
|
assert found_user.id == user.id
|
|
assert found_user.oidc_id == "oidc_identifier_12345"
|
|
|
|
{:ok, [found_user]} ->
|
|
assert found_user.id == user.id
|
|
assert found_user.oidc_id == "oidc_identifier_12345"
|
|
|
|
{:ok, []} ->
|
|
flunk("User should be found by oidc_id")
|
|
|
|
{:ok, nil} ->
|
|
flunk("User should be found by oidc_id")
|
|
|
|
{:error, error} ->
|
|
flunk("Unexpected error: #{inspect(error)}")
|
|
end
|
|
end
|
|
|
|
@tag :test_proposal
|
|
test "OIDC login creates new user when both email and oidc_id are new" do
|
|
# Completely new user from OIDC provider
|
|
user_info = %{
|
|
"sub" => "brand_new_oidc_789",
|
|
"preferred_username" => "newuser@example.com"
|
|
}
|
|
|
|
# Should create via register_with_oidc
|
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
|
|
{:ok, new_user} =
|
|
Mv.Accounts.create_register_with_oidc(
|
|
%{
|
|
user_info: user_info,
|
|
oauth_tokens: %{}
|
|
},
|
|
actor: system_actor
|
|
)
|
|
|
|
assert to_string(new_user.email) == "newuser@example.com"
|
|
assert new_user.oidc_id == "brand_new_oidc_789"
|
|
assert is_nil(new_user.hashed_password)
|
|
end
|
|
|
|
@tag :test_proposal
|
|
test "OIDC user can be uniquely identified by oidc_id" do
|
|
user1 =
|
|
create_test_user(%{
|
|
email: "user1@example.com",
|
|
oidc_id: "oidc_unique_1"
|
|
})
|
|
|
|
user2 =
|
|
create_test_user(%{
|
|
email: "user2@example.com",
|
|
oidc_id: "oidc_unique_2"
|
|
})
|
|
|
|
# Find by oidc_id
|
|
{:ok, users1} =
|
|
Mv.Accounts.User
|
|
|> Ash.Query.filter(oidc_id == "oidc_unique_1")
|
|
|> Ash.read(actor: user1)
|
|
|
|
{:ok, users2} =
|
|
Mv.Accounts.User
|
|
|> Ash.Query.filter(oidc_id == "oidc_unique_2")
|
|
|> Ash.read(actor: user2)
|
|
|
|
assert length(users1) == 1
|
|
assert length(users2) == 1
|
|
assert List.first(users1).id == user1.id
|
|
assert List.first(users2).id == user2.id
|
|
end
|
|
end
|
|
|
|
describe "Mixed authentication scenarios" do
|
|
@tag :test_proposal
|
|
test "user with oidc_id cannot be found by email-only query in sign_in_with_oidc" do
|
|
# This test verifies the security fix: sign_in_with_oidc should NOT
|
|
# match users by email, only by oidc_id
|
|
|
|
_user =
|
|
create_test_user(%{
|
|
email: "secure@example.com",
|
|
oidc_id: "secure_oidc_999"
|
|
})
|
|
|
|
# Try to sign in with DIFFERENT oidc_id but SAME email
|
|
user_info = %{
|
|
# Different oidc_id!
|
|
"sub" => "attacker_oidc_888",
|
|
# Same email
|
|
"preferred_username" => "secure@example.com"
|
|
}
|
|
|
|
# Should NOT find the user (security requirement)
|
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
|
|
result =
|
|
Mv.Accounts.read_sign_in_with_oidc(
|
|
%{
|
|
user_info: user_info,
|
|
oauth_tokens: %{}
|
|
},
|
|
actor: system_actor
|
|
)
|
|
|
|
# Either returns empty/nil OR authentication error - both mean "user not found"
|
|
case result do
|
|
{:ok, []} ->
|
|
:ok
|
|
|
|
{:ok, nil} ->
|
|
:ok
|
|
|
|
{:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} ->
|
|
:ok
|
|
|
|
other ->
|
|
flunk("sign_in_with_oidc should not match by email alone, got: #{inspect(other)}")
|
|
end
|
|
end
|
|
|
|
@tag :test_proposal
|
|
test "password user (oidc_id=nil) is not found by sign_in_with_oidc" do
|
|
# Create a password-only user
|
|
_user =
|
|
create_test_user(%{
|
|
email: "password.only@example.com",
|
|
password: "securepass123",
|
|
oidc_id: nil
|
|
})
|
|
|
|
# Try OIDC sign-in with this email
|
|
user_info = %{
|
|
"sub" => "new_oidc_777",
|
|
"preferred_username" => "password.only@example.com"
|
|
}
|
|
|
|
# Should NOT find the user because oidc_id is nil
|
|
system_actor = Mv.Helpers.SystemActor.get_system_actor()
|
|
|
|
result =
|
|
Mv.Accounts.read_sign_in_with_oidc(
|
|
%{
|
|
user_info: user_info,
|
|
oauth_tokens: %{}
|
|
},
|
|
actor: system_actor
|
|
)
|
|
|
|
# Either returns empty/nil OR authentication error - both mean "user not found"
|
|
case result do
|
|
{:ok, []} ->
|
|
:ok
|
|
|
|
{:ok, nil} ->
|
|
:ok
|
|
|
|
{:error, %Ash.Error.Forbidden{errors: [%AshAuthentication.Errors.AuthenticationFailed{}]}} ->
|
|
:ok
|
|
|
|
other ->
|
|
flunk(
|
|
"Password-only user should not be found by sign_in_with_oidc, got: #{inspect(other)}"
|
|
)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "register_with_password when OIDC-only is enabled" do
|
|
alias Mv.Membership
|
|
|
|
test "returns error when OIDC-only is enabled" do
|
|
{:ok, settings} = Membership.get_settings()
|
|
original_oidc_only = Map.get(settings, :oidc_only, false)
|
|
{:ok, _} = Membership.update_settings(settings, %{oidc_only: true})
|
|
|
|
try do
|
|
attrs = %{
|
|
email: "newuser#{System.unique_integer([:positive])}@example.com",
|
|
password: "SecurePassword123"
|
|
}
|
|
|
|
result =
|
|
Mv.Accounts.User
|
|
|> Ash.Changeset.for_create(:register_with_password, attrs)
|
|
|> Ash.create()
|
|
|
|
assert {:error, _} = result
|
|
after
|
|
{:ok, s} = Membership.get_settings()
|
|
Membership.update_settings(s, %{oidc_only: original_oidc_only})
|
|
end
|
|
end
|
|
end
|
|
end
|