From 0a161e88143d28349c49805ea7cc83c5106e075a Mon Sep 17 00:00:00 2001 From: Azul Date: Fri, 28 Jul 2017 11:35:14 +0200 Subject: prevent token conflicts --- app/models/token.rb | 51 ++++++++++++++++++++++++++++++++++++------------ public/favicon.ico | Bin 0 -> 1014 bytes test/unit/token_test.rb | 27 +++++++++++++++++++++++-- 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 Binary files a/public/favicon.ico and b/public/favicon.ico differ 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 -- cgit v1.2.3