FIX: log proper error message when SSO nonce verification fails (#14077)

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

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)

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
 

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