Server settings → Sign-in and Defaults: saving overwrites the stored value of a field the environment pins #492

New issue

Closed

opened 2026-10-01 10:47:04 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:04 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the Server settings forms and posting them in a test with a variable set.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The forms and templates are where they were; the views are now at core/server_views.py:453-457 (SignInView) and core/server_views.py:564-568 (DefaultsView).

When the environment pins a field on the Sign-in page (POSTULO_REGISTRATION_OPEN, POSTULO_OIDC_IS_SECOND_FACTOR) or on Defaults (POSTULO_TIME_ZONE), the template draws a notice in place of the input, but SignInForm and DefaultsForm still carry the field. The missing value is cleaned to an empty answer and written to the row, so any save of those pages quietly replaces what an administrator had stored. CaptureForm and EmailForm already refuse pinned fields for exactly this reason; these two forms were never given the rule. Related to #84 (closed), which set the rule for the email settings: "the server must refuse the write regardless".

Where: core/server_forms.py:39-73 (SignInForm) and core/server_forms.py:248-271 (DefaultsForm), neither of which computes self.pinned or drops it in clean(); compare CaptureForm.clean at core/server_forms.py:146-151 and EmailForm.clean at core/server_forms.py:458-467. The inputs are left out by templates/server/signin.html:69-81 and templates/server/defaults.html:36-45; the views are SignInView (core/server_views.py:452-456) and DefaultsView (core/server_views.py:563-567).
What happens: an administrator opened registration from the page, so the row holds registration_open = True. The operator later sets POSTULO_REGISTRATION_OPEN=false. Somebody saves the Sign-in page to switch on email sign-in; the row now holds False (checked: posting the form with the variable set left registration_open at False). When the variable is removed, registration stays closed rather than returning to what the administrator chose. In the same way, with default_time_zone = "Europe/Lisbon" stored and POSTULO_TIME_ZONE set, saving Defaults to change the instance name stores default_time_zone = "".
Why it matters: the notice on the page says the value is "Set by the environment … Remove it from the environment to change this here", which promises that the stored value is only shadowed. Saving an unrelated field destroys it instead, and the loss shows only later, when the variable goes.
Fix: give the policy forms one base (for example PinnedPolicyForm) that computes self.pinned = {field: variable for field in self.fields if (variable := site.overridden_by(field))} in __init__ and pops those names from cleaned_data and errors in clean(), and have SignInForm, DefaultsForm, CaptureForm and EmailForm inherit it, so the rule cannot drift again. Fixing the empty-value handling of PolicyField alone would not help: the pinned field would then be written as None rather than False, still overwriting the stored answer.

Done when

tests/test_server_settings.py has a test that stores registration_open = True, sets POSTULO_REGISTRATION_OPEN, posts the Sign-in page and finds True still stored, and one that stores a default time zone, sets POSTULO_TIME_ZONE, posts Defaults and finds the time zone unchanged. Both fail today.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the Server settings forms and posting them in a test with a variable set. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The forms and templates are where they were; the views are now at `core/server_views.py:453-457` (`SignInView`) and `core/server_views.py:564-568` (`DefaultsView`). When the environment pins a field on the Sign-in page (`POSTULO_REGISTRATION_OPEN`, `POSTULO_OIDC_IS_SECOND_FACTOR`) or on Defaults (`POSTULO_TIME_ZONE`), the template draws a notice in place of the input, but `SignInForm` and `DefaultsForm` still carry the field. The missing value is cleaned to an empty answer and written to the row, so any save of those pages quietly replaces what an administrator had stored. `CaptureForm` and `EmailForm` already refuse pinned fields for exactly this reason; these two forms were never given the rule. Related to #84 (closed), which set the rule for the email settings: "the server must refuse the write regardless". - **Where:** `core/server_forms.py:39-73` (`SignInForm`) and `core/server_forms.py:248-271` (`DefaultsForm`), neither of which computes `self.pinned` or drops it in `clean()`; compare `CaptureForm.clean` at `core/server_forms.py:146-151` and `EmailForm.clean` at `core/server_forms.py:458-467`. The inputs are left out by `templates/server/signin.html:69-81` and `templates/server/defaults.html:36-45`; the views are `SignInView` (`core/server_views.py:452-456`) and `DefaultsView` (`core/server_views.py:563-567`). - **What happens:** an administrator opened registration from the page, so the row holds `registration_open = True`. The operator later sets `POSTULO_REGISTRATION_OPEN=false`. Somebody saves the Sign-in page to switch on email sign-in; the row now holds `False` (checked: posting the form with the variable set left `registration_open` at `False`). When the variable is removed, registration stays closed rather than returning to what the administrator chose. In the same way, with `default_time_zone = "Europe/Lisbon"` stored and `POSTULO_TIME_ZONE` set, saving Defaults to change the instance name stores `default_time_zone = ""`. - **Why it matters:** the notice on the page says the value is "Set by the environment … Remove it from the environment to change this here", which promises that the stored value is only shadowed. Saving an unrelated field destroys it instead, and the loss shows only later, when the variable goes. - **Fix:** give the policy forms one base (for example `PinnedPolicyForm`) that computes `self.pinned = {field: variable for field in self.fields if (variable := site.overridden_by(field))}` in `__init__` and pops those names from `cleaned_data` and `errors` in `clean()`, and have `SignInForm`, `DefaultsForm`, `CaptureForm` and `EmailForm` inherit it, so the rule cannot drift again. Fixing the empty-value handling of `PolicyField` alone would not help: the pinned field would then be written as `None` rather than `False`, still overwriting the stored answer. ## Done when - `tests/test_server_settings.py` has a test that stores `registration_open = True`, sets `POSTULO_REGISTRATION_OPEN`, posts the Sign-in page and finds `True` still stored, and one that stores a default time zone, sets `POSTULO_TIME_ZONE`, posts Defaults and finds the time zone unchanged. Both fail today.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:04 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:47:04 +00:00

tiagoagueda referenced this issue

2026-10-03 01:59:09 +00:00

Say which language and time zone “the default” is, with its flag and name, in Settings and Server settings #698

tiagoagueda referenced this issue from a commit

2026-10-04 01:59:02 +00:00