RoR forms project - Update CSRF token related sections #29165

jg-k · 2024-12-08T16:36:45Z

Because

Turbo is enabled by default from Rails 7 but tasks related to CSRF token in the first rails form project do not take into account default form behavior with Turbo.

This PR

Extends the tasks of the rails form project by getting the student to submit the manually created form without then with turbo enabled to provide a better understanding of which CSRF token is needed where.
Removes the paragraph explaining how to add a hidden input with a CSRF token from the lesson and relocates it to the project section. This avoids introducing too much details related to Turbo in the lesson, as it would extend beyond the core topic of the lesson. It is proposed to address the impact of Turbo more comprehensively separately.

Issue

Closes #28932

Additional Information

NA

Pull Request Requirements

I have thoroughly read and understand The Odin Project curriculum contributing guide
The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
The Because section summarizes the reason for this PR
The This PR section has a bullet point list describing the changes in this PR
If this PR addresses an open issue, it is linked in the Issue section
If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
If any lesson files are included in this PR, they follow the Layout Style Guide

rlmoser99

Thank you for taking the time to make these changes to these two lessons. There are a few small things that I think we need to change, so that the layout matches the rest of the lesson/curriculum. I left a few suggestions to try and illustrate why I am requesting a change, but I am open to other ideas as well.

rlmoser99 · 2024-12-12T23:30:57Z

ruby_on_rails/forms_and_authentication/project_forms.md

+1. CSRF Safety:
+From Rails 7, Turbo is enabled by default in new apps. Turbo intercepts form submission and makes a partial XHR request instead of a standard HTTP request with full page reload. To get a better grasp of Rails protection against [cross-site request forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery), let's take a small detour and disable Turbo for this form by setting the data attribute `data-turbo=false`.
+In the dev tools network tab, compare the request type with and without the `data-turbo=false` attribute to confirm it works as expected.


When using our markdown preview tool, I don't really like having the "CSRF Safety:" and the last sentence on different lines look. I propose rewording slightly to putting them all on 1 line, something like this:

Suggested change

1. CSRF Safety:

From Rails 7, Turbo is enabled by default in new apps. Turbo intercepts form submission and makes a partial XHR request instead of a standard HTTP request with full page reload. To get a better grasp of Rails protection against [cross-site request forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery), let's take a small detour and disable Turbo for this form by setting the data attribute `data-turbo=false`.

In the dev tools network tab, compare the request type with and without the `data-turbo=false` attribute to confirm it works as expected.

1. For CSRF safety with Rails 7, Turbo is enabled by default in new apps. Turbo intercepts form submission and makes a partial XHR request instead of a standard HTTP request with full page reload. To get a better grasp of Rails protection against [cross-site request forgery](https://en.wikipedia.org/wiki/Cross-site_request_forgery), let's take a small detour and disable Turbo for this form by setting the data attribute `data-turbo=false`. Then, in the dev tools network tab, compare the request type with and without the `data-turbo=false` attribute to confirm it works as expected.

Updated as proposed

rlmoser99 · 2024-12-12T23:38:40Z

ruby_on_rails/forms_and_authentication/form_basics.md

-### Making forms into params
-
-What about the other form inputs, the ones we actually care about?
+### Railsifying your form - Making forms input into params


Instead of combining these two titles with a hyphen, I'd prefer to combine them together. Maybe something like this:

Suggested change

### Railsifying your form - Making forms input into params

### Railsifying your form by making the forms input into params

Updated as proposed

rlmoser99 · 2024-12-12T23:45:04Z

ruby_on_rails/forms_and_authentication/project_forms.md

+1. Check your inspector and your `application.html.erb` template. See a CSRF token that s always available?
+Remove this one too from `application.html.erb`, and verify that the server hits back with a CSRF error.


There is a typo here: "that s always available?".

Also, I'm not a big fan of how this looks in the markdown preview. I would either put these two lines on the same line, or add an empty line in between (line lines 118-120).

Updated and put in a single paragraph.

rlmoser99 · 2024-12-12T23:46:26Z

ruby_on_rails/forms_and_authentication/project_forms.md

+1. Re-enable form submission with Turbo by removing the `data-turbo=false` attribute on the form tag, then also remove the hidden input with CSRF token tag and submit.
+
+   No more CSRF error!?!
+   The from is now submitted with Turbo, yet Rails still protects you by verifying a CSRF token. Where does this token comes from?


I am not a fan of how these lines lay out in the markdown preview either, though I don't really have a great solution in mind. Maybe add an empty line between 120 and 121?

Here I have added an empty line to keep the "No more CSRF error" as a standalone observation that the reader should pause on. I did not put it on a separate numbered list item as it s not a new task.

jg-k · 2024-12-13T09:43:47Z

Thanks for the review 🙏

jg-k · 2024-12-20T13:36:52Z

@rlmoser99
please let me know if you have any further comments :-)

Or if you think the changes are not necessary and the PR should be closed.

Update CSRF token related sections

7cf4484

github-actions bot added the Content: Ruby on Rails Involves the Ruby on Rails course label Dec 8, 2024

jg-k changed the title ~~RoR forms project - Update CSRF related sections~~ RoR forms project - Update CSRF token related sections Dec 8, 2024

wise-king-sullyman requested review from a team and rlmoser99 and removed request for a team December 10, 2024 01:57

Remove sentence for clarity

b3558b6

rlmoser99 requested changes Dec 12, 2024

View reviewed changes

Implement review fixes. Fix links.

bf69c3f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RoR forms project - Update CSRF token related sections #29165

RoR forms project - Update CSRF token related sections #29165

jg-k commented Dec 8, 2024 •

edited

Loading

rlmoser99 left a comment

rlmoser99 Dec 12, 2024

jg-k Dec 13, 2024

rlmoser99 Dec 12, 2024

jg-k Dec 13, 2024

rlmoser99 Dec 12, 2024

jg-k Dec 13, 2024

rlmoser99 Dec 12, 2024

jg-k Dec 13, 2024

jg-k commented Dec 13, 2024

jg-k commented Dec 20, 2024

	### Railsifying your form - Making forms input into params
	### Railsifying your form by making the forms input into params

		1. Check your inspector and your `application.html.erb` template. See a CSRF token that s always available?
		Remove this one too from `application.html.erb`, and verify that the server hits back with a CSRF error.

RoR forms project - Update CSRF token related sections #29165

Are you sure you want to change the base?

RoR forms project - Update CSRF token related sections #29165

Conversation

jg-k commented Dec 8, 2024 • edited Loading

Because

This PR

Issue

Additional Information

Pull Request Requirements

rlmoser99 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jg-k commented Dec 13, 2024

jg-k commented Dec 20, 2024

jg-k commented Dec 8, 2024 •

edited

Loading