diff --git a/app/services/zxcvbn_service.rb b/app/services/zxcvbn_service.rb index 0b11d4135..a6097ccc0 100644 --- a/app/services/zxcvbn_service.rb +++ b/app/services/zxcvbn_service.rb @@ -1,4 +1,32 @@ class ZxcvbnService + @tester_mutex = Mutex.new + + class << self + # Returns an Zxcvbn instance cached between classes instances and between threads. + # + # The tester weights ~20 Mo, and we'd like to save some memory – so rather + # that storing it in a per-thread accessor, we prefer to use a mutex + # to cache it between threads. + def tester + @tester_mutex.synchronize do + @tester ||= build_tester + end + end + + private + + # Returns a fully initializer tester from the on-disk dictionary. + # + # This is slow: loading and parsing the dictionary may take around 1s. + def build_tester + dictionaries = YAML.safe_load(File.read(Rails.root.join("config", "initializers", "zxcvbn_dictionnaries.yaml"))) + + tester = Zxcvbn::Tester.new + tester.add_word_lists(dictionaries) + tester + end + end + def initialize(password) @password = password end @@ -18,6 +46,6 @@ class ZxcvbnService private def compute_zxcvbn - Zxcvbn.test(@password, [], ZXCVBN_DICTIONNARIES) + self.class.tester.test(@password) end end diff --git a/config/initializers/zxcvbn.rb b/config/initializers/zxcvbn.rb deleted file mode 100644 index 416e80192..000000000 --- a/config/initializers/zxcvbn.rb +++ /dev/null @@ -1 +0,0 @@ -ZXCVBN_DICTIONNARIES = YAML.safe_load(File.read(Rails.root.join("config", "initializers", "zxcvbn_dictionnaries.yaml"))) diff --git a/spec/services/zxcvbn_service_spec.rb b/spec/services/zxcvbn_service_spec.rb new file mode 100644 index 000000000..337da8c35 --- /dev/null +++ b/spec/services/zxcvbn_service_spec.rb @@ -0,0 +1,47 @@ +describe ZxcvbnService do + let(:password) { 'medium-strength-password' } + subject(:service) { ZxcvbnService.new(password) } + + describe '#score' do + it 'returns the password complexity score' do + expect(service.score).to eq 3 + end + end + + describe '#complexity' do + it 'returns the password score, vulnerability and length' do + expect(service.complexity).to eq [3, 'medium, strength, password', 24] + end + end + + describe 'caching' do + it 'lazily caches the tester between calls and instances' do + allow(Zxcvbn::Tester).to receive(:new).and_call_original + allow(YAML).to receive(:safe_load).and_call_original + + first_service = ZxcvbnService.new('some-password') + first_service.score + first_service.complexity + other_service = ZxcvbnService.new('other-password') + other_service.score + other_service.complexity + + expect(Zxcvbn::Tester).to have_received(:new).at_most(:once) + expect(YAML).to have_received(:safe_load).at_most(:once) + end + + it 'lazily caches the tester between threads' do + allow(Zxcvbn::Tester).to receive(:new).and_call_original + + threads = 1.upto(4).map do + Thread.new do + ZxcvbnService.new(password).score + end + end.map(&:join) + + scores = threads.map(&:value) + expect(scores).to eq([3, 3, 3, 3]) + expect(Zxcvbn::Tester).to have_received(:new).at_most(:once) + end + end +end