Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6-0-stable] Backport Upgrade-safe URL-safe CSRF tokens #39076 #41806

Merged
merged 3 commits into from Mar 31, 2021

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Mar 31, 2021

This is the backport of #39076 to make apps upgrade-safe from Rails 5.2.5.

I've made a PR #41805 to make CSRF tokens forward compatible, that is enough for 6.0.x -> 6.0.next.

But for 5.2.4.x and 5.2.5. #39076 is required to make apps upgrade-safe both 5.2.4.x -> 5.2.next (skip 5.2.5) and 5.2.5 -> 5.2.next.

If this way is preferable, I'll merge this and backport to 5-2-stable (with making this Ruby 2.2 compatible).

dragonsinth and others added 2 commits March 31, 2021 11:21
Base64 strict-encoded CSRF tokens are not inherently websafe, which makes
them difficult to deal with. For example, the common practice of sending
the CSRF token to a browser in a client-readable cookie does not work properly
out of the box: the value has to be url-encoded and decoded to survive transport.

Now, we generate Base64 urlsafe-encoded CSRF tokens, which are inherently safe
to transport.  Validation accepts both urlsafe tokens, and strict-encoded tokens
for backwards compatibility.
…e-csrf-tokens

Upgrade-safe URL-safe CSRF tokens
@kamipo kamipo merged commit ce2d72a into rails:6-0-stable Mar 31, 2021
@kamipo kamipo deleted the backport_39076_to_6-0-stable branch March 31, 2021 04:09
kamipo added a commit to kamipo/rails that referenced this pull request Mar 31, 2021
[6-0-stable] Backport Upgrade-safe URL-safe CSRF tokens rails#39076
@yevhene
Copy link
Contributor

yevhene commented Apr 12, 2021

@rafaelfranca Please release it. We are about to upgrade to Rails 6.0, but stuck on 5.2 with this issue.

@GCorbel
Copy link
Contributor

GCorbel commented May 27, 2021

Let me know if I didn't understand correct. The fix has been merged in branch 6-0-stable but wasn't in release 6.0.3.7 so, If we want to upgrade from Rails 5.2 to Rails 6.0, we have to use 6-0-stable or wait for the next release, right ?

@yevhene
Copy link
Contributor

yevhene commented May 27, 2021

@GCorbel As I understood 6.0.3.7 not included the fix because it is 4 level release. We need to wait for 6.0.4.

@zzak
Copy link
Member

zzak commented May 27, 2021

@yevhene @GCorbel In the mean time, have you tried out the 6-0-stable branch and let us know if there's any issues before a release is cut? 🙇

@yevhene
Copy link
Contributor

yevhene commented May 28, 2021

@zzak This feature works fine now in 6-0-stable branch

@GCorbel
Copy link
Contributor

GCorbel commented May 28, 2021

@zzak I saw no issue with 6-0-stable branch.

@zzak
Copy link
Member

zzak commented May 28, 2021

Thank you both for testing! I will leave it up to rails-core to decide when to release this 🙏

@GCorbel
Copy link
Contributor

GCorbel commented Jun 16, 2021

Rails 6.0.4 has been released with this commit in it. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants