Browse Source

Do not log device changes if geo ip service is out of order.

Martin Edenhofer 9 years ago
parent
commit
61f443dc89
2 changed files with 45 additions and 15 deletions
  1. 17 13
      app/models/user_device.rb
  2. 28 2
      test/unit/user_device_test.rb

+ 17 - 13
app/models/user_device.rb

@@ -29,7 +29,7 @@ store new device for user if device not already known
     # get location info
     location_details = Service::GeoIp.location(ip)
     location = 'unknown'
-    if location_details
+    if location_details && location_details['country_name']
       location = location_details['country_name']
     end
 
@@ -154,19 +154,23 @@ log user device action
     if user_device.ip != ip
       user_device.ip = ip
       location_details = Service::GeoIp.location(ip)
-      user_device.location_details = location_details
 
-      location = location_details['country_name']
-
-      # notify if country has changed
-      if user_device.location != location
-        return UserDevice.add(
-          user_agent,
-          ip,
-          user_id,
-          user_device.fingerprint,
-          type,
-        )
+      # if we do not have any data from backend (e. g. geo ip ist out of service), ignore log
+      if location_details && location_details['country_name']
+
+        user_device.location_details = location_details
+        location = location_details['country_name']
+
+        # notify if country has changed
+        if user_device.location != location
+          return UserDevice.add(
+            user_agent,
+            ip,
+            user_id,
+            user_device.fingerprint,
+            type,
+          )
+        end
       end
     end
 

+ 28 - 2
test/unit/user_device_test.rb

@@ -5,7 +5,7 @@ class UserDeviceTest < ActiveSupport::TestCase
 
     # create agent
     groups = Group.all
-    roles = Role.where( name: 'Agent' )
+    roles = Role.where(name: 'Agent')
 
     UserInfo.current_user_id = 1
 
@@ -207,6 +207,8 @@ class UserDeviceTest < ActiveSupport::TestCase
       'fingerprint1234',
       'session',
     )
+
+    # action with same fingerprint -> same device
     user_device1_1 = UserDevice.action(
       user_device1.id,
       'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
@@ -215,6 +217,8 @@ class UserDeviceTest < ActiveSupport::TestCase
       'session',
     )
     assert_equal(user_device1.id, user_device1_1.id)
+
+    # signin with same fingerprint -> same device
     user_device1_2 = UserDevice.add(
       'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
       '91.115.248.231',
@@ -223,6 +227,8 @@ class UserDeviceTest < ActiveSupport::TestCase
       'session',
     )
     assert_equal(user_device1.id, user_device1_2.id)
+
+    # action with different fingerprint -> new device
     user_device1_3 = UserDevice.add(
       'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
       '91.115.248.231',
@@ -232,7 +238,17 @@ class UserDeviceTest < ActiveSupport::TestCase
     )
     assert_not_equal(user_device1.id, user_device1_3.id)
 
-    # log with fingerprint A from country B via session -> new device #2
+    # signin with without accessable location -> new device
+    user_device1_4 = UserDevice.add(
+      'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
+      'not_existing_ip',
+      @agent.id,
+      'fingerprintABC',
+      'session',
+    )
+    assert_not_equal(user_device1.id, user_device1_4.id)
+
+    # action with fingerprint A from country B via session -> new device #2
     user_device2 = UserDevice.action(
       user_device1.id,
       'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
@@ -242,6 +258,16 @@ class UserDeviceTest < ActiveSupport::TestCase
     )
     assert_not_equal(user_device1.id, user_device2.id)
 
+    # action with fingerprint A without accessable location -> use current device #2
+    user_device3 = UserDevice.action(
+      user_device2.id,
+      'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.107 Safari/537.36',
+      'not_existing_ip',
+      @agent.id,
+      'session',
+    )
+    assert_equal(user_device2.id, user_device3.id)
+
   end
 
 end