Skip to content
Snippets Groups Projects
Commit 66b05aa9 authored by azul's avatar azul
Browse files

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

prevent token conflicts

Closes #8792

See merge request !42
parents 38ce3a14 0a161e88
No related branches found
No related tags found
1 merge request!42prevent token conflicts
Pipeline #
...@@ -35,8 +35,13 @@ class Token < CouchRest::Model::Base ...@@ -35,8 +35,13 @@ class Token < CouchRest::Model::Base
by_last_seen_at.endkey(expires_after.minutes.ago) by_last_seen_at.endkey(expires_after.minutes.ago)
end 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 def self.destroy_all_expired
self.expired.each do |token| self.to_cleanup.each do |token|
token.destroy token.destroy
end end
end end
...@@ -46,27 +51,29 @@ class Token < CouchRest::Model::Base ...@@ -46,27 +51,29 @@ class Token < CouchRest::Model::Base
end end
def authenticate def authenticate
if expired? return if expired?
destroy
return nil
else
touch touch
return user return user
end 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 end
# Tokens can be cleaned up in different ways. # Tokens can be cleaned up in different ways.
# So let's make sure we don't crash if they disappeared # So let's make sure we don't crash if they disappeared
def destroy_with_rescue def destroy_with_rescue
destroy_without_rescue destroy_without_rescue
rescue CouchRest::NotFound
rescue CouchRest::Conflict # do nothing - it's been updated - #7670 rescue CouchRest::Conflict # do nothing - it's been updated - #7670
try_to_reload && retry
rescue CouchRest::NotFound
end end
alias_method_chain :destroy, :rescue alias_method_chain :destroy, :rescue
def touch def touch
self.last_seen_at = Time.now update_attributes last_seen_at: Time.now
save rescue CouchRest::Conflict
reload && retry
end end
def expired? def expired?
...@@ -82,5 +89,25 @@ class Token < CouchRest::Model::Base ...@@ -82,5 +89,25 @@ class Token < CouchRest::Model::Base
self.last_seen_at = Time.now self.last_seen_at = Time.now
end 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 end
protected
def try_to_reload
reload
rescue CouchRest::NotFound
return false
end
end
No preview for this file type
...@@ -59,7 +59,6 @@ class TokenTest < ActiveSupport::TestCase ...@@ -59,7 +59,6 @@ class TokenTest < ActiveSupport::TestCase
sample = Token.new(user_id: @user.id) sample = Token.new(user_id: @user.id)
sample.last_seen_at = 2.hours.ago sample.last_seen_at = 2.hours.ago
with_config auth: {token_expires_after: 60} do with_config auth: {token_expires_after: 60} do
sample.expects(:destroy)
assert_nil sample.authenticate assert_nil sample.authenticate
end end
end end
...@@ -83,7 +82,6 @@ class TokenTest < ActiveSupport::TestCase ...@@ -83,7 +82,6 @@ class TokenTest < ActiveSupport::TestCase
fresh.destroy fresh.destroy
end end
test "Token.destroy_all_expired does not interfere with expired.authenticate" do test "Token.destroy_all_expired does not interfere with expired.authenticate" do
expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago expired = FactoryGirl.create :token, last_seen_at: 2.hours.ago
with_config auth: {token_expires_after: 60} do with_config auth: {token_expires_after: 60} do
...@@ -92,4 +90,29 @@ class TokenTest < ActiveSupport::TestCase ...@@ -92,4 +90,29 @@ class TokenTest < ActiveSupport::TestCase
assert_nil expired.authenticate assert_nil expired.authenticate
end 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 end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment