diff --git a/app/assets/javascripts/discourse/app/lib/logout.js b/app/assets/javascripts/discourse/app/lib/logout.js index d8aa425e8ec..e1d73befef1 100644 --- a/app/assets/javascripts/discourse/app/lib/logout.js +++ b/app/assets/javascripts/discourse/app/lib/logout.js @@ -1,30 +1,16 @@ import getURL from "discourse-common/lib/get-url"; import { isEmpty } from "@ember/utils"; -import { findAll } from "discourse/models/login-method"; import { helperContext } from "discourse-common/lib/helpers"; -export default function logout() { +export default function logout({ redirect } = {}) { const ctx = helperContext(); - let siteSettings = ctx.siteSettings; let keyValueStore = ctx.keyValueStore; keyValueStore.abandonLocal(); - const redirect = siteSettings.logout_redirect; if (!isEmpty(redirect)) { window.location.href = redirect; return; } - const sso = siteSettings.enable_sso; - const oneAuthenticator = - !siteSettings.enable_local_logins && findAll().length === 1; - - if (siteSettings.login_required && (sso || oneAuthenticator)) { - // In this situation visiting most URLs will start the auth process again - // Go to the `/login` page to avoid an immediate redirect - window.location.href = getURL("/login"); - return; - } - window.location.href = getURL("/"); } diff --git a/app/assets/javascripts/discourse/app/routes/application.js b/app/assets/javascripts/discourse/app/routes/application.js index fdbe5d2814a..8c8a21c80a0 100644 --- a/app/assets/javascripts/discourse/app/routes/application.js +++ b/app/assets/javascripts/discourse/app/routes/application.js @@ -278,7 +278,9 @@ const ApplicationRoute = DiscourseRoute.extend(OpenComposer, { _handleLogout() { if (this.currentUser) { - this.currentUser.destroySession().then(() => logout()); + this.currentUser + .destroySession() + .then((response) => logout({ redirect: response["redirect_url"] })); } }, }); diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 01bd6df4bf3..7bb72f9787f 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -442,12 +442,30 @@ class SessionController < ApplicationController end def destroy + redirect_url = params[:return_url].presence || SiteSetting.logout_redirect.presence + + sso = SiteSetting.enable_sso + only_one_authenticator = !SiteSetting.enable_local_logins && Discourse.enabled_authenticators.length == 1 + if sso || only_one_authenticator + # In this situation visiting most URLs will start the auth process again + # Go to the `/login` page to avoid an immediate redirect + redirect_url ||= path("/login") + end + + redirect_url ||= path("/") + + event_data = { redirect_url: redirect_url, user: current_user } + DiscourseEvent.trigger(:before_session_destroy, event_data) + redirect_url = event_data[:redirect_url] + reset_session log_off_user if request.xhr? - render body: nil + render json: { + redirect_url: redirect_url + } else - redirect_to (params[:return_url] || path("/")) + redirect_to redirect_url end end diff --git a/spec/requests/session_controller_spec.rb b/spec/requests/session_controller_spec.rb index 6ec3f4c73d9..710874b57fb 100644 --- a/spec/requests/session_controller_spec.rb +++ b/spec/requests/session_controller_spec.rb @@ -1751,6 +1751,44 @@ RSpec.describe SessionController do expect(session[:current_user_id]).to be_blank expect(response.cookies["_t"]).to be_blank end + + it 'returns the redirect URL in the body for XHR requests' do + user = sign_in(Fabricate(:user)) + delete "/session/#{user.username}.json", xhr: true + + expect(response.status).to eq(200) + expect(session[:current_user_id]).to be_blank + expect(response.cookies["_t"]).to be_blank + + expect(response.parsed_body["redirect_url"]).to eq("/") + end + + it 'redirects to /login for SSO' do + SiteSetting.sso_url = "https://example.com/sso" + SiteSetting.enable_sso = true + + user = sign_in(Fabricate(:user)) + delete "/session/#{user.username}.json", xhr: true + + expect(response.status).to eq(200) + expect(response.parsed_body["redirect_url"]).to eq("/login") + end + + it 'allows plugins to manipulate redirect URL' do + callback = -> (data) do + data[:redirect_url] = "/myredirect/#{data[:user].username}" + end + + DiscourseEvent.on(:before_session_destroy, &callback) + + user = sign_in(Fabricate(:user)) + delete "/session/#{user.username}.json", xhr: true + + expect(response.status).to eq(200) + expect(response.parsed_body["redirect_url"]).to eq("/myredirect/#{user.username}") + ensure + DiscourseEvent.off(:before_session_destroy, &callback) + end end describe '#one_time_password' do