diff --git a/Changelog.md b/Changelog.md index 8735c31ee..d867286af 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,6 +12,7 @@ * Indicate proper way to report bugs in the sidebar [#7039](https://github.com/diaspora/diaspora/pull/7039) * Remove text color from notification mails and fix sender avatar [#7054](https://github.com/diaspora/diaspora/pull/7054) * Make the session cookies HttpOnly again [#7041](https://github.com/diaspora/diaspora/pull/7041) +* Invalidate sessions with invalid CSRF tokens [#7050](https://github.com/diaspora/diaspora/pull/7050) ## Bug fixes * Post comments no longer get collapsed when interacting with a post [#7040](https://github.com/diaspora/diaspora/pull/7040) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ebd5b0343..bc79acdb2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -5,7 +5,17 @@ class ApplicationController < ActionController::Base before_action :force_tablet_html has_mobile_fu - protect_from_forgery except: :receive + protect_from_forgery except: :receive, with: :exception + + rescue_from ActionController::InvalidAuthenticityToken do + if user_signed_in? + logger.warn "#{current_user.diaspora_handle} CSRF token fail. referer: #{request.referer || 'empty'}" + Workers::Mail::CsrfTokenFail.perform_async(current_user.id) + sign_out current_user + end + flash[:error] = I18n.t("error_messages.csrf_token_fail") + redirect_to new_user_session_path format: request[:format] + end before_action :ensure_http_referer_is_set before_action :set_locale diff --git a/app/mailers/notification_mailers/csrf_token_fail.rb b/app/mailers/notification_mailers/csrf_token_fail.rb new file mode 100644 index 000000000..435f9d87b --- /dev/null +++ b/app/mailers/notification_mailers/csrf_token_fail.rb @@ -0,0 +1,7 @@ +module NotificationMailers + class CsrfTokenFail < NotificationMailers::Base + def set_headers + @headers[:subject] = I18n.t("notifier.csrf_token_fail.subject", name: @recipient.name) + end + end +end diff --git a/app/mailers/notifier.rb b/app/mailers/notifier.rb index e92795a18..80630ebe2 100644 --- a/app/mailers/notifier.rb +++ b/app/mailers/notifier.rb @@ -87,6 +87,10 @@ class Notifier < ActionMailer::Base send_notification(:confirm_email, recipient_id) end + def csrf_token_fail(recipient_id) + send_notification(:csrf_token_fail, recipient_id) + end + private def send_notification(type, *args) @notification = NotificationMailers.const_get(type.to_s.camelize).new(*args) diff --git a/app/views/notifier/csrf_token_fail.markerb b/app/views/notifier/csrf_token_fail.markerb new file mode 100644 index 000000000..22f3164ea --- /dev/null +++ b/app/views/notifier/csrf_token_fail.markerb @@ -0,0 +1 @@ +<%= t("notifier.csrf_token_fail.body", name: @notification.recipient_first_name, link: "https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)") %> diff --git a/app/workers/mail/csrf_token_fail.rb b/app/workers/mail/csrf_token_fail.rb new file mode 100644 index 000000000..a3372b322 --- /dev/null +++ b/app/workers/mail/csrf_token_fail.rb @@ -0,0 +1,11 @@ +module Workers + module Mail + class CsrfTokenFail < Base + sidekiq_options queue: :low + + def perform(user_id) + Notifier.csrf_token_fail(user_id).deliver_now + end + end + end +end diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 8d952a298..62d697149 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -75,6 +75,7 @@ en: helper: correct_the_following_errors_and_try_again: "Correct the following errors and try again." need_javascript: "This website requires JavaScript to function properly. If you disabled JavaScript, please enable it and refresh this page." + csrf_token_fail: "The CSRF token is invalid. Please sign in and try again." admins: admin_bar: @@ -695,6 +696,18 @@ en: confirm_email: subject: "Please activate your new email address %{unconfirmed_email}" click_link: "To activate your new email address %{unconfirmed_email}, please follow this link:" + csrf_token_fail: + subject: "We received an unauthorized request from your account, %{name}" + body: |- + Hello %{name}, + + We received a request with a wrong/missing CSRF token from your account. To prevent any possible damage you have been logged out. + + For more information on CSRF see [%{link}](%{link}). + + Sorry, + + The diaspora* email robot! report_email: type: post: "post" diff --git a/spec/integration/application_spec.rb b/spec/integration/application_spec.rb new file mode 100644 index 000000000..7f5527896 --- /dev/null +++ b/spec/integration/application_spec.rb @@ -0,0 +1,54 @@ +require "spec_helper" + +describe ApplicationController, type: :request do + describe "csrf token validation" do + context "without a current user" do + before do + @user = alice + @user.password = "evankorth" + @user.password_confirmation = "evankorth" + @user.save + end + + it "redirects to the new session page on validation fails" do + expect_any_instance_of(SessionsController).to receive(:verified_request?).and_return(false) + post "/users/sign_in", user: {remember_me: 0, username: @user.username, password: "evankorth"} + expect(response).to redirect_to new_user_session_path + expect(flash[:error]).to eq(I18n.t("error_messages.csrf_token_fail")) + end + + it "doesn't redirect to the new session page if the validation succeeded" do + expect_any_instance_of(SessionsController).to receive(:verified_request?).and_return(true) + post "/users/sign_in", user: {remember_me: 0, username: @user.username, password: "evankorth"} + expect(response).to redirect_to stream_path + expect(flash[:error]).to be_blank + end + end + + context "with a current user" do + before do + sign_in alice + end + + it "signs out users if a wrong token was given" do + expect_any_instance_of(UsersController).to receive(:verified_request?).and_return(false) + put edit_user_path, user: {language: "en"} + expect(response).to redirect_to new_user_session_path + expect(flash[:error]).to eq(I18n.t("error_messages.csrf_token_fail")) + end + + it "sends an email to the current user if the token validation failed" do + expect_any_instance_of(UsersController).to receive(:verified_request?).and_return(false) + expect(Workers::Mail::CsrfTokenFail).to receive(:perform_async).with(alice.id) + put edit_user_path, user: {language: "en"} + end + + it "doesn't sign out users if the token was correct" do + expect_any_instance_of(UsersController).to receive(:verified_request?).and_return(true) + put edit_user_path, user: {language: "en"} + expect(response).not_to be_redirect + expect(flash[:error]).to be_blank + end + end + end +end diff --git a/spec/mailers/notifier_spec.rb b/spec/mailers/notifier_spec.rb index 61595a7eb..3669fddd9 100644 --- a/spec/mailers/notifier_spec.rb +++ b/spec/mailers/notifier_spec.rb @@ -432,6 +432,26 @@ describe Notifier, type: :mailer do end end + describe ".csrf_token_fail" do + let(:email) { Notifier.csrf_token_fail(alice.id) } + + it "goes to the right person" do + expect(email.to).to eq([alice.email]) + end + + it "has the correct subject" do + expect(email.subject).to eq(I18n.translate("notifier.csrf_token_fail.subject", name: alice.name)) + end + + it "has the receivers name in the body" do + expect(email.body.encoded).to include(alice.person.profile.first_name) + end + + it "has some informative text in the body" do + expect(email.body.encoded).to include("https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)") + end + end + describe "hashtags" do it "escapes hashtags" do mails = Notifier.admin("#Welcome to bureaucracy!", [bob]) diff --git a/spec/workers/mail/csrf_token_fail_spec.rb b/spec/workers/mail/csrf_token_fail_spec.rb new file mode 100644 index 000000000..321b06dc4 --- /dev/null +++ b/spec/workers/mail/csrf_token_fail_spec.rb @@ -0,0 +1,18 @@ +# Copyright (c) 2010-2011, Diaspora Inc. This file is +# licensed under the Affero General Public License version 3 or later. See +# the COPYRIGHT file. + +require "spec_helper" + +describe Workers::Mail::CsrfTokenFail do + describe "#perfom" do + it "should call .deliver on the notifier object" do + user = alice + mail_double = double + expect(mail_double).to receive(:deliver_now) + expect(Notifier).to receive(:csrf_token_fail).with(user.id).and_return(mail_double) + + Workers::Mail::CsrfTokenFail.new.perform(user.id) + end + end +end