diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index 58574161954..1f099228bb3 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -156,7 +156,7 @@ class SessionController < ApplicationController if !sso.nonce_valid? if SiteSetting.verbose_discourse_connect_logging - Rails.logger.warn("Verbose SSO log: Nonce has already expired\n\n#{sso.diagnostics}") + Rails.logger.warn("Verbose SSO log: #{sso.nonce_error}\n\n#{sso.diagnostics}") end return render_sso_error(text: I18n.t("discourse_connect.timeout_expired"), status: 419) end diff --git a/app/models/discourse_single_sign_on.rb b/app/models/discourse_single_sign_on.rb index 7be3b50537e..b30286f2e0b 100644 --- a/app/models/discourse_single_sign_on.rb +++ b/app/models/discourse_single_sign_on.rb @@ -47,6 +47,14 @@ class DiscourseSingleSignOn < SingleSignOn end end + def nonce_error + if Discourse.cache.read(used_nonce_key).present? + "Nonce has already been used" + else + "Nonce has expired" + end + end + def return_path if SiteSetting.discourse_connect_csrf_protection @secure_session[nonce_key] || "/" @@ -62,6 +70,8 @@ class DiscourseSingleSignOn < SingleSignOn else Discourse.cache.delete nonce_key end + + Discourse.cache.write(used_nonce_key, return_path, expires_in: SingleSignOn.used_nonce_expiry_time) end end @@ -69,6 +79,10 @@ class DiscourseSingleSignOn < SingleSignOn "SSO_NONCE_#{nonce}" end + def used_nonce_key + "USED_SSO_NONCE_#{nonce}" + end + BANNED_EXTERNAL_IDS = %w{none nil blank null} def lookup_or_create_user(ip_address = nil) diff --git a/lib/single_sign_on.rb b/lib/single_sign_on.rb index a4c0bab2e75..3cdfff08271 100644 --- a/lib/single_sign_on.rb +++ b/lib/single_sign_on.rb @@ -50,6 +50,10 @@ class SingleSignOn @nonce_expiry_time = v end + def self.used_nonce_expiry_time + 24.hours + end + attr_accessor(*ACCESSORS) attr_writer :sso_secret, :sso_url diff --git a/spec/models/discourse_single_sign_on_spec.rb b/spec/models/discourse_single_sign_on_spec.rb index e1d03c12e73..c9095653826 100644 --- a/spec/models/discourse_single_sign_on_spec.rb +++ b/spec/models/discourse_single_sign_on_spec.rb @@ -406,6 +406,28 @@ describe DiscourseSingleSignOn do expect(sso.nonce).to_not be_nil end + context 'nonce error' do + it "generates correct error message when nonce has already been used" do + _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + + sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + expect(sso.nonce_valid?).to eq true + + sso.expire_nonce! + expect(sso.nonce_error).to eq("Nonce has already been used") + end + + it "generates correct error message when nonce is expired" do + _ , payload = DiscourseSingleSignOn.generate_url(secure_session: secure_session).split("?") + + sso = DiscourseSingleSignOn.parse(payload, secure_session: secure_session) + expect(sso.nonce_valid?).to eq true + + Discourse.cache.delete(sso.used_nonce_key) + expect(sso.nonce_error).to eq("Nonce has expired") + end + end + context 'user locale' do it 'sets default user locale if specified' do SiteSetting.allow_user_locale = true