Skip to content

Commit

Permalink
Remove legacy session identifier support (#3866)
Browse files Browse the repository at this point in the history
* remove legacy session identifier support

* remove redundant test

* redirect to login to support any invalid session identifiers

* be more specific with caught errors
  • Loading branch information
Omer Lachish authored and arikfr committed Jun 3, 2019
1 parent e433efe commit 05f6ef0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 30 deletions.
26 changes: 3 additions & 23 deletions redash/authentication/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,34 +43,14 @@ def sign(key, path, expires):
def load_user(user_id_with_identity):
org = current_org._get_current_object()

'''
Users who logged in prior to https://github.com/getredash/redash/pull/3174 going live are going
to have their (integer) user_id as their session user identifier.
These session user identifiers will be updated the first time they visit any page so we add special
logic to allow a frictionless transition.
This logic will be removed 2-4 weeks after going live, and users who haven't
visited any page during that time will simply have to log in again.
'''

is_legacy_session_identifier = str(user_id_with_identity).find('-') < 0

if is_legacy_session_identifier:
user_id = user_id_with_identity
else:
user_id, _ = user_id_with_identity.split("-")

try:
user_id, _ = user_id_with_identity.split("-")
user = models.User.get_by_id_and_org(user_id, org)
if user.is_disabled:
return None

if is_legacy_session_identifier:
login_user(user, remember=True)
elif user.get_id() != user_id_with_identity:
if user.is_disabled or user.get_id() != user_id_with_identity:
return None

return user
except models.NoResultFound:
except (models.NoResultFound, ValueError, AttributeError):
return None


Expand Down
14 changes: 7 additions & 7 deletions tests/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ def test_responds_with_success_for_signed_in_user(self):

self.assertEquals(200, rv.status_code)

def test_responds_with_success_for_signed_in_user_with_a_legacy_identity_session(self):
def test_redirects_for_nonsigned_in_user(self):
rv = self.client.get("/default/")
self.assertEquals(302, rv.status_code)

def test_redirects_for_invalid_session_identifier(self):
with self.client as c:
with c.session_transaction() as sess:
sess['user_id'] = str(self.factory.user.id)
sess['user_id'] = 100
rv = self.client.get("/default/")

self.assertEquals(200, rv.status_code)

def test_redirects_for_nonsigned_in_user(self):
rv = self.client.get("/default/")
self.assertEquals(302, rv.status_code)
self.assertEquals(302, rv.status_code)


class PingTest(BaseTestCase):
Expand Down

0 comments on commit 05f6ef0

Please sign in to comment.