Code-review follow-ups: policy, docs, seed_admin behaviour

- Use OidcRoleSyncContext for set_role_from_oidc_sync; document JWT peek risk.
- seed_admin without password sets Admin role on existing user (OIDC-only); update docs and test.
- Fix DE translation for 'access this page'; add get? true comment in User.

docs/admin-bootstrap-and-oidc-role-sync.md

@@ -10,12 +10,12 @@
 ### Environment Variables
 
 - `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 user is created in production.
+- `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 Task
 
-- `Mv.Release.seed_admin/0` – Reads ADMIN_EMAIL and password from ADMIN_PASSWORD or ADMIN_PASSWORD_FILE. If both are set, creates or updates the user with the Admin role. 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
 

lib/accounts/user.ex

@@ -258,6 +258,7 @@ defmodule Mv.Accounts.User do
     end
 
     read :sign_in_with_rauthy do
+      # Single record expected; required for AshAuthentication OAuth2 strategy (returns list of 0 or 1).
       get? true
       argument :user_info, :map, allow_nil?: false
       argument :oauth_tokens, :map, allow_nil?: false
@@ -356,10 +357,10 @@ defmodule Mv.Accounts.User do
     end
 
     # set_role_from_oidc_sync: internal only (called from Mv.OidcRoleSync on registration/sign-in).
-    # Not exposed in code_interface; must never be callable by clients.
+    # Not exposed in code_interface; only allowed when context.private.oidc_role_sync is set.
     bypass action(:set_role_from_oidc_sync) do
       description "Internal: OIDC role sync (server-side only)"
-      authorize_if always()
+      authorize_if Mv.Authorization.Checks.OidcRoleSyncContext
     end
 
     # UPDATE/DESTROY via HasPermission (evaluates PermissionSets scope)

lib/mv/authorization/checks/oidc_role_sync_context.ex

@@ -1,9 +1,9 @@
 defmodule Mv.Authorization.Checks.OidcRoleSyncContext do
   @moduledoc """
-  Policy check: true when the action is being run from OIDC role sync (context.private.oidc_role_sync).
+  Policy check: true when the action is run from OIDC role sync (context.private.oidc_role_sync).
 
-  Used to allow the internal set_role_from_oidc_sync action when called by Mv.OidcRoleSync
+  Used to allow the internal set_role_from_oidc_sync action only when called by Mv.OidcRoleSync,
-  without an actor.
+  which sets context.private.oidc_role_sync when performing the update.
   """
   use Ash.Policy.SimpleCheck
 
@@ -12,11 +12,7 @@ defmodule Mv.Authorization.Checks.OidcRoleSyncContext do
 
   @impl true
   def match?(_actor, authorizer, _opts) do
-    # Context from opts (e.g. Ash.update!(..., context: %{private: %{oidc_role_sync: true}}))
     context = Map.get(authorizer, :context) || %{}
-    from_context = get_in(context, [:private, :oidc_role_sync]) == true
+    get_in(context, [:private, :oidc_role_sync]) == true
-    # When update runs inside create's after_action, context may not be passed; use process dict.
-    from_process = Process.get(:oidc_role_sync) == true
-    from_context or from_process
   end
 end

lib/mv/oidc_role_sync.ex

@@ -10,6 +10,16 @@ defmodule Mv.OidcRoleSync do
   the access_token from oauth_tokens is decoded as JWT and the groups claim is
   read from there (e.g. Rauthy puts groups in the access token when scope
   includes "groups").
+
+  ## JWT access token (security)
+
+  The access_token payload is read without signature verification (peek only).
+  We rely on the fact that `oauth_tokens` is only ever passed from the
+  verified OIDC callback (Assent/AshAuthentication after provider token
+  exchange). If callers passed untrusted or tampered tokens, group claims
+  could be forged and a user could be assigned the Admin role. Therefore:
+  do not call this module with user-supplied tokens; it is intended only
+  for the internal flow from the OIDC callback.
   """
   alias Mv.Accounts.User
   alias Mv.Authorization.Role

lib/mv/release.ex

@@ -52,14 +52,37 @@ defmodule Mv.Release do
         :ok
 
       is_nil(admin_password) or admin_password == "" ->
-        # Do not create or update any user without a password (no fallback in production)
+        ensure_admin_role_only(admin_email)
-        :ok
 
       true ->
         ensure_admin_user(admin_email, admin_password)
     end
   end
 
+  defp ensure_admin_role_only(email) do
+    case Role.get_admin_role() do
+      {:ok, nil} ->
+        :ok
+
+      {:ok, %Role{} = admin_role} ->
+        case get_user_by_email(email) do
+          {:ok, %User{} = user} ->
+            user
+            |> Ash.Changeset.for_update(:update, %{})
+            |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
+            |> Ash.update!(authorize?: false)
+
+            :ok
+
+          _ ->
+            :ok
+        end
+
+      {:error, _} ->
+        :ok
+    end
+  end
+
   defp ensure_admin_user(email, password) do
     if is_nil(password) or password == "" do
       :ok

priv/gettext/de/LC_MESSAGES/default.po

@@ -2306,7 +2306,7 @@ msgstr "Import/Export"
 #: lib/mv_web/live/import_export_live.ex
 #, elixir-autogen, elixir-format, fuzzy
 msgid "You do not have permission to access this page."
-msgstr "Du hast keine Berechtigung, auf diese*n Benutzer*in zuzugreifen"
+msgstr "Du hast keine Berechtigung, auf diese Seite zuzugreifen."
 
 #: lib/mv_web/live/import_export_live.ex
 #, elixir-autogen, elixir-format, fuzzy

test/mv/release_test.exs

@@ -44,18 +44,20 @@ defmodule Mv.ReleaseTest do
              "seed_admin must not create any user when ADMIN_PASSWORD is unset (expected #{user_count_before}, got #{count_users()})"
     end
 
-    test "with ADMIN_EMAIL but without ADMIN_PASSWORD and user exists: leaves user and role unchanged" do
+    test "with ADMIN_EMAIL but without ADMIN_PASSWORD and user exists: sets Admin role (OIDC-only bootstrap)" do
+      System.delete_env("ADMIN_PASSWORD")
+      System.delete_env("ADMIN_PASSWORD_FILE")
+
       email = "existing-admin-#{System.unique_integer([:positive])}@test.example.com"
       System.put_env("ADMIN_EMAIL", email)
       on_exit(fn -> System.delete_env("ADMIN_EMAIL") end)
 
-      {:ok, user} = create_user_with_mitglied_role(email)
+      {:ok, _user} = create_user_with_mitglied_role(email)
-      role_id_before = user.role_id
 
       Mv.Release.seed_admin()
 
       {:ok, updated} = get_user_by_email(email)
-      assert updated.role_id == role_id_before
+      assert updated.role_id == admin_role_id()
     end
 
     test "with ADMIN_EMAIL and ADMIN_PASSWORD: creates user with Admin role and sets password" do