[FIX] website: make handle_visibility private

This commit: - make handle_visibility a private function (even if not exploitable in rpc easily since it uses request.website) - encrypt the password in db (even if not critic, since this password could be shared on twitter, ... it doesn't cost anything to secure it a bit more) - remove useless sudo, since handle_visibility does a sudo itself. In the future, this notion of visibility should be handled on controller layer, and no more on the View layer (during rendering) closes odoo/odoo#48145 X-original-commit: 06e0e48217f25f17dff23331111f5dcb98e8cecd Signed-off-by: Martin Trigaux (mat) <mat@odoo.com> Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
2020-03-21 10:23:32 +00:00
parent 92f9d9b21b
commit 94f8f15036
7 changed files with 66 additions and 16 deletions
@@ -10,6 +10,7 @@ from odoo import api, fields, models
 from odoo import tools
 from odoo.addons import website
 from odoo.addons.http_routing.models.ir_http import url_for
+from odoo.exceptions import AccessError
 from odoo.osv import expression
 from odoo.http import request

@@ -27,8 +28,20 @@ class View(models.Model):
    first_page_id = fields.Many2one('website.page', string='Website Page', help='First page linked to this view', compute='_compute_first_page_id')
    track = fields.Boolean(string='Track', default=False, help="Allow to specify for one page of the website to be trackable or not")
    visibility = fields.Selection([('', 'All'), ('connected', 'Signed In'), ('restricted_group', 'Restricted Group'), ('password', 'With Password')], default='')
-    visibility_group = fields.Many2one('res.groups')
-    visibility_password = fields.Char(groups='website.group_website_publisher')
+    visibility_group = fields.Many2one('res.groups', copy=False)
+    visibility_password = fields.Char(groups='base.group_system', copy=False)
+    visibility_password_display = fields.Char(compute='_get_pwd', inverse='_set_pwd', groups='website.group_website_designer')
+
+    @api.depends('visibility_password')
+    def _get_pwd(self):
+        for r in self:
+            r.visibility_password_display = r.sudo().visibility_password and '********' or ''
+
+    def _set_pwd(self):
+        crypt_context = self.env.user._crypt_context()
+        for r in self:
+            r.sudo().visibility_password = crypt_context.encrypt(r.visibility_password_display)
+            r.visibility = r.visibility  # double check access

    def _compute_first_page_id(self):
        for view in self:
@@ -324,8 +337,8 @@ class View(models.Model):

    @api.model
    def read_template(self, xml_id):
-        view_sudo = self._view_obj(self.get_view_id(xml_id)).sudo()
-        if view_sudo.visibility and view_sudo.handle_visibility(do_raise=False):
+        view = self._view_obj(self.get_view_id(xml_id))
+        if view.visibility and view._handle_visibility(do_raise=False):
            self = self.sudo()
        return super(View, self).read_template(xml_id)

@@ -338,14 +351,19 @@ class View(models.Model):
        domain = [('key', '=', self.key), ('model_data_id', '!=', None)]
        return self.with_context(active_test=False).search(domain, limit=1)  # Useless limit has multiple xmlid should not be possible

-    def handle_visibility(self, do_raise=True):
+    def _handle_visibility(self, do_raise=True):
        """ Check the visibility set on the main view and raise 403 if you should not have access.
            Order is: Public, Connected, Has group, Password

            It only check the visibility on the main content, others views called stay available in rpc.
        """
        error = False
-        self = self.sudo()
+
+        try:
+            self.visibility  # avoid useless sudo() in case page is public
+        except AccessError:
+            self = self.sudo()
+
        if self.visibility and not request.env.user.has_group('website.group_website_designer'):
            if (self.visibility == 'connected' and request.website.is_public_user()):
                error = werkzeug.exceptions.Forbidden()
@@ -356,7 +374,9 @@ class View(models.Model):
                    error = werkzeug.exceptions.Forbidden()
            elif self.visibility == 'password' and \
                    (request.website.is_public_user() or self.id not in request.session.get('views_unlock', [])):
-                if self.sudo().visibility_password == request.params.get('visibility_password'):
+                pwd = request.params.get('visibility_password')
+                if pwd and self.env.user._crypt_context().verify(
+                        pwd, self.sudo().visibility_password):
                    request.session.setdefault('views_unlock', list()).append(self.id)
                else:
                    error = werkzeug.exceptions.Forbidden('website_visibility_password_required')
@@ -369,7 +389,7 @@ class View(models.Model):

    def render(self, values=None, engine='ir.qweb', minimal_qcontext=False):
        """ Render the template. If website is enabled on request, then extend rendering context with website values. """
-        self.handle_visibility(do_raise=True)
+        self._handle_visibility(do_raise=True)
        new_context = dict(self._context)
        if request and getattr(request, 'is_frontend', False):

@@ -113,7 +113,7 @@ class Menu(models.Model):
            visible = True
            if menu.page_id and not menu.user_has_groups('base.group_user') and \
                (not menu.page_id.sudo().is_visible or
-                 (not menu.page_id.view_id.handle_visibility(do_raise=False) and
+                 (not menu.page_id.view_id._handle_visibility(do_raise=False) and
                 menu.page_id.view_id.visibility != "password")):
                visible = False
            menu.is_visible = visible
@@ -66,10 +66,11 @@ class Page(models.Model):
        self.ensure_one()
        res = self.read([
            'id', 'name', 'url', 'website_published', 'website_indexed', 'date_publish',
-            'menu_ids', 'is_homepage', 'website_id', 'visibility', 'visibility_password', 'visibility_group'
+            'menu_ids', 'is_homepage', 'website_id', 'visibility', 'visibility_group'
        ])[0]
        if not res['visibility_group']:
            res['visibility_group'] = self.env.ref('base.group_user').name_get()[0]
+        res['visibility_password'] = res['visibility'] == 'password' and self.visibility_password_display or ''
        return res

    @api.model
@@ -122,9 +123,11 @@ class Page(models.Model):
            'date_publish': data['date_publish'] or None,
            'is_homepage': data['is_homepage'],
            'visibility': data['visibility'],
-            'visibility_password': data['visibility'] == "password" and data['visibility_password'] or '',
            'visibility_group': data['visibility'] == "restricted_group" and data['visibility_group'],
        }
+        if 'visibility_pwd' in data:
+            w_vals['visibility_password_display'] = data['visibility_pwd'] or ''
+
        page.with_context(no_cow=True).write(w_vals)

        # Create redirect if needed
@@ -75,7 +75,7 @@
    <record id="view_rule_visibility_public" model="ir.rule">
        <field name="name">Website View Visibility Public</field>
        <field name="model_id" ref="base.model_ir_ui_view"/>
-        <field name="domain_force">[('type', '=', 'qweb'), ('visibility', 'in', ('public', False))]</field>
+        <field name="domain_force">['|', ('type', '!=', 'qweb'), ('visibility', 'in', ('public', False))]</field>
        <field name="perm_read" eval="True"/>
        <field name="perm_write" eval="False"/>
        <field name="perm_create" eval="False"/>
@@ -85,7 +85,7 @@
    <record id="view_rule_visibility_connected" model="ir.rule">
        <field name="name">Website View Visibility Connected</field>
        <field name="model_id" ref="base.model_ir_ui_view"/>
-        <field name="domain_force">[('type', '=', 'qweb'), ('visibility', 'in', ('public', 'connected', False))]</field>
+        <field name="domain_force">['|', ('type', '!=', 'qweb'), ('visibility', 'in', ('public', 'connected', False))]</field>
        <field name="perm_read" eval="True"/>
        <field name="perm_write" eval="False"/>
        <field name="perm_create" eval="False"/>
@@ -22,6 +22,8 @@ var PagePropertiesDialog = weWidgets.Dialog.extend({
        'keyup input#page_name': '_onNameChanged',
        'keyup input#page_url': '_onUrlChanged',
        'change input#create_redirect': '_onCreateRedirectChanged',
+        'click input#visibility_password': '_onPasswordClicked',
+        'change input#visibility_password': '_onPasswordChanged',
        'change select#visibility': '_onVisibilityChanged',
    }),

@@ -234,10 +236,19 @@ var PagePropertiesDialog = weWidgets.Dialog.extend({
            redirect_type: this.$('#redirect_type').val(),
            website_indexed: this.$('#is_indexed').prop('checked'),
            visibility: this.$('#visibility').val(),
-            visibility_password: this.$('#visibility').val() === 'password' ? this.$('#visibility_password').val() : false,
            visibility_group: this.$('#visibility').val() === 'restricted_group' ? this.$('#visibility_group').data('group-id') : false,
            date_publish: datePublish,
        };
+        if (this.$('#visibility').val() === 'password') {
+            var field_pwd = $('#visibility_password');
+            if (!field_pwd.get(0).reportValidity()) {
+                return;
+            }
+            if (field_pwd.data('dirty')) {
+                params['visibility_pwd'] = field_pwd.val();
+            }
+        }
+
        this._rpc({
            model: 'website.page',
            method: 'save_page_info',
@@ -408,6 +419,19 @@ var PagePropertiesDialog = weWidgets.Dialog.extend({
    _onVisibilityChanged: function (ev) {
        this.$('.show_visibility_password').toggleClass('d-none', ev.target.value !== 'password');
        this.$('.show_visibility_group').toggleClass('d-none', ev.target.value !== 'restricted_group');
+        this.$('#visibility_password').attr('required', ev.target.value === 'password');
+    },
+    /**
+     * @private
+     */
+    _onPasswordClicked: function (ev) {
+        ev.target.value = '';
+        this._onPasswordChanged();
+    },    /**
+     * @private
+     */
+    _onPasswordChanged: function () {
+        this.$('#visibility_password').data('dirty', 1);
    },
 });

@@ -152,7 +152,10 @@
                            <div class="ml-1 input-group-prepend show_visibility_password" >
                                <div class="input-group-text"><i class="fa fa-key"></i></div>
                            </div>
-                            <input type="text" t-att-value='widget.page.visibility_password ? widget.page.visibility_password : ""' class="form-control show_visibility_password" id="visibility_password"/>
+                            <input type="password" id="visibility_password"
+                                   t-att-value='widget.page.visibility_password'
+                                   t-att-required="widget.page.visibility == 'password' ? 'required' : None"
+                                   class="form-control show_visibility_password"/>
                            <div class="ml-1 input-group-prepend show_visibility_group">
                                <div class="input-group-text"><i class="fa fa-group"></i></div>
                            </div>
@@ -273,7 +273,7 @@
                    <field name="first_page_id" attrs="{'invisible': [('page_ids', '=', [])]}" />
                    <field name="visibility" attrs="{'invisible': [('type', '!=', 'qweb')]}" />
                    <field name="visibility_group" attrs="{'invisible': [('visibility', '!=', 'restricted_group')]}" />
-                    <field name="visibility_password" attrs="{'invisible': [('visibility', '!=', 'password')]}" />
+                    <field name="visibility_password_display" attrs="{'invisible': [('visibility', '!=', 'password')]}" password="True"/>
                </field>
                <sheet position="before">
                    <header>