diff --git a/.env.example b/.env.example index d63e019..e24b118 100644 --- a/.env.example +++ b/.env.example @@ -14,7 +14,6 @@ ASSOCIATION_NAME="Sportsclub XYZ" # Optional: Admin user (created/updated on container start via Release.seed_admin) # In production, set these so the first admin can log in. Change password without redeploy: # bin/mv eval "Mv.Release.seed_admin()" (with new ADMIN_PASSWORD or ADMIN_PASSWORD_FILE) -# FORCE_SEEDS=true re-runs bootstrap seeds even when admin user exists (e.g. after changing roles/custom fields). # ADMIN_EMAIL=admin@example.com # ADMIN_PASSWORD=secure-password # ADMIN_PASSWORD_FILE=/run/secrets/admin_password @@ -42,15 +41,3 @@ ASSOCIATION_NAME="Sportsclub XYZ" # VEREINFACHT_API_KEY=your-api-key # VEREINFACHT_CLUB_ID=2 # VEREINFACHT_APP_URL=https://app.verein.visuel.dev - -# Optional: Mail / SMTP (transactional emails). If set, overrides Settings UI. -# Export current UI settings to .env: mix mv.export_smtp_to_env -# SMTP_HOST=smtp.example.com -# SMTP_PORT=587 -# SMTP_USERNAME=user -# SMTP_PASSWORD=secret -# SMTP_PASSWORD_FILE=/run/secrets/smtp_password -# SMTP_SSL=tls -# SMTP_VERIFY_PEER=false -# MAIL_FROM_EMAIL=noreply@example.com -# MAIL_FROM_NAME=Mila diff --git a/CHANGELOG.md b/CHANGELOG.md index b94ce50..681169f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,19 +5,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.1.1] - 2026-03-16 - -### Added -- **FORCE_SEEDS** – Environment variable. When set to `"true"`, bootstrap (and optionally dev) seeds are run even when the admin user already exists, so you can re-apply changed seed data (e.g. new roles or custom fields) without deleting the admin user. -- **Improved OIDC-only mode** – Admin can enable “Only OIDC sign-in” in settings; when enabled, direct registration is disabled and sign-in page redirects to OIDC when configured. -- **Success toast auto-dismiss** – Success flash messages (e.g. “Settings saved”) hide automatically after 5 seconds instead of requiring the user to close them. - -### Changed -- **Seeds run only when needed** – Bootstrap and dev seeds are skipped on application start when the admin user already exists (`Mv.Release.bootstrap_seeds_applied?/0`). This avoids duplicate data and speeds up startup in dev and production after the first run. Set `FORCE_SEEDS=true` to override and re-run. -- **Unauthenticated access** – Users who are not logged in are redirected to sign-in without showing a “no permission” message; the message is only shown to logged-in users who lack access. - -### Fixed -- **SMTP configuration** – Repaired so that both port 587 (TLS/STARTTLS) and 465 (SSL) work correctly. +## [Unreleased] ## [1.1.0] - 2026-03-13 diff --git a/CODE_GUIDELINES.md b/CODE_GUIDELINES.md index f84c5ad..8d53484 100644 --- a/CODE_GUIDELINES.md +++ b/CODE_GUIDELINES.md @@ -130,8 +130,6 @@ lib/ │ ├── constants.ex # Application constants (member_fields, custom_field_prefix, vereinfacht_required_member_fields) │ ├── application.ex # OTP application │ ├── mailer.ex # Email mailer -│ ├── smtp/ -│ │ └── config_builder.ex # SMTP adapter opts (TLS/sockopts); used by runtime.exs and Mailer │ ├── release.ex # Release tasks │ ├── repo.ex # Database repository │ ├── secrets.ex # Secret management @@ -284,13 +282,13 @@ end ### 1.2.1 Database Seeds -Seeds are split into **bootstrap** and **dev**. They run on every start (e.g. `just run`, Docker entrypoint) but **exit early** if already applied so startup stays fast and no duplicate data is created. +Seeds are split into **bootstrap** and **dev**: -- **`priv/repo/seeds.exs`** – Entrypoint. If the admin user (ADMIN_EMAIL or default) already exists, skips entirely (unless `FORCE_SEEDS=true`); otherwise runs `seeds_bootstrap.exs` and, in dev/test, `seeds_dev.exs`. +- **`priv/repo/seeds.exs`** – Entrypoint. Runs `seeds_bootstrap.exs` always; runs `seeds_dev.exs` only when `Mix.env()` is `:dev` or `:test`. - **`priv/repo/seeds_bootstrap.exs`** – Creates only data required for system startup: membership fee types, custom fields, roles, admin user, system user, global settings (including default membership fee type). No members, no groups. Used in all environments (dev, test, prod). - **`priv/repo/seeds_dev.exs`** – Creates 20 sample members, groups, and optional custom field values. Run only in dev and test. -In production, running `mix run priv/repo/seeds.exs` (or `Mv.Release.run_seeds/0`) executes only the bootstrap part when not yet applied (no dev seeds unless `RUN_DEV_SEEDS=true`). The “already applied” check uses `Mv.Release.bootstrap_seeds_applied?/0` (admin user exists). Set `FORCE_SEEDS=true` to re-run seeds even when already applied. +In production, running `mix run priv/repo/seeds.exs` executes only the bootstrap part (no dev seeds). ### 1.3 Domain-Driven Design diff --git a/assets/js/app.js b/assets/js/app.js index 4c7e3c5..87f2c25 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -113,25 +113,6 @@ Hooks.FocusRestore = { } } -// FlashAutoDismiss: after a delay, clear the flash so the toast hides without user clicking X (e.g. success toasts) -Hooks.FlashAutoDismiss = { - mounted() { - const ms = this.el.dataset.autoClearMs - if (!ms) return - const delay = parseInt(ms, 10) - if (delay > 0) { - this.timer = setTimeout(() => { - const key = this.el.dataset.clearFlashKey || "success" - this.pushEvent("lv:clear-flash", {key}) - }, delay) - } - }, - - destroyed() { - if (this.timer) clearTimeout(this.timer) - } -} - // TabListKeydown hook: WCAG tab pattern — prevent default for ArrowLeft/ArrowRight so the server can handle tab switch (roving tabindex) Hooks.TabListKeydown = { mounted() { diff --git a/config/runtime.exs b/config/runtime.exs index 6a434fa..1c55f64 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -226,7 +226,11 @@ if config_env() == :prod do # SMTP configuration from environment variables (overrides base adapter in prod). # When SMTP_HOST is set, configure Swoosh to use the SMTP adapter at boot time. # If SMTP is configured only via Settings (Admin UI), the mailer builds the config - # per-send at runtime using Mv.Mailer.smtp_config/0 (which uses the same Mv.Smtp.ConfigBuilder). + # per-send at runtime using Mv.Config.smtp_*() helpers. + # + # TLS/SSL options (tls_options, sockopts) are duplicated here and in Mv.Mailer.smtp_config/0 + # because boot config must be set in this file; the Mailer uses the same logic for + # Settings-only config. Keep verify behaviour in sync (see SMTP_VERIFY_PEER below). smtp_host_env = System.get_env("SMTP_HOST") if smtp_host_env && String.trim(smtp_host_env) != "" do @@ -260,14 +264,20 @@ if config_env() == :prod do verify_mode = if smtp_verify_peer, do: :verify_peer, else: :verify_none smtp_opts = - Mv.Smtp.ConfigBuilder.build_opts( - host: String.trim(smtp_host_env), + [ + adapter: Swoosh.Adapters.SMTP, + relay: String.trim(smtp_host_env), port: smtp_port_env, username: System.get_env("SMTP_USERNAME"), password: smtp_password_env, - ssl_mode: smtp_ssl_mode, - verify_mode: verify_mode - ) + ssl: smtp_ssl_mode == "ssl", + tls: if(smtp_ssl_mode == "tls", do: :always, else: :never), + auth: :always, + # tls_options: STARTTLS (587); sockopts: direct SSL (465). + tls_options: [verify: verify_mode], + sockopts: [verify: verify_mode] + ] + |> Enum.reject(fn {_k, v} -> is_nil(v) end) config :mv, Mv.Mailer, smtp_opts end diff --git a/docs/admin-bootstrap-and-oidc-role-sync.md b/docs/admin-bootstrap-and-oidc-role-sync.md index 5413f91..5e26c85 100644 --- a/docs/admin-bootstrap-and-oidc-role-sync.md +++ b/docs/admin-bootstrap-and-oidc-role-sync.md @@ -2,7 +2,7 @@ ## Overview -- **Admin bootstrap:** In production, the Docker entrypoint runs migrate, then `Mv.Release.run_seeds/0` (skips if admin user already exists unless `FORCE_SEEDS=true`; set `RUN_DEV_SEEDS=true` to also run dev seeds), then `seed_admin/0` from ENV, then the server. Password can be changed without redeploy via `bin/mv eval "Mv.Release.seed_admin()"`. +- **Admin bootstrap:** In production, the Docker entrypoint runs migrate, then `Mv.Release.run_seeds/0` (bootstrap seeds; set `RUN_DEV_SEEDS=true` to also run dev seeds), then `seed_admin/0` from ENV, then the server. Password can be changed without redeploy via `bin/mv eval "Mv.Release.seed_admin()"`. - **OIDC role sync:** Optional mapping from OIDC groups (e.g. from Authentik profile scope) to the Admin role. Users in the configured admin group get the Admin role on registration and on each sign-in. ## Admin Bootstrap (Part A) @@ -10,14 +10,13 @@ ### Environment Variables - `RUN_DEV_SEEDS` – If set to `"true"`, `run_seeds/0` also runs dev seeds (members, groups, sample data). Otherwise only bootstrap seeds run. -- `FORCE_SEEDS` – If set to `"true"`, seeds are run even when the admin user already exists (e.g. after changing bootstrap data such as roles or custom fields). Otherwise seeds are skipped when bootstrap was already applied. - `ADMIN_EMAIL` – Email of the admin user to create/update. If unset, seed_admin/0 does nothing. - `ADMIN_PASSWORD` – Password for the admin user. If unset (and no file), no new user is created; if a user with ADMIN_EMAIL already exists (e.g. OIDC-only), their role is set to Admin (no password change). - `ADMIN_PASSWORD_FILE` – Path to a file containing the password (e.g. Docker secret). ### Release Tasks -- `Mv.Release.run_seeds/0` – If the admin user already exists (bootstrap already applied), skips unless `FORCE_SEEDS=true`; otherwise runs bootstrap seeds (fee types, custom fields, roles, settings). If `RUN_DEV_SEEDS` env is `"true"`, also runs dev seeds (members, groups, sample data). Safe to call on every start. +- `Mv.Release.run_seeds/0` – Runs bootstrap seeds (fee types, custom fields, roles, settings). If `RUN_DEV_SEEDS` env is `"true"`, also runs dev seeds (members, groups, sample data). Idempotent. - `Mv.Release.seed_admin/0` – Reads ADMIN_EMAIL and password from ADMIN_PASSWORD or ADMIN_PASSWORD_FILE. If both email and password are set: creates or updates the user with the Admin role. If only ADMIN_EMAIL is set: sets the Admin role on an existing user with that email (for OIDC-only admins); does not create a user. Idempotent. ### Entrypoint @@ -39,7 +38,6 @@ ### Sign-in page (OIDC-only mode) - `OIDC_ONLY` (or Settings → OIDC → "Only OIDC sign-in") – When set to true/1/yes and OIDC is configured, the sign-in page shows only the Single Sign-On button (password login is hidden). ENV takes precedence over Settings. -- **Redirect loop fix:** After an OIDC failure (e.g. provider down), the app redirects to `/sign-in?oidc_failed=1`. The plug `OidcOnlySignInRedirect` does not redirect that request back to OIDC, so the sign-in page is shown with the error (no endless redirect). ### Sync Logic diff --git a/docs/feature-roadmap.md b/docs/feature-roadmap.md index 2ec15a5..6383660 100644 --- a/docs/feature-roadmap.md +++ b/docs/feature-roadmap.md @@ -49,11 +49,6 @@ - ✅ **Page-level authorization** - LiveView page access control - ✅ **System role protection** - Critical roles cannot be deleted -**Planned: OIDC-only mode (TDD, tests first):** -- Admin Settings: When OIDC-only is enabled, disable "Allow direct registration" toggle and show hint (tests in `GlobalSettingsLiveTest`). -- Backend: Reject password sign-in and `register_with_password` when OIDC-only (tests in `AuthControllerTest`, `Accounts`). -- GET `/sign-in` redirect to OIDC when OIDC-only and OIDC configured (tests in `AuthControllerTest`). Implementation to follow after tests. - **Missing Features:** - ❌ Password reset flow - ❌ Email verification diff --git a/docs/smtp-configuration-concept.md b/docs/smtp-configuration-concept.md index 13b0d17..8832b5e 100644 --- a/docs/smtp-configuration-concept.md +++ b/docs/smtp-configuration-concept.md @@ -105,7 +105,7 @@ By default, TLS certificate verification is relaxed (`verify_none`) so self-sign - **ENV (prod):** Set `SMTP_VERIFY_PEER=true` (or `1`/`yes`) when configuring SMTP via environment variables in `config/runtime.exs`. This sets `config :mv, :smtp_verify_peer` and is used for both boot-time and per-send config. - **Default:** `false` (verify_none) for backward compatibility and internal/self-signed certs. -Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (implicit SSL), the initial connection is `ssl:connect`, so we also pass `sockopts: [verify: verify_mode]` so the SSL handshake uses the same mode. For 587 we must not pass `verify` in sockopts—gen_tcp is used first and rejects it (ArgumentError). The logic lives in `Mv.Smtp.ConfigBuilder.build_opts/1` (single source of truth), used by `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only). +Both `tls_options` (STARTTLS, port 587) and `sockopts` (direct SSL, port 465) use the same verify mode. The logic is duplicated in `config/runtime.exs` (boot) and `Mv.Mailer.smtp_config/0` (Settings-only); keep in sync. --- @@ -117,7 +117,7 @@ Verify mode is set in `tls_options` for port 587 (STARTTLS). For port 465 (impli - [x] Password from file: `SMTP_PASSWORD_FILE` supported in `runtime.exs`. - [x] Mailer: Swoosh SMTP adapter configured from merged ENV + Settings when SMTP is configured. - [x] Per-request SMTP config via `Mv.Mailer.smtp_config/0` for Settings-only scenarios. -- [x] TLS certificate validation relaxed for OTP 27 (tls_options for 587; sockopts with verify only for 465). +- [x] TLS certificate validation relaxed for OTP 27 (tls_options + sockopts). - [x] Prod warning: clear message in Settings when SMTP is not configured. - [x] Test email: form with recipient field, translatable content, classified success/error messages. - [x] Join confirmation email: uses `Mailer.smtp_config/0` (same as test mail); on failure returns `{:error, :email_delivery_failed}`, error shown in JoinLive, logged for admin. diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 0127796..29a2d4b 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -362,12 +362,6 @@ defmodule Mv.Accounts.User do # Authorization Policies # Order matters: Most specific policies first, then general permission check policies do - # When OIDC-only is active, password sign-in is forbidden (SSO only). - policy action(:sign_in_with_password) do - forbid_if Mv.Authorization.Checks.OidcOnlyActive - authorize_if always() - end - # AshAuthentication bypass (registration/login without actor) bypass AshAuthentication.Checks.AshAuthenticationInteraction do description "Allow AshAuthentication internal operations (registration, login)" @@ -415,10 +409,6 @@ defmodule Mv.Accounts.User do validate {Mv.Accounts.User.Validations.RegistrationEnabled, []}, where: [action_is(:register_with_password)] - # Block password registration when OIDC-only mode is active - validate {Mv.Accounts.User.Validations.OidcOnlyBlocksPasswordRegistration, []}, - where: [action_is(:register_with_password)] - # Email uniqueness check for all actions that change the email attribute # Validates that user email is not already used by another (unlinked) member validate Mv.Accounts.User.Validations.EmailNotUsedByOtherMember diff --git a/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex b/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex deleted file mode 100644 index e4d9a35..0000000 --- a/lib/accounts/user/validations/oidc_only_blocks_password_registration.ex +++ /dev/null @@ -1,27 +0,0 @@ -defmodule Mv.Accounts.User.Validations.OidcOnlyBlocksPasswordRegistration do - @moduledoc """ - Validation that blocks direct registration (register_with_password) when - OIDC-only mode is active. In OIDC-only mode, sign-in and registration are - only allowed via OIDC (SSO). - """ - use Ash.Resource.Validation - - @impl true - def init(opts), do: {:ok, opts} - - @impl true - def validate(_changeset, _opts, _context) do - if Mv.Config.oidc_only?() do - {:error, - field: :base, - message: - Gettext.dgettext( - MvWeb.Gettext, - "default", - "Registration with password is disabled when only OIDC sign-in is active." - )} - else - :ok - end - end -end diff --git a/lib/mv/authorization/checks/oidc_only_active.ex b/lib/mv/authorization/checks/oidc_only_active.ex deleted file mode 100644 index 8d56ca1..0000000 --- a/lib/mv/authorization/checks/oidc_only_active.ex +++ /dev/null @@ -1,16 +0,0 @@ -defmodule Mv.Authorization.Checks.OidcOnlyActive do - @moduledoc """ - Policy check: true when OIDC-only mode is active (Config.oidc_only?()). - - Used to forbid password sign-in when only OIDC (SSO) sign-in is allowed. - """ - use Ash.Policy.SimpleCheck - - alias Mv.Config - - @impl true - def describe(_opts), do: "OIDC-only mode is active" - - @impl true - def match?(_actor, _context, _opts), do: Config.oidc_only?() -end diff --git a/lib/mv/mailer.ex b/lib/mv/mailer.ex index 41a77cd..e5ac4e9 100644 --- a/lib/mv/mailer.ex +++ b/lib/mv/mailer.ex @@ -31,7 +31,6 @@ defmodule Mv.Mailer do import Swoosh.Email use Gettext, backend: MvWeb.Gettext, otp_app: :mv - alias Mv.Smtp.ConfigBuilder require Logger # Simple format check for test-email recipient only (e.g. allows a@b.c). Not for strict RFC validation. @@ -101,19 +100,31 @@ defmodule Mv.Mailer do @spec smtp_config() :: keyword() def smtp_config do if Mv.Config.smtp_configured?() and not boot_smtp_configured?() do + host = Mv.Config.smtp_host() + port = Mv.Config.smtp_port() || 587 + username = Mv.Config.smtp_username() + password = Mv.Config.smtp_password() + ssl_mode = Mv.Config.smtp_ssl() || "tls" + verify_mode = if Application.get_env(:mv, :smtp_verify_peer, false), do: :verify_peer, else: :verify_none - ConfigBuilder.build_opts( - host: Mv.Config.smtp_host(), - port: Mv.Config.smtp_port() || 587, - username: Mv.Config.smtp_username(), - password: Mv.Config.smtp_password(), - ssl_mode: Mv.Config.smtp_ssl() || "tls", - verify_mode: verify_mode - ) + [ + adapter: Swoosh.Adapters.SMTP, + relay: host, + port: port, + ssl: ssl_mode == "ssl", + tls: if(ssl_mode == "tls", do: :always, else: :never), + auth: :always, + username: username, + password: password, + # tls_options: STARTTLS (587); sockopts: direct SSL (465). Verify from :smtp_verify_peer (ENV SMTP_VERIFY_PEER). + tls_options: [verify: verify_mode], + sockopts: [verify: verify_mode] + ] + |> Enum.reject(fn {_k, v} -> is_nil(v) end) else [] end diff --git a/lib/mv/release.ex b/lib/mv/release.ex index 116b276..00dcadf 100644 --- a/lib/mv/release.ex +++ b/lib/mv/release.ex @@ -6,8 +6,8 @@ defmodule Mv.Release do ## Tasks - `migrate/0` - Runs all pending Ecto migrations. - - `bootstrap_seeds_applied?/0` - Returns whether bootstrap was already applied (admin user exists). Used to skip re-running seeds. - - `run_seeds/0` - If bootstrap already applied, skips; otherwise runs bootstrap seeds (fee types, custom fields, roles, settings). Set `FORCE_SEEDS=true` to re-run seeds even when already applied. In production, set `RUN_DEV_SEEDS=true` to also run dev seeds (members, groups, sample data). + - `run_seeds/0` - Runs bootstrap seeds (fee types, custom fields, roles, settings). + In production, set `RUN_DEV_SEEDS=true` to also run dev seeds (members, groups, sample data). - `seed_admin/0` - Ensures an admin user exists from ENV (ADMIN_EMAIL, ADMIN_PASSWORD or ADMIN_PASSWORD_FILE). Idempotent; can be run on every deployment or via shell to update the admin password without redeploying. @@ -19,7 +19,6 @@ defmodule Mv.Release do alias Mv.Authorization.Role require Ash.Query - require Logger def migrate do load_app() @@ -29,37 +28,13 @@ defmodule Mv.Release do end end - @doc """ - Returns whether bootstrap seeds have already been applied (admin user exists). - - We check for the admin user (from ADMIN_EMAIL or default), not the Admin role, - because migrations may create the Admin role for the system actor. Only seeds - create the admin (login) user. Used to skip re-running seeds on subsequent starts. - Call only when the application is already started. - """ - def bootstrap_seeds_applied? do - admin_email = get_env("ADMIN_EMAIL", "admin@localhost") - - case User - |> Ash.Query.filter(email == ^admin_email) - |> Ash.read_one(authorize?: false, domain: Mv.Accounts) do - {:ok, %User{}} -> true - _ -> false - end - rescue - e -> - Logger.warning("Could not check seed status (#{inspect(e)}), assuming not applied.") - false - end - @doc """ Runs seed scripts so the database has required bootstrap data (and optionally dev data). - - Skips if bootstrap was already applied (admin user exists); set `FORCE_SEEDS=true` to override and re-run. - - If `RUN_DEV_SEEDS` env is set to `"true"`, also runs dev seeds (members, groups, sample data) - when bootstrap is run. + - Always runs bootstrap seeds (fee types, custom fields, roles, system user, settings). + - If `RUN_DEV_SEEDS` env is set to `"true"`, also runs dev seeds (members, groups, sample data). - Uses paths from the application's priv dir so it works in releases (no Mix). + Uses paths from the application's priv dir so it works in releases (no Mix). Idempotent. """ def run_seeds do case Application.ensure_all_started(@app) do @@ -67,27 +42,23 @@ defmodule Mv.Release do {:error, {app, reason}} -> raise "Failed to start #{inspect(app)}: #{inspect(reason)}" end - if bootstrap_seeds_applied?() and System.get_env("FORCE_SEEDS") != "true" do - IO.puts("Seeds already applied. Skipping. (Set FORCE_SEEDS=true to override)") - else - priv = :code.priv_dir(@app) - bootstrap_path = Path.join(priv, "repo/seeds_bootstrap.exs") - dev_path = Path.join(priv, "repo/seeds_dev.exs") + priv = :code.priv_dir(@app) + bootstrap_path = Path.join(priv, "repo/seeds_bootstrap.exs") + dev_path = Path.join(priv, "repo/seeds_dev.exs") - prev = Code.compiler_options() - Code.compiler_options(ignore_module_conflict: true) + prev = Code.compiler_options() + Code.compiler_options(ignore_module_conflict: true) - try do - Code.eval_file(bootstrap_path) - IO.puts("✅ Bootstrap seeds completed.") + try do + Code.eval_file(bootstrap_path) + IO.puts("✅ Bootstrap seeds completed.") - if System.get_env("RUN_DEV_SEEDS") == "true" do - Code.eval_file(dev_path) - IO.puts("✅ Dev seeds completed.") - end - after - Code.compiler_options(prev) + if System.get_env("RUN_DEV_SEEDS") == "true" do + Code.eval_file(dev_path) + IO.puts("✅ Dev seeds completed.") end + after + Code.compiler_options(prev) end end diff --git a/lib/mv/smtp/config_builder.ex b/lib/mv/smtp/config_builder.ex deleted file mode 100644 index 5018dff..0000000 --- a/lib/mv/smtp/config_builder.ex +++ /dev/null @@ -1,58 +0,0 @@ -defmodule Mv.Smtp.ConfigBuilder do - @moduledoc """ - Builds Swoosh/gen_smtp SMTP adapter options from connection parameters. - - Single source of truth for TLS/sockopts logic (port 587 vs 465): - - Port 587 (STARTTLS): `gen_tcp` is used first; `sockopts` must NOT contain `:verify`. - - Port 465 (implicit SSL): initial connection is `ssl:connect`; `sockopts` must contain `:verify`. - - Used by `config/runtime.exs` (boot-time ENV) and `Mv.Mailer.smtp_config/0` (Settings-only). - """ - - @doc """ - Builds the keyword list of Swoosh SMTP adapter options. - - Options (keyword list): - - `:host` (required) — relay hostname - - `:port` (required) — port number (e.g. 587 or 465) - - `:ssl_mode` (required) — `"tls"` or `"ssl"` - - `:verify_mode` (required) — `:verify_peer` or `:verify_none` - - `:username` (optional) - - `:password` (optional) - - Nil values are stripped from the result. - """ - @spec build_opts(keyword()) :: keyword() - def build_opts(opts) do - host = Keyword.fetch!(opts, :host) - port = Keyword.fetch!(opts, :port) - username = Keyword.get(opts, :username) - password = Keyword.get(opts, :password) - ssl_mode = Keyword.fetch!(opts, :ssl_mode) - verify_mode = Keyword.fetch!(opts, :verify_mode) - - base_opts = [ - adapter: Swoosh.Adapters.SMTP, - relay: host, - port: port, - username: username, - password: password, - ssl: ssl_mode == "ssl", - tls: if(ssl_mode == "tls", do: :always, else: :never), - auth: :always, - # tls_options: used for STARTTLS (587). For 465, gen_smtp uses sockopts for initial ssl:connect. - tls_options: [verify: verify_mode] - ] - - # Port 465: initial connection is ssl:connect; pass verify in sockopts. - # Port 587: initial connection is gen_tcp; sockopts must NOT contain verify (gen_tcp rejects it). - opts = - if ssl_mode == "ssl" do - Keyword.put(base_opts, :sockopts, verify: verify_mode) - else - base_opts - end - - Enum.reject(opts, fn {_k, v} -> is_nil(v) end) - end -end diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index b5bd763..8c58c32 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -63,11 +63,6 @@ defmodule MvWeb.CoreComponents do values: [:info, :error, :success, :warning], doc: "used for styling and flash lookup" - attr :auto_clear_ms, :integer, - default: nil, - doc: - "when set, flash is auto-dismissed after this many milliseconds (e.g. 5000 for success toasts)" - attr :rest, :global, doc: "the arbitrary HTML attributes to add to the flash container" slot :inner_block, doc: "the optional inner block that renders the flash message" @@ -79,9 +74,6 @@ defmodule MvWeb.CoreComponents do
hide("##{@id}")} role="alert" class="pointer-events-auto" diff --git a/lib/mv_web/components/layouts.ex b/lib/mv_web/components/layouts.ex index 54f589d..5a96001 100644 --- a/lib/mv_web/components/layouts.ex +++ b/lib/mv_web/components/layouts.ex @@ -265,7 +265,7 @@ defmodule MvWeb.Layouts do aria-live="polite" class="z-50 toast toast-bottom toast-end flex flex-col gap-2 pointer-events-none" > - <.flash kind={:success} flash={@flash} auto_clear_ms={5000} /> + <.flash kind={:success} flash={@flash} /> <.flash kind={:warning} flash={@flash} /> <.flash kind={:info} flash={@flash} /> <.flash kind={:error} flash={@flash} /> diff --git a/lib/mv_web/components/layouts/root.html.heex b/lib/mv_web/components/layouts/root.html.heex index bb900aa..5419b73 100644 --- a/lib/mv_web/components/layouts/root.html.heex +++ b/lib/mv_web/components/layouts/root.html.heex @@ -74,7 +74,7 @@ aria-live="polite" class="z-50 flex flex-col gap-2 toast toast-bottom toast-end" > - <.flash id="flash-success-root" kind={:success} flash={@flash} auto_clear_ms={5000} /> + <.flash id="flash-success-root" kind={:success} flash={@flash} /> <.flash id="flash-warning-root" kind={:warning} flash={@flash} /> <.flash id="flash-info-root" kind={:info} flash={@flash} /> <.flash id="flash-error-root" kind={:error} flash={@flash} /> diff --git a/lib/mv_web/controllers/auth_controller.ex b/lib/mv_web/controllers/auth_controller.ex index adde4e8..20a76f5 100644 --- a/lib/mv_web/controllers/auth_controller.ex +++ b/lib/mv_web/controllers/auth_controller.ex @@ -15,23 +15,8 @@ defmodule MvWeb.AuthController do use AshAuthentication.Phoenix.Controller alias Mv.Accounts.User.Errors.PasswordVerificationRequired - alias Mv.Config - def success(conn, {:password, :sign_in} = _activity, user, token) do - if Config.oidc_only?() do - conn - |> put_flash(:error, gettext("Only sign-in via Single Sign-On (SSO) is allowed.")) - |> redirect(to: sign_in_path_after_oidc_failure()) - else - success_continue(conn, {:password, :sign_in}, user, token) - end - end - - def success(conn, activity, user, token) do - success_continue(conn, activity, user, token) - end - - defp success_continue(conn, activity, user, _token) do + def success(conn, activity, user, _token) do return_to = get_session(conn, :return_to) || ~p"/" message = @@ -149,7 +134,7 @@ defmodule MvWeb.AuthController do _ -> conn |> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again.")) - |> redirect(to: sign_in_path_after_oidc_failure()) + |> redirect(to: ~p"/sign-in") end end @@ -163,7 +148,7 @@ defmodule MvWeb.AuthController do :error, gettext("The authentication server is currently unavailable. Please try again later.") ) - |> redirect(to: sign_in_path_after_oidc_failure()) + |> redirect(to: ~p"/sign-in") end # Handle Assent invalid response errors (configuration or malformed responses) @@ -176,7 +161,7 @@ defmodule MvWeb.AuthController do :error, gettext("Authentication configuration error. Please contact the administrator.") ) - |> redirect(to: sign_in_path_after_oidc_failure()) + |> redirect(to: ~p"/sign-in") end # Catch-all clause for any other error types @@ -186,7 +171,7 @@ defmodule MvWeb.AuthController do conn |> put_flash(:error, gettext("Unable to authenticate with OIDC. Please try again.")) - |> redirect(to: sign_in_path_after_oidc_failure()) + |> redirect(to: ~p"/sign-in") end # Handle generic AuthenticationFailed errors @@ -226,14 +211,10 @@ defmodule MvWeb.AuthController do conn |> put_flash(:error, error_message) - |> redirect(to: sign_in_path_after_oidc_failure()) + |> redirect(to: ~p"/sign-in") end end - # Path used when redirecting to sign-in after an OIDC failure. The query param tells - # OidcOnlySignInRedirect to show the sign-in page instead of redirecting back to OIDC (avoids loop). - defp sign_in_path_after_oidc_failure, do: "/sign-in?oidc_failed=1" - # Extract meaningful error message from Ash errors defp extract_meaningful_error_message(errors) do # Look for specific error messages in InvalidAttribute errors diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index cb57631..f69b947 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -19,7 +19,6 @@ defmodule MvWeb.GlobalSettingsLive do ## Events - `validate` / `save` - Club settings form - `toggle_registration_enabled` - Enable/disable direct registration (/register) - - `toggle_oidc_only` - Enable/disable OIDC-only sign-in (immediate, outside OIDC form) - `toggle_join_form_enabled` - Enable/disable the join form - `add_join_form_field` / `remove_join_form_field` - Manage join form fields - `toggle_join_form_field_required` - Toggle required flag per field @@ -81,7 +80,6 @@ defmodule MvWeb.GlobalSettingsLive do |> assign(:oidc_admin_group_name_env_set, Mv.Config.oidc_admin_group_name_env_set?()) |> assign(:oidc_groups_claim_env_set, Mv.Config.oidc_groups_claim_env_set?()) |> assign(:oidc_only_env_set, Mv.Config.oidc_only_env_set?()) - |> assign(:oidc_only, Mv.Config.oidc_only?()) |> assign(:oidc_configured, Mv.Config.oidc_configured?()) |> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?()) |> assign(:registration_enabled, settings.registration_enabled != false) @@ -627,30 +625,11 @@ defmodule MvWeb.GlobalSettingsLive do class="checkbox checkbox-sm" checked={@registration_enabled} phx-click="toggle_registration_enabled" - disabled={@oidc_only} aria-label={gettext("Allow direct registration (/register)")} /> -

{gettext("OIDC (Single Sign-On)")}

@@ -659,38 +638,6 @@ defmodule MvWeb.GlobalSettingsLive do {gettext("Some values are set via environment variables. Those fields are read-only.")}

<% end %> -
- - -
-

- {gettext( - "When enabled and OIDC is configured, the sign-in page shows only the Single Sign-On button." - )} -

<.form for={@form} id="oidc-form" phx-change="validate" phx-submit="save">
<.input @@ -797,6 +744,27 @@ defmodule MvWeb.GlobalSettingsLive do ) } /> +
+ <.input + field={@form[:oidc_only]} + type="checkbox" + class="checkbox checkbox-sm" + disabled={@oidc_only_env_set or not @oidc_configured} + label={ + if @oidc_only_env_set do + gettext("Only OIDC sign-in (hide password login)") <> + " (" <> gettext("From OIDC_ONLY") <> ")" + else + gettext("Only OIDC sign-in (hide password login)") + end + } + /> +

+ {gettext( + "When enabled and OIDC is configured, the sign-in page shows only the Single Sign-On button." + )} +

+
<.button :if={ @@ -900,19 +868,18 @@ defmodule MvWeb.GlobalSettingsLive do saves_vereinfacht = vereinfacht_params?(setting_params_clean) case MvWeb.LiveHelpers.submit_form(socket.assigns.form, setting_params_clean, actor) do - {:ok, updated_settings} -> - # Use the returned record for the form so saved values show immediately; - # get_settings() can return cached data without the new attribute until reload. + {:ok, _updated_settings} -> + {:ok, fresh_settings} = Membership.get_settings() + test_result = if saves_vereinfacht, do: Mv.Vereinfacht.test_connection(), else: nil socket = socket - |> assign(:settings, updated_settings) - |> assign(:registration_enabled, updated_settings.registration_enabled != false) - |> assign(:vereinfacht_api_key_set, present?(updated_settings.vereinfacht_api_key)) + |> assign(:settings, fresh_settings) + |> assign(:registration_enabled, fresh_settings.registration_enabled != false) + |> assign(:vereinfacht_api_key_set, present?(fresh_settings.vereinfacht_api_key)) |> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?()) - |> assign(:oidc_only, Mv.Config.oidc_only?()) |> assign(:oidc_configured, Mv.Config.oidc_configured?()) |> assign(:smtp_configured, Mv.Config.smtp_configured?()) |> assign(:smtp_password_set, present?(Mv.Config.smtp_password())) @@ -949,53 +916,19 @@ defmodule MvWeb.GlobalSettingsLive do @impl true def handle_event("toggle_registration_enabled", _params, socket) do - if Mv.Config.oidc_only?() do - {:noreply, socket} - else - settings = socket.assigns.settings - new_value = not socket.assigns.registration_enabled + settings = socket.assigns.settings + new_value = not socket.assigns.registration_enabled - case Membership.update_settings(settings, %{registration_enabled: new_value}) do - {:ok, updated_settings} -> - {:noreply, - socket - |> assign(:settings, updated_settings) - |> assign(:registration_enabled, updated_settings.registration_enabled != false) - |> assign_form()} + case Membership.update_settings(settings, %{registration_enabled: new_value}) do + {:ok, updated_settings} -> + {:noreply, + socket + |> assign(:settings, updated_settings) + |> assign(:registration_enabled, updated_settings.registration_enabled != false) + |> assign_form()} - {:error, _} -> - {:noreply, put_flash(socket, :error, gettext("Failed to update setting."))} - end - end - end - - @impl true - def handle_event("toggle_oidc_only", _params, socket) do - if socket.assigns.oidc_only_env_set do - {:noreply, socket} - else - settings = socket.assigns.settings - new_value = not socket.assigns.oidc_only - - # When enabling OIDC-only, also disable direct registration; when disabling, only change oidc_only. - params = - if new_value, - do: %{oidc_only: true, registration_enabled: false}, - else: %{oidc_only: false} - - case Membership.update_settings(settings, params) do - {:ok, updated_settings} -> - {:noreply, - socket - |> assign(:settings, updated_settings) - |> assign(:oidc_only, updated_settings.oidc_only == true) - |> assign(:registration_enabled, updated_settings.registration_enabled != false) - |> assign_form() - |> put_flash(:success, gettext("Settings updated successfully"))} - - {:error, _} -> - {:noreply, put_flash(socket, :error, gettext("Failed to update setting."))} - end + {:error, _} -> + {:noreply, put_flash(socket, :error, gettext("Failed to update setting."))} end end diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex index 4206aa6..38a0157 100644 --- a/lib/mv_web/live_helpers.ex +++ b/lib/mv_web/live_helpers.ex @@ -74,7 +74,7 @@ defmodule MvWeb.LiveHelpers do socket = socket - |> maybe_put_access_denied_flash(user) + |> Phoenix.LiveView.put_flash(:error, "You don't have permission to access this page.") |> Phoenix.LiveView.push_navigate(to: redirect_to) {:halt, socket} @@ -82,13 +82,6 @@ defmodule MvWeb.LiveHelpers do end end - # Only show "no permission" when user is logged in; unauthenticated users are redirected to sign-in without flash. - defp maybe_put_access_denied_flash(socket, nil), do: socket - - defp maybe_put_access_denied_flash(socket, _user) do - Phoenix.LiveView.put_flash(socket, :error, "You don't have permission to access this page.") - end - defp ensure_user_role_loaded(socket) do user = socket.assigns[:current_user] diff --git a/lib/mv_web/plugs/check_page_permission.ex b/lib/mv_web/plugs/check_page_permission.ex index b2f37ec..ff6d47d 100644 --- a/lib/mv_web/plugs/check_page_permission.ex +++ b/lib/mv_web/plugs/check_page_permission.ex @@ -54,7 +54,7 @@ defmodule MvWeb.Plugs.CheckPagePermission do conn |> fetch_session() |> fetch_flash() - |> maybe_put_access_denied_flash(user) + |> put_flash(:error, "You don't have permission to access this page.") |> redirect(to: redirect_to) |> halt() end @@ -75,13 +75,6 @@ defmodule MvWeb.Plugs.CheckPagePermission do defp redirect_target(user), do: redirect_target_for_user(user) - # Only set "no permission" flash when user is logged in; unauthenticated users get redirect only, no flash. - defp maybe_put_access_denied_flash(conn, nil), do: conn - - defp maybe_put_access_denied_flash(conn, _user) do - put_flash(conn, :error, "You don't have permission to access this page.") - end - @doc """ Returns true if the path is public (no auth/permission check). Used by LiveView hook to skip redirect on sign-in etc. diff --git a/lib/mv_web/plugs/oidc_only_sign_in_redirect.ex b/lib/mv_web/plugs/oidc_only_sign_in_redirect.ex deleted file mode 100644 index 9cf2a16..0000000 --- a/lib/mv_web/plugs/oidc_only_sign_in_redirect.ex +++ /dev/null @@ -1,73 +0,0 @@ -defmodule MvWeb.Plugs.OidcOnlySignInRedirect do - @moduledoc """ - When OIDC-only mode is active: - - GET /sign-in redirects to the OIDC flow when OIDC is configured (sign-in page skipped). - - GET /sign-in?oidc_failed=1 is not redirected, so the sign-in page is shown after an OIDC - failure (avoids redirect loop when the provider is down or misconfigured). - - GET /auth/user/password/sign_in_with_token is rejected (redirect to /sign-in with error) - so password sign-in cannot complete. - """ - import Plug.Conn - import Phoenix.Controller - - alias Mv.Config - - def init(opts), do: opts - - def call(conn, _opts) do - conn - |> maybe_redirect_sign_in_to_oidc() - |> maybe_reject_password_token_sign_in() - end - - defp maybe_redirect_sign_in_to_oidc(conn) do - if conn.request_path != "/sign-in" or conn.method != "GET" do - conn - else - conn = fetch_query_params(conn) - maybe_redirect_sign_in_to_oidc_checked(conn) - end - end - - defp maybe_redirect_sign_in_to_oidc_checked(conn) do - cond do - # Show sign-in page when returning from OIDC failure to avoid redirect loop. - conn.query_params["oidc_failed"] -> conn - Config.oidc_only?() and Config.oidc_configured?() -> redirect_and_halt(conn) - true -> conn - end - end - - defp redirect_and_halt(conn) do - conn - |> redirect(to: "/auth/user/oidc") - |> halt() - end - - defp maybe_reject_password_token_sign_in(conn) do - if conn.halted, do: conn, else: reject_password_token_sign_in_if_applicable(conn) - end - - defp reject_password_token_sign_in_if_applicable(conn) do - path = conn.request_path - - password_token_path? = - path =~ ~r|/auth/user/password/sign_in_with_token| and conn.method == "GET" - - if password_token_path? and Config.oidc_only?() do - message = - Gettext.dgettext( - MvWeb.Gettext, - "default", - "Only sign-in via Single Sign-On (SSO) is allowed." - ) - - conn - |> put_flash(:error, message) - |> redirect(to: "/sign-in") - |> halt() - else - conn - end - end -end diff --git a/lib/mv_web/router.ex b/lib/mv_web/router.ex index 591dead..f115535 100644 --- a/lib/mv_web/router.ex +++ b/lib/mv_web/router.ex @@ -18,7 +18,6 @@ defmodule MvWeb.Router do plug MvWeb.Plugs.CheckPagePermission plug MvWeb.Plugs.JoinFormEnabled plug MvWeb.Plugs.RegistrationEnabled - plug MvWeb.Plugs.OidcOnlySignInRedirect end pipeline :api do diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index 383fb1c..078ef19 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -3854,7 +3854,7 @@ msgstr "Wenn deaktiviert, können sich Nutzer*innen nicht über /register anmeld #: lib/mv_web/controllers/page_controller.ex #, elixir-autogen, elixir-format msgid "Home" -msgstr "Startseite" +msgstr "" #: lib/mv_web/controllers/join_confirm_controller.ex #, elixir-autogen, elixir-format, fuzzy @@ -3895,13 +3895,3 @@ msgstr "Rolle %{name}" #, elixir-autogen, elixir-format msgid "User %{email}" msgstr "Benutzer*in %{email}" - -#: lib/mv_web/live/global_settings_live.ex -#, elixir-autogen, elixir-format -msgid "Only OIDC sign-in is active. This option is disabled." -msgstr "Nur OIDC-Anmeldung ist aktiv. Diese Option ist deaktiviert." - -#: lib/mv_web/controllers/auth_controller.ex -#, elixir-autogen, elixir-format -msgid "Only sign-in via Single Sign-On (SSO) is allowed." -msgstr "Nur Anmeldung per Single Sign-On (SSO) ist erlaubt." diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index 3e2eb5d..a845e1e 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -3895,13 +3895,3 @@ msgstr "" #, elixir-autogen, elixir-format msgid "User %{email}" msgstr "" - -#: lib/mv_web/live/global_settings_live.ex -#, elixir-autogen, elixir-format -msgid "Only OIDC sign-in is active. This option is disabled." -msgstr "" - -#: lib/mv_web/controllers/auth_controller.ex -#, elixir-autogen, elixir-format -msgid "Only sign-in via Single Sign-On (SSO) is allowed." -msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index 21314a5..d01da60 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -3895,13 +3895,3 @@ msgstr "Role %{name}" #, elixir-autogen, elixir-format msgid "User %{email}" msgstr "User %{email}" - -#: lib/mv_web/live/global_settings_live.ex -#, elixir-autogen, elixir-format -msgid "Only OIDC sign-in is active. This option is disabled." -msgstr "" - -#: lib/mv_web/controllers/auth_controller.ex -#, elixir-autogen, elixir-format -msgid "Only sign-in via Single Sign-On (SSO) is allowed." -msgstr "" diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index c562a43..7257f8b 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -3,9 +3,7 @@ # mix run priv/repo/seeds.exs # # Bootstrap runs in all environments. Dev seeds (members, groups, sample data) -# run only in dev and test. Skips entirely if bootstrap was already applied -# (admin user exists), so safe to run on every start. Set FORCE_SEEDS=true to -# re-run seeds even when already applied. +# run only in dev and test. # # In production (release): seeds are run via Mv.Release.run_seeds/0 from the # container entrypoint. Set RUN_DEV_SEEDS=true to also run dev seeds there. @@ -14,25 +12,19 @@ # so that eval_file of bootstrap/dev does not emit "redefining module" warnings; # it is always restored in `after` to avoid hiding real conflicts elsewhere. -_ = Application.ensure_all_started(:mv) +prev = Code.compiler_options() +Code.compiler_options(ignore_module_conflict: true) -if Mv.Release.bootstrap_seeds_applied?() and System.get_env("FORCE_SEEDS") != "true" do - IO.puts("Seeds already applied. Skipping. (Set FORCE_SEEDS=true to override)") -else - prev = Code.compiler_options() - Code.compiler_options(ignore_module_conflict: true) +try do + # Always run bootstrap (fee types, custom fields, roles, admin, system user, settings) + Code.eval_file("priv/repo/seeds_bootstrap.exs") - try do - # Always run bootstrap (fee types, custom fields, roles, admin, system user, settings) - Code.eval_file("priv/repo/seeds_bootstrap.exs") - - # In dev and test only: run dev seeds (20 members, groups, custom field values) - if Mix.env() in [:dev, :test] do - Code.eval_file("priv/repo/seeds_dev.exs") - end - - IO.puts("✅ All seeds completed.") - after - Code.compiler_options(prev) + # In dev and test only: run dev seeds (20 members, groups, custom field values) + if Mix.env() in [:dev, :test] do + Code.eval_file("priv/repo/seeds_dev.exs") end + + IO.puts("✅ All seeds completed.") +after + Code.compiler_options(prev) end diff --git a/test/accounts/user_authentication_test.exs b/test/accounts/user_authentication_test.exs index b759e8a..1b47165 100644 --- a/test/accounts/user_authentication_test.exs +++ b/test/accounts/user_authentication_test.exs @@ -288,31 +288,4 @@ defmodule Mv.Accounts.UserAuthenticationTest do 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 diff --git a/test/mv/mailer_smtp_config_test.exs b/test/mv/mailer_smtp_config_test.exs deleted file mode 100644 index 22325df..0000000 --- a/test/mv/mailer_smtp_config_test.exs +++ /dev/null @@ -1,89 +0,0 @@ -defmodule Mv.MailerSmtpConfigTest do - @moduledoc """ - Unit tests for Mv.Mailer.smtp_config/0. - - Ensures both port 587 (STARTTLS) and 465 (implicit SSL) work: - - 587: sockopts must NOT contain :verify (gen_tcp:connect would raise ArgumentError). - - 465: sockopts MUST contain :verify so initial ssl:connect uses verify_none/verify_peer. - Uses ENV to drive config; async: false. - """ - use Mv.DataCase, async: false - - alias Mv.Mailer - - defp set_smtp_env(key, value), do: System.put_env(key, value) - - defp clear_smtp_env do - System.delete_env("SMTP_HOST") - System.delete_env("SMTP_PORT") - System.delete_env("SMTP_USERNAME") - System.delete_env("SMTP_PASSWORD") - System.delete_env("SMTP_PASSWORD_FILE") - System.delete_env("SMTP_SSL") - end - - describe "smtp_config/0" do - test "port 587 (TLS): does not include :verify in sockopts so gen_tcp:connect does not crash" do - set_smtp_env("SMTP_HOST", "smtp.example.com") - set_smtp_env("SMTP_PORT", "587") - set_smtp_env("SMTP_SSL", "tls") - - config = Mailer.smtp_config() - - assert config != [], "expected non-empty config when SMTP_HOST is set" - - sockopts = Keyword.get(config, :sockopts, []) - - refute Keyword.has_key?(sockopts, :verify), - "for 587 gen_tcp is used first; sockopts must not contain :verify" - after - clear_smtp_env() - end - - test "port 465 (SSL): includes :verify in sockopts so initial ssl:connect accepts verify mode" do - set_smtp_env("SMTP_HOST", "smtp.example.com") - set_smtp_env("SMTP_PORT", "465") - set_smtp_env("SMTP_SSL", "ssl") - - config = Mailer.smtp_config() - - assert config != [] - sockopts = Keyword.get(config, :sockopts, []) - - assert Keyword.has_key?(sockopts, :verify), - "for 465 initial connection is ssl:connect; sockopts must contain :verify" - - assert Keyword.get(sockopts, :verify) in [:verify_none, :verify_peer] - after - clear_smtp_env() - end - - test "builds TLS mode for port 587 (STARTTLS)" do - set_smtp_env("SMTP_HOST", "smtp.example.com") - set_smtp_env("SMTP_PORT", "587") - set_smtp_env("SMTP_SSL", "tls") - - config = Mailer.smtp_config() - - assert config != [] - assert Keyword.get(config, :tls) == :always - assert Keyword.get(config, :ssl) == false - after - clear_smtp_env() - end - - test "builds SSL mode for port 465 (implicit SSL)" do - set_smtp_env("SMTP_HOST", "smtp.example.com") - set_smtp_env("SMTP_PORT", "465") - set_smtp_env("SMTP_SSL", "ssl") - - config = Mailer.smtp_config() - - assert config != [] - assert Keyword.get(config, :ssl) == true - assert Keyword.get(config, :tls) == :never - after - clear_smtp_env() - end - end -end diff --git a/test/mv/smtp/config_builder_test.exs b/test/mv/smtp/config_builder_test.exs deleted file mode 100644 index 0b12a52..0000000 --- a/test/mv/smtp/config_builder_test.exs +++ /dev/null @@ -1,83 +0,0 @@ -defmodule Mv.Smtp.ConfigBuilderTest do - @moduledoc """ - Unit tests for Mv.Smtp.ConfigBuilder.build_opts/1. - - Ensures the single source of truth for SMTP opts correctly handles: - - Port 587 (TLS): sockopts must NOT contain :verify (gen_tcp rejects it). - - Port 465 (SSL): sockopts MUST contain :verify for initial ssl:connect. - """ - use ExUnit.Case, async: true - - alias Mv.Smtp.ConfigBuilder - - describe "build_opts/1" do - test "port 587 (TLS): does not include :verify in sockopts" do - opts = - ConfigBuilder.build_opts( - host: "smtp.example.com", - port: 587, - username: "user", - password: "secret", - ssl_mode: "tls", - verify_mode: :verify_none - ) - - sockopts = Keyword.get(opts, :sockopts, []) - refute Keyword.has_key?(sockopts, :verify), "for 587 sockopts must not contain :verify" - assert Keyword.get(opts, :tls) == :always - assert Keyword.get(opts, :ssl) == false - end - - test "port 465 (SSL): includes :verify in sockopts" do - opts = - ConfigBuilder.build_opts( - host: "smtp.example.com", - port: 465, - username: "user", - password: "secret", - ssl_mode: "ssl", - verify_mode: :verify_peer - ) - - sockopts = Keyword.get(opts, :sockopts, []) - assert Keyword.has_key?(sockopts, :verify), "for 465 sockopts must contain :verify" - assert Keyword.get(sockopts, :verify) == :verify_peer - assert Keyword.get(opts, :ssl) == true - assert Keyword.get(opts, :tls) == :never - end - - test "strips nil values" do - opts = - ConfigBuilder.build_opts( - host: "smtp.example.com", - port: 587, - username: nil, - password: nil, - ssl_mode: "tls", - verify_mode: :verify_none - ) - - refute Keyword.has_key?(opts, :username) - refute Keyword.has_key?(opts, :password) - assert Keyword.get(opts, :relay) == "smtp.example.com" - end - - test "includes adapter and required keys" do - opts = - ConfigBuilder.build_opts( - host: "mail.example.com", - port: 587, - username: "u", - password: "p", - ssl_mode: "tls", - verify_mode: :verify_none - ) - - assert Keyword.get(opts, :adapter) == Swoosh.Adapters.SMTP - assert Keyword.get(opts, :relay) == "mail.example.com" - assert Keyword.get(opts, :port) == 587 - assert Keyword.get(opts, :auth) == :always - assert Keyword.get(opts, :tls_options) == [verify: :verify_none] - end - end -end diff --git a/test/mv_web/controllers/auth_controller_test.exs b/test/mv_web/controllers/auth_controller_test.exs index f1fbd76..e449284 100644 --- a/test/mv_web/controllers/auth_controller_test.exs +++ b/test/mv_web/controllers/auth_controller_test.exs @@ -283,141 +283,6 @@ defmodule MvWeb.AuthControllerTest do assert to =~ "/auth/user/password/sign_in_with_token" end - describe "when OIDC-only is enabled" do - setup %{conn: authenticated_conn} do - {:ok, settings} = Membership.get_settings() - original_oidc_only = Map.get(settings, :oidc_only, false) - {:ok, _} = Membership.update_settings(settings, %{oidc_only: true}) - - conn = build_unauthenticated_conn(authenticated_conn) - {:ok, conn: conn, original_oidc_only: original_oidc_only} - end - - test "password sign-in is rejected and redirects to sign-in with error", %{ - conn: conn, - original_oidc_only: original - } do - try do - _user = - create_test_user(%{ - email: "password@example.com", - password: "secret123", - oidc_id: nil - }) - - {:ok, view, _html} = live(conn, "/sign-in") - - result = - view - |> form("#user-password-sign-in-with-password", - user: %{email: "password@example.com", password: "secret123"} - ) - |> render_submit() - - # When OIDC-only is enabled, password sign-in must not succeed (no redirect to sign_in_with_token). - case result do - {:error, {:redirect, %{to: to}}} -> - refute to =~ "sign_in_with_token", - "Expected password sign-in to be rejected when OIDC-only, got redirect to: #{to}" - - _ -> - # LiveView re-rendered (e.g. with flash error) instead of redirecting to success - :ok - end - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, %{oidc_only: original}) - end - end - end - - describe "GET /sign-in when OIDC-only" do - test "redirects to OIDC flow when OIDC-only and OIDC are configured", %{ - conn: authenticated_conn - } do - {:ok, settings} = Membership.get_settings() - - prev = %{ - oidc_only: settings.oidc_only, - oidc_client_id: settings.oidc_client_id, - oidc_base_url: settings.oidc_base_url, - oidc_redirect_uri: settings.oidc_redirect_uri - } - - {:ok, _} = - Membership.update_settings(settings, %{ - oidc_only: true, - oidc_client_id: "test-client", - oidc_base_url: "https://idp.example.com", - oidc_redirect_uri: "http://localhost:4000/auth/user/oidc/callback", - oidc_client_secret: "test-secret" - }) - - try do - conn = build_unauthenticated_conn(authenticated_conn) - conn = get(conn, ~p"/sign-in") - assert redirected_to(conn) =~ "/auth/user/oidc" - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, prev) - end - end - - test "returns 200 when OIDC-only but oidc_failed=1 (avoids redirect loop)", %{ - conn: authenticated_conn - } do - {:ok, settings} = Membership.get_settings() - - prev = %{ - oidc_only: settings.oidc_only, - oidc_client_id: settings.oidc_client_id, - oidc_base_url: settings.oidc_base_url, - oidc_redirect_uri: settings.oidc_redirect_uri - } - - {:ok, _} = - Membership.update_settings(settings, %{ - oidc_only: true, - oidc_client_id: "test-client", - oidc_base_url: "https://idp.example.com", - oidc_redirect_uri: "http://localhost:4000/auth/user/oidc/callback", - oidc_client_secret: "test-secret" - }) - - try do - conn = build_unauthenticated_conn(authenticated_conn) - conn = get(conn, "/sign-in?oidc_failed=1") - assert conn.status == 200 - # Sign-in page is shown, not redirect to OIDC - assert conn.resp_body =~ "Sign in" or conn.resp_body =~ "sign-in" - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, prev) - end - end - - test "returns 200 when OIDC-only but OIDC not configured", %{conn: authenticated_conn} 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 - conn = build_unauthenticated_conn(authenticated_conn) - conn = get(conn, ~p"/sign-in") - assert conn.status == 200 - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, %{oidc_only: original_oidc_only}) - end - end - - test "returns 200 when OIDC-only is disabled", %{conn: authenticated_conn} do - conn = build_unauthenticated_conn(authenticated_conn) - conn = get(conn, ~p"/sign-in") - assert conn.status == 200 - end - end - # OIDC/Rauthy error handling tests describe "handle_oidc_failure/2" do test "Assent.ServerUnreachableError redirects to sign-in with error flash", %{ @@ -433,7 +298,7 @@ defmodule MvWeb.AuthControllerTest do conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error) - assert redirected_to(conn) == "/sign-in?oidc_failed=1" + assert redirected_to(conn) == ~p"/sign-in" assert Phoenix.Flash.get(conn.assigns.flash, :error) == "The authentication server is currently unavailable. Please try again later." @@ -455,7 +320,7 @@ defmodule MvWeb.AuthControllerTest do conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, error) - assert redirected_to(conn) == "/sign-in?oidc_failed=1" + assert redirected_to(conn) == ~p"/sign-in" assert Phoenix.Flash.get(conn.assigns.flash, :error) == "Authentication configuration error. Please contact the administrator." @@ -469,7 +334,7 @@ defmodule MvWeb.AuthControllerTest do conn = MvWeb.AuthController.failure(conn, {:oidc, :callback}, unknown_reason) - assert redirected_to(conn) == "/sign-in?oidc_failed=1" + assert redirected_to(conn) == ~p"/sign-in" assert Phoenix.Flash.get(conn.assigns.flash, :error) == "Unable to authenticate with OIDC. Please try again." diff --git a/test/mv_web/live/global_settings_live_test.exs b/test/mv_web/live/global_settings_live_test.exs index 92da11b..e48c44b 100644 --- a/test/mv_web/live/global_settings_live_test.exs +++ b/test/mv_web/live/global_settings_live_test.exs @@ -110,69 +110,4 @@ defmodule MvWeb.GlobalSettingsLiveTest do assert html =~ "SMTP" or html =~ "E-Mail" or html =~ "Settings" end end - - describe "Authentication section when OIDC-only is enabled" do - setup %{conn: conn} do - user = create_test_user(%{email: "admin@example.com"}) - conn = conn_with_oidc_user(conn, user) - {:ok, settings} = Membership.get_settings() - original_oidc_only = Map.get(settings, :oidc_only, false) - {:ok, _} = Membership.update_settings(settings, %{oidc_only: true}) - {:ok, conn: conn, original_oidc_only: original_oidc_only} - end - - @describetag :ui - test "registration checkbox is disabled when OIDC-only is enabled", %{ - conn: conn, - original_oidc_only: original - } do - try do - {:ok, view, _html} = live(conn, ~p"/settings") - assert has_element?(view, "#registration-enabled-checkbox[disabled]") - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, %{oidc_only: original}) - end - end - - @describetag :ui - test "OIDC-only hint is visible when OIDC-only is enabled", %{ - conn: conn, - original_oidc_only: original - } do - try do - {:ok, view, _html} = live(conn, ~p"/settings") - assert has_element?(view, "[data-testid='oidc-only-registration-hint']") - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, %{oidc_only: original}) - end - end - - test "when OIDC-only is disabled, registration checkbox is enabled and can be toggled", %{ - conn: conn, - original_oidc_only: original - } do - try do - {:ok, settings} = Membership.get_settings() - Membership.update_settings(settings, %{oidc_only: false}) - - {:ok, view, _html} = live(conn, ~p"/settings") - refute has_element?(view, "#registration-enabled-checkbox[disabled]") - - initial_checked = - view |> element("#registration-enabled-checkbox") |> render() =~ "checked" - - view - |> element("#registration-enabled-checkbox") - |> render_click() - - new_checked = view |> element("#registration-enabled-checkbox") |> render() =~ "checked" - assert new_checked != initial_checked - after - {:ok, s} = Membership.get_settings() - Membership.update_settings(s, %{oidc_only: original}) - end - end - end end diff --git a/test/mv_web/live/join_live_email_failure_test.exs b/test/mv_web/live/join_live_email_failure_test.exs index 50a209c..ef308c6 100644 --- a/test/mv_web/live/join_live_email_failure_test.exs +++ b/test/mv_web/live/join_live_email_failure_test.exs @@ -2,9 +2,6 @@ defmodule MvWeb.JoinLiveEmailFailureTest do @moduledoc """ When join confirmation email delivery fails, the user sees an error message and no success copy. Uses FailingMailAdapter; async: false to avoid config races. - - Ensures SMTP is not configured (ENV cleared, Settings smtp_host nil) so that - Mailer.smtp_config/0 returns [] and the Application-configured FailingMailAdapter is used. """ use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest @@ -15,21 +12,18 @@ defmodule MvWeb.JoinLiveEmailFailureTest do test "when confirmation email fails, user sees error flash and no success message", %{ conn: conn } do - # Clear SMTP config so Mailer.smtp_config() returns [] and FailingMailAdapter is used. - saved_env = clear_smtp_env_and_save() enable_join_form_for_test() - saved_mailer = Application.get_env(:mv, Mv.Mailer) + saved = Application.get_env(:mv, Mv.Mailer) Application.put_env( :mv, Mv.Mailer, - Keyword.put(saved_mailer || [], :adapter, Mv.TestSupport.FailingMailAdapter) + Keyword.put(saved || [], :adapter, Mv.TestSupport.FailingMailAdapter) ) on_exit(fn -> - Application.put_env(:mv, Mv.Mailer, saved_mailer) - restore_smtp_env(saved_env) + Application.put_env(:mv, Mv.Mailer, saved) end) {:ok, view, _html} = live(conn, "/join") @@ -52,36 +46,13 @@ defmodule MvWeb.JoinLiveEmailFailureTest do refute view |> element("[data-testid='join-success-message']") |> has_element?() end - defp clear_smtp_env_and_save do - keys = [ - "SMTP_HOST", - "SMTP_PORT", - "SMTP_USERNAME", - "SMTP_PASSWORD", - "SMTP_PASSWORD_FILE", - "SMTP_SSL" - ] - - saved = for k <- keys, into: %{}, do: {k, System.get_env(k)} - for k <- keys, do: System.delete_env(k) - saved - end - - defp restore_smtp_env(saved) do - for {k, v} <- saved do - if v != nil, do: System.put_env(k, v), else: System.delete_env(k) - end - end - defp enable_join_form_for_test do {:ok, settings} = Membership.get_settings() Membership.update_settings(settings, %{ join_form_enabled: true, join_form_field_ids: ["email", "first_name", "last_name"], - join_form_field_required: %{"email" => true, "first_name" => false, "last_name" => false}, - # Clear SMTP host so Mv.Config.smtp_configured?() is false and Mailer.smtp_config() returns []. - smtp_host: nil + join_form_field_required: %{"email" => true, "first_name" => false, "last_name" => false} }) end end diff --git a/test/mv_web/plugs/check_page_permission_test.exs b/test/mv_web/plugs/check_page_permission_test.exs index 76bed74..2798161 100644 --- a/test/mv_web/plugs/check_page_permission_test.exs +++ b/test/mv_web/plugs/check_page_permission_test.exs @@ -181,14 +181,13 @@ defmodule MvWeb.Plugs.CheckPagePermissionTest do end describe "unauthenticated user" do - test "nil current_user is denied and redirected to \"/sign-in\" without access-denied flash" do + test "nil current_user is denied and redirected to \"/sign-in\"" do conn = conn_without_user("/members") |> CheckPagePermission.call([]) assert conn.halted assert redirected_to(conn) == "/sign-in" - # Unauthenticated users are redirected to sign-in only; no "no permission" message. - refute Phoenix.Flash.get(conn.assigns[:flash] || %{}, :error) == + assert Phoenix.Flash.get(conn.assigns[:flash] || %{}, :error) == "You don't have permission to access this page." end end