From 0134e41286255111f2644a5b90e03640efef5db1 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 6 Mar 2018 16:49:31 +1100 Subject: [PATCH] FEATURE: detect when client thinks user is logged on but is not This cleans up an error condition where UI thinks a user is logged on but the user is not. If this happens user will be prompted to refresh. --- app/assets/javascripts/application.js | 1 + .../discourse/initializers/logout.js.es6 | 18 +++++++++++---- .../javascripts/discourse/lib/ajax.js.es6 | 23 +++++++++++++++++++ .../javascripts/discourse/lib/logout.js.es6 | 6 +++++ config/initializers/004-message_bus.rb | 17 +++++++++----- lib/auth/default_current_user_provider.rb | 6 ++++- lib/hijack.rb | 4 ++++ lib/middleware/request_tracker.rb | 5 ++++ spec/requests/session_controller_spec.rb | 13 +++++++++++ 9 files changed, 82 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index c6ca0b0c90c..f231c52bf8a 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -11,6 +11,7 @@ // Stuff we need to load first //= require ./discourse/lib/utilities //= require ./discourse/lib/page-visible +//= require ./discourse/lib/logout //= require ./discourse/lib/ajax //= require ./discourse/lib/text //= require ./discourse/lib/hash diff --git a/app/assets/javascripts/discourse/initializers/logout.js.es6 b/app/assets/javascripts/discourse/initializers/logout.js.es6 index dc0ff59c81d..db7d4de9608 100644 --- a/app/assets/javascripts/discourse/initializers/logout.js.es6 +++ b/app/assets/javascripts/discourse/initializers/logout.js.es6 @@ -1,5 +1,7 @@ import logout from 'discourse/lib/logout'; +let _showingLogout = false; + // Subscribe to "logout" change events via the Message Bus export default { name: "logout", @@ -7,14 +9,22 @@ export default { initialize: function (container) { const messageBus = container.lookup('message-bus:main'); - const siteSettings = container.lookup('site-settings:main'); - const keyValueStore = container.lookup('key-value-store:main'); if (!messageBus) { return; } - const callback = () => logout(siteSettings, keyValueStore); messageBus.subscribe("/logout", function () { - bootbox.dialog(I18n.t("logout"), {label: I18n.t("refresh"), callback}, {onEscape: callback, backdrop: 'static'}); + if (!_showingLogout) { + + _showingLogout = true; + + bootbox.dialog(I18n.t("logout"), { + label: I18n.t("refresh"), + callback: logout + }, { + onEscape: logout, + backdrop: 'static' + }); + } }); } }; diff --git a/app/assets/javascripts/discourse/lib/ajax.js.es6 b/app/assets/javascripts/discourse/lib/ajax.js.es6 index 98e65ed9c08..863d6eae7b1 100644 --- a/app/assets/javascripts/discourse/lib/ajax.js.es6 +++ b/app/assets/javascripts/discourse/lib/ajax.js.es6 @@ -1,7 +1,9 @@ import pageVisible from 'discourse/lib/page-visible'; +import logout from 'discourse/lib/logout'; let _trackView = false; let _transientHeader = null; +let _showingLogout = false; export function setTransientHeader(key, value) { _transientHeader = {key, value}; @@ -39,6 +41,10 @@ export function ajax() { args.headers = args.headers || {}; + if (Discourse.__container__.lookup('current-user:main')) { + args.headers['Discourse-Logged-In'] = "true"; + } + if (_transientHeader) { args.headers[_transientHeader.key] = _transientHeader.value; _transientHeader = null; @@ -54,7 +60,22 @@ export function ajax() { args.headers['Discourse-Visible'] = "true"; } + let handleLogoff = function(xhr) { + if (xhr.getResponseHeader('Discourse-Logged-Out') && !_showingLogout) { + _showingLogout = true; + bootbox.dialog( + I18n.t("logout"), {label: I18n.t("refresh"), callback: logout}, + { + onEscape: () => logout(), + backdrop: 'static' + } + ); + } + }; + args.success = (data, textStatus, xhr) => { + handleLogoff(xhr); + if (xhr.getResponseHeader('Discourse-Readonly')) { Ember.run(() => Discourse.Site.currentProp('isReadOnly', true)); } @@ -67,6 +88,8 @@ export function ajax() { }; args.error = (xhr, textStatus, errorThrown) => { + handleLogoff(xhr); + // note: for bad CSRF we don't loop an extra request right away. // this allows us to eliminate the possibility of having a loop. if (xhr.status === 403 && xhr.responseText === "[\"BAD CSRF\"]") { diff --git a/app/assets/javascripts/discourse/lib/logout.js.es6 b/app/assets/javascripts/discourse/lib/logout.js.es6 index 2fe4078a69f..51f397d186b 100644 --- a/app/assets/javascripts/discourse/lib/logout.js.es6 +++ b/app/assets/javascripts/discourse/lib/logout.js.es6 @@ -1,4 +1,10 @@ export default function logout(siteSettings, keyValueStore) { + if (!siteSettings || !keyValueStore) { + const container = Discourse.__container__; + siteSettings = siteSettings || container.lookup('site-settings:main'); + keyValueStore = keyValueStore || container.lookup('key-value-store:main'); + } + keyValueStore.abandonLocal(); const redirect = siteSettings.logout_redirect; diff --git a/config/initializers/004-message_bus.rb b/config/initializers/004-message_bus.rb index fabada48a02..d9adc7a8c13 100644 --- a/config/initializers/004-message_bus.rb +++ b/config/initializers/004-message_bus.rb @@ -22,13 +22,18 @@ def setup_message_bus_env(env) user.groups.pluck('groups.id') end + extra_headers = { + "Access-Control-Allow-Origin" => Discourse.base_url_no_prefix, + "Access-Control-Allow-Methods" => "GET, POST", + "Access-Control-Allow-Headers" => "X-SILENCE-LOGGER, X-Shared-Session-Key, Dont-Chunk, Discourse-Visible" + } + + if env[Auth::DefaultCurrentUserProvider::BAD_TOKEN] + extra_headers['Discourse-Logged-Out'] = '1' + end + hash = { - extra_headers: - { - "Access-Control-Allow-Origin" => Discourse.base_url_no_prefix, - "Access-Control-Allow-Methods" => "GET, POST", - "Access-Control-Allow-Headers" => "X-SILENCE-LOGGER, X-Shared-Session-Key, Dont-Chunk, Discourse-Visible" - }, + extra_headers: extra_headers, user_id: user_id, group_ids: group_ids, is_admin: is_admin, diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index cacfe81962d..06781518f39 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -14,6 +14,7 @@ class Auth::DefaultCurrentUserProvider TOKEN_COOKIE ||= "_t" PATH_INFO ||= "PATH_INFO" COOKIE_ATTEMPTS_PER_MIN ||= 10 + BAD_TOKEN ||= "_DISCOURSE_BAD_TOKEN" # do all current user initialization here def initialize(env) @@ -58,7 +59,8 @@ class Auth::DefaultCurrentUserProvider current_user = @user_token.try(:user) end - unless current_user + if !current_user + @env[BAD_TOKEN] = true begin limiter.performed! rescue RateLimiter::LimitExceeded @@ -69,6 +71,8 @@ class Auth::DefaultCurrentUserProvider ) end end + elsif @env['HTTP_DISCOURSE_LOGGED_IN'] + @env[BAD_TOKEN] = true end if current_user && should_update_last_seen? diff --git a/lib/hijack.rb b/lib/hijack.rb index 25a6acc7dbc..e0641182d11 100644 --- a/lib/hijack.rb +++ b/lib/hijack.rb @@ -78,6 +78,10 @@ module Hijack headers['Content-Length'] = body.bytesize headers['Connection'] = "close" + if env[Auth::DefaultCurrentUserProvider::BAD_TOKEN] + headers['Discourse-Logged-Out'] = '1' + end + status_string = Rack::Utils::HTTP_STATUS_CODES[response.status.to_i] || "Unknown" io.write "#{response.status} #{status_string}\r\n" diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb index 0ab468b482e..2ad01b05215 100644 --- a/lib/middleware/request_tracker.rb +++ b/lib/middleware/request_tracker.rb @@ -169,6 +169,11 @@ class Middleware::RequestTracker if info && (headers = result[1]) headers["X-Runtime"] = "%0.6f" % info[:total_duration] end + + if env[Auth::DefaultCurrentUserProvider::BAD_TOKEN] && (headers = result[1]) + headers['Discourse-Logged-Out'] = '1' + end + result ensure if (limiters = env['DISCOURSE_RATE_LIMITERS']) && env['DISCOURSE_IS_ASSET_PATH'] diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 5b003d39ffc..b315609fb77 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -182,4 +182,17 @@ RSpec.describe SessionController do end end end + + context 'logoff support' do + it 'can log off users cleanly' do + user = Fabricate(:user) + sign_in(user) + + UserAuthToken.destroy_all + + # we need a route that will call current user + post '/draft.json', params: {} + expect(response.headers['Discourse-Logged-Out']).to eq("1") + end + end end