diff --git a/.github/workflows/server-mariadb-tests.yml b/.github/workflows/server-mariadb-tests.yml deleted file mode 100644 index ae101d003b..0000000000 --- a/.github/workflows/server-mariadb-tests.yml +++ /dev/null @@ -1,148 +0,0 @@ -name: Server (MariaDB) - -on: - pull_request: - workflow_dispatch: - push: - branches: [ develop ] - -concurrency: - group: server-mariadb-develop-${{ github.event_name }}-${{ github.event.number }} - cancel-in-progress: true - - -permissions: - contents: read - -jobs: - checkrun: - name: Build Check - runs-on: ubuntu-latest - - outputs: - build: ${{ steps.check-build.outputs.build }} - - steps: - - name: Clone - uses: actions/checkout@v3 - - - name: Check if build should be run - id: check-build - run: | - python "${GITHUB_WORKSPACE}/.github/helper/roulette.py" - env: - TYPE: "server" - PR_NUMBER: ${{ github.event.number }} - REPO_NAME: ${{ github.repository }} - - test: - name: Unit Tests - runs-on: ubuntu-latest - needs: checkrun - if: ${{ needs.checkrun.outputs.build == 'strawberry' }} - timeout-minutes: 60 - - strategy: - fail-fast: false - matrix: - container: [1, 2] - - services: - mariadb: - image: mariadb:10.6 - env: - MARIADB_ROOT_PASSWORD: travis - ports: - - 3306:3306 - options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 - - steps: - - name: Clone - uses: actions/checkout@v3 - - - name: Setup Python - uses: actions/setup-python@v4 - with: - python-version: '3.11' - - - name: Check for valid Python & Merge Conflicts - run: | - python -m compileall -q -f "${GITHUB_WORKSPACE}" - if grep -lr --exclude-dir=node_modules "^<<<<<<< " "${GITHUB_WORKSPACE}" - then echo "Found merge conflicts" - exit 1 - fi - - - uses: actions/setup-node@v3 - with: - node-version: 16 - check-latest: true - - - name: Add to Hosts - run: | - echo "127.0.0.1 test_site" | sudo tee -a /etc/hosts - - - name: Cache pip - uses: actions/cache@v3 - with: - path: ~/.cache/pip - key: ${{ runner.os }}-pip-${{ hashFiles('**/*requirements.txt', '**/pyproject.toml', '**/setup.py') }} - restore-keys: | - ${{ runner.os }}-pip- - ${{ runner.os }}- - - - name: Get yarn cache directory path - id: yarn-cache-dir-path - run: echo "::set-output name=dir::$(yarn cache dir)" - - - uses: actions/cache@v3 - id: yarn-cache - with: - path: ${{ steps.yarn-cache-dir-path.outputs.dir }} - key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }} - restore-keys: | - ${{ runner.os }}-yarn- - - - name: Install Dependencies - run: | - bash ${GITHUB_WORKSPACE}/.github/helper/install_dependencies.sh - bash ${GITHUB_WORKSPACE}/.github/helper/install.sh - env: - BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} - AFTER: ${{ env.GITHUB_EVENT_PATH.after }} - TYPE: server - DB: mariadb - - - name: Run Tests - run: cd ~/frappe-bench/sites && ../env/bin/python3 ../apps/frappe/.github/helper/ci.py - env: - SITE: test_site - CI_BUILD_ID: ${{ github.run_id }} - BUILD_NUMBER: ${{ matrix.container }} - TOTAL_BUILDS: 2 - - - name: Upload coverage data - uses: actions/upload-artifact@v3 - with: - name: coverage-${{ matrix.container }} - path: /home/runner/frappe-bench/sites/coverage.xml - - coverage: - name: Coverage Wrap Up - needs: [test, checkrun] - runs-on: ubuntu-latest - if: ${{ needs.checkrun.outputs.build == 'strawberry' }} - steps: - - name: Clone - uses: actions/checkout@v3 - - - name: Download artifacts - uses: actions/download-artifact@v3 - - - name: Upload coverage data - uses: codecov/codecov-action@v3 - with: - name: MariaDB - fail_ci_if_error: true - verbose: true - flags: server-mariadb diff --git a/.github/workflows/server-postgres-tests.yml b/.github/workflows/server-tests.yml similarity index 87% rename from .github/workflows/server-postgres-tests.yml rename to .github/workflows/server-tests.yml index dcc078ad2a..a003393782 100644 --- a/.github/workflows/server-postgres-tests.yml +++ b/.github/workflows/server-tests.yml @@ -1,4 +1,4 @@ -name: Server (Postgres) +name: Server on: pull_request: @@ -7,9 +7,10 @@ on: branches: [ develop ] concurrency: - group: server-postgres-develop-${{ github.event_name }}-${{ github.event.number }} + group: server-develop-${{ github.event_name }}-${{ github.event.number }} cancel-in-progress: true + permissions: contents: read @@ -44,9 +45,18 @@ jobs: strategy: fail-fast: false matrix: + db: ["mariadb", "postgres"] container: [1, 2] services: + mariadb: + image: mariadb:10.6 + env: + MARIADB_ROOT_PASSWORD: travis + ports: + - 3306:3306 + options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3 + postgres: image: postgres:12.4 env: @@ -78,7 +88,7 @@ jobs: - uses: actions/setup-node@v3 with: - node-version: '16' + node-version: 16 check-latest: true - name: Add to Hosts @@ -114,7 +124,7 @@ jobs: BEFORE: ${{ env.GITHUB_EVENT_PATH.before }} AFTER: ${{ env.GITHUB_EVENT_PATH.after }} TYPE: server - DB: postgres + DB: ${{ matrix.db }} - name: Run Tests run: cd ~/frappe-bench/sites && ../env/bin/python3 ../apps/frappe/.github/helper/ci.py @@ -127,14 +137,14 @@ jobs: - name: Upload coverage data uses: actions/upload-artifact@v3 with: - name: coverage-${{ matrix.container }} + name: coverage-${{ matrix.db }}-${{ matrix.container }} path: /home/runner/frappe-bench/sites/coverage.xml coverage: name: Coverage Wrap Up needs: [test, checkrun] - if: ${{ needs.checkrun.outputs.build == 'strawberry' }} runs-on: ubuntu-latest + if: ${{ needs.checkrun.outputs.build == 'strawberry' }} steps: - name: Clone uses: actions/checkout@v3 @@ -145,7 +155,7 @@ jobs: - name: Upload coverage data uses: codecov/codecov-action@v3 with: - name: Postgres + name: Server fail_ci_if_error: true verbose: true - flags: server-postgres + flags: server diff --git a/frappe/email/doctype/email_template/email_template.py b/frappe/email/doctype/email_template/email_template.py index fcc6ce5010..1ef8ec062b 100644 --- a/frappe/email/doctype/email_template/email_template.py +++ b/frappe/email/doctype/email_template/email_template.py @@ -9,33 +9,33 @@ from frappe.utils.jinja import validate_template class EmailTemplate(Document): + @property + def response_(self): + return self.response_html if self.use_html else self.response + def validate(self): - if self.use_html: - validate_template(self.response_html) - else: - validate_template(self.response) + validate_template(self.subject) + validate_template(self.response_) def get_formatted_subject(self, doc): return frappe.render_template(self.subject, doc) def get_formatted_response(self, doc): - if self.use_html: - return frappe.render_template(self.response_html, doc) - - return frappe.render_template(self.response, doc) + return frappe.render_template(self.response_, doc) def get_formatted_email(self, doc): if isinstance(doc, str): doc = json.loads(doc) - return {"subject": self.get_formatted_subject(doc), "message": self.get_formatted_response(doc)} + return { + "subject": self.get_formatted_subject(doc), + "message": self.get_formatted_response(doc), + } @frappe.whitelist() def get_email_template(template_name, doc): """Returns the processed HTML of a email template with the given doc""" - if isinstance(doc, str): - doc = json.loads(doc) email_template = frappe.get_doc("Email Template", template_name) return email_template.get_formatted_email(doc) diff --git a/frappe/permissions.py b/frappe/permissions.py index 2997165dc9..d734f79a4e 100644 --- a/frappe/permissions.py +++ b/frappe/permissions.py @@ -707,8 +707,10 @@ def has_child_permission( parent_meta = frappe.get_meta(parent_doctype) - if parent_meta.istable or all( - df.options != child_doctype for df in parent_meta.get_table_fields() + if parent_meta.istable or not ( + valid_parentfields := [ + df.fieldname for df in parent_meta.get_table_fields() if df.options == child_doctype + ] ): push_perm_check_log( _("{0} is not a valid parent DocType for {1}").format( @@ -717,15 +719,30 @@ def has_child_permission( ) return False - if ( - child_doc - and (permlevel := parent_meta.get_field(child_doc.parentfield).permlevel) > 0 - and permlevel not in parent_meta.get_permlevel_access(ptype, user=user) - ): - push_perm_check_log( - _("Insufficient Permission Level for {0}").format(frappe.bold(parent_doctype)) - ) - return False + if child_doc: + parentfield = child_doc.parentfield + if not parentfield: + push_perm_check_log( + _("Parentfield not specified in {0}: {1}").format( + frappe.bold(child_doctype), frappe.bold(child_doc.name) + ) + ) + return False + + if parentfield not in valid_parentfields: + push_perm_check_log( + _("{0} is not a valid parentfield for {1}").format( + frappe.bold(parentfield), frappe.bold(child_doctype) + ) + ) + return False + + permlevel = parent_meta.get_field(parentfield).permlevel + if permlevel > 0 and permlevel not in parent_meta.get_permlevel_access(ptype, user=user): + push_perm_check_log( + _("Insufficient Permission Level for {0}").format(frappe.bold(parent_doctype)) + ) + return False return has_permission( parent_doctype, diff --git a/frappe/public/js/frappe/form/controls/base_input.js b/frappe/public/js/frappe/form/controls/base_input.js index 43fb4f54dc..0b9da726d7 100644 --- a/frappe/public/js/frappe/form/controls/base_input.js +++ b/frappe/public/js/frappe/form/controls/base_input.js @@ -109,7 +109,7 @@ frappe.ui.form.ControlInput = class ControlInput extends frappe.ui.form.Control "title", __( "This value is fetched from {0}'s {1} field", - me.df.fetch_from.split(".") + me.df.fetch_from.split(".").map((value) => __(frappe.unscrub(value))) ) ); } diff --git a/frappe/public/js/frappe/form/sidebar/share.js b/frappe/public/js/frappe/form/sidebar/share.js index d2ed9cb349..dd310531f0 100644 --- a/frappe/public/js/frappe/form/sidebar/share.js +++ b/frappe/public/js/frappe/form/sidebar/share.js @@ -17,9 +17,12 @@ frappe.ui.form.Share = class Share { this.parent.find(".share-doc-btn").hide(); } - this.parent.find(".share-doc-btn").on("click", () => { - this.frm.share_doc(); - }); + this.parent + .find(".share-doc-btn") + .off("click") + .on("click", () => { + this.frm.share_doc(); + }); this.shares.empty(); @@ -41,6 +44,8 @@ frappe.ui.form.Share = class Share { this.dialog = d; this.dirty = false; + $(d.body).html('

' + __("Loading...") + "

"); + frappe.call({ method: "frappe.share.get_users", args: { @@ -52,8 +57,6 @@ frappe.ui.form.Share = class Share { }, }); - $(d.body).html('

' + __("Loading...") + "

"); - d.onhide = function () { // reload comments if (me.dirty) me.frm.sidebar.reload_docinfo(); @@ -188,7 +191,6 @@ frappe.ui.form.Share = class Share { } me.dirty = true; - me.render_shared(); me.frm.shared.refresh(); }, }); diff --git a/frappe/public/js/frappe/ui/page.html b/frappe/public/js/frappe/ui/page.html index 06d1baec69..daea9fe03a 100644 --- a/frappe/public/js/frappe/ui/page.html +++ b/frappe/public/js/frappe/ui/page.html @@ -52,8 +52,8 @@
- \ No newline at end of file + diff --git a/frappe/query_builder/utils.py b/frappe/query_builder/utils.py index be0403a291..91cdfd0a54 100644 --- a/frappe/query_builder/utils.py +++ b/frappe/query_builder/utils.py @@ -104,9 +104,8 @@ def patch_query_execute(): # frame1: execute_query() # frame2: frame that called `query.run()` # - # if frame2 is server script it wont have a filename and hence + # if frame2 is server script is set as the filename # it shouldn't be allowed. - # p.s. stack() returns `""` as filename if not a file. pass else: raise frappe.PermissionError("Only SELECT SQL allowed in scripting") diff --git a/frappe/share.py b/frappe/share.py index 2556d1d484..2cdca9af91 100644 --- a/frappe/share.py +++ b/frappe/share.py @@ -82,7 +82,7 @@ def remove(doctype, name, user, flags=None): @frappe.whitelist() def set_permission(doctype, name, user, permission_to, value=1, everyone=0): """Expose function without flags to the client-side""" - set_docshare_permission(doctype, name, user, permission_to, value=value, everyone=everyone) + return set_docshare_permission(doctype, name, user, permission_to, value=value, everyone=everyone) def set_docshare_permission(doctype, name, user, permission_to, value=1, everyone=0, flags=None): diff --git a/frappe/tests/test_permissions.py b/frappe/tests/test_permissions.py index f48273135a..dbe3f20519 100644 --- a/frappe/tests/test_permissions.py +++ b/frappe/tests/test_permissions.py @@ -665,6 +665,16 @@ class TestPermissions(FrappeTestCase): doc = user.append("defaults") doc.check_permission() + # false due to missing parentfield + doc = user.append("roles") + doc.parentfield = None + self.assertRaises(frappe.PermissionError, doc.check_permission) + + # false due to invalid parentfield + doc = user.append("roles") + doc.parentfield = "first_name" + self.assertRaises(frappe.PermissionError, doc.check_permission) + # false by permlevel doc = user.append("roles") self.assertRaises(frappe.PermissionError, doc.check_permission) diff --git a/frappe/tests/test_safe_exec.py b/frappe/tests/test_safe_exec.py index fcd5832680..ad09aaea59 100644 --- a/frappe/tests/test_safe_exec.py +++ b/frappe/tests/test_safe_exec.py @@ -75,3 +75,10 @@ class TestSafeExec(FrappeTestCase): def test_unsafe_objects(self): unsafe_global = {"frappe": frappe} self.assertRaises(SyntaxError, safe_exec, """frappe.msgprint("Hello")""", unsafe_global) + + def test_attrdict(self): + # jinja + frappe.render_template("{% set my_dict = _dict() %} {{- my_dict.works -}}") + + # RestrictedPython + safe_exec("my_dict = _dict()") diff --git a/frappe/translations/de.csv b/frappe/translations/de.csv index fd81f8e87c..ffda3ca127 100644 --- a/frappe/translations/de.csv +++ b/frappe/translations/de.csv @@ -4828,3 +4828,4 @@ CSV Quoting,Anführungszeichen, CSV Preview,Vorschau, Non-numeric,Nicht-numerische, Minimal,Minimal, +This value is fetched from {0}'s {1} field,Dieser Wert ergibt sich aus dem Feld {1} von {0}, diff --git a/frappe/utils/jinja.py b/frappe/utils/jinja.py index 5e3a70554d..33bb929bc4 100644 --- a/frappe/utils/jinja.py +++ b/frappe/utils/jinja.py @@ -60,7 +60,7 @@ def validate_template(html): frappe.throw(frappe._("Syntax error in template")) -def render_template(template, context, is_path=None, safe_render=True): +def render_template(template, context=None, is_path=None, safe_render=True): """Render a template using Jinja :param template: path or HTML containing the jinja template @@ -76,6 +76,9 @@ def render_template(template, context, is_path=None, safe_render=True): if not template: return "" + if context is None: + context = {} + if is_path or guess_is_path(template): return get_jenv().get_template(template).render(context) else: diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py index 890f16a1c3..9e99754c67 100644 --- a/frappe/utils/safe_exec.py +++ b/frappe/utils/safe_exec.py @@ -8,6 +8,7 @@ from functools import lru_cache import RestrictedPython.Guards from RestrictedPython import compile_restricted, safe_globals +from RestrictedPython.transformer import RestrictingNodeTransformer import frappe import frappe.exceptions @@ -45,6 +46,14 @@ class NamespaceDict(frappe._dict): return ret +class FrappeTransformer(RestrictingNodeTransformer): + def check_name(self, node, name, *args, **kwargs): + if name == "_dict": + return + + return super().check_name(node, name, *args, **kwargs) + + def safe_exec(script, _globals=None, _locals=None, restrict_commit_rollback=False): # server scripts can be disabled via site_config.json # they are enabled by default @@ -69,7 +78,11 @@ def safe_exec(script, _globals=None, _locals=None, restrict_commit_rollback=Fals with safe_exec_flags(), patched_qb(): # execute script compiled by RestrictedPython - exec(compile_restricted(script), exec_globals, _locals) # pylint: disable=exec-used + exec( + compile_restricted(script, filename="", policy=FrappeTransformer), + exec_globals, + _locals, + ) return exec_globals, _locals @@ -106,6 +119,7 @@ def get_safe_globals(): as_json=frappe.as_json, dict=dict, log=frappe.log, + _dict=frappe._dict, args=form_dict, frappe=NamespaceDict( call=call_whitelisted_function, @@ -116,7 +130,6 @@ def get_safe_globals(): time_format=time_format, format_date=frappe.utils.data.global_date_format, form_dict=form_dict, - as_dict=frappe._dict, bold=frappe.bold, copy_doc=frappe.copy_doc, errprint=frappe.errprint, diff --git a/frappe/website/doctype/website_theme/website_theme.py b/frappe/website/doctype/website_theme/website_theme.py index ca78adf94e..7abfab93e3 100644 --- a/frappe/website/doctype/website_theme/website_theme.py +++ b/frappe/website/doctype/website_theme/website_theme.py @@ -62,10 +62,6 @@ class WebsiteTheme(Document): def generate_bootstrap_theme(self): from subprocess import PIPE, Popen - self.theme_scss = frappe.render_template( - "frappe/website/doctype/website_theme/website_theme_template.scss", self.as_dict() - ) - # create theme file in site public files folder folder_path = abspath(frappe.utils.get_files_path("website_theme", is_private=False)) # create folder if not exist