From a47e0a3fda9dfabd35174c6465f87150847c889c Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 20 Feb 2020 16:35:30 -0500 Subject: [PATCH] FIX: TOTP could not be used on sites with colons in their names This is because the TOTP gem identifies as a colon as an addressable protocol. The solution for now is to remove the colon in the issuer name. Changing the issuer changes the token values, but now it was completely broken for colons so this should not be breaking anyone new. --- app/models/concerns/second_factor_manager.rb | 2 +- .../components/concern/second_factor_manager_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/second_factor_manager.rb b/app/models/concerns/second_factor_manager.rb index 3db84af886f..8d2429026eb 100644 --- a/app/models/concerns/second_factor_manager.rb +++ b/app/models/concerns/second_factor_manager.rb @@ -20,7 +20,7 @@ module SecondFactorManager def get_totp_object(data) require_rotp - ROTP::TOTP.new(data, issuer: SiteSetting.title) + ROTP::TOTP.new(data, issuer: SiteSetting.title.gsub(":", "")) end def totp_provisioning_uri(data) diff --git a/spec/components/concern/second_factor_manager_spec.rb b/spec/components/concern/second_factor_manager_spec.rb index 36e4f066ce8..5a049e2c41b 100644 --- a/spec/components/concern/second_factor_manager_spec.rb +++ b/spec/components/concern/second_factor_manager_spec.rb @@ -47,6 +47,18 @@ RSpec.describe SecondFactorManager do "otpauth://totp/#{SiteSetting.title}:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=#{SiteSetting.title}" ) end + it 'should handle a colon in the site title' do + SiteSetting.title = 'Spaceballs: The Discourse' + expect(user.user_second_factors.totps.first.totp_provisioning_uri).to eq( + "otpauth://totp/Spaceballs%20The%20Discourse:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=Spaceballs+The+Discourse" + ) + end + it 'should handle a two words before a colon in the title' do + SiteSetting.title = 'Our Spaceballs: The Discourse' + expect(user.user_second_factors.totps.first.totp_provisioning_uri).to eq( + "otpauth://totp/Our%20Spaceballs%20The%20Discourse:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=Our+Spaceballs+The+Discourse" + ) + end end describe '#authenticate_totp' do