Skip to content

Commit

Permalink
Merge branch 'philnash-replace-custom-code-with-pwned'
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelbanfield committed Mar 25, 2018
2 parents c24c6c3 + 0709048 commit 37bda54
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 32 deletions.
12 changes: 7 additions & 5 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ PATH
specs:
devise-pwned_password (0.1.4)
devise (~> 4)
pwned (~> 1.2.1)

GEM
remote: https://rubygems.org/
Expand Down Expand Up @@ -61,7 +62,7 @@ GEM
activesupport (>= 4.2.0)
i18n (0.9.5)
concurrent-ruby (~> 1.0)
loofah (2.2.1)
loofah (2.2.2)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.0)
Expand All @@ -75,9 +76,10 @@ GEM
mini_portile2 (~> 2.3.0)
orm_adapter (0.5.0)
parallel (1.12.1)
parser (2.5.0.4)
parser (2.5.0.5)
ast (~> 2.4.0)
powerpack (0.1.1)
pwned (1.2.1)
rack (2.0.4)
rack-test (0.8.3)
rack (>= 1.0, < 3)
Expand All @@ -96,16 +98,16 @@ GEM
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
nokogiri (>= 1.6)
rails-html-sanitizer (1.0.3)
loofah (~> 2.0)
rails-html-sanitizer (1.0.4)
loofah (~> 2.2, >= 2.2.2)
railties (5.1.5)
actionpack (= 5.1.5)
activesupport (= 5.1.5)
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (3.0.0)
rake (12.3.0)
rake (12.3.1)
responders (2.4.0)
actionpack (>= 4.2.0, < 5.3)
railties (>= 4.2.0, < 5.3)
Expand Down
1 change: 1 addition & 0 deletions devise-pwned_password.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Gem::Specification.new do |s|
s.files = Dir["{app,config,db,lib}/**/*", "MIT-LICENSE", "Rakefile", "README.md"]

s.add_dependency "devise", "~> 4"
s.add_dependency "pwned", "~> 1.2.1"

s.add_development_dependency "rails", "~> 5.1.2"
s.add_development_dependency "sqlite3"
Expand Down
37 changes: 10 additions & 27 deletions lib/devise/pwned_password/model.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require "net/http"
require "pwned"
require "devise/pwned_password/hooks/pwned_password"

module Devise
Expand Down Expand Up @@ -31,22 +31,16 @@ def pwned?
# Implement retry behaviour described here https://haveibeenpwned.com/API/v2#RateLimiting
def password_pwned?(password)
@pwned = false
hash = Digest::SHA1.hexdigest(password.to_s).upcase
prefix, suffix = hash.slice!(0..4), hash

userAgent = "devise_pwned_password"

uri = URI.parse("https://api.pwnedpasswords.com/range/#{prefix}")

options = {
"User-Agent" => "devise_pwned_password",
read_timeout: self.class.pwned_password_read_timeout,
open_timeout: self.class.pwned_password_open_timeout
}
pwned_password = Pwned::Password.new(password.to_s, options)
begin
Net::HTTP.start(uri.host, uri.port, use_ssl: true, open_timeout: self.class.pwned_password_open_timeout, read_timeout: self.class.pwned_password_read_timeout) do |http|
request = Net::HTTP::Get.new(uri.request_uri, "User-Agent" => userAgent)
response = http.request request
return false unless response.is_a?(Net::HTTPSuccess)
@pwned = usage_count(response.read_body, suffix) >= self.class.min_password_matches
return @pwned
end
rescue StandardError
@pwned = pwned_password.pwned_count >= self.class.min_password_matches
return @pwned
rescue Pwned::Error
return false
end

Expand All @@ -55,17 +49,6 @@ def password_pwned?(password)

private

def usage_count(response, suffix)
count = 0
response.each_line do |line|
if line.start_with? suffix
count = line.strip.split(":").last.to_i
break
end
end
count
end

def not_pwned_password
# This deliberately fails silently on 500's etc. Most apps wont want to tie the ability to sign up customers to the availability of a third party API
if password_pwned?(password)
Expand Down

0 comments on commit 37bda54

Please sign in to comment.