Started POST "/u/create_second_factor_totp.json" for 209.249.35.130 at 2020-02-19 14:21:20 +0000
Processing by UsersController#create_second_factor_totp as JSON
Completed 500 Internal Server Error in 5ms (ActiveRecord: 0.0ms | Allocations: 1781)
Addressable::URI::InvalidURIError (Invalid scheme format: Our Great Forum)
/var/www/discourse/vendor/bundle/ruby/2.6.0/gems/addressable-2.7.0/lib/addressable/uri.rb:901:in `scheme='
Rendering text template
Rendered text template (Duration: 0.0ms | Allocations: 1)
“Our Great Forum” is almost the forum title in system settings. The actual value is something like
“Our Great Forum: Our fine tagline”, so what’s in the log is just the part before the colon. I don’t see “Out Great Forum” anywhere else in system settings, so I’m confused about why it’d be there without the : tagline part in the log.
Sigh. This is 2.4.0.beta10 (6301477b4c). I’m going to upgrade and get them to try again. I’ll edit this if I can get the user to try again on latest.
That’s all I see in the logs. I don’t see a backtrace. I get the error just clicking the “new authenticator” in the “token based authenticators” section, though.
diff --git a/spec/components/concern/second_factor_manager_spec.rb b/spec/components/concern/second_factor_manager_spec.rb
index 36e4f066ce..a657a0fdc8 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/#{URI::escape SiteSetting.title}:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=#{CGI::escape SiteSetting.title}"
+ )
+ end
+ it 'should handle a space before a colon in the site title' do
+ SiteSetting.title = 'Our Spaceballs: The Discourse'
+ expect(user.user_second_factors.totps.first.totp_provisioning_uri).to eq(
+ "otpauth://totp/#{URI::escape SiteSetting.title}:#{user.email}?secret=#{user_second_factor_totp.data}&issuer=#{CGI::escape SiteSetting.title}"
+ )
+ end
end
describe '#authenticate_totp' do
→ RAILS_ENV=test rspec ./spec/components/concern/second_factor_manager_spec.rb
Randomized with seed 47333
...................F......................
Failures:
1) SecondFactorManager#totp_provisioning_uri should handle a space before a colon in the site title
Failure/Error: totp_object.provisioning_uri(user.email)
Addressable::URI::InvalidURIError:
Invalid scheme format: Our Spaceballs
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:901:in `scheme='
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:826:in `block in initialize'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:2392:in `defer_validation'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:823:in `initialize'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:138:in `new'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:138:in `parse'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/addressable-2.7.0/lib/addressable/uri.rb:607:in `encode'
# /home/michael/.rvm/gems/ruby-2.6.3/gems/rotp-5.1.0/lib/rotp/totp.rb:61:in `provisioning_uri'
# ./app/models/user_second_factor.rb:32:in `totp_provisioning_uri'
# ./spec/components/concern/second_factor_manager_spec.rb:58:in `block (3 levels) in <top (required)>'
Finished in 0.75471 seconds (files took 2.18 seconds to load)
42 examples, 1 failure
Failed examples:
rspec ./spec/components/concern/second_factor_manager_spec.rb:56 # SecondFactorManager#totp_provisioning_uri should handle a space before a colon in the site title
Randomized with seed 47333
Changing the issuer changes the token values, but now it was completely
broken for colons so this should not be breaking anyone new.
Note that it worked previously for titles with a colon such as Spaceballs: The Discourse (it worked fine for me and I was able to add it to my phone) but would fail when there were two words before the colon.
I haven’t gotten confirmation that it worked for the user that was compaining, but it worked for me.
What I don’t understand, though, is that I understand that
this has always been broken for title with : in it
if site title changes, authenticator gets broken, as it’s tied to the site title
SiteSettings.title hasn’t changed in a long time
UserSecondFactor.where(method: 1) returns a bunch of entries from last August
I don’t understand how those can all be true. Is it the case that changing the site title broke all of the existing authenticator UserSecondFactors? EDIT: No, it doesn’t. I just changed the title and the authenticator 2fa that I made yesterday still works.
Well, I’m pretty certain that the title had multiple words and a colon forever (no changes in logs since at least May 2019) and that it worked as recently as last August (because there are authenticator tokens generated then), but whatever. It seems to work now and that changing the title doesn’t break existing tokens, so this seems like a win.
