[Django] #37162: Docs ContactForm example demonstrates poor practices

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike | Owner: Mike Edmunds
Edmunds |
Type: | Status: assigned
Cleanup/optimization |
Component: | Version: 6.0
Documentation |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
The `ContactForm` used as a running example starting in the
[https://docs.djangoproject.com/en/6.1/topics/forms/#more-on-fields More
on fields] forms topic demonstrates poor practices for implementing a
contact form that sends email:

* It uses the contact's email address (`sender`) as the message's
`from_email`. At best, this
[https://stackoverflow.com/questions/54549797/smtpdataerror-553-brelaying-
disallowed-as-abcemail-com-while-using-contact won't work] (the message
will be rejected by the outgoing server or silently ignored as spam at the
receiving end). At worst, it creates vulnerabilities.
* It passes through `subject` and `message` without any indication they
originated in a web contact form. This can allow high-fidelity phishing
attacks against the organization deploying the form (particularly when
combined with an arbitrary sender address).

I realize the example needs to be kept simple, but Django's docs probably
shouldn't be illustrating insecure approaches.

I'd suggest changing the current:

{{{#!python
from django.core.mail import send_mail

if form.is_valid():
subject = form.cleaned_data["subject"]
message = form.cleaned_data["message"]
sender = form.cleaned_data["sender"]
cc_myself = form.cleaned_data["cc_myself"]

recipients = ["in...@example.com"]
if cc_myself:
recipients.append(sender)

send_mail(subject, message, sender, recipients)
return HttpResponseRedirect("/thanks/")
}}}

to something like (also renaming the `sender` form field to `email`
throughout the page):

{{{#!python
from django.core.mail import EmailMessage

if form.is_valid():
subject = form.cleaned_data["subject"]
message = form.cleaned_data["message"]
email = form.cleaned_data["email"]
cc_myself = form.cleaned_data["cc_myself"]

# Send the message, directing replies to the contact's email.
EmailMessage(
subject=f"[via contact form] {subject}",
body=f"Contact email: {email}\n\n{message}",
to=["in...@example.com"],
cc=[email] if cc_myself else None,
reply_to=[email],
).send()

return HttpResponseRedirect("/thanks/")
}}}

Note that the `cc` handling there (and in the original) is also not ideal.
Maybe we could come up with some other reason to include a boolean field
in the form? ("Urgent"? "No reply necessary"?)

Or if something like that seems too complicated, maybe we should consider
rewriting the page to use something other than a contact form.

[Noticed this while I was working on #34753. I was considering removing
the contact-form-like example from the
[https://docs.djangoproject.com/en/6.1/topics/email/#preventing-header-
injection Email header injection] section and replacing it with a ref to
this actual-contact-form example.]
--
Ticket URL: <https://code.djangoproject.com/ticket/37162>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------

Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:1>

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mike Edmunds):

* has_patch: 0 => 1

Comment:

https://github.com/django/django/pull/21522

I ended up replacing the `cc_myself` field with an `urgent` option.
--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:2>

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mike Edmunds):

A version of that same `ContactForm` also appears in
[https://www.djangoproject.com/start/#:~:text=class%20BandContactForm(forms.Form)%3A%0A%20%20%20%20subject%20%3D%20forms.CharField(max_length%3D100)%0A%20%20%20%20message%20%3D%20forms.TextField()%0A%20%20%20%20sender%20%3D%20forms.EmailField()%0A%20%20%20%20cc_myself%20%3D%20forms.BooleanField(required%3DFalse)
djangoproject.com/start/] under ''Intro to Django > Forms'' (but without
the email sending function).

We may want to update it in
djangoproject.com:[https://github.com/django/djangoproject.com/blob/69746e315450ee9ce8074d1f4fe1dc229e504d90/djangoproject/templates/start.html#L217-L221
djangoproject/templates/start.html].
--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:3>

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1

Comment:

Setting as patch needs improvement to ensure other related references of
the ContactForm are updated to match. On the djangoproject.com start page
from comment:3, my recommendation is to remove the Python code for the
form altogether and just leave the actual form definition to be found once
the user follows the "read more" link.
--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:4>

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Mike Edmunds):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:5>

[Django]

#37162: Docs ContactForm example demonstrates poor practices
-------------------------------------+-------------------------------------
Reporter: Mike Edmunds | Owner: Mike
Type: | Edmunds
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 6.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mike Edmunds):

Opened separate issue
https://github.com/django/djangoproject.com/issues/2684 for
djangoproject.com example.
--
Ticket URL: <https://code.djangoproject.com/ticket/37162#comment:6>