diff --git a/.github/helper/semgrep_rules/README.md b/.github/helper/semgrep_rules/README.md new file mode 100644 index 0000000000..670d8d280f --- /dev/null +++ b/.github/helper/semgrep_rules/README.md @@ -0,0 +1,38 @@ +# Semgrep linting + +## What is semgrep? +Semgrep or "semantic grep" is language agnostic static analysis tool. In simple terms semgrep is syntax-aware `grep`, so unlike regex it doesn't get confused by different ways of writing same thing or whitespaces or code split in multiple lines etc. + +Example: + +To check if a translate function is using f-string or not the regex would be `r"_\(\s*f[\"']"` while equivalent rule in semgrep would be `_(f"...")`. As semgrep knows grammer of language it takes care of unnecessary whitespace, type of quotation marks etc. + +You can read more such examples in `.github/helper/semgrep_rules` directory. + +# Why/when to use this? +We want to maintain quality of contributions, at the same time remembering all the good practices can be pain to deal with while evaluating contributions. Using semgrep if you can translate "best practice" into a rule then it can automate the task for us. + +## Running locally + +Install semgrep using homebrew `brew install semgrep` or pip `pip install semgrep`. + +To run locally use following command: + +`semgrep --config=.github/helper/semgrep_rules [file/folder names]` + +## Testing +semgrep allows testing the tests. Refer to this page: https://semgrep.dev/docs/writing-rules/testing-rules/ + +When writing new rules you should write few positive and few negative cases as shown in the guide and current tests. + +To run current tests: `semgrep --test --test-ignore-todo .github/helper/semgrep_rules` + + +## Reference + +If you are new to Semgrep read following pages to get started on writing/modifying rules: + +- https://semgrep.dev/docs/getting-started/ +- https://semgrep.dev/docs/writing-rules/rule-syntax +- https://semgrep.dev/docs/writing-rules/pattern-examples/ +- https://semgrep.dev/docs/writing-rules/rule-ideas/#common-use-cases diff --git a/.github/helper/semgrep_rules/security.py b/.github/helper/semgrep_rules/security.py new file mode 100644 index 0000000000..f477d7c176 --- /dev/null +++ b/.github/helper/semgrep_rules/security.py @@ -0,0 +1,6 @@ +def function_name(input): + # ruleid: frappe-codeinjection-eval + eval(input) + +# ok: frappe-codeinjection-eval +eval("1 + 1") diff --git a/.github/helper/semgrep_rules/security.yml b/.github/helper/semgrep_rules/security.yml new file mode 100644 index 0000000000..1937fc0e52 --- /dev/null +++ b/.github/helper/semgrep_rules/security.yml @@ -0,0 +1,14 @@ +rules: +- id: frappe-codeinjection-eval + patterns: + - pattern-not: eval("...") + - pattern: eval(...) + message: | + Detected the use of eval(). eval() can be dangerous if used to evaluate + dynamic content. Avoid it or use safe_eval(). + languages: [python] + severity: ERROR + paths: + exclude: + - frappe/__init__.py + - frappe/commands/utils.py diff --git a/.github/helper/semgrep_rules/translate.js b/.github/helper/semgrep_rules/translate.js new file mode 100644 index 0000000000..7b92fe2dff --- /dev/null +++ b/.github/helper/semgrep_rules/translate.js @@ -0,0 +1,37 @@ +// ruleid: frappe-translation-empty-string +__("") +// ruleid: frappe-translation-empty-string +__('') + +// ok: frappe-translation-js-formatting +__('Welcome {0}, get started with ERPNext in just a few clicks.', [full_name]); + +// ruleid: frappe-translation-js-formatting +__(`Welcome ${full_name}, get started with ERPNext in just a few clicks.`); + +// ok: frappe-translation-js-formatting +__('This is fine'); + + +// ok: frappe-translation-trailing-spaces +__('This is fine'); + +// ruleid: frappe-translation-trailing-spaces +__(' this is not ok '); +// ruleid: frappe-translation-trailing-spaces +__('this is not ok '); +// ruleid: frappe-translation-trailing-spaces +__(' this is not ok'); + +// ok: frappe-translation-js-splitting +__('You have {0} subscribers in your mailing list.', [subscribers.length]) + +// todoruleid: frappe-translation-js-splitting +__('You have') + subscribers.length + __('subscribers in your mailing list.') + +// ruleid: frappe-translation-js-splitting +__('You have' + 'subscribers in your mailing list.') + +// ruleid: frappe-translation-js-splitting +__('You have {0} subscribers' + + 'in your mailing list', [subscribers.length]) diff --git a/.github/helper/semgrep_rules/translate.py b/.github/helper/semgrep_rules/translate.py new file mode 100644 index 0000000000..bd6cd9126c --- /dev/null +++ b/.github/helper/semgrep_rules/translate.py @@ -0,0 +1,53 @@ +# Examples taken from https://frappeframework.com/docs/user/en/translations +# This file is used for testing the tests. + +from frappe import _ + +full_name = "Jon Doe" +# ok: frappe-translation-python-formatting +_('Welcome {0}, get started with ERPNext in just a few clicks.').format(full_name) + +# ruleid: frappe-translation-python-formatting +_('Welcome %s, get started with ERPNext in just a few clicks.' % full_name) +# ruleid: frappe-translation-python-formatting +_('Welcome %(name)s, get started with ERPNext in just a few clicks.' % {'name': full_name}) + +# ruleid: frappe-translation-python-formatting +_('Welcome {0}, get started with ERPNext in just a few clicks.'.format(full_name)) + + +subscribers = ["Jon", "Doe"] +# ok: frappe-translation-python-formatting +_('You have {0} subscribers in your mailing list.').format(len(subscribers)) + +# ruleid: frappe-translation-python-splitting +_('You have') + len(subscribers) + _('subscribers in your mailing list.') + +# ruleid: frappe-translation-python-splitting +_('You have {0} subscribers \ + in your mailing list').format(len(subscribers)) + +# ok: frappe-translation-python-splitting +_('You have {0} subscribers') \ + + 'in your mailing list' + +# ruleid: frappe-translation-trailing-spaces +msg = _(" You have {0} pending invoice ") +# ruleid: frappe-translation-trailing-spaces +msg = _("You have {0} pending invoice ") +# ruleid: frappe-translation-trailing-spaces +msg = _(" You have {0} pending invoice") + +# ok: frappe-translation-trailing-spaces +msg = ' ' + _("You have {0} pending invoices") + ' ' + +# ruleid: frappe-translation-python-formatting +_(f"can not format like this - {subscribers}") +# ruleid: frappe-translation-python-splitting +_(f"what" + f"this is also not cool") + + +# ruleid: frappe-translation-empty-string +_("") +# ruleid: frappe-translation-empty-string +_('') diff --git a/.github/helper/semgrep_rules/translate.yml b/.github/helper/semgrep_rules/translate.yml new file mode 100644 index 0000000000..3737da5a7e --- /dev/null +++ b/.github/helper/semgrep_rules/translate.yml @@ -0,0 +1,63 @@ +rules: +- id: frappe-translation-empty-string + pattern-either: + - pattern: _("") + - pattern: __("") + message: | + Empty string is useless for translation. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python, javascript, json] + severity: ERROR + +- id: frappe-translation-trailing-spaces + pattern-either: + - pattern: _("=~/(^[ \t]+|[ \t]+$)/") + - pattern: __("=~/(^[ \t]+|[ \t]+$)/") + message: | + Trailing or leading whitespace not allowed in translate strings. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python, javascript, json] + severity: ERROR + +- id: frappe-translation-python-formatting + pattern-either: + - pattern: _("..." % ...) + - pattern: _("...".format(...)) + - pattern: _(f"...") + message: | + Only positional formatters are allowed and formatting should not be done before translating. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python] + severity: ERROR + +- id: frappe-translation-js-formatting + patterns: + - pattern: __(`...`) + - pattern-not: __("...") + message: | + Template strings are not allowed for text formatting. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [javascript, json] + severity: ERROR + +- id: frappe-translation-python-splitting + pattern-either: + - pattern: _(...) + ... + _(...) + - pattern: _("..." + "...") + - pattern-regex: '_\([^\)]*\\\s*' + message: | + Do not split strings inside translate function. Do not concatenate using translate functions. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [python] + severity: ERROR + +- id: frappe-translation-js-splitting + pattern-either: + - pattern-regex: '__\([^\)]*[\+\\]\s*' + - pattern: __('...' + '...') + - pattern: __('...') + __('...') + message: | + Do not split strings inside translate function. Do not concatenate using translate functions. + Please refer: https://frappeframework.com/docs/user/en/translations + languages: [javascript, json] + severity: ERROR diff --git a/.github/stale.yml b/.github/stale.yml index dd1ab9e9e7..2d776759e4 100644 --- a/.github/stale.yml +++ b/.github/stale.yml @@ -1,7 +1,7 @@ # Configuration for probot-stale - https://github.com/probot/stale # Number of days of inactivity before an Issue or Pull Request becomes stale -daysUntilStale: 10 +daysUntilStale: 7 # Number of days of inactivity before a stale Issue or Pull Request is closed. # Set to false to disable. If disabled, issues still need to be closed manually, but will remain marked as stale. @@ -28,7 +28,7 @@ markComment: > you can always reopen the PR when you're ready. Thank you for contributing. # Limit the number of actions per hour, from 1-30. Default is 30 -limitPerRun: 30 +limitPerRun: 10 # Limit to only `issues` or `pulls` only: pulls diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 665e7b6c10..bfe2002f69 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -1,6 +1,10 @@ name: CI -on: [pull_request, workflow_dispatch, push] +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + workflow_dispatch: + push: jobs: test: @@ -101,6 +105,7 @@ jobs: ${{ runner.os }}-yarn- - name: Cache cypress binary + if: matrix.TYPE == 'ui' uses: actions/cache@v2 with: path: ~/.cache @@ -129,6 +134,10 @@ jobs: DB: ${{ matrix.DB }} TYPE: ${{ matrix.TYPE }} + - name: Setup tmate session + if: contains(github.event.pull_request.labels.*.name, 'debug-gha') + uses: mxschmitt/action-tmate@v3 + - name: Run Tests run: cd ~/frappe-bench/ && ${{ matrix.RUN_COMMAND }} env: diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml index 321dfb567b..1d5694f521 100644 --- a/.github/workflows/semgrep.yml +++ b/.github/workflows/semgrep.yml @@ -19,4 +19,4 @@ jobs: python -m pip install -q semgrep git fetch origin $GITHUB_BASE_REF:$GITHUB_BASE_REF -q files=$(git diff --name-only --diff-filter=d $GITHUB_BASE_REF) - if [ -f .semgrep.yml ]; then semgrep --config=.semgrep.yml --quiet --error $files; fi + [[ -d .github/helper/semgrep_rules ]] && semgrep --config=.github/helper/semgrep_rules --quiet --error $files diff --git a/.semgrep.yml b/.semgrep.yml deleted file mode 100644 index 99d237251e..0000000000 --- a/.semgrep.yml +++ /dev/null @@ -1,29 +0,0 @@ -#Reference: https://semgrep.dev/docs/writing-rules/rule-syntax/ - -rules: -- id: eval - patterns: - - pattern-not: eval("...") - - pattern: eval(...) - message: | - Detected the use of eval(). eval() can be dangerous if used to evaluate - dynamic content. Avoid it or use safe_eval(). - languages: - - python - severity: ERROR - -# translations -- id: frappe-translation-syntax-python - pattern-either: - - pattern: _(f"...") # f-strings not allowed - - pattern: _("..." + "...") # concatenation not allowed - - pattern: _("") # empty string is meaningless - - pattern: _("..." % ...) # Only positional formatters are allowed. - - pattern: _("...".format(...)) # format should not be used before translating - - pattern: _("...") + ... + _("...") # don't split strings - message: | - Incorrect use of translation function detected. - Please refer: https://frappeframework.com/docs/user/en/translations - languages: - - python - severity: ERROR diff --git a/cypress/integration/table_multiselect.js b/cypress/integration/table_multiselect.js index bdcf5d1ff0..faa72d63a5 100644 --- a/cypress/integration/table_multiselect.js +++ b/cypress/integration/table_multiselect.js @@ -45,6 +45,6 @@ context('Table MultiSelect', () => { cy.get(`.list-subject:contains("table multiselect")`).last().find('a').click(); cy.get('.frappe-control[data-fieldname="users"] .form-control .tb-selected-value').as('existing_value'); cy.get('@existing_value').find('.btn-link-to-form').click(); - cy.location('pathname').should('contain', '/user/test%40erpnext.com'); + cy.location('pathname').should('contain', '/user/test@erpnext.com'); }); }); diff --git a/frappe/__init__.py b/frappe/__init__.py index 871d1b9e92..844a9238e3 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -854,8 +854,8 @@ def get_meta_module(doctype): import frappe.modules return frappe.modules.load_doctype_module(doctype) -def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, - for_reload=False, ignore_permissions=False, flags=None, ignore_on_trash=False, ignore_missing=True): +def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, for_reload=False, + ignore_permissions=False, flags=None, ignore_on_trash=False, ignore_missing=True, delete_permanently=False): """Delete a document. Calls `frappe.model.delete_doc.delete_doc`. :param doctype: DocType of document to be delete. @@ -863,10 +863,11 @@ def delete_doc(doctype=None, name=None, force=0, ignore_doctypes=None, :param force: Allow even if document is linked. Warning: This may lead to data integrity errors. :param ignore_doctypes: Ignore if child table is one of these. :param for_reload: Call `before_reload` trigger before deleting. - :param ignore_permissions: Ignore user permissions.""" + :param ignore_permissions: Ignore user permissions. + :param delete_permanently: Do not create a Deleted Document for the document.""" import frappe.model.delete_doc frappe.model.delete_doc.delete_doc(doctype, name, force, ignore_doctypes, for_reload, - ignore_permissions, flags, ignore_on_trash, ignore_missing) + ignore_permissions, flags, ignore_on_trash, ignore_missing, delete_permanently) def delete_doc_if_exists(doctype, name, force=0): """Delete document if exists.""" diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index cbcfa350f5..c0a82c594a 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -67,7 +67,7 @@ class DocType(Document): self.scrub_field_names() self.set_default_in_list_view() self.set_default_translatable() - self.validate_series() + validate_series(self) self.validate_document_type() validate_fields(self) @@ -238,44 +238,6 @@ class DocType(Document): # unique is automatically an index if d.unique: d.search_index = 0 - def validate_series(self, autoname=None, name=None): - """Validate if `autoname` property is correctly set.""" - if not autoname: autoname = self.autoname - if not name: name = self.name - - if not autoname and self.get("fields", {"fieldname":"naming_series"}): - self.autoname = "naming_series:" - elif self.autoname == "naming_series:" and not self.get("fields", {"fieldname":"naming_series"}): - frappe.throw(_("Invalid fieldname '{0}' in autoname").format(self.autoname)) - - # validate field name if autoname field:fieldname is used - # Create unique index on autoname field automatically. - if autoname and autoname.startswith('field:'): - field = autoname.split(":")[1] - if not field or field not in [ df.fieldname for df in self.fields ]: - frappe.throw(_("Invalid fieldname '{0}' in autoname").format(field)) - else: - for df in self.fields: - if df.fieldname == field: - df.unique = 1 - break - - if autoname and (not autoname.startswith('field:')) \ - and (not autoname.startswith('eval:')) \ - and (not autoname.lower() in ('prompt', 'hash')) \ - and (not autoname.startswith('naming_series:')) \ - and (not autoname.startswith('format:')): - - prefix = autoname.split('.')[0] - used_in = frappe.db.sql(""" - SELECT `name` - FROM `tabDocType` - WHERE `autoname` LIKE CONCAT(%s, '.%%') - AND `name`!=%s - """, (prefix, name)) - if used_in: - frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) - def on_update(self): """Update database schema, make controller templates if `custom` is not set and clear cache.""" try: @@ -666,6 +628,46 @@ class DocType(Document): validate_route_conflict(self.doctype, self.name) +def validate_series(dt, autoname=None, name=None): + """Validate if `autoname` property is correctly set.""" + if not autoname: + autoname = dt.autoname + if not name: + name = dt.name + + if not autoname and dt.get("fields", {"fieldname":"naming_series"}): + dt.autoname = "naming_series:" + elif dt.autoname == "naming_series:" and not dt.get("fields", {"fieldname":"naming_series"}): + frappe.throw(_("Invalid fieldname '{0}' in autoname").format(dt.autoname)) + + # validate field name if autoname field:fieldname is used + # Create unique index on autoname field automatically. + if autoname and autoname.startswith('field:'): + field = autoname.split(":")[1] + if not field or field not in [df.fieldname for df in dt.fields]: + frappe.throw(_("Invalid fieldname '{0}' in autoname").format(field)) + else: + for df in dt.fields: + if df.fieldname == field: + df.unique = 1 + break + + if autoname and (not autoname.startswith('field:')) \ + and (not autoname.startswith('eval:')) \ + and (not autoname.lower() in ('prompt', 'hash')) \ + and (not autoname.startswith('naming_series:')) \ + and (not autoname.startswith('format:')): + + prefix = autoname.split('.')[0] + used_in = frappe.db.sql(""" + SELECT `name` + FROM `tabDocType` + WHERE `autoname` LIKE CONCAT(%s, '.%%') + AND `name`!=%s + """, (prefix, name)) + if used_in: + frappe.throw(_("Series {0} already used in {1}").format(prefix, used_in[0][0])) + def validate_links_table_fieldnames(meta): """Validate fieldnames in Links table""" if frappe.flags.in_patch: return diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py index c237b8e436..0cf38508b8 100755 --- a/frappe/core/doctype/file/file.py +++ b/frappe/core/doctype/file/file.py @@ -718,7 +718,7 @@ def delete_file(path): os.remove(path) -def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False): +def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_delete=False, delete_permanently=False): """Remove file and File entry""" file_name = None if not (attached_to_doctype and attached_to_name): @@ -736,7 +736,7 @@ def remove_file(fid=None, attached_to_doctype=None, attached_to_name=None, from_ if not file_name: file_name = frappe.db.get_value("File", fid, "file_name") comment = doc.add_comment("Attachment Removed", _("Removed {0}").format(file_name)) - frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions) + frappe.delete_doc("File", fid, ignore_permissions=ignore_permissions, delete_permanently=delete_permanently) return comment @@ -745,17 +745,18 @@ def get_max_file_size(): return cint(conf.get('max_file_size')) or 10485760 -def remove_all(dt, dn, from_delete=False): +def remove_all(dt, dn, from_delete=False, delete_permanently=False): """remove all files in a transaction""" try: for fid in frappe.db.sql_list("""select name from `tabFile` where attached_to_doctype=%s and attached_to_name=%s""", (dt, dn)): if from_delete: # If deleting a doc, directly delete files - frappe.delete_doc("File", fid, ignore_permissions=True) + frappe.delete_doc("File", fid, ignore_permissions=True, delete_permanently=delete_permanently) else: # Removes file and adds a comment in the document it is attached to - remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn, from_delete=from_delete) + remove_file(fid=fid, attached_to_doctype=dt, attached_to_name=dn, + from_delete=from_delete, delete_permanently=delete_permanently) except Exception as e: if e.args[0]!=1054: raise # (temp till for patched) diff --git a/frappe/core/doctype/prepared_report/prepared_report.py b/frappe/core/doctype/prepared_report/prepared_report.py index 1d0d6ebb09..e947cee8ed 100644 --- a/frappe/core/doctype/prepared_report/prepared_report.py +++ b/frappe/core/doctype/prepared_report/prepared_report.py @@ -24,8 +24,6 @@ class PreparedReport(Document): def enqueue_report(self): enqueue(run_background, prepared_report=self.name, timeout=6000) - def on_trash(self): - remove_all("Prepared Report", self.name) def run_background(prepared_report): @@ -100,7 +98,7 @@ def delete_expired_prepared_reports(): def delete_prepared_reports(reports): reports = frappe.parse_json(reports) for report in reports: - frappe.delete_doc('Prepared Report', report['name'], ignore_permissions=True) + frappe.delete_doc('Prepared Report', report['name'], ignore_permissions=True, delete_permanently=True) def create_json_gz_file(data, dt, dn): # Storing data in CSV file causes information loss diff --git a/frappe/core/doctype/system_settings/system_settings.json b/frappe/core/doctype/system_settings/system_settings.json index 13dbc32620..7116cd908c 100644 --- a/frappe/core/doctype/system_settings/system_settings.json +++ b/frappe/core/doctype/system_settings/system_settings.json @@ -1,5 +1,7 @@ { "actions": [], + "allow_read": 1, + "allow_workflow": 1, "creation": "2014-04-17 16:53:52.640856", "doctype": "DocType", "document_type": "System", @@ -67,8 +69,7 @@ "enable_prepared_report_auto_deletion", "prepared_report_expiry_period", "chat", - "enable_chat", - "use_socketio_to_upload_file" + "enable_chat" ], "fields": [ { @@ -394,12 +395,6 @@ "fieldtype": "Check", "label": "Enable Chat" }, - { - "default": "1", - "fieldname": "use_socketio_to_upload_file", - "fieldtype": "Check", - "label": "Use socketio to upload file" - }, { "fieldname": "column_break_21", "fieldtype": "Column Break" @@ -446,7 +441,7 @@ { "default": "30", "depends_on": "enable_prepared_report_auto_deletion", - "description": "System will automatically delete Prepared Reports after these many days since creation", + "description": "System will auto-delete Prepared Reports permanently after these many days since creation", "fieldname": "prepared_report_expiry_period", "fieldtype": "Int", "label": "Prepared Report Expiry Period (Days)" @@ -469,13 +464,6 @@ "fieldtype": "Data", "label": "App Name" }, - { - "default": "3", - "description": "Hourly rate limit for generating password reset links", - "fieldname": "password_reset_limit", - "fieldtype": "Int", - "label": "Password Reset Link Generation Limit" - }, { "default": "1", "fieldname": "strip_exif_metadata_from_uploaded_images", @@ -486,7 +474,7 @@ "icon": "fa fa-cog", "issingle": 1, "links": [], - "modified": "2020-12-30 18:52:22.161391", + "modified": "2021-03-25 17:54:32.668876", "modified_by": "Administrator", "module": "Core", "name": "System Settings", @@ -504,4 +492,4 @@ "sort_field": "modified", "sort_order": "ASC", "track_changes": 1 -} +} \ No newline at end of file diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index c103ad7e4a..c7bc6c43c0 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -534,24 +534,36 @@ class User(Document): @classmethod def find_by_credentials(cls, user_name: str, password: str, validate_password: bool = True): """Find the user by credentials. - """ - login_with_mobile = cint(frappe.db.get_value("System Settings", "System Settings", "allow_login_using_mobile_number")) - filter = {"mobile_no": user_name} if login_with_mobile else {"name": user_name} - user = frappe.db.get_value("User", filters=filter, fieldname=['name', 'enabled'], as_dict=True) or {} - if not user: + This is a login utility that needs to check login related system settings while finding the user. + 1. Find user by email ID by default + 2. If allow_login_using_mobile_number is set, you can use mobile number while finding the user. + 3. If allow_login_using_user_name is set, you can use username while finding the user. + """ + + login_with_mobile = cint(frappe.db.get_value("System Settings", "System Settings", "allow_login_using_mobile_number")) + login_with_username = cint(frappe.db.get_value("System Settings", "System Settings", "allow_login_using_user_name")) + + or_filters = [{"name": user_name}] + if login_with_mobile: + or_filters.append({"mobile_no": user_name}) + if login_with_username: + or_filters.append({"username": user_name}) + + users = frappe.db.get_all('User', fields=['name', 'enabled'], or_filters=or_filters, limit=1) + if not users: return + user = users[0] user['is_authenticated'] = True if validate_password: try: - check_password(user_name, password) + check_password(user['name'], password) except frappe.AuthenticationError: user['is_authenticated'] = False return user - @frappe.whitelist() def get_timezones(): import pytz @@ -863,11 +875,12 @@ def reset_password(user): @frappe.whitelist() @frappe.validate_and_sanitize_search_inputs def user_query(doctype, txt, searchfield, start, page_len, filters): - from frappe.desk.reportview import get_match_cond - + from frappe.desk.reportview import get_match_cond, get_filters_cond + conditions=[] user_type_condition = "and user_type = 'System User'" if filters and filters.get('ignore_user_type'): user_type_condition = '' + filters.pop('ignore_user_type') txt = "%{}%".format(txt) return frappe.db.sql("""SELECT `name`, CONCAT_WS(' ', first_name, middle_name, last_name) @@ -878,17 +891,22 @@ def user_query(doctype, txt, searchfield, start, page_len, filters): AND `name` NOT IN ({standard_users}) AND ({key} LIKE %(txt)s OR CONCAT_WS(' ', first_name, middle_name, last_name) LIKE %(txt)s) - {mcond} + {fcond} {mcond} ORDER BY CASE WHEN `name` LIKE %(txt)s THEN 0 ELSE 1 END, CASE WHEN concat_ws(' ', first_name, middle_name, last_name) LIKE %(txt)s THEN 0 ELSE 1 END, NAME asc - LIMIT %(page_len)s OFFSET %(start)s""".format( + LIMIT %(page_len)s OFFSET %(start)s + """.format( user_type_condition = user_type_condition, standard_users=", ".join([frappe.db.escape(u) for u in STANDARD_USERS]), - key=searchfield, mcond=get_match_cond(doctype)), - dict(start=start, page_len=page_len, txt=txt)) + key=searchfield, + fcond=get_filters_cond(doctype, filters, conditions), + mcond=get_match_cond(doctype) + ), + dict(start=start, page_len=page_len, txt=txt) + ) def get_total_users(): """Returns total no. of system users""" diff --git a/frappe/core/doctype/user_permission/test_user_permission.py b/frappe/core/doctype/user_permission/test_user_permission.py index 7e0b4a49c6..2e9b832acc 100644 --- a/frappe/core/doctype/user_permission/test_user_permission.py +++ b/frappe/core/doctype/user_permission/test_user_permission.py @@ -2,8 +2,9 @@ # Copyright (c) 2017, Frappe Technologies and Contributors # See license.txt from __future__ import unicode_literals -from frappe.core.doctype.user_permission.user_permission import add_user_permissions +from frappe.core.doctype.user_permission.user_permission import add_user_permissions, remove_applicable from frappe.permissions import has_user_permission +from frappe.core.doctype.doctype.test_doctype import new_doctype import frappe import unittest @@ -17,6 +18,8 @@ class TestUserPermission(unittest.TestCase): 'nested_doc_user@example.com')""") frappe.delete_doc_if_exists("DocType", "Person") frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabPerson`") + frappe.delete_doc_if_exists("DocType", "Doc A") + frappe.db.sql_ddl("DROP TABLE IF EXISTS `tabDoc A`") def test_default_user_permission_validation(self): user = create_user('test_default_permission@example.com') @@ -153,16 +156,98 @@ class TestUserPermission(unittest.TestCase): self.assertTrue(has_user_permission(frappe.get_doc("Person", parent_record.name), user.name)) self.assertFalse(has_user_permission(frappe.get_doc("Person", child_record.name), user.name)) -def create_user(email, role="System Manager"): + def test_user_perm_on_new_doc_with_field_default(self): + """Test User Perm impact on frappe.new_doc. with *field* default value""" + frappe.set_user('Administrator') + user = create_user("new_doc_test@example.com", "Blogger") + + # make a doctype "Doc A" with 'doctype' link field and default value ToDo + if not frappe.db.exists("DocType", "Doc A"): + doc = new_doctype("Doc A", + fields=[ + { + "label": "DocType", + "fieldname": "doc", + "fieldtype": "Link", + "options": "DocType", + "default": "ToDo" + } + ], unique=0) + doc.insert() + + # make User Perm on DocType 'ToDo' in Assignment Rule (unrelated doctype) + add_user_permissions(get_params(user, "DocType", "ToDo", applicable=["Assignment Rule"])) + frappe.set_user("new_doc_test@example.com") + + new_doc = frappe.new_doc("Doc A") + + # User perm is created on ToDo but for doctype Assignment Rule only + # it should not have impact on Doc A + self.assertEquals(new_doc.doc, "ToDo") + + frappe.set_user('Administrator') + remove_applicable(["Assignment Rule"], "new_doc_test@example.com", "DocType", "ToDo") + + def test_user_perm_on_new_doc_with_user_default(self): + """Test User Perm impact on frappe.new_doc. with *user* default value""" + from frappe.core.doctype.session_default_settings.session_default_settings import (clear_session_defaults, + set_session_default_values) + + frappe.set_user('Administrator') + user = create_user("user_default_test@example.com", "Blogger") + + # make a doctype "Doc A" with 'doctype' link field + if not frappe.db.exists("DocType", "Doc A"): + doc = new_doctype("Doc A", + fields=[ + { + "label": "DocType", + "fieldname": "doc", + "fieldtype": "Link", + "options": "DocType", + } + ], unique=0) + doc.insert() + + # create a 'DocType' session default field + if not frappe.db.exists("Session Default", {"ref_doctype": "DocType"}): + settings = frappe.get_single('Session Default Settings') + settings.append("session_defaults", { + "ref_doctype": "DocType" + }) + settings.save() + + # make User Perm on DocType 'ToDo' in Assignment Rule (unrelated doctype) + add_user_permissions(get_params(user, "DocType", "ToDo", applicable=["Assignment Rule"])) + + # User default Doctype value is ToDo via Session Defaults + frappe.set_user("user_default_test@example.com") + set_session_default_values({"doc": "ToDo"}) + + new_doc = frappe.new_doc("Doc A") + + # User perm is created on ToDo but for doctype Assignment Rule only + # it should not have impact on Doc A + self.assertEquals(new_doc.doc, "ToDo") + + frappe.set_user('Administrator') + clear_session_defaults() + remove_applicable(["Assignment Rule"], "user_default_test@example.com", "DocType", "ToDo") + +def create_user(email, *roles): ''' create user with role system manager ''' if frappe.db.exists('User', email): return frappe.get_doc('User', email) - else: - user = frappe.new_doc('User') - user.email = email - user.first_name = email.split("@")[0] - user.add_roles(role) - return user + + user = frappe.new_doc('User') + user.email = email + user.first_name = email.split("@")[0] + + if not roles: + roles = ('System Manager',) + + user.add_roles(*roles) + return user def get_params(user, doctype, docname, is_default=0, hide_descendants=0, applicable=None): ''' Return param to insert ''' diff --git a/frappe/core/doctype/version/test_version.py b/frappe/core/doctype/version/test_version.py index 97aa69fd9c..51b3c21f58 100644 --- a/frappe/core/doctype/version/test_version.py +++ b/frappe/core/doctype/version/test_version.py @@ -9,6 +9,7 @@ from frappe.core.doctype.version.version import get_diff class TestVersion(unittest.TestCase): def test_get_diff(self): + frappe.set_user('Administrator') test_records = make_test_objects('Event', reset = True) old_doc = frappe.get_doc("Event", test_records[0]) new_doc = copy.deepcopy(old_doc) diff --git a/frappe/custom/doctype/customize_form/customize_form.json b/frappe/custom/doctype/customize_form/customize_form.json index ff102b3c08..77f62b3ec3 100644 --- a/frappe/custom/doctype/customize_form/customize_form.json +++ b/frappe/custom/doctype/customize_form/customize_form.json @@ -23,6 +23,8 @@ "allow_import", "fields_section_break", "fields", + "naming_section", + "autoname", "view_settings_section", "title_field", "image_field", @@ -261,6 +263,18 @@ "fieldtype": "Table", "label": "Actions", "options": "DocType Action" + }, + { + "collapsible": 1, + "fieldname": "naming_section", + "fieldtype": "Section Break", + "label": "Naming" + }, + { + "description": "Naming Options:\n