[15.0][IMP] `bi_view_editor`: Restrict access to non BI View Editor Manager users #801

yankinmax · 2023-10-19T14:34:44Z

No description provided.

legalsylvain

too much files changed. Could you fix it ?

thanks !

yankinmax · 2023-10-19T14:41:18Z

@legalsylvain It's because I regenerated translations. The group name was added to the translation files. I can remove them.

yankinmax · 2023-10-19T14:47:46Z

@legalsylvain sorry, I forgot translations will be regenerated automatically, too much translations generated on projects recently 😄

yankinmax · 2023-10-23T09:39:20Z

@legalsylvain is this ok now?

legalsylvain · 2023-10-23T11:41:39Z

I don't use personaly this module. I prefer to have a point of view of the initial developers.

bi_view_editor/security/ir.model.access.csv

vvrossem

LGTM

yankinmax · 2023-10-24T08:30:36Z

@astirpe @monen17 can you pls take a look?

monen17

Thanks for the PR!
Right now all users can edit the views so, in order to avoid unexpected behaviors when the module is updated, I think a migration is needed to assign the new group to everyone; what do you think?

bi_view_editor/security/res_groups.xml

legalsylvain · 2023-10-24T21:22:12Z

Right now all users can edit the views so, in order to avoid unexpected behaviors when the module is updated, I think a migration is needed to assign the new group to everyone; what do you think?

Not sure. It will generate errors, if base_user_role (other OCA module) is installed.
If you fix a security issue, it's "normal" that the accesses change. don't you think ?

monen17 · 2023-10-24T21:41:27Z

Right now all users can edit the views so, in order to avoid unexpected behaviors when the module is updated, I think a migration is needed to assign the new group to everyone; what do you think?

Not sure. It will generate errors, if base_user_role (other OCA module) is installed.
If you fix a security issue, it's "normal" that the accesses change. don't you think ?

Is this fixing a security issue?
Whoever has this module installed right now is expecting that everyone can work on the views.
It would be at least unexpected that only admin and Odoobot can access the views after an update.
I still think that current behaviour should be kept for whoever has the module already installed.

New installs should be aware of the module's behaviour, and it should be mentioned in the README; otherwise a normal user installing the module wouldn't be able to access the menu mentioned in the Usage section.

yankinmax · 2023-10-25T06:08:04Z

@monen17 I think it's normal when some functionality of module changes/new feature appears.
When someone is updating the module of course he can expect that something can change.
New fields added for example.
It's the way OCA modules evolve.
Assigning this group to everyone is not what we need.
What I propose:

We can restrict access not to menu, but to bve.view records CRUD operations. So, everyone will still have access to the menu, but only allowed Users to create custom reports.
I agree with you we should update README.
I'll update PR, add noupdate.

leemannd

Seems legit. Thank you

Camille0907

LGTM :)

yankinmax · 2023-10-26T06:41:32Z

@monen17 is 4df35fa commit ok for you?

monen17

Thanks for taking care of #801 (review), everyone seeing the menu is a good compromise between not seeing anything and everyone being a view manager.

Here you have just a couple of notes, the only blocking comment is the fact that changes to the README are overwritten.

Also please fix the commits:

Fix up the fixup! commit.
You probably did it to let me see the clean changes (thanks!) but the commit history is part of what has to be reviewed so I can't really approve a PR having a fixup! commit.
For the future, in my opinion you should always organize the commits as you want them to be merged.
The commit message is too long, see https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#commit-message
I suggest [IMP] bi_view_editor: Restrict access to BI View Editor Managers
Then you can write any explanation in the 2nd and following lines fo the message.

monen17 · 2023-10-28T11:58:35Z

bi_view_editor/README.rst

README file is overwritten by files in the readme folder, so those files should be edited instead (there is also a warning about it at the top of this file).
You can have a look at https://github.com/OCA/maintainer-tools#readme-generator for more details

monen17 · 2023-10-28T11:58:48Z

bi_view_editor/README.rst

@@ -76,6 +76,8 @@ Usage
 To graphically design your analysis data-set:

 - From the Dashboards menu, select "Custom BI Views"
+- BI Views creation is restricted to admin users and members of "BI View Editor Manager" group.


Suggested change

- BI Views creation is restricted to admin users and members of "BI View Editor Manager" group.

- BI Views creation is restricted to members of "BI View Editor Manager" group.

monen17 · 2023-10-28T11:59:01Z

bi_view_editor/README.rst

@@ -88,6 +90,7 @@ To access the created BI View with a dedicated menu:

 - If module Dashboard (board) is installed, the standard "Add to My Dashboard"
  functionality would be available
+- "Create a menu" is restricted to admin users and members of "BI View Editor Manager" group.


Suggested change

- "Create a menu" is restricted to admin users and members of "BI View Editor Manager" group.

- "Create a menu" is restricted to members of "BI View Editor Manager" group.

yankinmax · 2023-10-30T11:04:32Z

@monen17 thanks for the review, your suggestions are applied and PR is updated

yankinmax · 2023-11-02T10:12:19Z

@monen17 @legalsylvain can we finish this PR review and merge?

yankinmax · 2023-11-07T09:46:57Z

@HviorForgeFlow can you pls also pay attention on this PR? It's updated with @monen17 suggestions as maintainer and it will also be forwarded to v16

HviorForgeFlow

Code review 👍

HviorForgeFlow · 2023-11-07T09:56:10Z

/ocabot merge minor

OCA-git-bot · 2023-11-07T09:56:19Z

What a great day to merge this nice PR. Let's do it!
Prepared branch 15.0-ocabot-merge-pr-801-by-HviorForgeFlow-bump-minor, awaiting test results.

OCA-git-bot · 2023-11-07T10:03:46Z

Congratulations, your PR was merged at 776b468. Thanks a lot for contributing to OCA. ❤️

HviorForgeFlow · 2023-11-07T10:05:51Z

@yankinmax ready to be forward ported

Forward-port changes from 15.0 OCA#801

[MIG+IMP] bi_view_editor: Migration to v10 + enhancements * Add menu items creation feature * Added selection of fields of a tree view * Improved usability and strings made translatable * Avoid display duplicated nodes * Robustness * Updated Dutch translation * Avoid possible sql injection in bi_view_editor * Removed deprecated RegistryManager OCA Transbot updated translations from Transifex [MIG] bi_view_editor: Migration to 11.0 [FIX] bi_view_editor: Apostrophe + migration script Apostrophe in model name raised ValueError. Added needed migration script. Remove logger warnings during update all [FIX] Field widget, new readme + Make enabled in list view default true Drop bi views when uninstalling module [UPD] README.rst [UPD] Update bi_view_editor.pot [12.0][MIG] bi_view_editor Add extra functionalities Add LEFT JOIN capabilities Add sums and avg capabilities for tree views Robustness and code review Provide ER diagram view for table relations [UPD] Update bi_view_editor.pot [UPD] README.rst Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-12.0/reporting-engine-12.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-12-0/reporting-engine-12-0-bi_view_editor/ bi_view_editor: window functions without ORDER BY clause do not have reliable behavior bi_view_editor: Better error message if the query is not correct pre-commit [13.0][MIG] bi_view_editor [14.0][MIG] bi_view_editor pre-commit [UPD] Update bi_view_editor.pot [UPD] README.rst bi_view_editor: add deb external dependencies Translated using Weblate (Italian) Currently translated at 18.6% (17 of 91 strings) Translation: reporting-engine-14.0/reporting-engine-14.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-14-0/reporting-engine-14-0-bi_view_editor/it/ [IMP] bi_view_editor : black, isort, prettier [MIG] bi_view_editor: Migration to 15.0 [UPD] Update bi_view_editor.pot [UPD] README.rst [MIG] bi_view_editor: migration to 16.0 [UPD] Update bi_view_editor.pot [BOT] post-merge updates Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/ [IMP] bi_view_editor: Restrict access to BI View Editor Managers Forward-port changes from 15.0 OCA#801 [UPD] Update bi_view_editor.pot [BOT] post-merge updates Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/ Translated using Weblate (Spanish) Currently translated at 100.0% (91 of 91 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/es/ Translated using Weblate (Italian) Currently translated at 18.6% (17 of 91 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/it/ Translated using Weblate (Japanese) Currently translated at 53.8% (49 of 91 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/ja/ Translated using Weblate (Italian) Currently translated at 100.0% (91 of 91 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/it/ [UPD] Update bi_view_editor.pot Update translation files Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/ Translated using Weblate (Italian) Currently translated at 100.0% (92 of 92 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/it/ Translated using Weblate (Swedish) Currently translated at 100.0% (92 of 92 strings) Translation: reporting-engine-16.0/reporting-engine-16.0-bi_view_editor Translate-URL: https://translation.odoo-community.org/projects/reporting-engine-16-0/reporting-engine-16-0-bi_view_editor/sv/ [IMP] bi_view_editor: pre-commit auto fixes [17.0][MIG] bi_view_editor: Migrate [17.0][MIG] bi_view_editor: Migrate

yankinmax mentioned this pull request Oct 19, 2023

[16.0][MIG] bi_view_editor #761

Merged

legalsylvain requested changes Oct 19, 2023

View reviewed changes

yankinmax force-pushed the 15.0-imp-bi_view_editor-add-mamager-group branch from ebdc95a to 4eadad5 Compare October 19, 2023 14:46

ivantodorovich reviewed Oct 23, 2023

View reviewed changes

bi_view_editor/security/ir.model.access.csv Outdated Show resolved Hide resolved

yankinmax force-pushed the 15.0-imp-bi_view_editor-add-mamager-group branch from 4eadad5 to a5612c8 Compare October 23, 2023 14:46

ivantodorovich approved these changes Oct 23, 2023

View reviewed changes

vvrossem approved these changes Oct 24, 2023

View reviewed changes

legalsylvain self-requested a review October 24, 2023 09:47

monen17 suggested changes Oct 24, 2023

View reviewed changes

bi_view_editor/security/res_groups.xml Outdated Show resolved Hide resolved

yankinmax force-pushed the 15.0-imp-bi_view_editor-add-mamager-group branch from acbfe97 to 4df35fa Compare October 25, 2023 07:12

leemannd approved these changes Oct 25, 2023

View reviewed changes

Camille0907 approved these changes Oct 25, 2023

View reviewed changes

monen17 suggested changes Oct 28, 2023

View reviewed changes

[IMP] bi_view_editor: Restrict access to BI View Editor Managers

fecc083

yankinmax force-pushed the 15.0-imp-bi_view_editor-add-mamager-group branch from 4df35fa to fecc083 Compare October 30, 2023 11:03

HviorForgeFlow approved these changes Nov 7, 2023

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Nov 7, 2023

OCA-git-bot merged commit f53027c into OCA:15.0 Nov 7, 2023

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Nov 7, 2023

yankinmax mentioned this pull request Nov 7, 2023

[IMP] bi_view_editor: Restrict access to BI View Editor Managers #807

Merged

1 task

yankinmax added a commit to yankinmax/reporting-engine that referenced this pull request Nov 7, 2023

[IMP] bi_view_editor: Restrict access to BI View Editor Managers

e878ba6

Forward-port changes from 15.0 OCA#801

bealdav pushed a commit to akretion/reporting-engine that referenced this pull request May 31, 2024

[IMP] bi_view_editor: Restrict access to BI View Editor Managers

effdbc4

Forward-port changes from 15.0 OCA#801

cormaza pushed a commit to cormaza/reporting-engine that referenced this pull request Jan 6, 2025

[IMP] bi_view_editor: Restrict access to BI View Editor Managers

6ded4af

Forward-port changes from 15.0 OCA#801

antoniodavid pushed a commit to BinhexTeam/reporting-engine that referenced this pull request Jan 21, 2025

[IMP] bi_view_editor: Restrict access to BI View Editor Managers

5bd31e5

Forward-port changes from 15.0 OCA#801

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[15.0][IMP] `bi_view_editor`: Restrict access to non BI View Editor Manager users #801

[15.0][IMP] `bi_view_editor`: Restrict access to non BI View Editor Manager users #801

yankinmax commented Oct 19, 2023

legalsylvain left a comment

yankinmax commented Oct 19, 2023 •

edited

Loading

yankinmax commented Oct 19, 2023 •

edited

Loading

yankinmax commented Oct 23, 2023

legalsylvain commented Oct 23, 2023

vvrossem left a comment

yankinmax commented Oct 24, 2023

monen17 left a comment

legalsylvain commented Oct 24, 2023

monen17 commented Oct 24, 2023

yankinmax commented Oct 25, 2023 •

edited

Loading

leemannd left a comment

Camille0907 left a comment

yankinmax commented Oct 26, 2023

monen17 left a comment

monen17 Oct 28, 2023

monen17 Oct 28, 2023

monen17 Oct 28, 2023

yankinmax commented Oct 30, 2023

yankinmax commented Nov 2, 2023

yankinmax commented Nov 7, 2023

HviorForgeFlow left a comment

HviorForgeFlow commented Nov 7, 2023

OCA-git-bot commented Nov 7, 2023

OCA-git-bot commented Nov 7, 2023

HviorForgeFlow commented Nov 7, 2023

	- BI Views creation is restricted to admin users and members of "BI View Editor Manager" group.
	- BI Views creation is restricted to members of "BI View Editor Manager" group.

	- "Create a menu" is restricted to admin users and members of "BI View Editor Manager" group.
	- "Create a menu" is restricted to members of "BI View Editor Manager" group.

[15.0][IMP] bi_view_editor: Restrict access to non BI View Editor Manager users #801

[15.0][IMP] bi_view_editor: Restrict access to non BI View Editor Manager users #801

Conversation

yankinmax commented Oct 19, 2023

legalsylvain left a comment

Choose a reason for hiding this comment

yankinmax commented Oct 19, 2023 • edited Loading

yankinmax commented Oct 19, 2023 • edited Loading

yankinmax commented Oct 23, 2023

legalsylvain commented Oct 23, 2023

vvrossem left a comment

Choose a reason for hiding this comment

yankinmax commented Oct 24, 2023

monen17 left a comment

Choose a reason for hiding this comment

legalsylvain commented Oct 24, 2023

monen17 commented Oct 24, 2023

yankinmax commented Oct 25, 2023 • edited Loading

leemannd left a comment

Choose a reason for hiding this comment

Camille0907 left a comment

Choose a reason for hiding this comment

yankinmax commented Oct 26, 2023

monen17 left a comment

Choose a reason for hiding this comment

monen17 Oct 28, 2023

Choose a reason for hiding this comment

monen17 Oct 28, 2023

Choose a reason for hiding this comment

monen17 Oct 28, 2023

Choose a reason for hiding this comment

yankinmax commented Oct 30, 2023

yankinmax commented Nov 2, 2023

yankinmax commented Nov 7, 2023

HviorForgeFlow left a comment

Choose a reason for hiding this comment

HviorForgeFlow commented Nov 7, 2023

OCA-git-bot commented Nov 7, 2023

OCA-git-bot commented Nov 7, 2023

HviorForgeFlow commented Nov 7, 2023

[15.0][IMP] `bi_view_editor`: Restrict access to non BI View Editor Manager users #801

[15.0][IMP] `bi_view_editor`: Restrict access to non BI View Editor Manager users #801

yankinmax commented Oct 19, 2023 •

edited

Loading

yankinmax commented Oct 19, 2023 •

edited

Loading

yankinmax commented Oct 25, 2023 •

edited

Loading