From be889efe45a6fae3bfc242b51a125e2712c832d7 Mon Sep 17 00:00:00 2001
From: Florian Charlier <flch@odoo.com>
Date: Wed, 18 May 2022 09:07:03 +0000
Subject: [PATCH] [FIX] website_slides: fix resource ACLs
This commit fixes ACLs for slide resources:
* Access to resources should be blocked for non-members (except publishers
for the course: responsible and managers)
* Tests are included
Task-2818136
Part of Task-2663320
Part-of: odoo/odoo#88454
---
addons/website_slides/models/slide_slide.py | 6 ++
.../security/ir.model.access.csv | 4 +-
.../security/website_slides_security.xml | 35 +++++++++++-
addons/website_slides/tests/test_security.py | 57 ++++++++++++++++++-
.../views/website_slides_templates_lesson.xml | 15 ++---
...ite_slides_templates_lesson_fullscreen.xml | 11 ++--
6 files changed, 112 insertions(+), 16 deletions(-)
diff --git a/addons/website_slides/models/slide_slide.py b/addons/website_slides/models/slide_slide.py
index dc931ae91e7d..7b209064b93a 100644
--- a/addons/website_slides/models/slide_slide.py
+++ b/addons/website_slides/models/slide_slide.py
@@ -240,6 +240,12 @@ class Slide(models.Model):
for slide in self:
slide.questions_count = len(slide.question_ids)
+ def _has_additional_resources(self):
+ """Sudo required for public user to know if the course has additional
+ resources that they will be able to access once a member."""
+ self.ensure_one()
+ return bool(self.sudo().slide_resource_ids)
+
@api.depends('website_message_ids.res_id', 'website_message_ids.model', 'website_message_ids.message_type')
def _compute_comments_count(self):
for slide in self:
diff --git a/addons/website_slides/security/ir.model.access.csv b/addons/website_slides/security/ir.model.access.csv
index 355ebc5ade44..aa3abd42fa67 100644
--- a/addons/website_slides/security/ir.model.access.csv
+++ b/addons/website_slides/security/ir.model.access.csv
@@ -23,7 +23,9 @@ access_slide_embed_all,slide.embed.all,model_slide_embed,,1,0,0,0
access_slide_embed_user,slide.embed.user,model_slide_embed,base.group_user,1,1,1,1
access_slide_slide_link_all,slide.slide.link.all,model_slide_slide_link,,1,0,0,0
access_slide_slide_link_officer,slide.slide.link.officer,model_slide_slide_link,website_slides.group_website_slides_officer,1,1,1,1
-access_slide_slide_resource_all,slide.slide.resource.all,model_slide_slide_resource,,1,0,0,0
+access_slide_slide_resource_all,slide.slide.resource.all,model_slide_slide_resource,,0,0,0,0
access_slide_slide_resource_public,slide.slide.resource.public,model_slide_slide_resource,base.group_public,0,0,0,0
+access_slide_slide_resource_portal,slide.slide.resource.portal,model_slide_slide_resource,base.group_portal,1,0,0,0
+access_slide_slide_resource_internal,slide.slide.resource.internal,model_slide_slide_resource,base.group_user,1,0,0,0
access_slide_slide_resource_publisher,slide.slide.resource.publisher,model_slide_slide_resource,website_slides.group_website_slides_officer,1,1,1,1
access_slide_channel_invite,access.slide.channel.invite,model_slide_channel_invite,base.group_user,1,1,1,0
diff --git a/addons/website_slides/security/website_slides_security.xml b/addons/website_slides/security/website_slides_security.xml
index ab800865dd00..fad254027607 100644
--- a/addons/website_slides/security/website_slides_security.xml
+++ b/addons/website_slides/security/website_slides_security.xml
@@ -178,7 +178,7 @@
<!--SLIDE RESOURCE-->
<record id="rule_slide_slide_resource_downloadable" model="ir.rule">
- <field name="name">Resource: restricted to channel members and channel responsible</field>
+ <field name="name">Resource: read restricted to channel members</field>
<field name="model_id" ref="model_slide_slide_resource"/>
<field name="domain_force">[('slide_id.channel_id.partner_ids', '=', user.partner_id.id)]</field>
<field name="groups" eval="[(4, ref('base.group_portal')), (4, ref('base.group_user'))]"/>
@@ -187,5 +187,38 @@
<field name="perm_create" eval="False"/>
<field name="perm_unlink" eval="False"/>
</record>
+
+ <record id="rule_slide_slide_resource_officer_read" model="ir.rule">
+ <field name="name">Resource: officer: read all</field>
+ <field name="model_id" ref="model_slide_slide_resource"/>
+ <field name="domain_force">[(1, '=', 1)]</field>
+ <field name="groups" eval="[(4, ref('group_website_slides_officer'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="False"/>
+ <field name="perm_create" eval="False"/>
+ <field name="perm_unlink" eval="False"/>
+ </record>
+
+ <record id="rule_slide_slide_resource_officer_crud" model="ir.rule">
+ <field name="name">Resource: officer: crud own only</field>
+ <field name="model_id" ref="model_slide_slide_resource"/>
+ <field name="domain_force">[('slide_id.channel_id.user_id', '=', user.id)]</field>
+ <field name="groups" eval="[(4, ref('group_website_slides_officer'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="True"/>
+ <field name="perm_create" eval="True"/>
+ <field name="perm_unlink" eval="True"/>
+ </record>
+
+ <record id="rule_slide_slide_resource_manager" model="ir.rule">
+ <field name="name">Resource: manager: crud all</field>
+ <field name="model_id" ref="model_slide_slide_resource"/>
+ <field name="domain_force">[(1, '=', 1)]</field>
+ <field name="groups" eval="[(4, ref('group_website_slides_manager'))]"/>
+ <field name="perm_read" eval="True"/>
+ <field name="perm_write" eval="True"/>
+ <field name="perm_create" eval="True"/>
+ <field name="perm_unlink" eval="True"/>
+ </record>
</data>
</odoo>
diff --git a/addons/website_slides/tests/test_security.py b/addons/website_slides/tests/test_security.py
index 1f5eecc31436..9ffdccba6c66 100644
--- a/addons/website_slides/tests/test_security.py
+++ b/addons/website_slides/tests/test_security.py
@@ -1,10 +1,11 @@
# -*- coding: utf-8 -*-
# Part of Odoo. See LICENSE file for full copyright and licensing details.
+import base64
+from odoo.addons.mail.tests.common import mail_new_test_user
from odoo.addons.website_slides.tests import common
-from odoo.exceptions import AccessError, UserError
+from odoo.exceptions import AccessError
from odoo.tests import tagged
-from odoo.tests.common import users
from odoo.tools import mute_logger
@@ -298,3 +299,55 @@ class TestAccessFeatures(common.SlidesCase):
channel_superuser.invalidate_cache(['can_upload', 'can_publish'])
self.assertTrue(channel_superuser.can_upload)
self.assertTrue(channel_superuser.can_publish)
+
+ @mute_logger('odoo.models.unlink', 'odoo.addons.base.models.ir_rule', 'odoo.addons.base.models.ir_model')
+ def test_resource_access(self):
+ resource_values = {
+ 'name': 'Image',
+ 'slide_id': self.slide_3.id,
+ 'data': base64.b64encode(b'Some content')
+ }
+ resource1, resource2 = self.env['slide.slide.resource'].with_user(self.user_officer).create(
+ [resource_values for _ in range(2)])
+
+ # No public access
+ with self.assertRaises(AccessError):
+ resource1.with_user(self.user_public).read(['name'])
+ with self.assertRaises(AccessError):
+ resource1.with_user(self.user_public).write({'name': 'other name'})
+
+ # No random portal access
+ with self.assertRaises(AccessError):
+ resource1.with_user(self.user_portal).read(['name'])
+
+ # Members can only read
+ self.env['slide.channel.partner'].create({
+ 'channel_id': self.channel.id,
+ 'partner_id': self.user_portal.partner_id.id,
+ })
+ resource1.with_user(self.user_portal).read(['name'])
+ with self.assertRaises(AccessError):
+ resource1.with_user(self.user_portal).write({'name': 'other name'})
+
+ # Other officers can only read
+ user_officer_other = mail_new_test_user(
+ self.env, name='Ornella Officer', login='user_officer_2', email='officer2@example.com',
+ groups='base.group_user,website_slides.group_website_slides_officer'
+ )
+ resource1.with_user(user_officer_other).read(['name'])
+ with self.assertRaises(AccessError):
+ resource1.with_user(user_officer_other).write({'name': 'Another name'})
+
+ with self.assertRaises(AccessError):
+ self.env['slide.slide.resource'].with_user(user_officer_other).create(resource_values)
+ with self.assertRaises(AccessError):
+ resource1.with_user(user_officer_other).unlink()
+
+ # Responsible officer can do anything on their own channels
+ resource1.with_user(self.user_officer).write({'name': 'other name'})
+ resource1.with_user(self.user_officer).unlink()
+
+ # Managers can do anything on all channels
+ resource2.with_user(self.user_manager).write({'name': 'Another name'})
+ resource2.with_user(self.user_manager).unlink()
+ self.env['slide.slide.resource'].with_user(self.user_manager).create(resource_values)
diff --git a/addons/website_slides/views/website_slides_templates_lesson.xml b/addons/website_slides/views/website_slides_templates_lesson.xml
index a8e9ad2a662e..8fd7575d1c86 100644
--- a/addons/website_slides/views/website_slides_templates_lesson.xml
+++ b/addons/website_slides/views/website_slides_templates_lesson.xml
@@ -138,10 +138,11 @@
</t>
</a>
<ul class="collapse show p-0 m-0 list-unstyled" t-att-id="('collapse-%s') % (category.id if category else 0)" >
+ <t t-set="is_member" t-value="slide.channel_id.is_member"/>
+ <t t-set="can_access_channel" t-value="is_member or slide.channel_id.can_publish"/>
<t t-foreach="category_slide_ids" t-as="aside_slide">
<t t-set="slide_completed" t-value="channel_progress[aside_slide.id].get('completed')"/>
- <t t-set="is_member" t-value="slide.channel_id.is_member"/>
- <t t-set="can_access" t-value="aside_slide.is_preview or is_member or slide.channel_id.can_publish"/>
+ <t t-set="can_access" t-value="can_access_channel or aside_slide.is_preview"/>
<li class="p-0 pb-1">
<a t-att-href="'/slides/slide/%s' % (slug(aside_slide)) if can_access else '#'"
t-att-class="'o_wslides_lesson_aside_list_link d-flex align-items-top px-2 pt-1 text-decoration-none %s%s' % (('bg-100 py-1 active' if aside_slide == slide else ''), 'text-muted' if not can_access else '')">
@@ -162,10 +163,10 @@
</span>
</div>
</a>
- <ul t-if="aside_slide.link_ids or aside_slide.slide_resource_ids or aside_slide.question_ids" class="list-group px-2 mb-1 list-unstyled">
+ <ul t-if="aside_slide.link_ids or aside_slide._has_additional_resources() or aside_slide.question_ids" class="list-group px-2 mb-1 list-unstyled">
<t t-foreach="aside_slide.link_ids" t-as="resource">
<li class="pl-4">
- <a t-if="can_access" t-att-href="resource.link" target="new" class="text-decoration-none small">
+ <a t-if="can_access_channel" t-att-href="resource.link" target="new" class="text-decoration-none small">
<i class="fa fa-link mr-1"/><span t-field="resource.name"/>
</a>
<span t-else="" class="text-decoration-none text-muted small">
@@ -173,8 +174,8 @@
</span>
</li>
</t>
- <div class="o_wslides_js_course_join pl-4" t-if="aside_slide.slide_resource_ids">
- <t t-if="is_member or aside_slide.channel_id.can_publish">
+ <div class="o_wslides_js_course_join pl-4" t-if="aside_slide._has_additional_resources()">
+ <t t-if="can_access_channel">
<li t-foreach="aside_slide.slide_resource_ids" t-as="resource">
<a t-attf-href="/web/content/slide.slide.resource/#{resource.id}/data?download=true" class="text-decoration-none small">
<i class="fa fa-download mr-1"/><span t-field="resource.name"/>
@@ -403,7 +404,7 @@
</t>
</div>
</div>
- <div class="col-12 col-md d-flex align-items-start mb-4 mb-md-0 o_wslides_js_course_join" t-if="len(slide.slide_resource_ids)">
+ <div class="col-12 col-md d-flex align-items-start mb-4 mb-md-0 o_wslides_js_course_join" t-if="slide._has_additional_resources()">
<span t-if="slide.channel_id.is_member or slide.channel_id.can_publish or slide.is_preview or slide.channel_id.enroll in ['private', 'payment']" class="text-muted font-weight-bold mr-3">
Additional Resources
</span>
diff --git a/addons/website_slides/views/website_slides_templates_lesson_fullscreen.xml b/addons/website_slides/views/website_slides_templates_lesson_fullscreen.xml
index 2fa732b2ac5e..3d8d405908e7 100644
--- a/addons/website_slides/views/website_slides_templates_lesson_fullscreen.xml
+++ b/addons/website_slides/views/website_slides_templates_lesson_fullscreen.xml
@@ -84,10 +84,11 @@
<b t-field="category.name"/>
</a>
<ul class="o_wslides_fs_sidebar_section_slides collapse show position-relative px-0 pb-1 my-0 mx-n3" t-att-id="('collapse-%s') % (category.id if category else 0)">
+ <t t-set="is_member" t-value="current_slide.channel_id.is_member"/>
+ <t t-set="can_access_channel" t-value="is_member or current_slide.channel_id.can_publish"/>
<t t-foreach="slides" t-as="slide">
<t t-set="slide_completed" t-value="channel_progress[slide.id].get('completed')"/>
- <t t-set="is_member" t-value="current_slide.channel_id.is_member"/>
- <t t-set="can_access" t-value="slide.is_preview or is_member or current_slide.channel_id.can_publish"/>
+ <t t-set="can_access" t-value="can_access_channel or slide.is_preview"/>
<li t-att-class="'o_wslides_fs_sidebar_list_item d-flex align-items-top py-1 %s' % ('active' if slide.id == current_slide.id else '')"
t-att-data-id="slide.id"
t-att-data-can-access="can_access"
@@ -118,7 +119,7 @@
<div class="o_wslides_fs_slide_name text-600" t-esc="slide.name"/>
</div>
</span>
- <ul class="list-unstyled w-100 pt-2 small" t-if="slide.link_ids or slide.slide_resource_ids or (slide.question_ids and not slide.slide_type =='quiz')" >
+ <ul class="list-unstyled w-100 pt-2 small" t-if="slide.link_ids or slide._has_additional_resources() or (slide.question_ids and not slide.slide_type =='quiz')" >
<li t-if="slide.link_ids" t-foreach="slide.link_ids" t-as="link" class="pl-0 mb-1">
<a t-if="can_access" class="o_wslides_fs_slide_link" t-att-href="link.link" target="_blank">
<i class="fa fa-link mr-2"/><span t-esc="link.name"/>
@@ -127,8 +128,8 @@
<i class="fa fa-link mr-2"/><span t-esc="link.name"/>
</span>
</li>
- <div class="o_wslides_js_course_join pl-0" t-if="slide.slide_resource_ids">
- <t t-if="is_member or slide.channel_id.can_publish">
+ <div class="o_wslides_js_course_join pl-0" t-if="slide._has_additional_resources()">
+ <t t-if="can_access_channel">
<li t-foreach="slide.slide_resource_ids" t-as="resource" class="mb-1">
<a class="o_wslides_fs_slide_link" t-attf-href="/web/content/slide.slide.resource/#{resource.id}/data?download=true">
<i class="fa fa-download mr-2"/><span t-esc="resource.name"/>
--
GitLab