From abf7d498395cb1207045feaeaff778943cb55c93 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Fri, 7 Feb 2025 17:32:23 +0000 Subject: [PATCH 1/2] Enable CSRF protection globally by default Enable Pyramid's `config.set_default_csrf_options(require_csrf=True)` which causes it to require a valid CSRF token for all requests with a request method that is *not* one of `GET`, `HEAD`, `OPTIONS` or `TRACE`. The CSRF token must be in a csrf_token POST parameter or an X-CSRF-Token header, and must match the CSRF token stored in the signed session cookie. It also checks that the request's `Referer` (if any) is the current host. See: * https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/security.html#checking-csrf-tokens-automatically * https://docs.pylonsproject.org/projects/pyramid/en/latest/api/config.html#pyramid.config.Configurator.set_default_csrf_options This is a safer default. The current implementation requires all views receiving form submissions to use a Colander schema that's a subclass of `CSRFSchema`. It's too easy to forget to add CSRF protection to a form if it doesn't use Colander (for example: perhaps there are no parameters to be validated) or if it has a schema that doesn't subclass `CSRFSchema`. Even if the view's schema *does* sublass `CSRFSchema`, if it wants to have a `validate()` method it must remember to call `super().validate()` or it'll disable `CSRFSchema`'s CSRF protection. This commit removes the CSRF protection code form `CSRFSchema` (that schema is now only used to *serialize* the CSRF tokens into the forms, but doesn't do any CSRF validation at *deserialization* time) and instead enables Pyramid's global CSRF protection option. CSRF protection can be disabled for individual views by passing `require_csrf=False` to `@view_config`. This has been added to h's custom `@api_config` decorator so that CSRF protection is disabled for all API endpoints. --- h/accounts/schemas.py | 4 -- h/app.py | 2 + h/schemas/auth_client.py | 2 - h/schemas/base.py | 44 +++++++++++++++---- h/schemas/forms/accounts/forgot_password.py | 2 - h/schemas/forms/accounts/login.py | 2 - h/schemas/forms/admin/group.py | 1 - h/views/api/auth.py | 1 + h/views/api/config.py | 1 + .../h/views/admin/permissions_test.py | 5 +-- tests/unit/h/accounts/schemas_test.py | 17 +------ tests/unit/h/schemas/base_test.py | 24 ---------- .../h/schemas/forms/accounts/login_test.py | 7 --- .../unit/h/schemas/forms/admin/group_test.py | 13 ------ 14 files changed, 41 insertions(+), 84 deletions(-) diff --git a/h/accounts/schemas.py b/h/accounts/schemas.py index 81e9303bdc0..8fcf259884f 100644 --- a/h/accounts/schemas.py +++ b/h/accounts/schemas.py @@ -203,7 +203,6 @@ class EmailChangeSchema(CSRFSchema): password = password_node(title=_("Confirm password"), hide_until_form_active=True) def validator(self, node, value): - super().validator(node, value) exc = colander.Invalid(node) request = node.bindings["request"] svc = request.find_service(name="user_password") @@ -231,7 +230,6 @@ class PasswordChangeSchema(CSRFSchema): ) def validator(self, node, value): # pragma: no cover - super().validator(node, value) exc = colander.Invalid(node) request = node.bindings["request"] svc = request.find_service(name="user_password") @@ -251,8 +249,6 @@ class DeleteAccountSchema(CSRFSchema): password = password_node(title=_("Confirm password")) def validator(self, node, value): - super().validator(node, value) - request = node.bindings["request"] svc = request.find_service(name="user_password") diff --git a/h/app.py b/h/app.py index c9db4b8c2c0..bf4ddf65dcc 100644 --- a/h/app.py +++ b/h/app.py @@ -30,6 +30,8 @@ def create_app(_global_config, **settings): # pragma: no cover def includeme(config): # pragma: no cover + config.set_default_csrf_options(require_csrf=True) + config.scan("h.subscribers") config.add_tween("h.tweens.conditional_http_tween_factory", under=EXCVIEW) diff --git a/h/schemas/auth_client.py b/h/schemas/auth_client.py index ebe28a8ea98..f880862cef9 100644 --- a/h/schemas/auth_client.py +++ b/h/schemas/auth_client.py @@ -57,8 +57,6 @@ class CreateAuthClientSchema(CSRFSchema): ) def validator(self, node, value): - super().validator(node, value) - grant_type = value.get("grant_type") redirect_url = value.get("redirect_url") diff --git a/h/schemas/base.py b/h/schemas/base.py index e1d118acf68..4b581da55db 100644 --- a/h/schemas/base.py +++ b/h/schemas/base.py @@ -6,7 +6,7 @@ import deform import jsonschema from pyramid import httpexceptions -from pyramid.csrf import check_csrf_token, get_csrf_token +from pyramid.csrf import get_csrf_token @colander.deferred @@ -21,23 +21,49 @@ class ValidationError(httpexceptions.HTTPBadRequest): class CSRFSchema(colander.Schema): """ - A CSRFSchema backward-compatible with the one from the hem module. + Add a hidden CSRF token to forms when seralized using Deform. - Unlike hem, this doesn't require that the csrf_token appear in the - serialized appstruct. + This is intended as a base class for other schemas to inherit from if the + schema's form needs a CSRF token (by default all form submissions do need a + CSRF token). + + This schema *does not* implement CSRF verification when receiving requests. + That's enabled globally for non-GET requests by + config.set_default_csrf_options(require_csrf=True). """ csrf_token = colander.SchemaNode( colander.String(), widget=deform.widget.HiddenWidget(), + # When serializing (i.e. when rendering a form) if there's no + # csrf_token then call deferred_csrf_token() to get one. default=deferred_csrf_token, - missing=None, + # Allow data with no "csrf_token" field to be *deserialized* successfully + # (the deserialized data will contain no "csrf_token" field.) + # + # CSRF protection isn't provided by this schema, it's provided by + # Pyramid's config.set_default_csrf_options(require_csrf=True). + # + # Nonetheless, without a `missing` value, when deserializing any + # subclass of this schema Colander would require a csrf_token field to + # be present in the data (even if this schema doesn't actually check + # that the token is valid). + # + # In production any request missing a CSRF token would be rejected by + # Pyramid's CSRF protection before even reaching schema + # deserialization. So by the time we get to schema deserialization + # there must be a CSRF token and this `missing` value is seemingly + # unnecessary. + # + # However: + # + # 1. The CSRF token may be in an X-CSRF-Token header rather than in a + # POST param. + # 2. Unittests for schemas often don't set a CSRF token and would fail + # if this `missing` value wasn't here. + missing=colander.drop, ) - def validator(self, node, _value): - request = node.bindings["request"] - check_csrf_token(request) - class JSONSchema: """ diff --git a/h/schemas/forms/accounts/forgot_password.py b/h/schemas/forms/accounts/forgot_password.py index 574d6a89e9f..e81258fa667 100644 --- a/h/schemas/forms/accounts/forgot_password.py +++ b/h/schemas/forms/accounts/forgot_password.py @@ -17,8 +17,6 @@ class ForgotPasswordSchema(CSRFSchema): ) def validator(self, node, value): - super().validator(node, value) - request = node.bindings["request"] email = value.get("email") user = models.User.get_by_email(request.db, email, request.default_authority) diff --git a/h/schemas/forms/accounts/login.py b/h/schemas/forms/accounts/login.py index ba950bedaa7..7a171f5a2bc 100644 --- a/h/schemas/forms/accounts/login.py +++ b/h/schemas/forms/accounts/login.py @@ -36,8 +36,6 @@ class LoginSchema(CSRFSchema): ) def validator(self, node, value): - super().validator(node, value) - request = node.bindings["request"] username = value.get("username") password = value.get("password") diff --git a/h/schemas/forms/admin/group.py b/h/schemas/forms/admin/group.py index 98c2e321f23..100a45e677e 100644 --- a/h/schemas/forms/admin/group.py +++ b/h/schemas/forms/admin/group.py @@ -219,5 +219,4 @@ def __init__(self, *args): ) def validator(self, node, value): - super().validator(node, value) username_validator(node, value) diff --git a/h/views/api/auth.py b/h/views/api/auth.py index 54bb97af10d..87546f77ad7 100644 --- a/h/views/api/auth.py +++ b/h/views/api/auth.py @@ -104,6 +104,7 @@ def post(self): request_param="response_mode=web_message", is_authenticated=True, renderer="h:templates/oauth/authorize_web_message.html.jinja2", + require_csrf=False, ) def post_web_message(self): """ diff --git a/h/views/api/config.py b/h/views/api/config.py index 28e7298e28a..bf8ac485dfa 100644 --- a/h/views/api/config.py +++ b/h/views/api/config.py @@ -65,6 +65,7 @@ def add_api_view( # noqa: PLR0913 `route_name` must be specified. :param dict **settings: Arguments to pass on to ``config.add_view`` """ + settings.setdefault("require_csrf", False) settings.setdefault("renderer", "json") settings.setdefault("decorator", (cors_policy, version_media_type_header(subtype))) diff --git a/tests/functional/h/views/admin/permissions_test.py b/tests/functional/h/views/admin/permissions_test.py index 52ce2a6e69f..d23cb6206ac 100644 --- a/tests/functional/h/views/admin/permissions_test.py +++ b/tests/functional/h/views/admin/permissions_test.py @@ -35,10 +35,7 @@ def test_accessible_by_staff(self, app, url, accessible): assert res.status_code == 200 if accessible else 404 - GROUP_PAGES = ( - ("POST", "/admin/groups/delete/{pubid}", 302), - ("GET", "/admin/groups/{pubid}", 200), - ) + GROUP_PAGES = (("GET", "/admin/groups/{pubid}", 200),) @pytest.mark.usefixtures("with_logged_in_admin") @pytest.mark.parametrize("method,url_template,success_code", GROUP_PAGES) diff --git a/tests/unit/h/accounts/schemas_test.py b/tests/unit/h/accounts/schemas_test.py index 3d089b1bc93..2a7f2df10aa 100644 --- a/tests/unit/h/accounts/schemas_test.py +++ b/tests/unit/h/accounts/schemas_test.py @@ -3,7 +3,6 @@ import colander import pytest -from pyramid.exceptions import BadCSRFToken from h.accounts import schemas from h.services.user_password import UserPasswordService @@ -156,9 +155,7 @@ def test_it_validates_with_valid_payload( result = schema.deserialize(valid_params) - assert result == dict( - valid_params, privacy_accepted=True, comms_opt_in=None, csrf_token=None - ) + assert result == dict(valid_params, privacy_accepted=True, comms_opt_in=None) @pytest.fixture def valid_params(self): @@ -194,18 +191,6 @@ def test_it_is_valid_if_email_same_as_users_existing_email( schema.deserialize({"email": user.email, "password": "flibble"}) - def test_it_is_invalid_if_csrf_token_missing(self, pyramid_request, schema): - del pyramid_request.headers["X-CSRF-Token"] - - with pytest.raises(BadCSRFToken): - schema.deserialize({"email": "foo@bar.com", "password": "flibble"}) - - def test_it_is_invalid_if_csrf_token_wrong(self, pyramid_request, schema): - pyramid_request.headers["X-CSRF-Token"] = "WRONG" - - with pytest.raises(BadCSRFToken): - schema.deserialize({"email": "foo@bar.com", "password": "flibble"}) - def test_it_is_invalid_if_password_wrong(self, schema, user_password_service): user_password_service.check_password.return_value = False diff --git a/tests/unit/h/schemas/base_test.py b/tests/unit/h/schemas/base_test.py index e660a12705f..f8a301b3f80 100644 --- a/tests/unit/h/schemas/base_test.py +++ b/tests/unit/h/schemas/base_test.py @@ -3,8 +3,6 @@ import colander import pytest -from pyramid import csrf -from pyramid.exceptions import BadCSRFToken from h.schemas import ValidationError from h.schemas.base import CSRFSchema, JSONSchema, enum_type @@ -25,28 +23,6 @@ class ExampleJSONSchema(JSONSchema): } -class TestCSRFSchema: - def test_raises_badcsrf_with_bad_csrf(self, pyramid_request): - schema = ExampleCSRFSchema().bind(request=pyramid_request) - - with pytest.raises(BadCSRFToken): - schema.deserialize({}) - - def test_ok_with_good_csrf(self, pyramid_request): - csrf_token = csrf.get_csrf_token(pyramid_request) - pyramid_request.POST["csrf_token"] = csrf_token - schema = ExampleCSRFSchema().bind(request=pyramid_request) - - # Does not raise - schema.deserialize({}) - - def test_ok_with_good_csrf_from_header(self, pyramid_csrf_request): - schema = ExampleCSRFSchema().bind(request=pyramid_csrf_request) - - # Does not raise - schema.deserialize({}) - - class TestJSONSchema: def test_it_raises_for_unsupported_schema_versions(self): class BadSchema(JSONSchema): diff --git a/tests/unit/h/schemas/forms/accounts/login_test.py b/tests/unit/h/schemas/forms/accounts/login_test.py index 864f06d1c36..8fe2d153cac 100644 --- a/tests/unit/h/schemas/forms/accounts/login_test.py +++ b/tests/unit/h/schemas/forms/accounts/login_test.py @@ -2,7 +2,6 @@ import colander import pytest -from pyramid.exceptions import BadCSRFToken from h.schemas.forms.accounts import LoginSchema from h.services.user import UserNotActivated @@ -46,12 +45,6 @@ def test_it_returns_user_when_valid( assert result["user"] is user - def test_invalid_with_bad_csrf(self, pyramid_request): - schema = LoginSchema().bind(request=pyramid_request) - - with pytest.raises(BadCSRFToken): - schema.deserialize({"username": "jeannie", "password": "cake"}) - def test_invalid_with_inactive_user(self, pyramid_csrf_request, user_service): schema = LoginSchema().bind(request=pyramid_csrf_request) user_service.fetch_for_login.side_effect = UserNotActivated() diff --git a/tests/unit/h/schemas/forms/admin/group_test.py b/tests/unit/h/schemas/forms/admin/group_test.py index 9eaa1cd2b4f..b92a9c766c8 100644 --- a/tests/unit/h/schemas/forms/admin/group_test.py +++ b/tests/unit/h/schemas/forms/admin/group_test.py @@ -3,7 +3,6 @@ import colander import pytest -from pyramid.exceptions import BadCSRFToken from h.models.group import ( GROUP_DESCRIPTION_MAX_LENGTH, @@ -18,18 +17,6 @@ class TestAdminGroupSchema: def test_it_allows_with_valid_data(self, group_data, bound_schema): bound_schema.deserialize(group_data) - def test_it_raises_if_csrf_token_missing(self, group_data, bound_schema): - del bound_schema.bindings["request"].headers["X-CSRF-Token"] - - with pytest.raises(BadCSRFToken): - bound_schema.deserialize(group_data) - - def test_it_raises_if_csrf_token_wrong(self, group_data, bound_schema): - bound_schema.bindings["request"].headers["X-CSRF-Token"] = "foobar" - - with pytest.raises(BadCSRFToken): - bound_schema.deserialize(group_data) - def test_it_raises_if_name_too_short(self, group_data, bound_schema): too_short_name = "a" * (GROUP_NAME_MIN_LENGTH - 1) group_data["name"] = too_short_name From d20457ef5a999baa10e986187afd4eb51da62fe4 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Fri, 7 Feb 2025 19:22:49 +0000 Subject: [PATCH 2/2] Factor out CSRFSchema base class There's no need for this base class. This commit just replaces it with a line in h/templates/deform/form.jinja2 that adds a CSRF token to every form. --- h/accounts/schemas.py | 11 ++--- h/form.py | 9 +++- h/schemas/auth_client.py | 4 +- h/schemas/base.py | 54 --------------------- h/schemas/forms/accounts/edit_profile.py | 3 +- h/schemas/forms/accounts/forgot_password.py | 3 +- h/schemas/forms/accounts/login.py | 3 +- h/schemas/forms/accounts/reset_password.py | 3 +- h/schemas/forms/admin/group.py | 3 +- h/schemas/forms/admin/organization.py | 3 +- h/templates/deform/form.jinja2 | 1 + tests/unit/h/form_test.py | 46 +++++++++++++++--- tests/unit/h/schemas/base_test.py | 6 +-- 13 files changed, 61 insertions(+), 88 deletions(-) diff --git a/h/accounts/schemas.py b/h/accounts/schemas.py index 8fcf259884f..db755ddc8d7 100644 --- a/h/accounts/schemas.py +++ b/h/accounts/schemas.py @@ -16,7 +16,6 @@ USERNAME_PATTERN, ) from h.schemas import validators -from h.schemas.base import CSRFSchema from h.schemas.forms.accounts.util import PASSWORD_MIN_LENGTH from h.util.user import format_userid @@ -153,7 +152,7 @@ def _privacy_accepted_message(): return privacy_msg -class RegisterSchema(CSRFSchema): +class RegisterSchema(colander.Schema): username = colander.SchemaNode( colander.String(), validator=colander.All( @@ -197,7 +196,7 @@ class RegisterSchema(CSRFSchema): ) -class EmailChangeSchema(CSRFSchema): +class EmailChangeSchema(colander.Schema): email = email_node(title=_("Email address")) # No validators: all validation is done on the email field password = password_node(title=_("Confirm password"), hide_until_form_active=True) @@ -215,7 +214,7 @@ def validator(self, node, value): raise exc -class PasswordChangeSchema(CSRFSchema): +class PasswordChangeSchema(colander.Schema): password = password_node(title=_("Current password"), inactive_label=_("Password")) new_password = new_password_node( title=_("New password"), hide_until_form_active=True @@ -245,7 +244,7 @@ def validator(self, node, value): # pragma: no cover raise exc -class DeleteAccountSchema(CSRFSchema): +class DeleteAccountSchema(colander.Schema): password = password_node(title=_("Confirm password")) def validator(self, node, value): @@ -258,7 +257,7 @@ def validator(self, node, value): raise exc -class NotificationsSchema(CSRFSchema): +class NotificationsSchema(colander.Schema): types = (("reply", _("Email me when someone replies to one of my annotations.")),) notifications = colander.SchemaNode( diff --git a/h/form.py b/h/form.py index e40736b38db..0fc3f7d2f0f 100644 --- a/h/form.py +++ b/h/form.py @@ -5,10 +5,13 @@ form templates in preference to the defaults. """ +from functools import partial + import deform import pyramid_jinja2 from markupsafe import Markup from pyramid import httpexceptions +from pyramid.csrf import get_csrf_token from pyramid.path import AssetResolver from h import i18n @@ -46,6 +49,10 @@ def __call__(self, template_name, **kwargs): context = self._system.copy() context.update(kwargs) + context.setdefault( + "get_csrf_token", partial(get_csrf_token, context["request"]) + ) + return Markup(template.render(context)) @@ -75,7 +82,7 @@ def create_form(request, *args, **kwargs): default) will use the renderer configured in the :py:mod:`h.form` module. """ env = request.registry[ENVIRONMENT_KEY] - renderer = Jinja2Renderer(env, {"feature": request.feature}) + renderer = Jinja2Renderer(env, {"feature": request.feature, "request": request}) kwargs.setdefault("renderer", renderer) return deform.Form(*args, **kwargs) diff --git a/h/schemas/auth_client.py b/h/schemas/auth_client.py index f880862cef9..251e3a37925 100644 --- a/h/schemas/auth_client.py +++ b/h/schemas/auth_client.py @@ -3,13 +3,13 @@ from h import i18n from h.models.auth_client import GrantType -from h.schemas.base import CSRFSchema, enum_type +from h.schemas.base import enum_type _ = i18n.TranslationString GrantTypeSchemaType = enum_type(GrantType) -class CreateAuthClientSchema(CSRFSchema): +class CreateAuthClientSchema(colander.Schema): name = colander.SchemaNode( colander.String(), title=_("Name"), diff --git a/h/schemas/base.py b/h/schemas/base.py index 4b581da55db..644171da428 100644 --- a/h/schemas/base.py +++ b/h/schemas/base.py @@ -3,68 +3,14 @@ import copy import colander -import deform import jsonschema from pyramid import httpexceptions -from pyramid.csrf import get_csrf_token - - -@colander.deferred -def deferred_csrf_token(_node, kwargs): - request = kwargs.get("request") - return get_csrf_token(request) class ValidationError(httpexceptions.HTTPBadRequest): pass -class CSRFSchema(colander.Schema): - """ - Add a hidden CSRF token to forms when seralized using Deform. - - This is intended as a base class for other schemas to inherit from if the - schema's form needs a CSRF token (by default all form submissions do need a - CSRF token). - - This schema *does not* implement CSRF verification when receiving requests. - That's enabled globally for non-GET requests by - config.set_default_csrf_options(require_csrf=True). - """ - - csrf_token = colander.SchemaNode( - colander.String(), - widget=deform.widget.HiddenWidget(), - # When serializing (i.e. when rendering a form) if there's no - # csrf_token then call deferred_csrf_token() to get one. - default=deferred_csrf_token, - # Allow data with no "csrf_token" field to be *deserialized* successfully - # (the deserialized data will contain no "csrf_token" field.) - # - # CSRF protection isn't provided by this schema, it's provided by - # Pyramid's config.set_default_csrf_options(require_csrf=True). - # - # Nonetheless, without a `missing` value, when deserializing any - # subclass of this schema Colander would require a csrf_token field to - # be present in the data (even if this schema doesn't actually check - # that the token is valid). - # - # In production any request missing a CSRF token would be rejected by - # Pyramid's CSRF protection before even reaching schema - # deserialization. So by the time we get to schema deserialization - # there must be a CSRF token and this `missing` value is seemingly - # unnecessary. - # - # However: - # - # 1. The CSRF token may be in an X-CSRF-Token header rather than in a - # POST param. - # 2. Unittests for schemas often don't set a CSRF token and would fail - # if this `missing` value wasn't here. - missing=colander.drop, - ) - - class JSONSchema: """ Validate data according to a JSON Schema. diff --git a/h/schemas/forms/accounts/edit_profile.py b/h/schemas/forms/accounts/edit_profile.py index 826f5e81a7a..011178ff9c0 100644 --- a/h/schemas/forms/accounts/edit_profile.py +++ b/h/schemas/forms/accounts/edit_profile.py @@ -5,7 +5,6 @@ from h.accounts import util from h.models.user import DISPLAY_NAME_MAX_LENGTH from h.schemas import validators -from h.schemas.base import CSRFSchema _ = i18n.TranslationString @@ -24,7 +23,7 @@ def validate_orcid(node, cstruct): raise colander.Invalid(node, str(exc)) # noqa: B904 -class EditProfileSchema(CSRFSchema): +class EditProfileSchema(colander.Schema): display_name = colander.SchemaNode( colander.String(), missing=None, diff --git a/h/schemas/forms/accounts/forgot_password.py b/h/schemas/forms/accounts/forgot_password.py index e81258fa667..d8e99246205 100644 --- a/h/schemas/forms/accounts/forgot_password.py +++ b/h/schemas/forms/accounts/forgot_password.py @@ -3,12 +3,11 @@ from h import i18n, models from h.schemas import validators -from h.schemas.base import CSRFSchema _ = i18n.TranslationString -class ForgotPasswordSchema(CSRFSchema): +class ForgotPasswordSchema(colander.Schema): email = colander.SchemaNode( colander.String(), validator=colander.All(validators.Email()), diff --git a/h/schemas/forms/accounts/login.py b/h/schemas/forms/accounts/login.py index 7a171f5a2bc..accec254649 100644 --- a/h/schemas/forms/accounts/login.py +++ b/h/schemas/forms/accounts/login.py @@ -2,7 +2,6 @@ import deform from h import i18n -from h.schemas.base import CSRFSchema from h.services.user import UserNotActivated _ = i18n.TranslationString @@ -25,7 +24,7 @@ def _deferred_password_widget(_node, kwargs): ) -class LoginSchema(CSRFSchema): +class LoginSchema(colander.Schema): username = colander.SchemaNode( colander.String(), title=_("Username / email"), diff --git a/h/schemas/forms/accounts/reset_password.py b/h/schemas/forms/accounts/reset_password.py index d425e0d65ab..61a44676d38 100644 --- a/h/schemas/forms/accounts/reset_password.py +++ b/h/schemas/forms/accounts/reset_password.py @@ -4,7 +4,6 @@ from itsdangerous import BadData, SignatureExpired from h import i18n, models -from h.schemas.base import CSRFSchema from h.schemas.forms.accounts import util _ = i18n.TranslationString @@ -60,7 +59,7 @@ def deserialize(self, node, cstruct): return user -class ResetPasswordSchema(CSRFSchema): +class ResetPasswordSchema(colander.Schema): # N.B. this is the field into which the user puts their reset code, but we # call it `user` because when validated, it will return a `User` object. user = colander.SchemaNode( diff --git a/h/schemas/forms/admin/group.py b/h/schemas/forms/admin/group.py index 100a45e677e..48812a13499 100644 --- a/h/schemas/forms/admin/group.py +++ b/h/schemas/forms/admin/group.py @@ -14,7 +14,6 @@ GROUP_NAME_MIN_LENGTH, ) from h.schemas import validators -from h.schemas.base import CSRFSchema from h.util import group_scope _ = i18n.TranslationString @@ -137,7 +136,7 @@ def group_organization_select_widget(_node, kwargs): return SelectWidget(values=list(zip(org_pubids, org_labels, strict=False))) -class AdminGroupSchema(CSRFSchema): +class AdminGroupSchema(colander.Schema): def __init__(self, *args): super().__init__(*args) diff --git a/h/schemas/forms/admin/organization.py b/h/schemas/forms/admin/organization.py index d26322a9a5f..5a4f014ff23 100644 --- a/h/schemas/forms/admin/organization.py +++ b/h/schemas/forms/admin/organization.py @@ -6,7 +6,6 @@ import h.i18n from h.models.organization import Organization from h.schemas import validators -from h.schemas.base import CSRFSchema _ = h.i18n.TranslationString @@ -36,7 +35,7 @@ def validate_logo(node, value): raise colander.Invalid(node, _("Logo does not start with tag")) -class OrganizationSchema(CSRFSchema): +class OrganizationSchema(colander.Schema): authority = colander.SchemaNode(colander.String(), title=_("Authority")) name = colander.SchemaNode( diff --git a/h/templates/deform/form.jinja2 b/h/templates/deform/form.jinja2 index 5288b2c4f73..a24850c26e9 100644 --- a/h/templates/deform/form.jinja2 +++ b/h/templates/deform/form.jinja2 @@ -8,6 +8,7 @@ class="form {{ field.css_class or '' }} {%- if field.use_inline_editing %} js-form {% endif %}"> +
diff --git a/tests/unit/h/form_test.py b/tests/unit/h/form_test.py index 164d71778ac..49b3ef8adf9 100644 --- a/tests/unit/h/form_test.py +++ b/tests/unit/h/form_test.py @@ -8,7 +8,7 @@ class TestJinja2Renderer: def test_call_fetches_correct_templates(self, jinja2_env): - renderer = form.Jinja2Renderer(jinja2_env) + renderer = form.Jinja2Renderer(jinja2_env, {"request": mock.sentinel.request}) renderer("foo") renderer("foo.jinja2") @@ -23,23 +23,52 @@ def test_call_fetches_correct_templates(self, jinja2_env): ] def test_call_passes_kwargs_to_render(self, jinja2_env, jinja2_template): - renderer = form.Jinja2Renderer(jinja2_env) + renderer = form.Jinja2Renderer(jinja2_env, {"request": mock.sentinel.request}) renderer("textinput", foo="foo", bar="bar") - jinja2_template.render.assert_called_once_with({"foo": "foo", "bar": "bar"}) + jinja2_template.render.assert_called_once_with( + { + "foo": "foo", + "bar": "bar", + "request": mock.sentinel.request, + "get_csrf_token": mock.ANY, + } + ) def test_call_passes_system_context_to_render(self, jinja2_env, jinja2_template): - renderer = form.Jinja2Renderer(jinja2_env, {"bar": "default"}) + renderer = form.Jinja2Renderer( + jinja2_env, {"bar": "default", "request": mock.sentinel.request} + ) renderer("textinput") renderer("textinput", foo="foo") renderer("textinput", foo="foo", bar="bar") assert jinja2_template.render.call_args_list == [ - mock.call({"bar": "default"}), - mock.call({"foo": "foo", "bar": "default"}), - mock.call({"foo": "foo", "bar": "bar"}), + mock.call( + { + "bar": "default", + "request": mock.sentinel.request, + "get_csrf_token": mock.ANY, + } + ), + mock.call( + { + "foo": "foo", + "bar": "default", + "request": mock.sentinel.request, + "get_csrf_token": mock.ANY, + } + ), + mock.call( + { + "foo": "foo", + "bar": "bar", + "request": mock.sentinel.request, + "get_csrf_token": mock.ANY, + } + ), ] @pytest.fixture @@ -92,7 +121,8 @@ def test_adds_feature_client_to_system_context(self, patch, pyramid_request): form.create_form(pyramid_request, mock.sentinel.schema) Jinja2Renderer.assert_called_once_with( - mock.sentinel.jinja2_env, {"feature": pyramid_request.feature} + mock.sentinel.jinja2_env, + {"feature": pyramid_request.feature, "request": pyramid_request}, ) @pytest.fixture(autouse=True) diff --git a/tests/unit/h/schemas/base_test.py b/tests/unit/h/schemas/base_test.py index f8a301b3f80..90d8b1fda3c 100644 --- a/tests/unit/h/schemas/base_test.py +++ b/tests/unit/h/schemas/base_test.py @@ -5,15 +5,11 @@ import pytest from h.schemas import ValidationError -from h.schemas.base import CSRFSchema, JSONSchema, enum_type +from h.schemas.base import JSONSchema, enum_type pytestmark = pytest.mark.usefixtures("pyramid_config") -class ExampleCSRFSchema(CSRFSchema): - pass - - class ExampleJSONSchema(JSONSchema): schema = { # noqa: RUF012 "$schema": "http://json-schema.org/draft-04/schema#",