From 42494b5bb1495a60caeecd324245891dc3ca07d8 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 3 May 2013 16:43:11 +1000 Subject: [PATCH] we can't trust CSRF for anon the way it is designed. The page they have loaded may be cached we need a different way of delivering the CSRF potentially --- app/controllers/session_controller.rb | 5 +++++ app/controllers/users_controller.rb | 5 +++++ app/helpers/application_helper.rb | 9 +++++++++ app/views/layouts/application.html.erb | 2 +- app/views/layouts/no_js.html.erb | 2 +- 5 files changed, 21 insertions(+), 2 deletions(-) diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 46c1085f3d4..a7c6f66e8ac 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -1,4 +1,9 @@ class SessionController < ApplicationController + + # we need to allow account login with bad CSRF tokens, if people are caching, the CSRF token on the + # page is going to be empty, this means that server will see an invalid CSRF and blow the session + # once that happens you can't log in with social + skip_before_filter :verify_authenticity_token, only: [:create] def create requires_parameter(:login, :password) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index e34c798c2d7..4227335121b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -8,6 +8,11 @@ class UsersController < ApplicationController before_filter :ensure_logged_in, only: [:username, :update, :change_email, :user_preferences_redirect] + # we need to allow account creation with bad CSRF tokens, if people are caching, the CSRF token on the + # page is going to be empty, this means that server will see an invalid CSRF and blow the session + # once that happens you can't log in with social + skip_before_filter :verify_authenticity_token, only: [:create] + def show @user = fetch_user_from_params user_serializer = UserSerializer.new(@user, scope: guardian, root: 'user') diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 4377e8a4a1a..074329b9e66 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -10,6 +10,15 @@ module ApplicationHelper include CanonicalURL::Helpers include ConfigurableUrls + def discourse_csrf_tags + # anon can not have a CSRF token cause these are all pages + # that may be cached, causing a mismatch between session CSRF + # and CSRF on page and horrible impossible to debug login issues + if current_user + csrf_meta_tags + end + end + def with_format(format, &block) old_formats = formats self.formats = [format] diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 609aff54b4c..eaa103cc904 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -15,7 +15,7 @@ <%= render :partial => "common/special_font_face" %> <%= render :partial => "common/discourse_stylesheet" %> - <%= csrf_meta_tags %> + <%= discourse_csrf_tags %> <%= yield :head %> diff --git a/app/views/layouts/no_js.html.erb b/app/views/layouts/no_js.html.erb index 863f786d0b9..91060ccb0b2 100644 --- a/app/views/layouts/no_js.html.erb +++ b/app/views/layouts/no_js.html.erb @@ -11,7 +11,7 @@ <%= render :partial => "common/special_font_face" %> <%= render :partial => "common/discourse_stylesheet" %> - <%=csrf_meta_tags%> + <%= discourse_csrf_tags %>