diff --git a/app/models/user_auth_token.rb b/app/models/user_auth_token.rb index cc484b6eb26..ffe26e07cd0 100644 --- a/app/models/user_auth_token.rb +++ b/app/models/user_auth_token.rb @@ -30,23 +30,23 @@ class UserAuthToken < ActiveRecord::Base end end - # Returns the login location as it will be used by the the system to detect - # suspicious login. - # - # This should not be very specific because small variations in location - # (i.e. changes of network, small trips, etc) will be detected as suspicious - # logins. - # - # On the other hand, if this is too broad it will not report any suspicious - # logins at all. - # - # For example, let's choose the country as the only component in login - # locations. In general, this should be a pretty good choce with the - # exception that for users from huge countries it might not be specific - # enoguh. For US users where the real user and the malicious one could - # happen to live both in USA, this will not detect any suspicious activity. + RAD_PER_DEG = Math::PI / 180 + EARTH_RADIUS_KM = 6371 # kilometers + def self.login_location(ip) - DiscourseIpInfo.get(ip)[:country] + ipinfo = DiscourseIpInfo.get(ip) + + ipinfo['latitude'] && ipinfo['longitude'] ? [ipinfo['latitude'], ipinfo['longitude']] : nil + end + + def self.distance(loc1, loc2) + lat1_rad, lon1_rad = loc1[0] * RAD_PER_DEG, loc1[1] * RAD_PER_DEG + lat2_rad, lon2_rad = loc2[0] * RAD_PER_DEG, loc2[1] * RAD_PER_DEG + + a = Math.sin((lat2_rad - lat1_rad) / 2)**2 + Math.cos(lat1_rad) * Math.cos(lat2_rad) * Math.sin((lon2_rad - lon1_rad) / 2)**2 + c = 2 * Math::atan2(Math::sqrt(a), Math::sqrt(1 - a)) + + c * EARTH_RADIUS_KM end def self.is_suspicious(user_id, user_ip) @@ -57,8 +57,13 @@ class UserAuthToken < ActiveRecord::Base ips.uniq! return false if ips.empty? # first login is never suspicious - user_location = login_location(user_ip) - ips.none? { |ip| user_location == login_location(ip) } + if user_location = login_location(user_ip) + ips.none? do |ip| + if location = login_location(ip) + distance(user_location, location) < SiteSetting.max_suspicious_distance_km + end + end + end end def self.generate!(user_id: , user_agent: nil, client_ip: nil, path: nil, staff: nil, impersonate: false) diff --git a/config/site_settings.yml b/config/site_settings.yml index 2c134a29282..aab7b5c0fe0 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -381,6 +381,9 @@ login: verbose_auth_token_logging: hidden: true default: true + max_suspicious_distance_km: + hidden: true + default: 500 sso_url: default: '' regex: '^https?:\/\/.+[^\/]$' diff --git a/spec/jobs/suspicious_login_spec.rb b/spec/jobs/suspicious_login_spec.rb index be4fdfaed78..06b2bd76250 100644 --- a/spec/jobs/suspicious_login_spec.rb +++ b/spec/jobs/suspicious_login_spec.rb @@ -4,10 +4,25 @@ describe Jobs::SuspiciousLogin do let(:user) { Fabricate(:moderator) } + let(:zurich) { [47.3686498, 8.5391825] } # Zurich, Switzerland + let(:bern) { [46.947922, 7.444608] } # Bern, Switzerland + let(:london) { [51.5073509, -0.1277583] } # London, United Kingdom + before do - UserAuthToken.stubs(:login_location).with("1.1.1.1").returns("Location 1") - UserAuthToken.stubs(:login_location).with("1.1.1.2").returns("Location 1") - UserAuthToken.stubs(:login_location).with("1.1.2.1").returns("Location 2") + UserAuthToken.stubs(:login_location).with("1.1.1.1").returns(zurich) + UserAuthToken.stubs(:login_location).with("1.1.1.2").returns(bern) + UserAuthToken.stubs(:login_location).with("1.1.2.1").returns(london) + end + + it "will correctly compute distance" do + def expect_distance(from, to, distance) + expect(UserAuthToken.distance(from, to).to_i).to eq(distance) + expect(UserAuthToken.distance(to, from).to_i).to eq(distance) + end + + expect_distance(zurich, bern, 95) + expect_distance(zurich, london, 776) + expect_distance(bern, london, 747) end it "will not send an email on first login" do