[ADD] sql_shell: get easy access to database without having access to server #115744

william-andre · 2023-03-17T17:15:15Z

No description provided.

robodoo · 2023-03-17T17:15:23Z

legalsylvain · 2023-03-18T13:32:34Z

If you want, you can take a look on some community modules developed some years ago :
https://github.com/OCA/reporting-engine/tree/16.0/sql_export (close with your implémentation)
https://github.com/OCA/reporting-engine/tree/16.0/bi_sql_editor (more technical, allowing to create materialized views to improve performance)

Note : in your current implementation, it is possible to call INSERT, UPDATE, TRUNCATE. Is it intended ?

(I'm available to talk with you if you want some feedbacks, talk about the design, etc...)

ajepe · 2023-03-18T14:05:22Z

Hi @william-andre Nice move !

If you want, you can take a look on some community modules developed some years ago : https://github.com/OCA/reporting-engine/tree/16.0/sql_export (close with your implémentation) https://github.com/OCA/reporting-engine/tree/16.0/bi_sql_editor (more technical, allowing to create materialized views to improve performance)

Note : in your current implementation, it is possible to call INSERT, UPDATE, TRUNCATE. Is it intended ?

(I'm available to talk with you if you want some feedbacks, talk about the design, etc...)
The UI is looks better than the available options in the community

william-andre · 2023-03-18T20:16:24Z

The purpose is indeed to be able to execute any SQL queries, such as INSERT, UPDATE, DELETE FROM, but also EXPLAIN ANALYZE or CREATE INDEX.
It is not intended to be only reading data but also a debugging tool (doing a SELECT is actually the least useful feature of this module since it can already be done using a standard read() through the ORM)
Even though some of these queried can already be done with a server action, it would be easier and faster using this, for the technical support for instance who doesn't always have an easy access to the server.

legalsylvain · 2023-03-18T21:05:47Z

Hi @ajepe.

The UI is looks better than the available options in the community

Thanks for your quick review. Could you elaborate on this, though ? I don't get your point.

The purpose is indeed to be able to execute any SQL queries, such as INSERT, UPDATE, DELETE FROM, but also EXPLAIN ANALYZE or CREATE INDEX.

I see. Thanks for the explanation.

Even though some of these queried can already be done with a server action

Indeed, but the ORM "protects" data inconsistency. account_move.unlink() will fail if the account move is posted. pos_order_line.write() will faill if french certification is installed.
With this module, it is possible by the UI, to destroy the data, violating ACL, security checks and business checks. It is not a neutral change.

In the other hand, I understand the need, that is valid. But as you propose the PR against a stable version, I think odoo SA should communicate on that topic. I'm sure some integrators will disable this module to prevent their client from accessing this functionality.

In other words, if today, someone would find the possibility to perform SQL write/read queries on the database via UI or xml-rpc, this would be considered as a severe security flaw. @rco-odoo, a point of view on this topic?

Thanks.

william-andre · 2023-03-18T21:14:32Z

You can run any SQL query in a server action by running env.cr.execute. This has always been possible.
This module only makes it easier to get the response from the query.

legalsylvain · 2023-03-18T21:19:51Z

You can run any SQL query in a server action by running env.cr.execute. This has always been possible.

You're right. Sorry for the noice, then.

fmdl · 2023-03-19T19:31:05Z

addons/sql_shell/models/sql_shell.py

+                self.env.cr.execute(self.query)
+                self.result = {
+                    'header': [d.name for d in self.env.cr.description],
+                    'rows': self.env.cr.fetchall(),


some request (update, delete) doesn't return rows

note : maybe add a limit to prevent browser crash
'rows': self.env.cr.fetchall()[:1000],

some request (update, delete) doesn't return rows

They return how many rows were updated, for instance

note : maybe add a limit to prevent browser crash
'rows': self.env.cr.fetchall()[:1000],

This depends on the browser/computer. Use at your own risks.
The purpose is not to fetch a bajillion rows. The way it is (temporarily) currently implemented would make the server's memory explode too btw.

My bad, I was too quick in testing, I fixed both remarks in the latest commit

Levizar

my 2 cents when reading it

addons/web/static/lib/ace/mode-sql.js

This commit adds a new widget to the field registry. This button is represented with an icon and acts as a boolean field widget. The label is shown as a tooltip when hovered. A test file has also been added to verify its behavior. task #3121078 backport of 1f02263

… server

legalsylvain · 2023-08-30T20:15:09Z

addons/sql_shell/views/sql_shell_views.xml

+        <field name="view_mode">form</field>
+    </record>
+
+    <menuitem name="SQL Shell" id="sql_shell_menu" parent="base.menu_custom" groups="base.group_no_one" action="sql_shell_action" sequence="10000"/>


Suggested change

<menuitem name="SQL Shell" id="sql_shell_menu" parent="base.menu_custom" groups="base.group_no_one" action="sql_shell_action" sequence="10000"/>

<menuitem name="SQL Shell" id="sql_shell_menu" parent="base.next_id_9" groups="base.group_no_one" action="sql_shell_action" sequence="100"/>

For the time being, this entry is set at the very last position of the technical menu, that doesn't make much sense.
Propose to replace : "Setting > Technical > SQL Shell" by "Setting > Technical > Database Structure > SQL Shell"

legalsylvain · 2023-08-30T20:18:15Z

addons/sql_shell/models/sql_shell.py

+
+from odoo import models, fields, api
+
+class SQLShell(models.Model):


Naive question : Is there a reason to not use classic Transient Model like other wizard ?

legalsylvain · 2023-08-30T20:42:11Z

hi @william-andre. What is the state of this PR ?

Some functional review. Something is wrong if the max rows is small. could you take a look ?

Otherwise, LGTM.

SELECT with max > 40 : OK

SELECT with max < 40 : KO

EXPLAIN ANALYZE : OK (nice feature, BTW)

Destroy the database : OK
(yes, I know, still possible by server action).

Then

william-andre · 2023-08-31T07:33:53Z

The issue you raised with "Max Rows" is actually a protection to avoid crashing the browser and making too much traffic on the network; increase the limit at your own risk

And this PR will probably never get accepted because of you TRUNCATE TABLE example. Even if it is already possible in a server action, this makes it a bit too easy to do...

We might implement it someday only for Odoo support and only on duplicate databases.
Partners using their own hosting are welcome to do the same if they want to.

legalsylvain · 2023-08-31T07:50:13Z

Hi @william-andre, thanks for your answer !

The issue you raised with "Max Rows" is actually a protection to avoid crashing the browser and making too much traffic on the network; increase the limit at your own risk

No : take a look again on my prinscreen. I set 5 to max rows, and an error is raised and I can not see the result. That's not expected, don't you think ?

And this PR will probably never get accepted because of you TRUNCATE TABLE example.

Well, indeed, there is no warning, but we could imagine quite simply to add a warning with some confirmation, if it is a "write" or a "delete" requests. (similar algorithm than the community module that prohibit the non select requests : https://github.com/OCA/reporting-engine/blob/16.0/sql_request_abstract/models/sql_request_mixin.py#L39)

Even if it is already possible in a server action, this makes it a bit too easy to do...

well, go to "application > select product module > click on uninstall" will do basically similar "apocalypse by UI".
IMO if correct warning is displayed, that is OK. Don't you think ?

william-andre force-pushed the 16.0-sql-shell-wan branch 3 times, most recently from 243a376 to d10ce92 Compare March 17, 2023 17:32

C3POdoo added the RD research & development, internal work label Mar 17, 2023

fmdl reviewed Mar 19, 2023

View reviewed changes

Levizar reviewed Mar 20, 2023

View reviewed changes

addons/web/static/lib/ace/mode-sql.js Outdated Show resolved Hide resolved

addons/web/static/lib/ace/mode-sql.js Outdated Show resolved Hide resolved

william-andre force-pushed the 16.0-sql-shell-wan branch 3 times, most recently from 71b4221 to f1355f5 Compare March 20, 2023 18:20

Iucapad and others added 2 commits March 20, 2023 19:25

[IMP] web: add support for SQL in ACE fields

ab930d2

william-andre force-pushed the 16.0-sql-shell-wan branch 3 times, most recently from e2309d2 to f5b1fbf Compare March 21, 2023 12:32

[ADD] sql_shell: get easy access to database without having access to…

6f8d110

… server

william-andre force-pushed the 16.0-sql-shell-wan branch from f5b1fbf to 6f8d110 Compare March 21, 2023 13:50

legalsylvain reviewed Aug 30, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ADD] sql_shell: get easy access to database without having access to server #115744

[ADD] sql_shell: get easy access to database without having access to server #115744

william-andre commented Mar 17, 2023

robodoo commented Mar 17, 2023

legalsylvain commented Mar 18, 2023

ajepe commented Mar 18, 2023

william-andre commented Mar 18, 2023 •

edited

legalsylvain commented Mar 18, 2023

william-andre commented Mar 18, 2023

legalsylvain commented Mar 18, 2023

fmdl Mar 19, 2023

fmdl Mar 19, 2023

william-andre Mar 19, 2023

william-andre Mar 19, 2023

william-andre Mar 20, 2023

Levizar left a comment

legalsylvain Aug 30, 2023

legalsylvain Aug 30, 2023

legalsylvain commented Aug 30, 2023

william-andre commented Aug 31, 2023

legalsylvain commented Aug 31, 2023

	<menuitem name="SQL Shell" id="sql_shell_menu" parent="base.menu_custom" groups="base.group_no_one" action="sql_shell_action" sequence="10000"/>
	<menuitem name="SQL Shell" id="sql_shell_menu" parent="base.next_id_9" groups="base.group_no_one" action="sql_shell_action" sequence="100"/>


		from odoo import models, fields, api

		class SQLShell(models.Model):

[ADD] sql_shell: get easy access to database without having access to server #115744

Are you sure you want to change the base?

[ADD] sql_shell: get easy access to database without having access to server #115744

Conversation

william-andre commented Mar 17, 2023

robodoo commented Mar 17, 2023

legalsylvain commented Mar 18, 2023

ajepe commented Mar 18, 2023

william-andre commented Mar 18, 2023 • edited

legalsylvain commented Mar 18, 2023

william-andre commented Mar 18, 2023

legalsylvain commented Mar 18, 2023

fmdl Mar 19, 2023

Choose a reason for hiding this comment

fmdl Mar 19, 2023

Choose a reason for hiding this comment

william-andre Mar 19, 2023

Choose a reason for hiding this comment

william-andre Mar 19, 2023

Choose a reason for hiding this comment

william-andre Mar 20, 2023

Choose a reason for hiding this comment

Levizar left a comment

Choose a reason for hiding this comment

legalsylvain Aug 30, 2023

Choose a reason for hiding this comment

legalsylvain Aug 30, 2023

Choose a reason for hiding this comment

legalsylvain commented Aug 30, 2023

william-andre commented Aug 31, 2023

legalsylvain commented Aug 31, 2023

william-andre commented Mar 18, 2023 •

edited