Merge branch 'fix/token-conflict' into 'master'

prevent token conflicts

Closes #8792

See merge request !42

3 files changed, 64 insertions, 14 deletions

diff --git a/app/models/token.rb b/app/models/token.rb
index 8ac32b8..6d9f0fe 100644
--- a/app/models/token.rb
+++ b/app/models/token.rb
@@ -35,8 +35,13 @@ class Token < CouchRest::Model::Base
     by_last_seen_at.endkey(expires_after.minutes.ago)
   end
 
+  def self.to_cleanup
+    return [] unless expires_after
+    by_last_seen_at.endkey((expires_after + 5).minutes.ago)
+  end
+
   def self.destroy_all_expired
-    self.expired.each do |token|
+    self.to_cleanup.each do |token|
       token.destroy
     end
   end
@@ -46,27 +51,29 @@ class Token < CouchRest::Model::Base
   end
 
   def authenticate
-    if expired?
-      destroy
-      return nil
-    else
-      touch
-      return user
-    end
+    return if expired?
+    touch
+    return user
+  rescue CouchRest::NotFound
+    # Reload in touch failed - token has been deleted.
+    # That's either an active logout or account destruction.
+    # We don't accept the token anymore.
   end
 
   # Tokens can be cleaned up in different ways.
   # So let's make sure we don't crash if they disappeared
   def destroy_with_rescue
     destroy_without_rescue
-  rescue CouchRest::NotFound
   rescue CouchRest::Conflict # do nothing - it's been updated - #7670
+    try_to_reload && retry
+  rescue CouchRest::NotFound
   end
   alias_method_chain :destroy, :rescue
 
   def touch
-    self.last_seen_at = Time.now
-    save
+    update_attributes last_seen_at: Time.now
+  rescue CouchRest::Conflict
+    reload && retry
   end
 
   def expired?
@@ -82,5 +89,25 @@ class Token < CouchRest::Model::Base
       self.last_seen_at = Time.now
     end
   end
-end
 
+  # UPGRADE: the underlying code here changes between CouchRest::Model
+  # 2.1.0.rc1 and 2.2.0.beta2
+  # Hopefully we'll also get a pr merged that pushes this workaround
+  # upstream:
+  # https://github.com/couchrest/couchrest_model/pull/223
+  def reload
+    prepare_all_attributes(
+      database.get!(id), :directly_set_attributes => true
+    )
+    self
+  end
+
+  protected
+
+  def try_to_reload
+    reload
+  rescue CouchRest::NotFound
+    return false
+  end
+
+end
diff --git a/public/favicon.ico b/public/favicon.ico
index e69de29..61e3374 100644
--- a/public/favicon.ico
+++ b/public/favicon.ico
diff --git a/test/unit/token_test.rb b/test/unit/token_test.rb
index 51c8d8e..ce7edaa 100644
--- a/test/unit/token_test.rb
+++ b/test/unit/token_test.rb
@@ -59,7 +59,6 @@ class TokenTest < ActiveSupport::TestCase
     sample = Token.new(user_id: @user.id)
     sample.last_seen_at = 2.hours.ago
     with_config auth: {token_expires_after: 60} do
-      sample.expects(:destroy)
       assert_nil sample.authenticate
     end
   end
@@ -83,7 +82,6 @@ class TokenTest < ActiveSupport::TestCase
     fresh.destroy
   end
 
-
   test "Token.destroy_all_expired does not interfere with expired.authenticate" do
     expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
     with_config auth: {token_expires_after: 60} do
@@ -92,4 +90,29 @@ class TokenTest < ActiveSupport::TestCase
     assert_nil expired.authenticate
   end
 
+  test "active logout (destroy) prevents reuse" do
+    token = FactoryGirl.create :token
+    same = Token.find(token.id)
+    token.destroy
+    assert_raises CouchRest::NotFound do
+      same.touch
+    end
+  end
+
+  test "logout works on prolonged token" do
+    token = FactoryGirl.create :token
+    same = Token.find(token.id)
+    token.touch
+    same.destroy
+    assert_nil Token.find(same.id)
+  end
+
+  test 'second destroy carries on' do
+    token = FactoryGirl.create :token
+    same = Token.find(token.id)
+    token.destroy
+    same.destroy
+    assert_nil Token.find(same.id)
+  end
+
 end