summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAzul <azul@riseup.net>2017-07-28 11:35:14 +0200
committerAzul <azul@riseup.net>2017-08-07 10:30:16 +0200
commit0a161e88143d28349c49805ea7cc83c5106e075a (patch)
treed6442dbd54d2cf6224714ce2b96643f548148f09
parent38ce3a14652aca9b3b8d8ad42f9968cfbcc44478 (diff)
prevent token conflicts
-rw-r--r--app/models/token.rb51
-rw-r--r--public/favicon.icobin0 -> 1014 bytes
-rw-r--r--test/unit/token_test.rb27
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
Binary files 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