FIX: attempt to handle ios edge case where token is seen but unsaved

This relaxes our security in the following way

- prev auth token is always accepted as long as rotation
date is within our window of SiteSetting.maximum_session_age.hours
(previously old token expired within a minute of new one being seen)

- new auth token is marked unseen if we are presented with an old token
after we already saw new one

This attempts to fix an issue where ios webkit is not committing new cookies
This commit is contained in:
Sam Saffron 2017-02-26 17:09:57 -05:00
parent fdf749770b
commit 7e8f0dc967
2 changed files with 51 additions and 11 deletions

View File

@ -46,18 +46,10 @@ class UserAuthToken < ActiveRecord::Base
user_token = find_by("(auth_token = :token OR
prev_auth_token = :token OR
(auth_token = :unhashed_token AND legacy)) AND created_at > :expire_before",
(auth_token = :unhashed_token AND legacy)) AND rotated_at > :expire_before",
token: token, unhashed_token: unhashed_token, expire_before: expire_before)
token_expired =
user_token &&
user_token.seen_at &&
user_token.auth_token_seen &&
user_token.prev_auth_token == token &&
user_token.prev_auth_token != user_token.auth_token &&
user_token.seen_at < 1.minute.ago
if token_expired || !user_token
if !user_token
if SiteSetting.verbose_auth_token_logging
UserAuthTokenLog.create(
@ -72,6 +64,23 @@ class UserAuthToken < ActiveRecord::Base
return nil
end
if user_token.prev_auth_token == token && user_token.auth_token_seen
changed_rows = UserAuthToken
.where(id: user_token.id, prev_auth_token: token)
.update_all(auth_token_seen: false)
# not updating AR model cause we want to give it one more req
# with wrong cookie
UserAuthTokenLog.create(
action: changed_rows == 0 ? "prev seen token unchanged" : "prev seen token",
user_auth_token_id: user_token.id,
user_id: user_token.user_id,
auth_token: user_token.auth_token,
user_agent: opts && opts[:user_agent],
client_ip: opts && opts[:client_ip]
)
end
if mark_seen && user_token && !user_token.auth_token_seen && user_token.auth_token == token
# we must protect against concurrency issues here
changed_rows = UserAuthToken

View File

@ -85,6 +85,34 @@ describe UserAuthToken do
expect(user_token.user_agent).to eq("some user agent 2")
end
it "expires correctly" do
user = Fabricate(:user)
user_token = UserAuthToken.generate!(user_id: user.id,
user_agent: "some user agent 2",
client_ip: "1.1.2.3")
UserAuthToken.lookup(user_token.unhashed_auth_token, seen: true)
freeze_time (SiteSetting.maximum_session_age.hours - 1).from_now
user_token.reload
user_token.rotate!
UserAuthToken.lookup(user_token.unhashed_auth_token, seen: true)
freeze_time (SiteSetting.maximum_session_age.hours - 1).from_now
still_good = UserAuthToken.lookup(user_token.unhashed_auth_token, seen: true)
expect(still_good).not_to eq(nil)
freeze_time 2.hours.from_now
not_good = UserAuthToken.lookup(user_token.unhashed_auth_token, seen: true)
expect(not_good).to eq(nil)
end
it "can properly rotate tokens" do
user = Fabricate(:user)
@ -127,7 +155,10 @@ describe UserAuthToken do
freeze_time(2.minute.from_now)
looked_up = UserAuthToken.lookup(unhashed_prev)
expect(looked_up).to eq(nil)
expect(looked_up).not_to eq(nil)
looked_up.reload
expect(looked_up.auth_token_seen).to eq(false)
rotated = user_token.rotate!(user_agent: "a new user agent", client_ip: "1.1.2.4")
expect(rotated).to eq(true)