From 33db89b9e48489b442ae361f17fefbe6b23d3573 Mon Sep 17 00:00:00 2001 From: David Beguin Date: Wed, 13 Nov 2019 14:40:32 +0100 Subject: [PATCH] [FIX] website: fix authentication of archived visitor If a visitor logs in (and has a visitor_id in his cookie), the partner will be linked to the visitor. If, after 1 week, the visitor tries to connect again with a different session (or another visitor_id in cookies), the authenticate will crash because * _cron_archive_visitors applies on visitor inactive since at least a week * there can be only one visitor per partner (sql constraint) * the visitor linked to the partner is not retrieved (because archived) and we try to link the partner to a new visitor. Further than that, if the visitor is archived and the linked partner wants to login again with a new visitor_id, we should * reactivate the previous visitor, * copy history from newest to previous one, * delete the newest one Note that last two points were already done before this commit. Task ID: 2120464 PR #40199 Fixes #40077 Fixes #40301 --- addons/website/models/res_users.py | 6 +- addons/website/tests/test_website_visitor.py | 75 +++++++++++++++----- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/addons/website/models/res_users.py b/addons/website/models/res_users.py index 39eaf5babd6d2..a3435c9130a35 100644 --- a/addons/website/models/res_users.py +++ b/addons/website/models/res_users.py @@ -72,10 +72,14 @@ def authenticate(cls, db, login, password, user_agent_env): visitor_sudo = env['website.visitor']._get_visitor_from_request() if visitor_sudo: partner = env.user.partner_id - partner_visitor = env['website.visitor'].sudo().search([('partner_id', '=', partner.id)]) + partner_visitor = env['website.visitor'].with_context(active_test=False).sudo().search([('partner_id', '=', partner.id)]) if partner_visitor and partner_visitor.id != visitor_sudo.id: + # Link history to older Visitor and delete the newest visitor_sudo.website_track_ids.write({'visitor_id': partner_visitor.id}) visitor_sudo.unlink() + # If archived (most likely by the cron for inactivity reasons), reactivate the partner's visitor + if not partner_visitor.active: + partner_visitor.write({'active': True}) else: vals = { 'partner_id': partner.id, diff --git a/addons/website/tests/test_website_visitor.py b/addons/website/tests/test_website_visitor.py index 427816a045239..ca46b40f5f119 100644 --- a/addons/website/tests/test_website_visitor.py +++ b/addons/website/tests/test_website_visitor.py @@ -1,13 +1,15 @@ # coding: utf-8 -from odoo.tests import HttpCase +from odoo import tests +from datetime import datetime, timedelta -class WebsiteVisitorTests(HttpCase): - def test_create_visitor_on_tracked_page(self): +class WebsiteVisitorTests(tests.HttpCase): + def setUp(self): + super(WebsiteVisitorTests, self).setUp() Page = self.env['website.page'] View = self.env['ir.ui.view'] - Visitor = self.env['website.visitor'] - Track = self.env['website.track'] + self.Visitor = self.env['website.visitor'] + self.Track = self.env['website.track'] untracked_view = View.create({ 'name': 'Base', 'type': 'qweb', @@ -30,7 +32,7 @@ def test_create_visitor_on_tracked_page(self): 'key': 'test.base_view', 'track': True, }) - [untracked_view, tracked_view] = Page.create([ + [self.untracked_view, self.tracked_view] = Page.create([ { 'view_id': untracked_view.id, 'url': '/untracked_view', @@ -43,14 +45,53 @@ def test_create_visitor_on_tracked_page(self): }, ]) - self.assertEqual(len(Visitor.search([])), 0, "No visitor at the moment") - self.assertEqual(len(Track.search([])), 0, "No track at the moment") - self.url_open(untracked_view.url) - self.assertEqual(len(Visitor.search([])), 0, "No visitor created after visiting an untracked view") - self.assertEqual(len(Track.search([])), 0, "No track created after visiting an untracked view") - self.url_open(tracked_view.url) - self.assertEqual(len(Visitor.search([])), 1, "A visitor should be created after visiting a tracked view") - self.assertEqual(len(Track.search([])), 1, "A track should be created after visiting a tracked view") - self.url_open(tracked_view.url) - self.assertEqual(len(Visitor.search([])), 1, "No visitor should be created after visiting another tracked view") - self.assertEqual(len(Track.search([])), 1, "No track should be created after visiting another tracked view before 30 min") + def test_create_visitor_on_tracked_page(self): + self.assertEqual(len(self.Visitor.search([])), 0, "No visitor at the moment") + self.assertEqual(len(self.Track.search([])), 0, "No track at the moment") + self.url_open(self.untracked_view.url) + self.assertEqual(len(self.Visitor.search([])), 0, "No visitor created after visiting an untracked view") + self.assertEqual(len(self.Track.search([])), 0, "No track created after visiting an untracked view") + self.url_open(self.tracked_view.url) + self.assertEqual(len(self.Visitor.search([])), 1, "A visitor should be created after visiting a tracked view") + self.assertEqual(len(self.Track.search([])), 1, "A track should be created after visiting a tracked view") + self.url_open(self.tracked_view.url) + self.assertEqual(len(self.Visitor.search([])), 1, "No visitor should be created after visiting another tracked view") + self.assertEqual(len(self.Track.search([])), 1, "No track should be created after visiting another tracked view before 30 min") + + def test_long_period_inactivity(self): + # link visitor to partner + old_visitor = self.Visitor.create({ + 'lang_id': self.env.ref('base.lang_en').id, + 'country_id': self.env.ref('base.be').id, + 'website_id': 1, + }) + partner_demo = self.env.ref('base.partner_demo') + old_visitor.partner_id = partner_demo.id + partner_demo.visitor_ids = [(6, 0, [old_visitor.id])] # TODO DBE : Remove this line in Master (13.1) after visitor_ids field declaration master fix + self.assertEqual(partner_demo.visitor_ids.id, old_visitor.id, "The partner visitor should be set correctly.") + + # archive old visitor + old_visitor.last_connection_datetime = datetime.now() - timedelta(days=8) + self.Visitor._cron_archive_visitors() + self.assertEqual(old_visitor.active, False, "The visitor should be archived after one week of inactivity") + + # reconnect with new visitor. + self.url_open(self.tracked_view.url) + new_visitor = self.Visitor.search([('id', '!=', old_visitor.id)], limit=1, order="id desc") # get the last created visitor + new_visitor_id = new_visitor.id + self.assertEqual(new_visitor_id > old_visitor.id, True, "A new visitor should have been created.") + self.assertEqual(len(new_visitor), 1, "A visitor should be created after visiting a tracked view") + self.assertEqual(len(self.Track.search([('visitor_id', '=', new_visitor.id)])), 1, + "A track for the new visitor should be created after visiting a tracked view") + + # override the get_visitor_from_request to mock that is new_visitor that authenticates + def get_visitor_from_request(self_mock, force_create=False): + return new_visitor + self.patch(type(self.env['website.visitor']), '_get_visitor_from_request', get_visitor_from_request) + + self.authenticate('demo', 'demo') + self.assertEqual(partner_demo.visitor_ids.id, old_visitor.id, "The partner visitor should be back to the 'old' visitor.") + + new_visitor = self.Visitor.search([('id', '=', new_visitor_id)]) + self.assertEqual(len(new_visitor), 0, "The new visitor should be deleted when visitor authenticate once again.") + self.assertEqual(old_visitor.active, True, "The old visitor should be reactivated when visitor authenticates once again.")