From 299831d209b0568efcecf80df09aee5e84c21161 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 16 Nov 2022 19:24:12 +0530 Subject: [PATCH 001/136] fix: server method to return evaluated dict of perms for a document --- frappe/client.py | 9 +++++++++ .../js/frappe/views/kanban/kanban_board.bundle.js | 10 ++++++---- frappe/public/js/frappe/views/kanban/kanban_view.js | 1 + 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index 404617b68c..afbb0df5ee 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -302,6 +302,15 @@ def has_permission(doctype, docname, perm_type="read"): # perm_type can be one of read, write, create, submit, cancel, report return {"has_permission": frappe.has_permission(doctype, perm_type.lower(), docname)} +@frappe.whitelist() +def get_doc_permissions(doctype, docname): + """Returns an evaluated document permissions dict like `{"read":1, "write":1}` + + :param doctype: DocType of the document to be evaluated + :param docname: `name` of the document to be evaluated + """ + doc = frappe.get_doc(doctype, docname) + return {"permissions": frappe.permissions.get_doc_permissions(doc)} @frappe.whitelist() def get_password(doctype, name, fieldname): diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index 11e7fba198..971081f5b9 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -333,12 +333,13 @@ frappe.provide("frappe.views"); if (self.$kanban_board.length === 0) { self.$kanban_board = $(frappe.render_template("kanban_board")); + // add column self.$kanban_board.appendTo(self.wrapper); } self.$filter_area = self.cur_list.$page.find(".active-tag-filters"); bind_events(); - setup_sortable(); + setup_sortable(); // column } function make_columns() { @@ -355,7 +356,7 @@ frappe.provide("frappe.views"); bind_clickdrag(); } - function setup_sortable() { + function setup_sortable() { // drag column var sortable = new Sortable(self.$kanban_board.get(0), { group: "columns", animation: 150, @@ -518,7 +519,7 @@ frappe.provide("frappe.views"); function init() { make_dom(); - setup_sortable(); + // setup_sortable(); // drag card make_cards(); store.watch((state, getters) => { return state.cards; @@ -535,6 +536,7 @@ frappe.provide("frappe.views"); indicator: frappe.scrub(column.indicator, "-"), }) ).appendTo(wrapper); + // add task, archive self.$kanban_cards = self.$kanban_column.find(".kanban-cards"); } @@ -564,7 +566,7 @@ frappe.provide("frappe.views"); } } - function setup_sortable() { + function setup_sortable() { // drag card Sortable.create(self.$kanban_cards.get(0), { group: "cards", animation: 150, diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 7eccd7ebf8..f1be1d2849 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -56,6 +56,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { this.card_meta = this.get_card_meta(); this.page_length = 0; + // frappe run serially get/set perms > push menu items > get_board this.menu_items.push( ...[ { From 8d31e702e82e9e9d204908b16bd997595bbfb956 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 17 Nov 2022 18:46:45 +0530 Subject: [PATCH 002/136] fix: Kanban Board Menu Items Accessibility via perms - Save Filters and Delete Board btns render if 'write' and 'delete' perms exist on Kanban Board - `Create Kanban Board` renders only if 'create' perms exist on Kanban Board - Bind board permisions to `board_perms` property of board object - Get perms via backend call at board initialization, as frontend does not have document object (only doctype meta and perms are cached) --- .../public/js/frappe/list/list_view_select.js | 16 +++-- .../views/kanban/kanban_board.bundle.js | 2 +- .../js/frappe/views/kanban/kanban_view.js | 68 ++++++++++++------- 3 files changed, 55 insertions(+), 31 deletions(-) diff --git a/frappe/public/js/frappe/list/list_view_select.js b/frappe/public/js/frappe/list/list_view_select.js index 5265ace340..daa16432b5 100644 --- a/frappe/public/js/frappe/list/list_view_select.js +++ b/frappe/public/js/frappe/list/list_view_select.js @@ -191,12 +191,16 @@ frappe.views.ListViewSelect = class ListViewSelect { ); }); - this.page.add_custom_menu_item( - kanban_switcher, - __("Create New Kanban Board"), - () => frappe.views.KanbanView.show_kanban_dialog(this.doctype), - true - ); + let perms = this.list_view.board_perms; + let can_create = perms ? perms.create : true; + if (can_create) { + this.page.add_custom_menu_item( + kanban_switcher, + __("Create New Kanban Board"), + () => frappe.views.KanbanView.show_kanban_dialog(this.doctype), + true + ); + } } get_page_name() { diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index 971081f5b9..ae11c0fdc0 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -519,7 +519,7 @@ frappe.provide("frappe.views"); function init() { make_dom(); - // setup_sortable(); // drag card + setup_sortable(); // drag card make_cards(); store.watch((state, getters) => { return state.cards; diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index f1be1d2849..506d4ee1b2 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -56,33 +56,53 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { this.card_meta = this.get_card_meta(); this.page_length = 0; - // frappe run serially get/set perms > push menu items > get_board - this.menu_items.push( - ...[ - { - label: __("Save filters"), - action: () => { - this.save_kanban_board_filters(); - }, - }, - { - label: __("Delete Kanban Board"), - action: () => { - frappe.confirm("Are you sure you want to proceed?", () => { - frappe.db.delete_doc("Kanban Board", this.board_name).then(() => { - frappe.show_alert(`Kanban Board ${this.board_name} deleted.`); - frappe.set_route("List", this.doctype, "List"); - }); - }); - }, - }, - ] - ); - - return this.get_board(); + return frappe.run_serially([ + () => this.set_board_perms_and_push_menu_items(), + () => this.get_board(), + ]); }); } + set_board_perms_and_push_menu_items() { + // needs server-side call as client-side document instance is absent before kanban render + return frappe.call({ + method: "frappe.client.get_doc_permissions", + args: { + doctype: "Kanban Board", + docname: this.board_name + }, + callback: (result) => { + this.board_perms = result.message.permissions || {}; + this.push_menu_items(); + }, + }); + } + + push_menu_items() { + if (this.board_perms.write) { + this.menu_items.push({ + label: __("Save filters"), + action: () => { + this.save_kanban_board_filters(); + } + }); + } + + if (this.board_perms.delete) { + this.menu_items.push({ + label: __("Delete Kanban Board"), + action: () => { + frappe.confirm("Are you sure you want to proceed?", () => { + frappe.db.delete_doc("Kanban Board", this.board_name).then(() => { + frappe.show_alert(`Kanban Board ${this.board_name} deleted.`); + frappe.set_route("List", this.doctype, "List"); + }); + }); + }, + }); + } + } + setup_paging_area() { // pass } From dcbfcdf8b9098ef723dec4ca9566cd4bc18bac63 Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 23 Nov 2022 20:41:04 +0530 Subject: [PATCH 003/136] fix: Check perms on Kanban Column actions - Check Column options access (archive, indicators) - Check column dragability access (allow if write access to board) - Check card dragability access (allow if write access to board) - Hide "Add Column" if no write access to board - Avoid board update on load without write access --- .../views/kanban/kanban_board.bundle.js | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index ae11c0fdc0..52eb912c78 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -297,6 +297,7 @@ frappe.provide("frappe.views"); self.wrapper = opts.wrapper; self.cur_list = opts.cur_list; self.board_name = opts.board_name; + self.board_perms = self.cur_list.board_perms; self.update = function (cards) { // update cards internally @@ -325,7 +326,11 @@ frappe.provide("frappe.views"); store.watch((state, getters) => { return state.empty_state; }, show_empty_state); - store.dispatch("update_order"); + + if (self.board_perms.write) { + // If write access to Board, update Kanban cards order on load + store.dispatch("update_order"); + } } function prepare() { @@ -347,7 +352,7 @@ frappe.provide("frappe.views"); var columns = store.state.columns; columns.filter(is_active_column).map(function (col) { - frappe.views.KanbanBoardColumn(col, self.$kanban_board); + frappe.views.KanbanBoardColumn(col, self.$kanban_board, self.board_perms); }); } @@ -356,7 +361,10 @@ frappe.provide("frappe.views"); bind_clickdrag(); } - function setup_sortable() { // drag column + function setup_sortable() { + // If no write access, editing board (by dragging column) should be blocked + if (!self.board_perms.write) return; + var sortable = new Sortable(self.$kanban_board.get(0), { group: "columns", animation: 150, @@ -372,6 +380,12 @@ frappe.provide("frappe.views"); } function bind_add_column() { + if (!self.board_perms.write) { + // If no write access, editing board (by adding column) should be blocked + self.$kanban_board.find(".add-new-column").hide(); + return; + } + var $add_new_column = self.$kanban_board.find(".add-new-column"), $compose_column = $add_new_column.find(".compose-column"), $compose_column_form = $add_new_column.find(".compose-column-form").hide(); @@ -513,7 +527,7 @@ frappe.provide("frappe.views"); return self; }; - frappe.views.KanbanBoardColumn = function (column, wrapper) { + frappe.views.KanbanBoardColumn = function (column, wrapper, board_perms) { var self = {}; var filtered_cards = []; @@ -566,7 +580,10 @@ frappe.provide("frappe.views"); } } - function setup_sortable() { // drag card + function setup_sortable() { + // If no write access, editing board (by dragging card) should be blocked + if (!board_perms.write) return; + Sortable.create(self.$kanban_cards.get(0), { group: "cards", animation: 150, @@ -641,6 +658,12 @@ frappe.provide("frappe.views"); } function bind_options() { + if (!board_perms.write) { + // If no write access, column options should be hidden + self.$kanban_column.find(".column-options").hide(); + return; + } + self.$kanban_column .find(".column-options .dropdown-menu") .on("click", "[data-action]", function () { @@ -654,6 +677,7 @@ frappe.provide("frappe.views"); store.dispatch("set_indicator", { column, color }); } }); + get_column_indicators(function (indicators) { let html = `
  • ${indicators .map((indicator) => { From 77f3a74ecaa357a69b4cc5d067b5f59d43f26f2d Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 24 Nov 2022 14:08:05 +0530 Subject: [PATCH 004/136] fix: Hide Sort Selector in Kanban - It just re-renders the board that gets the card order from the db and makes no difference to the card order even temporarily --- frappe/public/js/frappe/views/kanban/kanban_view.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 506d4ee1b2..693326b062 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -69,7 +69,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { method: "frappe.client.get_doc_permissions", args: { doctype: "Kanban Board", - docname: this.board_name + docname: this.board_name, }, callback: (result) => { this.board_perms = result.message.permissions || {}; @@ -84,7 +84,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { label: __("Save filters"), action: () => { this.save_kanban_board_filters(); - } + }, }); } @@ -124,6 +124,7 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { this.hide_sidebar = true; this.hide_page_form = true; this.hide_card_layout = true; + this.hide_sort_selector = true; super.setup_page(); } From 0981f593ffa249eaa0d7732b1d58243c95fbfa14 Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 24 Nov 2022 20:30:24 +0530 Subject: [PATCH 005/136] fix: Allow card actions (adding/dragging) if access to reference doctype - allow user to update kanban column cards order even without write access --- frappe/desk/doctype/kanban_board/kanban_board.py | 16 +++++++++++----- .../frappe/views/kanban/kanban_board.bundle.js | 14 +++----------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index 83f0f46df0..c9eb48564d 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -88,6 +88,9 @@ def update_order(board_name, order): """Save the order of cards in columns""" board = frappe.get_doc("Kanban Board", board_name) doctype = board.reference_doctype + + frappe.has_permission(doctype, "write", throw=True) + fieldname = board.field_name order_dict = json.loads(order) @@ -103,8 +106,7 @@ def update_order(board_name, order): if column.column_name == col_name: column.order = json.dumps(cards) - board.save() - return board, updated_cards + return board.save(ignore_permissions=True), updated_cards @frappe.whitelist() @@ -114,6 +116,9 @@ def update_order_for_single_card( """Save the order of cards in columns""" board = frappe.get_doc("Kanban Board", board_name) doctype = board.reference_doctype + + frappe.has_permission(doctype, "write", throw=True) + fieldname = board.field_name old_index = frappe.parse_json(old_index) new_index = frappe.parse_json(new_index) @@ -130,7 +135,7 @@ def update_order_for_single_card( # save updated order board.columns[from_col_idx].order = frappe.as_json(from_col_order) board.columns[to_col_idx].order = frappe.as_json(to_col_order) - board.save() + board.save(ignore_permissions=True) # update changed value in doc frappe.set_value(doctype, docname, fieldname, to_colname) @@ -151,13 +156,14 @@ def get_kanban_column_order_and_index(board, colname): def add_card(board_name, docname, colname): board = frappe.get_doc("Kanban Board", board_name) + frappe.has_permission(board.reference_doctype, "write", throw=True) + col_order, col_idx = get_kanban_column_order_and_index(board, colname) col_order.insert(0, docname) board.columns[col_idx].order = frappe.as_json(col_order) - board.save() - return board + return board.save(ignore_permissions=True) @frappe.whitelist() diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index 52eb912c78..f8782e9f21 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -326,11 +326,7 @@ frappe.provide("frappe.views"); store.watch((state, getters) => { return state.empty_state; }, show_empty_state); - - if (self.board_perms.write) { - // If write access to Board, update Kanban cards order on load - store.dispatch("update_order"); - } + store.dispatch("update_order"); } function prepare() { @@ -338,13 +334,12 @@ frappe.provide("frappe.views"); if (self.$kanban_board.length === 0) { self.$kanban_board = $(frappe.render_template("kanban_board")); - // add column self.$kanban_board.appendTo(self.wrapper); } self.$filter_area = self.cur_list.$page.find(".active-tag-filters"); bind_events(); - setup_sortable(); // column + setup_sortable(); } function make_columns() { @@ -533,7 +528,7 @@ frappe.provide("frappe.views"); function init() { make_dom(); - setup_sortable(); // drag card + setup_sortable(); make_cards(); store.watch((state, getters) => { return state.cards; @@ -581,9 +576,6 @@ frappe.provide("frappe.views"); } function setup_sortable() { - // If no write access, editing board (by dragging card) should be blocked - if (!board_perms.write) return; - Sortable.create(self.$kanban_cards.get(0), { group: "cards", animation: 150, From d6bdd636dcc358a308ec72acade69af2875915b6 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 25 Nov 2022 12:09:36 +0530 Subject: [PATCH 006/136] fix: Check Reference Doctype perms & control indicator change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Don’t change indicator on filter change if user can’t write to board. They can’t save filters - Invoke `update_order` on Kanban board init() only if user has `write` access to reference doctype (non-deliberate invocation) - All deliberate invocations of `update_order` via UI actions are blocked/hidden without `write` access - Remove elements with no access instead of hiding to avoid inspect element hacks - Card Actions: Block card dragging if no `write` access to reference doctype - Card Actions: Block card adding if no `create` access to reference doctype --- frappe/client.py | 2 ++ .../views/kanban/kanban_board.bundle.js | 22 ++++++++++++++++--- .../js/frappe/views/kanban/kanban_view.js | 2 ++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/frappe/client.py b/frappe/client.py index afbb0df5ee..4dc118ea06 100644 --- a/frappe/client.py +++ b/frappe/client.py @@ -302,6 +302,7 @@ def has_permission(doctype, docname, perm_type="read"): # perm_type can be one of read, write, create, submit, cancel, report return {"has_permission": frappe.has_permission(doctype, perm_type.lower(), docname)} + @frappe.whitelist() def get_doc_permissions(doctype, docname): """Returns an evaluated document permissions dict like `{"read":1, "write":1}` @@ -312,6 +313,7 @@ def get_doc_permissions(doctype, docname): doc = frappe.get_doc(doctype, docname) return {"permissions": frappe.permissions.get_doc_permissions(doc)} + @frappe.whitelist() def get_password(doctype, name, fieldname): """Return a password type property. Only applicable for System Managers diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index f8782e9f21..651b94f28f 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -326,7 +326,12 @@ frappe.provide("frappe.views"); store.watch((state, getters) => { return state.empty_state; }, show_empty_state); - store.dispatch("update_order"); + + if (frappe.model.can_write(store.state.doctype)) { + // Check for reference doctype access before initiating + // non-deliberate action + store.dispatch("update_order"); + } } function prepare() { @@ -377,7 +382,7 @@ frappe.provide("frappe.views"); function bind_add_column() { if (!self.board_perms.write) { // If no write access, editing board (by adding column) should be blocked - self.$kanban_board.find(".add-new-column").hide(); + self.$kanban_board.find(".add-new-column").remove(); return; } @@ -576,6 +581,9 @@ frappe.provide("frappe.views"); } function setup_sortable() { + // Block card dragging/record editing without 'write' access + if (!frappe.model.can_write(store.state.doctype)) return; + Sortable.create(self.$kanban_cards.get(0), { group: "cards", animation: 150, @@ -609,6 +617,14 @@ frappe.provide("frappe.views"); var $wrapper = self.$kanban_column; var $btn_add = $wrapper.find(".add-card"); var $new_card_area = $wrapper.find(".new-card-area"); + + if (!frappe.model.can_create(store.state.doctype)) { + // Block record/card creation without 'create' access + $btn_add.remove(); + $new_card_area.remove(); + return; + } + var $textarea = $new_card_area.find("textarea"); //Add card button @@ -652,7 +668,7 @@ frappe.provide("frappe.views"); function bind_options() { if (!board_perms.write) { // If no write access, column options should be hidden - self.$kanban_column.find(".column-options").hide(); + self.$kanban_column.find(".column-options").remove(); return; } diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 693326b062..f0b0cb8adf 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -151,6 +151,8 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { render_list() {} on_filter_change() { + if (!this.board_perms.write) return; // avoid misleading ux + if (JSON.stringify(this.board.filters_array) !== JSON.stringify(this.filter_area.get())) { this.page.set_indicator(__("Not Saved"), "orange"); } else { From 203e73dacb3677209dcf32ca6bb8f9bd0d7a1c09 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 25 Nov 2022 19:35:55 +0530 Subject: [PATCH 007/136] chore: Change card cursor to default from `grab` if no `write` access to Ref. Doctype --- .../js/frappe/views/kanban/kanban_board.bundle.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index 651b94f28f..fc93f2a910 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -362,7 +362,7 @@ frappe.provide("frappe.views"); } function setup_sortable() { - // If no write access, editing board (by dragging column) should be blocked + // If no write access to board, editing board (by dragging column) should be blocked if (!self.board_perms.write) return; var sortable = new Sortable(self.$kanban_board.get(0), { @@ -381,7 +381,7 @@ frappe.provide("frappe.views"); function bind_add_column() { if (!self.board_perms.write) { - // If no write access, editing board (by adding column) should be blocked + // If no write access to board, editing board (by adding column) should be blocked self.$kanban_board.find(".add-new-column").remove(); return; } @@ -581,7 +581,7 @@ frappe.provide("frappe.views"); } function setup_sortable() { - // Block card dragging/record editing without 'write' access + // Block card dragging/record editing without 'write' access to reference doctype if (!frappe.model.can_write(store.state.doctype)) return; Sortable.create(self.$kanban_cards.get(0), { @@ -619,7 +619,7 @@ frappe.provide("frappe.views"); var $new_card_area = $wrapper.find(".new-card-area"); if (!frappe.model.can_create(store.state.doctype)) { - // Block record/card creation without 'create' access + // Block record/card creation without 'create' access to reference doctype $btn_add.remove(); $new_card_area.remove(); return; @@ -667,7 +667,7 @@ frappe.provide("frappe.views"); function bind_options() { if (!board_perms.write) { - // If no write access, column options should be hidden + // If no write access to board, column options should be hidden self.$kanban_column.find(".column-options").remove(); return; } @@ -721,6 +721,11 @@ frappe.provide("frappe.views"); }; self.$card = $(frappe.render_template("kanban_card", opts)).appendTo(wrapper); + + if (!frappe.model.can_write(card.doctype)) { + // Undraggable card without 'write' access to reference doctype + self.$card.find(".kanban-card-body").css("cursor", "default"); + } } function get_doc_content(card) { From 5d0ad77d62c682ed77ebc4e9f2e954cb058d6626 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 23 Dec 2022 00:44:53 +0530 Subject: [PATCH 008/136] fix: Hide Menu if empty & render columns without state change - 'Refresh' btn is always present in dropdown menu. Show menu if any other action is also present. - Without `update_order` state is not changed, and thus columns are not rendered on `init()` - If we are not updating order in the db, render columns with whatever information we have currently --- .../js/frappe/views/kanban/kanban_board.bundle.js | 6 ++++-- frappe/public/js/frappe/views/kanban/kanban_view.js | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index fc93f2a910..703f49f205 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -328,9 +328,11 @@ frappe.provide("frappe.views"); }, show_empty_state); if (frappe.model.can_write(store.state.doctype)) { - // Check for reference doctype access before initiating - // non-deliberate action + // Check for reference doctype access before trying to modify it's value store.dispatch("update_order"); + } else { + // Render columns without state change + make_columns(); } } diff --git a/frappe/public/js/frappe/views/kanban/kanban_view.js b/frappe/public/js/frappe/views/kanban/kanban_view.js index 7ba15d7270..fb15131394 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_view.js +++ b/frappe/public/js/frappe/views/kanban/kanban_view.js @@ -45,6 +45,16 @@ frappe.views.KanbanView = class KanbanView extends frappe.views.ListView { }); } + init() { + return super.init().then(() => { + let menu_length = this.page.menu.find(".dropdown-item").length; + if (menu_length === 1) { + // Only 'Refresh' (hidden) is present (always), dropdown is visibly empty + this.page.hide_menu(); + } + }); + } + setup_defaults() { return super.setup_defaults().then(() => { let get_board_name = () => { From 5c7b5835829c51b89d967cd1a93c794b80dfc10d Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Sat, 24 Dec 2022 12:32:51 +0530 Subject: [PATCH 009/136] fix(commands): require ngrok auth token in `site_config.json` to use ngrok command * ngrok authtoken is now required to use the host_header feature --- frappe/commands/site.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 1c0207ce4b..191be60c31 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1128,6 +1128,14 @@ def start_ngrok(context, bind_tls): site = get_site(context) frappe.init(site=site) + ngrok_auth_token = frappe.conf.ngrok_auth_token + if not ngrok_auth_token: + frappe.errprint( + "'ngrok_auth_token' not found in site config. Please register for a free ngrok account at: https://dashboard.ngrok.com/signup and place the obtained authtoken in site_config.json file." + ) + exit(0) + ngrok.set_auth_token(ngrok_auth_token) + port = frappe.conf.http_port or frappe.conf.webserver_port tunnel = ngrok.connect(addr=str(port), host_header=site, bind_tls=bind_tls) print(f"Public URL: {tunnel.public_url}") From b9a6db3c08645a813e1fb44246fba73104b9aa40 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Sat, 24 Dec 2022 12:45:24 +0530 Subject: [PATCH 010/136] refactor: `ngrok_auth_token` -> `ngrok_authtoken` * to be consistent with ngrok's naming convention --- frappe/commands/site.py | 96 +++++++++++++++++++++++++++++------------ 1 file changed, 69 insertions(+), 27 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 191be60c31..91e439dfeb 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -109,7 +109,8 @@ def new_site( "--with-public-files", help="Restores the public files of the site, given path to its tar file" ) @click.option( - "--with-private-files", help="Restores the private files of the site, given path to its tar file" + "--with-private-files", + help="Restores the private files of the site, given path to its tar file", ) @click.option( "--force", @@ -191,7 +192,8 @@ def _restore( fg="red", ) click.secho( - "Use `bench partial-restore` to restore a partial backup to an existing site.", fg="yellow" + "Use `bench partial-restore` to restore a partial backup to an existing site.", + fg="yellow", ) _backup.decryption_rollback() sys.exit(1) @@ -199,11 +201,15 @@ def _restore( except UnicodeDecodeError: _backup.decryption_rollback() if encryption_key: - click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") + click.secho( + "Encrypted backup file detected. Decrypting using provided key.", fg="yellow" + ) _backup.backup_decryption(encryption_key) else: - click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") + click.secho( + "Encrypted backup file detected. Decrypting using site config.", fg="yellow" + ) encryption_key = get_or_generate_backup_encryption_key() _backup.backup_decryption(encryption_key) @@ -222,7 +228,8 @@ def _restore( fg="red", ) click.secho( - "Use `bench partial-restore` to restore a partial backup to an existing site.", fg="yellow" + "Use `bench partial-restore` to restore a partial backup to an existing site.", + fg="yellow", ) _backup.decryption_rollback() sys.exit(1) @@ -324,7 +331,8 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): # Check for full backup file if "Partial Backup" not in header: click.secho( - "Full backup file detected.Use `bench restore` to restore a Frappe Site.", fg="red" + "Full backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", ) _backup.decryption_rollback() sys.exit(1) @@ -332,11 +340,15 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): except UnicodeDecodeError: _backup.decryption_rollback() if encryption_key: - click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") + click.secho( + "Encrypted backup file detected. Decrypting using provided key.", fg="yellow" + ) key = encryption_key else: - click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") + click.secho( + "Encrypted backup file detected. Decrypting using site config.", fg="yellow" + ) key = get_or_generate_backup_encryption_key() _backup.backup_decryption(key) @@ -355,7 +367,8 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): # Check for Full backup file. if "Partial Backup" not in header: click.secho( - "Full Backup file detected.Use `bench restore` to restore a Frappe Site.", fg="red" + "Full Backup file detected.Use `bench restore` to restore a Frappe Site.", + fg="red", ) _backup.decryption_rollback() sys.exit(1) @@ -387,17 +400,26 @@ def reinstall( ): "Reinstall site ie. wipe all data and start over" site = get_site(context) - _reinstall(site, admin_password, db_root_username, db_root_password, yes, verbose=context.verbose) + _reinstall( + site, admin_password, db_root_username, db_root_password, yes, verbose=context.verbose + ) def _reinstall( - site, admin_password=None, db_root_username=None, db_root_password=None, yes=False, verbose=False + site, + admin_password=None, + db_root_username=None, + db_root_password=None, + yes=False, + verbose=False, ): from frappe.installer import _new_site from frappe.utils.synchronization import filelock if not yes: - click.confirm("This will wipe your database. Are you sure you want to reinstall?", abort=True) + click.confirm( + "This will wipe your database. Are you sure you want to reinstall?", abort=True + ) try: frappe.init(site=site) frappe.connect() @@ -487,7 +509,9 @@ def list_apps(context, format): apps = frappe.get_single("Installed Applications").installed_applications if apps: - name_len, ver_len = (max(len(x.get(y)) for x in apps) for y in ["app_name", "app_version"]) + name_len, ver_len = ( + max(len(x.get(y)) for x in apps) for y in ["app_name", "app_version"] + ) template = f"{{0:{name_len}}} {{1:{ver_len}}} {{2}}" installed_applications = [ @@ -528,7 +552,9 @@ def add_system_manager(context, email, first_name, last_name, send_welcome_email for site in context.sites: frappe.connect(site=site) try: - frappe.utils.user.add_system_manager(email, first_name, last_name, send_welcome_email, password) + frappe.utils.user.add_system_manager( + email, first_name, last_name, send_welcome_email, password + ) frappe.db.commit() finally: frappe.destroy() @@ -554,7 +580,9 @@ def add_user_for_sites( for site in context.sites: frappe.connect(site=site) try: - add_new_user(email, first_name, last_name, user_type, send_welcome_email, password, add_role) + add_new_user( + email, first_name, last_name, user_type, send_welcome_email, password, add_role + ) frappe.db.commit() finally: frappe.destroy() @@ -719,7 +747,10 @@ def use(site, sites_path="."): @click.option("--backup-path-private-files", default=None, help="Set path for saving private file") @click.option("--backup-path-conf", default=None, help="Set path for saving config file") @click.option( - "--ignore-backup-conf", default=False, is_flag=True, help="Ignore excludes/includes set in config" + "--ignore-backup-conf", + default=False, + is_flag=True, + help="Ignore excludes/includes set in config", ) @click.option("--verbose", default=False, is_flag=True, help="Add verbosity") @click.option("--compress", default=False, is_flag=True, help="Compress private and public files") @@ -772,9 +803,13 @@ def backup( print(frappe.get_traceback()) exit_code = 1 continue - if frappe.get_system_settings("encrypt_backup") and frappe.get_site_config().encryption_key: + if ( + frappe.get_system_settings("encrypt_backup") + and frappe.get_site_config().encryption_key + ): click.secho( - "Backup encryption is turned on. Please note the backup encryption key.", fg="yellow" + "Backup encryption is turned on. Please note the backup encryption key.", + fg="yellow", ) odb.print_summary() @@ -835,7 +870,9 @@ def uninstall(context, app, dry_run, yes, no_backup, force): frappe.init(site=site) frappe.connect() with filelock("uninstall_app"): - remove_app(app_name=app, dry_run=dry_run, yes=yes, no_backup=no_backup, force=force) + remove_app( + app_name=app, dry_run=dry_run, yes=yes, no_backup=no_backup, force=force + ) finally: frappe.destroy() if not context.sites: @@ -1128,13 +1165,14 @@ def start_ngrok(context, bind_tls): site = get_site(context) frappe.init(site=site) - ngrok_auth_token = frappe.conf.ngrok_auth_token - if not ngrok_auth_token: - frappe.errprint( - "'ngrok_auth_token' not found in site config. Please register for a free ngrok account at: https://dashboard.ngrok.com/signup and place the obtained authtoken in site_config.json file." + ngrok_authtoken = frappe.conf.ngrok_authtoken + if not ngrok_authtoken: + click.echo( + f"{click.style('ngrok_authtoken', bold=True)} not found in site config. Please register for a free ngrok account at: https://dashboard.ngrok.com/signup and place the obtained authtoken in site_config.json file.\n", + err=True, ) - exit(0) - ngrok.set_auth_token(ngrok_auth_token) + sys.exit(1) + ngrok.set_auth_token(ngrok_authtoken) port = frappe.conf.http_port or frappe.conf.webserver_port tunnel = ngrok.connect(addr=str(port), host_header=site, bind_tls=bind_tls) @@ -1354,7 +1392,9 @@ def trim_tables(context, dry_run, format, no_backup): trimmed_data = trim_tables(dry_run=dry_run, quiet=format == "json") if format == "table" and not dry_run: - click.secho(f"The following data have been removed from {frappe.local.site}", fg="green") + click.secho( + f"The following data have been removed from {frappe.local.site}", fg="green" + ) handle_data(trimmed_data, format=format) finally: @@ -1369,7 +1409,9 @@ def handle_data(data: dict, format="json"): else: from frappe.utils.commands import render_table - data = [["DocType", "Fields"]] + [[table, ", ".join(columns)] for table, columns in data.items()] + data = [["DocType", "Fields"]] + [ + [table, ", ".join(columns)] for table, columns in data.items() + ] render_table(data) From ef75e0f9b847726b41c59e5e60d998fbe4997cfe Mon Sep 17 00:00:00 2001 From: marination Date: Mon, 26 Dec 2022 16:01:11 +0530 Subject: [PATCH 011/136] fix: Dispatch `update_order` always. Handle perm-wise action in backend - Currently, `update_order` is also responsible for fetching fresh column order from db as well updating card order - In the .py file, return board doc if no write permission as that is used to render thr right columns order - Without it, stale order (before moving columns) is restored as there is no db call getting columns that were last stored --- frappe/desk/doctype/kanban_board/kanban_board.py | 6 ++++-- .../public/js/frappe/views/kanban/kanban_board.bundle.js | 8 +------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/frappe/desk/doctype/kanban_board/kanban_board.py b/frappe/desk/doctype/kanban_board/kanban_board.py index c9eb48564d..e3257e25be 100644 --- a/frappe/desk/doctype/kanban_board/kanban_board.py +++ b/frappe/desk/doctype/kanban_board/kanban_board.py @@ -88,13 +88,15 @@ def update_order(board_name, order): """Save the order of cards in columns""" board = frappe.get_doc("Kanban Board", board_name) doctype = board.reference_doctype + updated_cards = [] - frappe.has_permission(doctype, "write", throw=True) + if not frappe.has_permission(doctype, "write"): + # Return board data from db + return board, updated_cards fieldname = board.field_name order_dict = json.loads(order) - updated_cards = [] for col_name, cards in order_dict.items(): for card in cards: column = frappe.get_value(doctype, {"name": card}, fieldname) diff --git a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js index 703f49f205..1810101820 100644 --- a/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js +++ b/frappe/public/js/frappe/views/kanban/kanban_board.bundle.js @@ -327,13 +327,7 @@ frappe.provide("frappe.views"); return state.empty_state; }, show_empty_state); - if (frappe.model.can_write(store.state.doctype)) { - // Check for reference doctype access before trying to modify it's value - store.dispatch("update_order"); - } else { - // Render columns without state change - make_columns(); - } + store.dispatch("update_order"); } function prepare() { From d5c4af53d8d1475f9740b95b9b75faf153ed5ce0 Mon Sep 17 00:00:00 2001 From: marination Date: Tue, 27 Dec 2022 17:00:00 +0530 Subject: [PATCH 012/136] chore: Cleaner Cypress commands - Clearer APIs `add_role` and `remove_role` --- cypress/support/commands.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 20de7508c0..5862d85564 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -371,6 +371,42 @@ Cypress.Commands.add("update_doc", (doctype, docname, args) => { }); }); +Cypress.Commands.add("add_role", (user, role) => { + cy.window() + .its("frappe") + .then((frappe) => { + const session_user = frappe.session.user; + add_remove_role("add", user, role, session_user); + }); +}); + +Cypress.Commands.add("remove_role", (user, role) => { + cy.window() + .its("frappe") + .then((frappe) => { + const session_user = frappe.session.user; + add_remove_role("remove", user, role, session_user); + }); +}); + +const add_remove_role = (action, user, role, session_user) => { + if (session_user !== "Administrator") { + cy.call("logout"); + cy.login("Administrator"); + } + + cy.call("frappe.tests.ui_test_helpers.add_remove_role", { + action: action, + user: user, + role: role, + }); + + if (session_user !== "Administrator") { + cy.call("logout"); + cy.login(session_user); + } +}; + Cypress.Commands.add("open_list_filter", () => { cy.get(".filter-section .filter-button").click(); cy.wait(300); From d13c3778481b42aef8d3fa1de894cf5ef7e235ff Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Tue, 27 Dec 2022 18:45:10 +0530 Subject: [PATCH 013/136] refactor: OAuth * Added typing hints for Oauth APIs and dependant utils * Simplify oauth core flows * Remove long commented code * Use newer, simpler syntax for better readability --- frappe/core/doctype/user/user.py | 2 +- frappe/integrations/oauth2_logins.py | 18 +-- frappe/utils/oauth.py | 210 ++++++++++++--------------- frappe/www/login.py | 12 +- 4 files changed, 108 insertions(+), 134 deletions(-) diff --git a/frappe/core/doctype/user/user.py b/frappe/core/doctype/user/user.py index 91de1d29be..7ba35f5181 100644 --- a/frappe/core/doctype/user/user.py +++ b/frappe/core/doctype/user/user.py @@ -582,7 +582,7 @@ class User(Document): if len(email_accounts) != len(set(email_accounts)): frappe.throw(_("Email Account added multiple times")) - def get_social_login_userid(self, provider): + def get_social_login_userid(self, provider: str): try: for p in self.social_logins: if p.provider == provider: diff --git a/frappe/integrations/oauth2_logins.py b/frappe/integrations/oauth2_logins.py index b668211c2b..942ad2b51b 100644 --- a/frappe/integrations/oauth2_logins.py +++ b/frappe/integrations/oauth2_logins.py @@ -1,4 +1,4 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import json @@ -9,42 +9,42 @@ from frappe.utils.oauth import login_via_oauth2, login_via_oauth2_id_token @frappe.whitelist(allow_guest=True) -def login_via_google(code, state): +def login_via_google(code: str, state: str): login_via_oauth2("google", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_github(code, state): +def login_via_github(code: str, state: str): login_via_oauth2("github", code, state) @frappe.whitelist(allow_guest=True) -def login_via_facebook(code, state): +def login_via_facebook(code: str, state: str): login_via_oauth2("facebook", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_frappe(code, state): +def login_via_frappe(code: str, state: str): login_via_oauth2("frappe", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_office365(code, state): +def login_via_office365(code: str, state: str): login_via_oauth2_id_token("office_365", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_salesforce(code, state): +def login_via_salesforce(code: str, state: str): login_via_oauth2("salesforce", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_fairlogin(code, state): +def login_via_fairlogin(code: str, state: str): login_via_oauth2("fairlogin", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def custom(code, state): +def custom(code: str, state: str): """ Callback for processing code and state for user added providers diff --git a/frappe/utils/oauth.py b/frappe/utils/oauth.py index 7034394fcf..45e93c2e48 100644 --- a/frappe/utils/oauth.py +++ b/frappe/utils/oauth.py @@ -1,8 +1,9 @@ -# Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors +# Copyright (c) 2022, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE import base64 import json +from typing import TYPE_CHECKING, Callable import jwt @@ -11,12 +12,15 @@ import frappe.utils from frappe import _ from frappe.utils.password import get_decrypted_password +if TYPE_CHECKING: + from frappe.core.doctype.user.user import User + class SignupDisabledError(frappe.PermissionError): - pass + ... -def get_oauth2_providers(): +def get_oauth2_providers() -> dict[str, dict]: out = {} providers = frappe.get_all("Social Login Key", fields=["*"]) for provider in providers: @@ -43,25 +47,19 @@ def get_oauth2_providers(): return out -def get_oauth_keys(provider): +def get_oauth_keys(provider: str) -> dict[str, str]: """get client_id and client_secret from database or conf""" - # try conf - keys = frappe.conf.get(f"{provider}_login") - - if not keys: - # try database - client_id, client_secret = frappe.get_value( - "Social Login Key", provider, ["client_id", "client_secret"] - ) - client_secret = get_decrypted_password("Social Login Key", provider, "client_secret") - keys = {"client_id": client_id, "client_secret": client_secret} - return keys - else: + if keys := frappe.conf.get(f"{provider}_login"): return {"client_id": keys["client_id"], "client_secret": keys["client_secret"]} + return { + "client_id": frappe.db.get_value("Social Login Key", provider, "client_id"), + "client_secret": get_decrypted_password("Social Login Key", provider, "client_secret"), + } -def get_oauth2_authorize_url(provider, redirect_to): + +def get_oauth2_authorize_url(provider: str, redirect_to: str) -> str: flow = get_oauth2_flow(provider) state = { @@ -84,7 +82,7 @@ def get_oauth2_authorize_url(provider, redirect_to): return flow.get_authorize_url(**data) -def get_oauth2_flow(provider): +def get_oauth2_flow(provider: str): from rauth import OAuth2Service # get client_id and client_secret @@ -99,33 +97,35 @@ def get_oauth2_flow(provider): return OAuth2Service(**params) -def get_redirect_uri(provider): +def get_redirect_uri(provider: str) -> str: keys = frappe.conf.get(f"{provider}_login") if keys and keys.get("redirect_uri"): # this should be a fully qualified redirect uri return keys["redirect_uri"] - else: - oauth2_providers = get_oauth2_providers() + oauth2_providers = get_oauth2_providers() + redirect_uri = oauth2_providers[provider]["redirect_uri"] - redirect_uri = oauth2_providers[provider]["redirect_uri"] - - # this uses the site's url + the relative redirect uri - return frappe.utils.get_url(redirect_uri) + # this uses the site's url + the relative redirect uri + return frappe.utils.get_url(redirect_uri) -def login_via_oauth2(provider, code, state, decoder=None): +def login_via_oauth2(provider: str, code: str, state: str, decoder: Callable | None = None): info = get_info_via_oauth(provider, code, decoder) login_oauth_user(info, provider=provider, state=state) -def login_via_oauth2_id_token(provider, code, state, decoder=None): +def login_via_oauth2_id_token( + provider: str, code: str, state: str, decoder: Callable | None = None +): info = get_info_via_oauth(provider, code, decoder, id_token=True) login_oauth_user(info, provider=provider, state=state) -def get_info_via_oauth(provider, code, decoder=None, id_token=False): +def get_info_via_oauth( + provider: str, code: str, decoder: Callable | None = None, id_token: bool = False +): flow = get_oauth2_flow(provider) oauth2_providers = get_oauth2_providers() @@ -144,14 +144,12 @@ def get_info_via_oauth(provider, code, decoder=None, id_token=False): if id_token: parsed_access = json.loads(session.access_token_response.text) - token = parsed_access["id_token"] - info = jwt.decode(token, flow.client_secret, options={"verify_signature": False}) + else: api_endpoint = oauth2_providers[provider].get("api_endpoint") api_endpoint_args = oauth2_providers[provider].get("api_endpoint_args") - info = session.get(api_endpoint, params=api_endpoint_args).json() if provider == "github" and not info.get("email"): @@ -166,22 +164,12 @@ def get_info_via_oauth(provider, code, decoder=None, id_token=False): def login_oauth_user( - data=None, provider=None, state=None, email_id=None, key=None, generate_login_token=False + data: dict | str, + *, + provider: str | None = None, + state: dict | str, + generate_login_token: bool = False, ): - # NOTE: This could lead to security issue as the signed in user can type any email address in complete_signup - # if email_id and key: - # data = json.loads(frappe.db.get_temp(key)) - # # What if data is missing because of an invalid key - # data["email"] = email_id - # - # elif not (data.get("email") and get_first_name(data)) and not frappe.db.exists("User", data.get("email")): - # # ask for user email - # key = frappe.db.set_temp(json.dumps(data)) - # frappe.db.commit() - # frappe.local.response["type"] = "redirect" - # frappe.local.response["location"] = "/complete_signup?key=" + key - # return - # json.loads data and state if isinstance(data, str): data = json.loads(data) @@ -237,110 +225,96 @@ def login_oauth_user( ) -def update_oauth_user(user, data, provider): - if isinstance(data.get("location"), dict): - data["location"] = data.get("location").get("name") - - save = False - - if not frappe.db.exists("User", user): - - # is signup disabled? - if frappe.utils.cint(frappe.db.get_single_value("Website Settings", "disable_signup")): +def get_user_record(user: str, data: dict) -> "User": + try: + return frappe.get_doc("User", user) + except frappe.DoesNotExistError: + if frappe.get_website_settings("disable_signup"): raise SignupDisabledError - save = True - user = frappe.new_doc("User") + user: "User" = frappe.new_doc("User") - gender = data.get("gender", "").title() - - if gender and not frappe.db.exists("Gender", gender): - doc = frappe.new_doc("Gender", {"gender": gender}) - doc.insert(ignore_permissions=True) - - user.update( - { - "doctype": "User", - "first_name": get_first_name(data), - "last_name": get_last_name(data), - "email": get_email(data), - "gender": gender, - "enabled": 1, - "new_password": frappe.generate_hash(), - "location": data.get("location"), - "user_type": "Website User", - "user_image": data.get("picture") or data.get("avatar_url"), - } + if gender := data.get("gender", "").title(): + frappe.get_doc({"doctype": "Gender", "gender": gender}).insert( + ignore_permissions=True, ignore_if_duplicate=True ) - else: - user = frappe.get_doc("User", user) - if not user.enabled: - frappe.respond_as_web_page(_("Not Allowed"), _("User {0} is disabled").format(user.email)) - return False + user.update( + { + "doctype": "User", + "first_name": get_first_name(data), + "last_name": get_last_name(data), + "email": get_email(data), + "gender": gender, + "enabled": 1, + "new_password": frappe.generate_hash(), + "location": data.get("location"), + "user_type": "Website User", + "user_image": data.get("picture") or data.get("avatar_url"), + } + ) - if provider == "facebook" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid=data["id"], username=data.get("username")) - user.update({"user_image": "https://graph.facebook.com/{id}/picture".format(id=data["id"])}) + return user - elif provider == "google" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid=data["id"]) - elif provider == "github" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid=data["id"], username=data.get("login")) +def update_oauth_user(user: str, data: dict, provider: str): + if isinstance(data.get("location"), dict): + data["location"] = data["location"].get("name") - elif provider == "frappe" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid=data["sub"]) + user: "User" = get_user_record(user, data) + update_user_record = user.is_new() - elif provider == "office_365" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid=data["sub"]) + if not user.enabled: + frappe.respond_as_web_page(_("Not Allowed"), _("User {0} is disabled").format(user.email)) + return False - elif provider == "salesforce" and not user.get_social_login_userid(provider): - save = True - user.set_social_login_userid(provider, userid="/".join(data["sub"].split("/")[-2:])) + if not user.get_social_login_userid(provider): + update_user_record = True + match provider: + case "facebook": + user.set_social_login_userid(provider, userid=data["id"], username=data.get("username")) + user.update({"user_image": f"https://graph.facebook.com/{data['id']}/picture"}) + case "google": + user.set_social_login_userid(provider, userid=data["id"]) + case "github": + user.set_social_login_userid(provider, userid=data["id"], username=data.get("login")) + case "frappe" | "office_365": + user.set_social_login_userid(provider, userid=data["sub"]) + case "salesforce": + user.set_social_login_userid(provider, userid="/".join(data["sub"].split("/")[-2:])) + case _: + user_id_property = ( + frappe.db.get_value("Social Login Key", provider, "user_id_property") or "sub" + ) + user.set_social_login_userid(provider, userid=data[user_id_property]) - elif not user.get_social_login_userid(provider): - save = True - user_id_property = frappe.db.get_value("Social Login Key", provider, "user_id_property") or "sub" - user.set_social_login_userid(provider, userid=data[user_id_property]) - - if save: + if update_user_record: user.flags.ignore_permissions = True user.flags.no_welcome_mail = True - # set default signup role as per Portal Settings - default_role = frappe.db.get_single_value("Portal Settings", "default_role") - if default_role: + if default_role := frappe.db.get_single_value("Portal Settings", "default_role"): user.add_roles(default_role) user.save() -def get_first_name(data): +def get_first_name(data: dict) -> str: return data.get("first_name") or data.get("given_name") or data.get("name") -def get_last_name(data): +def get_last_name(data: dict) -> str: return data.get("last_name") or data.get("family_name") -def get_email(data): +def get_email(data: dict) -> str: return data.get("email") or data.get("upn") or data.get("unique_name") -def redirect_post_login(desk_user, redirect_to=None, provider=None): - # redirect! +def redirect_post_login(desk_user: bool, redirect_to: str | None = None, provider: str = None): frappe.local.response["type"] = "redirect" if not redirect_to: - # the #desktop is added to prevent a facebook redirect bug desk_uri = "/app/workspace" if provider == "facebook" else "/app" - redirect_to = desk_uri if desk_user else "/me" - redirect_to = frappe.utils.get_url(redirect_to) + redirect_to = frappe.utils.get_url(desk_uri if desk_user else "/me") frappe.local.response["location"] = redirect_to diff --git a/frappe/www/login.py b/frappe/www/login.py index 32e4bb4344..7805c3c663 100644 --- a/frappe/www/login.py +++ b/frappe/www/login.py @@ -106,32 +106,32 @@ def get_context(context): @frappe.whitelist(allow_guest=True) -def login_via_google(code, state): +def login_via_google(code: str, state: str): login_via_oauth2("google", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_github(code, state): +def login_via_github(code: str, state: str): login_via_oauth2("github", code, state) @frappe.whitelist(allow_guest=True) -def login_via_facebook(code, state): +def login_via_facebook(code: str, state: str): login_via_oauth2("facebook", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_frappe(code, state): +def login_via_frappe(code: str, state: str): login_via_oauth2("frappe", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_office365(code, state): +def login_via_office365(code: str, state: str): login_via_oauth2_id_token("office_365", code, state, decoder=decoder_compat) @frappe.whitelist(allow_guest=True) -def login_via_token(login_token): +def login_via_token(login_token: str): sid = frappe.cache().get_value(f"login_token:{login_token}", expires=True) if not sid: frappe.respond_as_web_page(_("Invalid Request"), _("Invalid Login Token"), http_status_code=417) From f79185d79ceeb3dbe74098996f5af01c00833dc2 Mon Sep 17 00:00:00 2001 From: Revant Nandgaonkar Date: Thu, 3 Nov 2022 15:08:26 +0000 Subject: [PATCH 014/136] feat: use Connected App for OAuth based Email Account --- .../doctype/email_account/email_account.js | 71 ++++++++++------- .../doctype/email_account/email_account.json | 38 ++++----- .../doctype/email_account/email_account.py | 48 ++++++------ frappe/email/oauth.py | 77 +------------------ .../doctype/connected_app/connected_app.py | 29 ++++++- .../doctype/token_cache/token_cache.py | 9 ++- 6 files changed, 126 insertions(+), 146 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index 98160e5f46..92b7deece3 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -68,15 +68,19 @@ frappe.email_defaults_pop = { function oauth_access(frm) { return frappe.call({ - method: "frappe.email.oauth.oauth_access", + method: "frappe.client.get", args: { - email_account: frm.doc.name, - service: frm.doc.service || "", + doctype: "Connected App", + name: frm.doc.connected_app, }, - callback: function (r) { - if (!r.exc) { - window.open(r.message.url, "_self"); - } + callback: app => { + return frappe.call({ + method: "initiate_web_application_flow", + doc: app.message, + callback: function (r) { + window.open(r.message, "_self"); + }, + }); }, }); } @@ -105,7 +109,6 @@ frappe.ui.form.on("Email Account", { }); } frm.events.show_gmail_message_for_less_secure_apps(frm); - frm.events.toggle_auth_method(frm); }, use_imap: function (frm) { @@ -153,7 +156,6 @@ frappe.ui.form.on("Email Account", { frm.add_child("imap_folder", { folder_name: "INBOX" }); frm.refresh_field("imap_folder"); } - frm.toggle_display(["auth_method"], frm.doc.service === "GMail"); set_default_max_attachment_size(frm, "attachment_limit"); }, @@ -167,21 +169,25 @@ frappe.ui.form.on("Email Account", { delete frappe.route_flags.delete_user_from_locals; delete locals["User"][frappe.route_flags.linked_user]; } + + if (frm.doc.connected_app && !frm.doc.connected_user) { + frm.set_value("connected_user", frappe.session.user); + } }, after_save(frm) { - if (frm.doc.auth_method === "OAuth" && !frm.doc.refresh_token) { - oauth_access(frm); - } - }, - - toggle_auth_method: function (frm) { - if (frm.doc.service !== "GMail") { - frm.toggle_display(["auth_method"], false); - frm.doc.auth_method = "Basic"; - } else { - frm.toggle_display(["auth_method"], true); - } + frappe.call({ + method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", + args: { + connected_app: frm.doc.connected_app, + connected_user: frm.doc.connected_user, + }, + callback: r => { + if (!r.message) { + oauth_access(frm); + } + }, + }); }, show_gmail_message_for_less_secure_apps: function (frm) { @@ -197,13 +203,22 @@ frappe.ui.form.on("Email Account", { }, show_oauth_authorization_message(frm) { - if (frm.doc.auth_method === "OAuth" && !frm.doc.refresh_token) { - let msg = __( - 'OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.' - ); - frm.dashboard.clear_headline(); - frm.dashboard.set_headline_alert(msg, "yellow"); - } + frappe.call({ + method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", + args: { + connected_app: frm.doc.connected_app, + connected_user: frm.doc.connected_user, + }, + callback: r => { + if (frm.doc.auth_method === "OAuth" && !r.message) { + let msg = __( + 'OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.' + ); + frm.dashboard.clear_headline(); + frm.dashboard.set_headline_alert(msg, "yellow"); + } + }, + }); }, authorize_api_access: function (frm) { diff --git a/frappe/email/doctype/email_account/email_account.json b/frappe/email/doctype/email_account/email_account.json index da88ac680c..38345ea618 100644 --- a/frappe/email/doctype/email_account/email_account.json +++ b/frappe/email/doctype/email_account/email_account.json @@ -20,8 +20,8 @@ "awaiting_password", "ascii_encode_password", "column_break_10", - "refresh_token", - "access_token", + "connected_app", + "connected_user", "login_id_is_different", "login_id", "mailbox_settings", @@ -577,25 +577,11 @@ "label": "IMAP Details" }, { - "depends_on": "eval: doc.service === \"GMail\" && doc.auth_method === \"OAuth\" && !doc.__islocal && !doc.__unsaved", + "depends_on": "eval: doc.auth_method === \"OAuth\" && !doc.__islocal && !doc.__unsaved", "fieldname": "authorize_api_access", "fieldtype": "Button", "label": "Authorize API Access" }, - { - "fieldname": "refresh_token", - "fieldtype": "Small Text", - "hidden": 1, - "label": "Refresh Token", - "read_only": 1 - }, - { - "fieldname": "access_token", - "fieldtype": "Small Text", - "hidden": 1, - "label": "Access Token", - "read_only": 1 - }, { "default": "Basic", "fieldname": "auth_method", @@ -610,12 +596,28 @@ "fieldname": "use_starttls", "fieldtype": "Check", "label": "Use STARTTLS" + }, + { + "depends_on": "eval: doc.auth_method === \"OAuth\"", + "fieldname": "connected_app", + "fieldtype": "Link", + "label": "Connected App", + "mandatory_depends_on": "eval: doc.auth_method === \"OAuth\"", + "options": "Connected App" + }, + { + "depends_on": "eval: doc.auth_method === \"OAuth\"", + "fieldname": "connected_user", + "fieldtype": "Link", + "label": "Connected User", + "mandatory_depends_on": "eval: doc.auth_method === \"OAuth\"", + "options": "User" } ], "icon": "fa fa-inbox", "index_web_pages_for_search": 1, "links": [], - "modified": "2022-08-23 00:31:05.305462", + "modified": "2022-11-03 20:26:52.876668", "modified_by": "Administrator", "module": "Email", "name": "Email Account", diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 1db45604e1..30fad87565 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -11,6 +11,7 @@ from poplib import error_proto import frappe from frappe import _, are_emails_muted, safe_encode +from frappe.integrations.doctype.connected_app.connected_app import check_active_token from frappe.desk.form import assign_to from frappe.email.doctype.email_domain.email_domain import EMAIL_DOMAIN_FIELDS from frappe.email.receive import EmailServer, InboundMail, SentEmailInInboxError @@ -85,21 +86,13 @@ class EmailAccount(Document): use_oauth = self.auth_method == "OAuth" self.use_starttls = cint(self.use_imap and self.use_starttls and not self.use_ssl) - if getattr(self, "service", "") != "GMail" and use_oauth: - self.auth_method = "Basic" - use_oauth = False - if use_oauth: # no need for awaiting password for oauth self.awaiting_password = 0 self.password = None - elif self.refresh_token: - # clear access & refresh token - self.refresh_token = self.access_token = None - if not frappe.local.flags.in_install and not self.awaiting_password: - if self.refresh_token or self.password or self.smtp_server in ("127.0.0.1", "localhost"): + if self.auth_method == "OAuth" or self.password or self.smtp_server in ("127.0.0.1", "localhost"): if self.enable_incoming: self.get_incoming_server() self.no_failed = 0 @@ -188,6 +181,7 @@ class EmailAccount(Document): if frappe.cache().get_value("workers:no-internet") == True: return None + oauth_token = self.get_oauth_token() args = frappe._dict( { "email_account_name": self.email_account_name, @@ -202,8 +196,8 @@ class EmailAccount(Document): "incoming_port": get_port(self), "initial_sync_count": self.initial_sync_count or 100, "use_oauth": self.auth_method == "OAuth", - "refresh_token": decrypt(self.refresh_token) if self.refresh_token else None, - "access_token": decrypt(self.access_token) if self.access_token else None, + "refresh_token": oauth_token.get_password("refresh_token") if oauth_token else None, + "access_token": oauth_token.get_password("refresh_token") if oauth_token else None, } ) @@ -392,8 +386,6 @@ class EmailAccount(Document): }, "name": {"conf_names": ("email_sender_name",), "default": "Frappe"}, "auth_method": {"conf_names": ("auth_method"), "default": "Basic"}, - "access_token": {"conf_names": ("mail_access_token")}, - "refresh_token": {"conf_names": ("mail_refresh_token")}, "from_site_config": {"default": True}, } @@ -401,15 +393,13 @@ class EmailAccount(Document): for doc_field_name, d in field_to_conf_name_map.items(): conf_names, default = d.get("conf_names") or [], d.get("default") value = [frappe.conf.get(k) for k in conf_names if frappe.conf.get(k)] - - if doc_field_name in ("refresh_token", "access_token"): - account_details[doc_field_name] = value and encrypt(value[0]) - else: - account_details[doc_field_name] = (value and value[0]) or default + account_details[doc_field_name] = (value and value[0]) or default return account_details def sendmail_config(self): + oauth_token = self.get_oauth_token() + return { "email_account": self.name, "server": self.smtp_server, @@ -420,8 +410,8 @@ class EmailAccount(Document): "use_tls": cint(self.use_tls), "service": getattr(self, "service", ""), "use_oauth": self.auth_method == "OAuth", - "refresh_token": decrypt(self.refresh_token) if self.refresh_token else None, - "access_token": decrypt(self.access_token) if self.access_token else None, + "refresh_token": oauth_token.get_password("refresh_token") if oauth_token else None, + "access_token": oauth_token.get_password("refresh_token") if oauth_token else None, } def get_smtp_server(self): @@ -681,6 +671,13 @@ class EmailAccount(Document): except Exception: self.log_error("Unable to add to Sent folder") + def get_oauth_token(self): + token = None + if self.auth_method == "OAuth": + connected_app = frappe.get_doc("Connected App", self.connected_app) + token = connected_app.get_active_token(self.connected_user) + + return token @frappe.whitelist() def get_append_to( @@ -786,15 +783,20 @@ def pull(now=False): doctype = frappe.qb.DocType("Email Account") email_accounts = ( frappe.qb.from_(doctype) - .select(doctype.name) + .select(doctype.name, doctype.auth_method) .where(doctype.enable_incoming == 1) .where( - (doctype.awaiting_password == 0) - | ((doctype.auth_method == "OAuth") & (doctype.refresh_token.isnotnull())) + (doctype.awaiting_password == 0) | (doctype.auth_method == "OAuth") ) .run(as_dict=1) ) for email_account in email_accounts: + if email_account.auth_method == "OAuth" and not check_active_token( + connected_app=doctype.connected_app, + connected_user=doctype.connected_user, + ): + continue + if now: pull_from_email_account(email_account.name) diff --git a/frappe/email/oauth.py b/frappe/email/oauth.py index f5b60a9f3d..af86bc47d2 100644 --- a/frappe/email/oauth.py +++ b/frappe/email/oauth.py @@ -53,7 +53,7 @@ class Oauth: return f"user={self.email}\1auth=Bearer {self._access_token}\1\1" def connect(self, _retry: int = 0) -> None: - """Connection method with retry on exception for Oauth""" + """Connection method with retry on exception for connection errors""" try: if isinstance(self._conn, POP3): res = self._connect_pop() @@ -69,18 +69,14 @@ class Oauth: self._connect_smtp() except Exception as e: - # maybe the access token expired - refreshing - access_token = self._refresh_access_token() - - if not access_token or _retry > 0: + if _retry > 0: frappe.log_error( - "OAuth Error - Authentication Failed", str(e), "Email Account", self.email_account + "SMTP Connection Error - Authentication Failed", str(e), "Email Account", self.email_account ) # raising a bare exception here as we have a lot of exception handling present # where the connect method is called from - hence just logging and raising. raise - self._access_token = access_token self.connect(_retry + 1) def _connect_pop(self) -> bytes: @@ -98,70 +94,3 @@ class Oauth: def _connect_smtp(self) -> None: self._conn.auth(self._mechanism, lambda x: self._auth_string, initial_response_ok=False) - - def _refresh_access_token(self) -> str: - """Refreshes access token via calling `refresh_access_token` method of oauth service object""" - service_obj = self._get_service_object() - access_token = service_obj.refresh_access_token(self._refresh_token).get("access_token") - - if access_token: - # set the new access token in db - frappe.db.set_value( - "Email Account", - self.email_account, - "access_token", - encrypt(access_token), - update_modified=False, - ) - - return access_token - - def _get_service_object(self): - """Get Oauth service object""" - - return { - "GMail": GoogleOAuth("mail", validate=False), - }[self.service] - - -@frappe.whitelist(methods=["POST"]) -def oauth_access(email_account: str, service: str): - """Used as a default endpoint/caller for all oauth services. - Returns authorization url for redirection""" - - if not service: - frappe.throw(frappe._("No Service is selected. Please select one and try again!")) - - if service == "GMail": - return authorize_google_access(email_account) - - raise NotImplementedError(f"Service {service} currently doesn't have oauth implementation.") - - -def authorize_google_access(email_account: str, code: str = None): - """Facilitates google oauth for email. - This is invoked 2 times - first time when user clicks `Authorize API Access` for getting the authorization url - and second time for setting the refresh and access token in db when google redirects back with oauth code.""" - - doctype = "Email Account" - oauth_obj = GoogleOAuth("mail") - - if not code: - return oauth_obj.get_authentication_url( - { - "redirect": f"/app/Form/{quote(doctype)}/{quote(email_account)}", - "success_query_param": "successful_authorization=1", - "email_account": email_account, - }, - ) - - res = oauth_obj.authorize(code) - frappe.db.set_value( - doctype, - email_account, - { - "refresh_token": encrypt(res.get("refresh_token")), - "access_token": encrypt(res.get("access_token")), - }, - update_modified=False, - ) diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index 308d1ca84a..46894132e8 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -57,7 +57,7 @@ class ConnectedApp(Document): def initiate_web_application_flow(self, user=None, success_uri=None): """Return an authorization URL for the user. Save state in Token Cache.""" user = user or frappe.session.user - oauth = self.get_oauth2_session(init=True) + oauth = self.get_oauth2_session(user, init=True) query_params = self.get_query_params() authorization_url, state = oauth.authorization_url(self.authorization_uri, **query_params) token_cache = self.get_token_cache(user) @@ -102,6 +102,21 @@ class ConnectedApp(Document): def get_query_params(self): return {param.key: param.value for param in self.query_parameters} + def get_active_token(self, user=None): + user = user or frappe.session.user + token_cache = self.get_token_cache(user) + if token_cache and not token_cache.is_expired(): + return token_cache + elif token_cache and token_cache.get_expires_in(): + oauth_session = self.get_oauth2_session(user) + token = oauth_session.refresh_token( + body=f"redirect_uri={self.redirect_uri}", + token_url=self.token_uri, + refresh_token=token_cache.get_password("refresh_token"), + ) + token_cache.update_data(token) + return token_cache + @frappe.whitelist(allow_guest=True) def callback(code=None, state=None): @@ -142,3 +157,15 @@ def callback(code=None, state=None): frappe.local.response["type"] = "redirect" frappe.local.response["location"] = token_cache.get("success_uri") or connected_app.get_url() + + +@frappe.whitelist() +def check_active_token(connected_app, connected_user=None): + is_token_active = False + app = frappe.get_doc("Connected App", connected_app) + token = app.get_active_token(connected_user) + + if token: + is_token_active = True + + return is_token_active diff --git a/frappe/integrations/doctype/token_cache/token_cache.py b/frappe/integrations/doctype/token_cache/token_cache.py index 25f07a16ba..b01974e5ab 100644 --- a/frappe/integrations/doctype/token_cache/token_cache.py +++ b/frappe/integrations/doctype/token_cache/token_cache.py @@ -3,6 +3,8 @@ from datetime import datetime, timedelta +import pytz + import frappe from frappe import _ from frappe.model.document import Document @@ -50,8 +52,11 @@ class TokenCache(Document): return self def get_expires_in(self): - expiry_time = frappe.utils.get_datetime(self.modified) + timedelta(self.expires_in) - return (datetime.now() - expiry_time).total_seconds() + now_utc = datetime.utcnow().replace(tzinfo=pytz.utc) + expiry_time = frappe.utils.get_datetime(self.modified) + timedelta(seconds=self.expires_in) + expiry_local = expiry_time.replace(tzinfo=pytz.timezone(frappe.utils.get_time_zone())) + expiry_utc = expiry_local.astimezone(pytz.utc) + return (expiry_utc - now_utc).total_seconds() def is_expired(self): return self.get_expires_in() < 0 From a4e5df674b2c938b187cd2f07172bfc71ccb6e2a Mon Sep 17 00:00:00 2001 From: Revant Nandgaonkar Date: Sun, 6 Nov 2022 06:11:24 +0000 Subject: [PATCH 015/136] fix(email): oauth_access call and info message --- .../doctype/email_account/email_account.js | 86 +++++++++---------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index 92b7deece3..ba6ebd4de7 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -67,22 +67,16 @@ frappe.email_defaults_pop = { }; function oauth_access(frm) { - return frappe.call({ - method: "frappe.client.get", - args: { - doctype: "Connected App", - name: frm.doc.connected_app, - }, - callback: app => { - return frappe.call({ - method: "initiate_web_application_flow", - doc: app.message, - callback: function (r) { - window.open(r.message, "_self"); - }, - }); - }, - }); + frappe.model.with_doc("Connected App", frm.doc.connected_app, () => { + const connected_app = frappe.get_doc("Connected App", frm.doc.connected_app); + return frappe.call({ + doc: connected_app, + method: "initiate_web_application_flow", + callback: function(r) { + window.open(r.message, "_self"); + } + }); + }) } function set_default_max_attachment_size(frm, field) { @@ -176,18 +170,20 @@ frappe.ui.form.on("Email Account", { }, after_save(frm) { - frappe.call({ - method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", - args: { - connected_app: frm.doc.connected_app, - connected_user: frm.doc.connected_user, - }, - callback: r => { - if (!r.message) { - oauth_access(frm); - } - }, - }); + if (frm.doc.auth_method === "OAuth") { + frappe.call({ + method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", + args: { + connected_app: frm.doc.connected_app, + connected_user: frm.doc.connected_user, + }, + callback: r => { + if (!r.message) { + oauth_access(frm); + } + }, + }); + } }, show_gmail_message_for_less_secure_apps: function (frm) { @@ -203,22 +199,24 @@ frappe.ui.form.on("Email Account", { }, show_oauth_authorization_message(frm) { - frappe.call({ - method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", - args: { - connected_app: frm.doc.connected_app, - connected_user: frm.doc.connected_user, - }, - callback: r => { - if (frm.doc.auth_method === "OAuth" && !r.message) { - let msg = __( - 'OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.' - ); - frm.dashboard.clear_headline(); - frm.dashboard.set_headline_alert(msg, "yellow"); - } - }, - }); + if (frm.doc.auth_method === "OAuth") { + frappe.call({ + method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", + args: { + connected_app: frm.doc.connected_app, + connected_user: frm.doc.connected_user, + }, + callback: r => { + if (!r.message) { + let msg = __( + 'OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.' + ); + frm.dashboard.clear_headline(); + frm.dashboard.set_headline_alert(msg, "yellow"); + } + }, + }); + } }, authorize_api_access: function (frm) { From f0a17d7adb68a57c16322eb18f47f44469d63054 Mon Sep 17 00:00:00 2001 From: phot0n Date: Wed, 28 Dec 2022 14:48:16 +0530 Subject: [PATCH 016/136] fix: make connected app work with email account * removed (now) unnecessary things from oauth class * simplified get_active_token in connected_app * removed gmail banner from email account docs --- .../doctype/email_account/email_account.js | 62 +++---------------- .../doctype/email_account/email_account.json | 3 +- .../doctype/email_account/email_account.py | 26 +++----- frappe/email/oauth.py | 41 ++++-------- frappe/email/receive.py | 4 -- frappe/email/smtp.py | 8 +-- .../doctype/connected_app/connected_app.py | 9 ++- 7 files changed, 33 insertions(+), 120 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index ba6ebd4de7..67e2ffc863 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -72,20 +72,20 @@ function oauth_access(frm) { return frappe.call({ doc: connected_app, method: "initiate_web_application_flow", - callback: function(r) { + callback: function (r) { window.open(r.message, "_self"); - } + }, }); - }) + }); } -function set_default_max_attachment_size(frm, field) { - if (frm.doc.__islocal && !frm.doc[field]) { +function set_default_max_attachment_size(frm) { + if (frm.doc.__islocal && !frm.doc["attachment_limit"]) { frappe.call({ method: "frappe.core.api.file.get_max_file_size", callback: function (r) { if (!r.exc) { - frm.set_value(field, Number(r.message) / (1024 * 1024)); + frm.set_value("attachment_limit", Number(r.message) / (1024 * 1024)); } }, }); @@ -102,7 +102,6 @@ frappe.ui.form.on("Email Account", { frm.set_value(key, value); }); } - frm.events.show_gmail_message_for_less_secure_apps(frm); }, use_imap: function (frm) { @@ -130,12 +129,6 @@ frappe.ui.form.on("Email Account", { }, onload: function (frm) { - if (frappe.utils.get_query_params().successful_authorization === "1") { - frappe.show_alert(__("Successfully Authorized")); - // FIXME: find better alternative - window.history.replaceState(null, "", window.location.pathname); - } - frm.set_df_property("append_to", "only_select", true); frm.set_query( "append_to", @@ -150,23 +143,17 @@ frappe.ui.form.on("Email Account", { frm.add_child("imap_folder", { folder_name: "INBOX" }); frm.refresh_field("imap_folder"); } - set_default_max_attachment_size(frm, "attachment_limit"); + set_default_max_attachment_size(frm); }, refresh: function (frm) { frm.events.enable_incoming(frm); frm.events.notify_if_unreplied(frm); - frm.events.show_gmail_message_for_less_secure_apps(frm); - frm.events.show_oauth_authorization_message(frm); if (frappe.route_flags.delete_user_from_locals && frappe.route_flags.linked_user) { delete frappe.route_flags.delete_user_from_locals; delete locals["User"][frappe.route_flags.linked_user]; } - - if (frm.doc.connected_app && !frm.doc.connected_user) { - frm.set_value("connected_user", frappe.session.user); - } }, after_save(frm) { @@ -177,7 +164,7 @@ frappe.ui.form.on("Email Account", { connected_app: frm.doc.connected_app, connected_user: frm.doc.connected_user, }, - callback: r => { + callback: (r) => { if (!r.message) { oauth_access(frm); } @@ -186,39 +173,6 @@ frappe.ui.form.on("Email Account", { } }, - show_gmail_message_for_less_secure_apps: function (frm) { - frm.dashboard.clear_headline(); - let msg = __( - "GMail will only work if you enable 2-step authentication and use app-specific password OR use OAuth." - ); - let cta = __("Read the step by step guide here."); - msg += ` ${cta}`; - if (frm.doc.service === "GMail") { - frm.dashboard.set_headline_alert(msg); - } - }, - - show_oauth_authorization_message(frm) { - if (frm.doc.auth_method === "OAuth") { - frappe.call({ - method: "frappe.integrations.doctype.connected_app.connected_app.check_active_token", - args: { - connected_app: frm.doc.connected_app, - connected_user: frm.doc.connected_user, - }, - callback: r => { - if (!r.message) { - let msg = __( - 'OAuth has been enabled but not authorised. Please use "Authorise API Access" button to do the same.' - ); - frm.dashboard.clear_headline(); - frm.dashboard.set_headline_alert(msg, "yellow"); - } - }, - }); - } - }, - authorize_api_access: function (frm) { oauth_access(frm); }, diff --git a/frappe/email/doctype/email_account/email_account.json b/frappe/email/doctype/email_account/email_account.json index 38345ea618..f9e4f95ff0 100644 --- a/frappe/email/doctype/email_account/email_account.json +++ b/frappe/email/doctype/email_account/email_account.json @@ -203,7 +203,6 @@ "label": "Use SSL" }, { - "default": "1", "depends_on": "eval:!doc.domain && doc.enable_incoming", "description": "Ignore attachments over this size", "fetch_from": "domain.attachment_limit", @@ -617,7 +616,7 @@ "icon": "fa fa-inbox", "index_web_pages_for_search": 1, "links": [], - "modified": "2022-11-03 20:26:52.876668", + "modified": "2022-12-28 14:56:18.754804", "modified_by": "Administrator", "module": "Email", "name": "Email Account", diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 30fad87565..3ad2f67c0b 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -11,18 +11,17 @@ from poplib import error_proto import frappe from frappe import _, are_emails_muted, safe_encode -from frappe.integrations.doctype.connected_app.connected_app import check_active_token from frappe.desk.form import assign_to from frappe.email.doctype.email_domain.email_domain import EMAIL_DOMAIN_FIELDS from frappe.email.receive import EmailServer, InboundMail, SentEmailInInboxError from frappe.email.smtp import SMTPServer from frappe.email.utils import get_port +from frappe.integrations.doctype.connected_app.connected_app import check_active_token from frappe.model.document import Document from frappe.utils import cint, comma_or, cstr, parse_addr, validate_email_address from frappe.utils.background_jobs import enqueue, get_jobs from frappe.utils.error import raise_error_on_no_output from frappe.utils.jinja import render_template -from frappe.utils.password import decrypt, encrypt from frappe.utils.user import get_system_managers @@ -92,7 +91,7 @@ class EmailAccount(Document): self.password = None if not frappe.local.flags.in_install and not self.awaiting_password: - if self.auth_method == "OAuth" or self.password or self.smtp_server in ("127.0.0.1", "localhost"): + if use_oauth or self.password or self.smtp_server in ("127.0.0.1", "localhost"): if self.enable_incoming: self.get_incoming_server() self.no_failed = 0 @@ -190,14 +189,12 @@ class EmailAccount(Document): "use_ssl": self.use_ssl, "use_starttls": self.use_starttls, "username": getattr(self, "login_id", None) or self.email_id, - "service": getattr(self, "service", ""), "use_imap": self.use_imap, "email_sync_rule": email_sync_rule, "incoming_port": get_port(self), "initial_sync_count": self.initial_sync_count or 100, "use_oauth": self.auth_method == "OAuth", - "refresh_token": oauth_token.get_password("refresh_token") if oauth_token else None, - "access_token": oauth_token.get_password("refresh_token") if oauth_token else None, + "access_token": oauth_token.get_password("access_token") if oauth_token else None, } ) @@ -408,10 +405,8 @@ class EmailAccount(Document): "password": self._password, "use_ssl": cint(self.use_ssl_for_outgoing), "use_tls": cint(self.use_tls), - "service": getattr(self, "service", ""), "use_oauth": self.auth_method == "OAuth", - "refresh_token": oauth_token.get_password("refresh_token") if oauth_token else None, - "access_token": oauth_token.get_password("refresh_token") if oauth_token else None, + "access_token": oauth_token.get_password("access_token") if oauth_token else None, } def get_smtp_server(self): @@ -679,6 +674,7 @@ class EmailAccount(Document): return token + @frappe.whitelist() def get_append_to( doctype=None, txt=None, searchfield=None, start=None, page_len=None, filters=None @@ -783,20 +779,12 @@ def pull(now=False): doctype = frappe.qb.DocType("Email Account") email_accounts = ( frappe.qb.from_(doctype) - .select(doctype.name, doctype.auth_method) + .select(doctype.name) .where(doctype.enable_incoming == 1) - .where( - (doctype.awaiting_password == 0) | (doctype.auth_method == "OAuth") - ) + .where((doctype.awaiting_password == 0) | (doctype.auth_method == "OAuth")) .run(as_dict=1) ) for email_account in email_accounts: - if email_account.auth_method == "OAuth" and not check_active_token( - connected_app=doctype.connected_app, - connected_user=doctype.connected_user, - ): - continue - if now: pull_from_email_account(email_account.name) diff --git a/frappe/email/oauth.py b/frappe/email/oauth.py index af86bc47d2..c69ab2211e 100644 --- a/frappe/email/oauth.py +++ b/frappe/email/oauth.py @@ -2,11 +2,8 @@ import base64 from imaplib import IMAP4 from poplib import POP3 from smtplib import SMTP -from urllib.parse import quote import frappe -from frappe.integrations.google_oauth import GoogleOAuth -from frappe.utils.password import encrypt class OAuthenticationError(Exception): @@ -20,30 +17,21 @@ class Oauth: email_account: str, email: str, access_token: str, - refresh_token: str, - service: str, mechanism: str = "XOAUTH2", ) -> None: self.email_account = email_account self.email = email - self.service = service self._mechanism = mechanism self._conn = conn self._access_token = access_token - self._refresh_token = refresh_token self._validate() def _validate(self) -> None: - if self.service != "GMail": - raise NotImplementedError( - f"Service {self.service} currently doesn't have oauth implementation." - ) - - if not self._refresh_token: + if not self._access_token: frappe.throw( - frappe._("Please Authorize OAuth."), + frappe._("Please Authorize OAuth for Email Account {}").format(self.email_account), OAuthenticationError, frappe._("OAuth Error"), ) @@ -52,14 +40,11 @@ class Oauth: def _auth_string(self) -> str: return f"user={self.email}\1auth=Bearer {self._access_token}\1\1" - def connect(self, _retry: int = 0) -> None: + def connect(self) -> None: """Connection method with retry on exception for connection errors""" try: if isinstance(self._conn, POP3): - res = self._connect_pop() - - if not res.startswith(b"+OK"): - raise + self._connect_pop() elif isinstance(self._conn, IMAP4): self._connect_imap() @@ -69,15 +54,12 @@ class Oauth: self._connect_smtp() except Exception as e: - if _retry > 0: - frappe.log_error( - "SMTP Connection Error - Authentication Failed", str(e), "Email Account", self.email_account - ) - # raising a bare exception here as we have a lot of exception handling present - # where the connect method is called from - hence just logging and raising. - raise - - self.connect(_retry + 1) + frappe.log_error( + "Email Connection Error - Authentication Failed", str(e), "Email Account", self.email_account + ) + # raising a bare exception here as we have a lot of exception handling present + # where the connect method is called from - hence just logging and raising. + raise def _connect_pop(self) -> bytes: # poplib doesn't have AUTH command implementation @@ -87,7 +69,8 @@ class Oauth: ) ) - return res + if not res.startswith(b"+OK"): + raise def _connect_imap(self) -> None: self._conn.authenticate(self._mechanism, lambda x: self._auth_string) diff --git a/frappe/email/receive.py b/frappe/email/receive.py index 7028dc1f11..c635bdd98a 100644 --- a/frappe/email/receive.py +++ b/frappe/email/receive.py @@ -109,8 +109,6 @@ class EmailServer: self.settings.email_account, self.settings.username, self.settings.access_token, - self.settings.refresh_token, - self.settings.service, ).connect() else: @@ -142,8 +140,6 @@ class EmailServer: self.settings.email_account, self.settings.username, self.settings.access_token, - self.settings.refresh_token, - self.settings.service, ).connect() else: diff --git a/frappe/email/smtp.py b/frappe/email/smtp.py index 10eb2f7681..028b21b0ae 100644 --- a/frappe/email/smtp.py +++ b/frappe/email/smtp.py @@ -54,9 +54,7 @@ class SMTPServer: use_tls=None, use_ssl=None, use_oauth=0, - refresh_token=None, access_token=None, - service=None, ): self.login = login self.email_account = email_account @@ -66,9 +64,7 @@ class SMTPServer: self.use_tls = use_tls self.use_ssl = use_ssl self.use_oauth = use_oauth - self.refresh_token = refresh_token self.access_token = access_token - self.service = service self._session = None if not self.server: @@ -112,9 +108,7 @@ class SMTPServer: self.secure_session(_session) if self.use_oauth: - Oauth( - _session, self.email_account, self.login, self.access_token, self.refresh_token, self.service - ).connect() + Oauth(_session, self.email_account, self.login, self.access_token).connect() elif self.password: res = _session.login(str(self.login or ""), str(self.password or "")) diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index 46894132e8..e7da138fbc 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -105,9 +105,7 @@ class ConnectedApp(Document): def get_active_token(self, user=None): user = user or frappe.session.user token_cache = self.get_token_cache(user) - if token_cache and not token_cache.is_expired(): - return token_cache - elif token_cache and token_cache.get_expires_in(): + if token_cache and token_cache.is_expired(): oauth_session = self.get_oauth2_session(user) token = oauth_session.refresh_token( body=f"redirect_uri={self.redirect_uri}", @@ -115,7 +113,8 @@ class ConnectedApp(Document): refresh_token=token_cache.get_password("refresh_token"), ) token_cache.update_data(token) - return token_cache + + return token_cache @frappe.whitelist(allow_guest=True) @@ -151,7 +150,7 @@ def callback(code=None, state=None): code=code, client_secret=connected_app.get_password("client_secret"), include_client_id=True, - **query_params + **query_params, ) token_cache.update_data(token) From 49143922c5100e8cba8876ce90bf353fbd94d40a Mon Sep 17 00:00:00 2001 From: marination Date: Wed, 28 Dec 2022 23:00:00 +0530 Subject: [PATCH 017/136] chore: Kanban and ToDo UI test helpers - `create_admin_kanban` and `create_todo` UI test helpers - `switch_to_user` Cypress command: logs out and logs in as specified user - Used `remove_role` in permissions test - Used `switch_to_user` command in test helper --- cypress/integration/permissions.js | 20 ++---------------- cypress/support/commands.js | 11 ++++++---- frappe/tests/ui_test_helpers.py | 34 ++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 22 deletions(-) diff --git a/cypress/integration/permissions.js b/cypress/integration/permissions.js index 7a13239771..4e371cc17f 100644 --- a/cypress/integration/permissions.js +++ b/cypress/integration/permissions.js @@ -1,16 +1,7 @@ context("Permissions API", () => { before(() => { cy.visit("/login"); - - cy.login("Administrator"); - cy.call("frappe.tests.ui_test_helpers.add_remove_role", { - action: "remove", - user: "frappe@example.com", - role: "System Manager", - }); - cy.call("logout"); - - cy.login("frappe@example.com"); + cy.remove_role("frappe@example.com", "System Manager"); cy.visit("/app"); }); @@ -44,14 +35,7 @@ context("Permissions API", () => { }); after(() => { - cy.call("logout"); - - cy.login("Administrator"); - cy.call("frappe.tests.ui_test_helpers.add_remove_role", { - action: "add", - user: "frappe@example.com", - role: "System Manager", - }); + cy.add_role("frappe@example.com", "System Manager"); cy.call("logout"); }); }); diff --git a/cypress/support/commands.js b/cypress/support/commands.js index 5862d85564..0a25ff5cab 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -371,6 +371,11 @@ Cypress.Commands.add("update_doc", (doctype, docname, args) => { }); }); +Cypress.Commands.add("switch_to_user", (user) => { + cy.call("logout"); + cy.login(user); +}); + Cypress.Commands.add("add_role", (user, role) => { cy.window() .its("frappe") @@ -391,8 +396,7 @@ Cypress.Commands.add("remove_role", (user, role) => { const add_remove_role = (action, user, role, session_user) => { if (session_user !== "Administrator") { - cy.call("logout"); - cy.login("Administrator"); + cy.switch_to_user("Administrator"); } cy.call("frappe.tests.ui_test_helpers.add_remove_role", { @@ -402,8 +406,7 @@ const add_remove_role = (action, user, role, session_user) => { }); if (session_user !== "Administrator") { - cy.call("logout"); - cy.login(session_user); + cy.switch_to_user(session_user); } }; diff --git a/frappe/tests/ui_test_helpers.py b/frappe/tests/ui_test_helpers.py index 05d3dedad6..16a5688cec 100644 --- a/frappe/tests/ui_test_helpers.py +++ b/frappe/tests/ui_test_helpers.py @@ -581,6 +581,40 @@ def create_kanban(): ).insert() +@whitelist_for_tests +def create_todo(description): + frappe.get_doc({"doctype": "ToDo", "description": description}).insert() + + +@whitelist_for_tests +def create_admin_kanban(): + if not frappe.db.exists("Kanban Board", "Admin Kanban"): + frappe.get_doc( + { + "doctype": "Kanban Board", + "name": "Admin Kanban", + "owner": "Administrator", + "kanban_board_name": "Admin Kanban", + "reference_doctype": "ToDo", + "field_name": "status", + "private": 0, + "show_labels": 0, + "columns": [ + { + "column_name": "Open", + "status": "Active", + "indicator": "Gray", + }, + { + "column_name": "Closed", + "status": "Active", + "indicator": "Gray", + }, + ], + } + ).insert() + + @whitelist_for_tests def add_remove_role(action, user, role): user_doc = frappe.get_doc("User", user) From 048de262fd665a49b27d9a7202d1ff0d3fbb345b Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Thu, 29 Dec 2022 11:05:46 +0530 Subject: [PATCH 018/136] fix: don't set default in list view for fields not allowed --- frappe/core/doctype/doctype/doctype.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/frappe/core/doctype/doctype/doctype.py b/frappe/core/doctype/doctype/doctype.py index e3f8ffd503..e1bb23b388 100644 --- a/frappe/core/doctype/doctype/doctype.py +++ b/frappe/core/doctype/doctype/doctype.py @@ -195,10 +195,12 @@ class DocType(Document): def set_default_in_list_view(self): """Set default in-list-view for first 4 mandatory fields""" + not_allowed_in_list_view = get_fields_not_allowed_in_list_view(self.meta) + if not [d.fieldname for d in self.fields if d.in_list_view]: cnt = 0 for d in self.fields: - if d.reqd and not d.hidden and not d.fieldtype in table_fields: + if d.reqd and not d.hidden and not d.fieldtype in not_allowed_in_list_view: d.in_list_view = 1 cnt += 1 if cnt == 4: @@ -1446,10 +1448,7 @@ def validate_fields(meta): fields = meta.get("fields") fieldname_list = [d.fieldname for d in fields] - not_allowed_in_list_view = list(copy.copy(no_value_fields)) - not_allowed_in_list_view.append("Attach Image") - if meta.istable: - not_allowed_in_list_view.remove("Button") + not_allowed_in_list_view = get_fields_not_allowed_in_list_view(meta) for d in fields: if not d.permlevel: @@ -1490,6 +1489,14 @@ def validate_fields(meta): check_image_field(meta) +def get_fields_not_allowed_in_list_view(meta) -> list[str]: + not_allowed_in_list_view = list(copy.copy(no_value_fields)) + not_allowed_in_list_view.append("Attach Image") + if meta.istable: + not_allowed_in_list_view.remove("Button") + return not_allowed_in_list_view + + def validate_permissions_for_doctype(doctype, for_remove=False, alert=False): """Validates if permissions are set correctly.""" doctype = frappe.get_doc("DocType", doctype) From f1553c479c9a98fa8a91f9f8f1eca773312892a4 Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Thu, 29 Dec 2022 16:20:07 +0530 Subject: [PATCH 019/136] fix(test): override fields of test doctype if passed --- frappe/core/doctype/doctype/test_doctype.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 2e74fd3a6a..916c29ebff 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -759,8 +759,7 @@ def new_doctype( } ) - if fields: - for f in fields: - doc.append("fields", f) + if fields and len(fields) > 0: + doc.set("fields", fields) return doc From ca8842861a0bc34cda7103e2443283a8ae013766 Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 29 Dec 2022 17:03:16 +0530 Subject: [PATCH 020/136] refactor(minor): simplify check_active_token --- .../integrations/doctype/connected_app/connected_app.py | 8 +------- frappe/translations/ru.csv | 1 + frappe/website/doctype/blog_post/templates/blog_post.html | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index e7da138fbc..cd219263e9 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -160,11 +160,5 @@ def callback(code=None, state=None): @frappe.whitelist() def check_active_token(connected_app, connected_user=None): - is_token_active = False app = frappe.get_doc("Connected App", connected_app) - token = app.get_active_token(connected_user) - - if token: - is_token_active = True - - return is_token_active + return bool(app.get_active_token(connected_user)) diff --git a/frappe/translations/ru.csv b/frappe/translations/ru.csv index f83bf411ff..1c99690ff3 100644 --- a/frappe/translations/ru.csv +++ b/frappe/translations/ru.csv @@ -6,6 +6,7 @@ Account,Аккаунт, Accounts Manager,Диспетчер учетных записей, Accounts User,Пользователь Учетных записей, Action,Действие, +min read, Действие, Actions,Действия, Active,Активен, Add,Добавить, diff --git a/frappe/website/doctype/blog_post/templates/blog_post.html b/frappe/website/doctype/blog_post/templates/blog_post.html index 67236fb26d..708b896357 100644 --- a/frappe/website/doctype/blog_post/templates/blog_post.html +++ b/frappe/website/doctype/blog_post/templates/blog_post.html @@ -22,7 +22,7 @@ {%- if read_time -%}  · - {{ read_time }} min read + {{ read_time }} {{ _("min read") }} {%- endif -%} From b78f86a30790d95cc1b90aa3743395db220c9df2 Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 29 Dec 2022 17:14:28 +0530 Subject: [PATCH 021/136] fix: redirect back to email account after successful oauth --- frappe/email/doctype/email_account/email_account.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frappe/email/doctype/email_account/email_account.js b/frappe/email/doctype/email_account/email_account.js index 67e2ffc863..31da072692 100644 --- a/frappe/email/doctype/email_account/email_account.js +++ b/frappe/email/doctype/email_account/email_account.js @@ -72,6 +72,9 @@ function oauth_access(frm) { return frappe.call({ doc: connected_app, method: "initiate_web_application_flow", + args: { + success_uri: window.location.pathname, + }, callback: function (r) { window.open(r.message, "_self"); }, From e96ce833439d559bdd741398aae804d03562f31d Mon Sep 17 00:00:00 2001 From: Hussain Nagaria Date: Thu, 29 Dec 2022 16:22:06 +0530 Subject: [PATCH 022/136] test: not in list view by default for not allowed mandatory field types --- frappe/core/doctype/doctype/test_doctype.py | 22 +++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/frappe/core/doctype/doctype/test_doctype.py b/frappe/core/doctype/doctype/test_doctype.py index 916c29ebff..e8226d4f9d 100644 --- a/frappe/core/doctype/doctype/test_doctype.py +++ b/frappe/core/doctype/doctype/test_doctype.py @@ -722,6 +722,28 @@ class TestDocType(FrappeTestCase): self.assertEqual(frappe.get_meta(doctype).get_field(field).default, "DELETETHIS") frappe.delete_doc("DocType", doctype) + def test_not_in_list_view_for_not_allowed_mandatory_field(self): + doctype = new_doctype( + fields=[ + { + "fieldname": "cover_image", + "fieldtype": "Attach Image", + "label": "Cover Image", + "reqd": 1, # mandatory + }, + { + "fieldname": "book_name", + "fieldtype": "Data", + "label": "Book Name", + "reqd": 1, # mandatory + }, + ], + ).insert() + + self.assertFalse(doctype.fields[0].in_list_view) + self.assertTrue(doctype.fields[1].in_list_view) + frappe.delete_doc("DocType", doctype.name) + def new_doctype( name: str | None = None, From 231d90cb40eda39543a55ea6a5fd588bdc5fa514 Mon Sep 17 00:00:00 2001 From: phot0n Date: Thu, 29 Dec 2022 17:26:08 +0530 Subject: [PATCH 023/136] chore: handle get requests via frappe.whitelist decorator --- frappe/email/doctype/email_account/email_account.py | 1 - frappe/email/oauth.py | 2 +- frappe/integrations/doctype/connected_app/connected_app.py | 4 +--- frappe/translations/ru.csv | 1 - frappe/website/doctype/blog_post/templates/blog_post.html | 2 +- 5 files changed, 3 insertions(+), 7 deletions(-) diff --git a/frappe/email/doctype/email_account/email_account.py b/frappe/email/doctype/email_account/email_account.py index 3ad2f67c0b..a0fc8f162e 100755 --- a/frappe/email/doctype/email_account/email_account.py +++ b/frappe/email/doctype/email_account/email_account.py @@ -16,7 +16,6 @@ from frappe.email.doctype.email_domain.email_domain import EMAIL_DOMAIN_FIELDS from frappe.email.receive import EmailServer, InboundMail, SentEmailInInboxError from frappe.email.smtp import SMTPServer from frappe.email.utils import get_port -from frappe.integrations.doctype.connected_app.connected_app import check_active_token from frappe.model.document import Document from frappe.utils import cint, comma_or, cstr, parse_addr, validate_email_address from frappe.utils.background_jobs import enqueue, get_jobs diff --git a/frappe/email/oauth.py b/frappe/email/oauth.py index c69ab2211e..ad951d770e 100644 --- a/frappe/email/oauth.py +++ b/frappe/email/oauth.py @@ -61,7 +61,7 @@ class Oauth: # where the connect method is called from - hence just logging and raising. raise - def _connect_pop(self) -> bytes: + def _connect_pop(self) -> None: # poplib doesn't have AUTH command implementation res = self._conn._shortcmd( "AUTH {} {}".format( diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index cd219263e9..e8193c89f2 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -117,7 +117,7 @@ class ConnectedApp(Document): return token_cache -@frappe.whitelist(allow_guest=True) +@frappe.whitelist(methods=["GET"], allow_guest=True) def callback(code=None, state=None): """Handle client's code. @@ -125,8 +125,6 @@ def callback(code=None, state=None): transmit a code that can be used by the local server to obtain an access token. """ - if frappe.request.method != "GET": - frappe.throw(_("Invalid request method: {}").format(frappe.request.method)) if frappe.session.user == "Guest": frappe.local.response["type"] = "redirect" diff --git a/frappe/translations/ru.csv b/frappe/translations/ru.csv index 1c99690ff3..f83bf411ff 100644 --- a/frappe/translations/ru.csv +++ b/frappe/translations/ru.csv @@ -6,7 +6,6 @@ Account,Аккаунт, Accounts Manager,Диспетчер учетных записей, Accounts User,Пользователь Учетных записей, Action,Действие, -min read, Действие, Actions,Действия, Active,Активен, Add,Добавить, diff --git a/frappe/website/doctype/blog_post/templates/blog_post.html b/frappe/website/doctype/blog_post/templates/blog_post.html index 708b896357..67236fb26d 100644 --- a/frappe/website/doctype/blog_post/templates/blog_post.html +++ b/frappe/website/doctype/blog_post/templates/blog_post.html @@ -22,7 +22,7 @@ {%- if read_time -%}  · - {{ read_time }} {{ _("min read") }} + {{ read_time }} min read {%- endif -%} From 5b319f0830a870655e7f53e1341c1a6072e26ada Mon Sep 17 00:00:00 2001 From: marination Date: Thu, 29 Dec 2022 23:11:30 +0530 Subject: [PATCH 024/136] test: Check if Kanban Board is not editable by non-system user who does not own the board - User should only be able to view the board, and drag the cards - No column actions, no board actions --- cypress/integration/kanban.js | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/cypress/integration/kanban.js b/cypress/integration/kanban.js index 1b7e45ac55..04a72a9436 100644 --- a/cypress/integration/kanban.js +++ b/cypress/integration/kanban.js @@ -1,6 +1,6 @@ context("Kanban Board", () => { before(() => { - cy.login(); + cy.login("frappe@example.com"); cy.visit("/app"); }); @@ -96,4 +96,36 @@ context("Kanban Board", () => { .first() .should("not.contain", "ID:"); }); + + it("Checks if Kanban Board edits are blocked for non-System Manager and non-owner of the Board", () => { + // create admin kanban board + cy.call("frappe.tests.ui_test_helpers.create_todo", { description: "Frappe User ToDo" }); + + cy.switch_to_user("Administrator"); + cy.call("frappe.tests.ui_test_helpers.create_admin_kanban"); + // remove sys manager + cy.remove_role("frappe@example.com", "System Manager"); + + cy.switch_to_user("frappe@example.com"); + + cy.visit("/app/todo/view/kanban/Admin Kanban"); + + // Menu button should be hidden (dropdown for 'Save Filters' and 'Delete Kanban Board') + cy.get(".no-list-sidebar .menu-btn-group .btn-default[data-original-title='Menu']").should( + "have.length", + 0 + ); + // Kanban Columns should be visible (read-only) + cy.get(".kanban .kanban-column").should("have.length", 2); + // User should be able to add card (has access to ToDo) + cy.get(".kanban .add-card").should("have.length", 2); + // Column actions should be hidden (dropdown for 'Archive' and indicators) + cy.get(".kanban .column-options").should("have.length", 0); + + cy.add_role("frappe@example.com", "System Manager"); + }); + + after(() => { + cy.call("logout"); + }); }); From 31d1bd6cc767aa670871f9fb20b292ea7aaeeef1 Mon Sep 17 00:00:00 2001 From: phot0n Date: Fri, 30 Dec 2022 19:27:17 +0530 Subject: [PATCH 025/136] chore: linter --- frappe/commands/site.py | 53 ++++++++++------------------------------- 1 file changed, 13 insertions(+), 40 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 91e439dfeb..2f111200e5 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -201,15 +201,11 @@ def _restore( except UnicodeDecodeError: _backup.decryption_rollback() if encryption_key: - click.secho( - "Encrypted backup file detected. Decrypting using provided key.", fg="yellow" - ) + click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") _backup.backup_decryption(encryption_key) else: - click.secho( - "Encrypted backup file detected. Decrypting using site config.", fg="yellow" - ) + click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") encryption_key = get_or_generate_backup_encryption_key() _backup.backup_decryption(encryption_key) @@ -340,15 +336,11 @@ def partial_restore(context, sql_file_path, verbose, encryption_key=None): except UnicodeDecodeError: _backup.decryption_rollback() if encryption_key: - click.secho( - "Encrypted backup file detected. Decrypting using provided key.", fg="yellow" - ) + click.secho("Encrypted backup file detected. Decrypting using provided key.", fg="yellow") key = encryption_key else: - click.secho( - "Encrypted backup file detected. Decrypting using site config.", fg="yellow" - ) + click.secho("Encrypted backup file detected. Decrypting using site config.", fg="yellow") key = get_or_generate_backup_encryption_key() _backup.backup_decryption(key) @@ -400,9 +392,7 @@ def reinstall( ): "Reinstall site ie. wipe all data and start over" site = get_site(context) - _reinstall( - site, admin_password, db_root_username, db_root_password, yes, verbose=context.verbose - ) + _reinstall(site, admin_password, db_root_username, db_root_password, yes, verbose=context.verbose) def _reinstall( @@ -417,9 +407,7 @@ def _reinstall( from frappe.utils.synchronization import filelock if not yes: - click.confirm( - "This will wipe your database. Are you sure you want to reinstall?", abort=True - ) + click.confirm("This will wipe your database. Are you sure you want to reinstall?", abort=True) try: frappe.init(site=site) frappe.connect() @@ -509,9 +497,7 @@ def list_apps(context, format): apps = frappe.get_single("Installed Applications").installed_applications if apps: - name_len, ver_len = ( - max(len(x.get(y)) for x in apps) for y in ["app_name", "app_version"] - ) + name_len, ver_len = (max(len(x.get(y)) for x in apps) for y in ["app_name", "app_version"]) template = f"{{0:{name_len}}} {{1:{ver_len}}} {{2}}" installed_applications = [ @@ -552,9 +538,7 @@ def add_system_manager(context, email, first_name, last_name, send_welcome_email for site in context.sites: frappe.connect(site=site) try: - frappe.utils.user.add_system_manager( - email, first_name, last_name, send_welcome_email, password - ) + frappe.utils.user.add_system_manager(email, first_name, last_name, send_welcome_email, password) frappe.db.commit() finally: frappe.destroy() @@ -580,9 +564,7 @@ def add_user_for_sites( for site in context.sites: frappe.connect(site=site) try: - add_new_user( - email, first_name, last_name, user_type, send_welcome_email, password, add_role - ) + add_new_user(email, first_name, last_name, user_type, send_welcome_email, password, add_role) frappe.db.commit() finally: frappe.destroy() @@ -803,10 +785,7 @@ def backup( print(frappe.get_traceback()) exit_code = 1 continue - if ( - frappe.get_system_settings("encrypt_backup") - and frappe.get_site_config().encryption_key - ): + if frappe.get_system_settings("encrypt_backup") and frappe.get_site_config().encryption_key: click.secho( "Backup encryption is turned on. Please note the backup encryption key.", fg="yellow", @@ -870,9 +849,7 @@ def uninstall(context, app, dry_run, yes, no_backup, force): frappe.init(site=site) frappe.connect() with filelock("uninstall_app"): - remove_app( - app_name=app, dry_run=dry_run, yes=yes, no_backup=no_backup, force=force - ) + remove_app(app_name=app, dry_run=dry_run, yes=yes, no_backup=no_backup, force=force) finally: frappe.destroy() if not context.sites: @@ -1392,9 +1369,7 @@ def trim_tables(context, dry_run, format, no_backup): trimmed_data = trim_tables(dry_run=dry_run, quiet=format == "json") if format == "table" and not dry_run: - click.secho( - f"The following data have been removed from {frappe.local.site}", fg="green" - ) + click.secho(f"The following data have been removed from {frappe.local.site}", fg="green") handle_data(trimmed_data, format=format) finally: @@ -1409,9 +1384,7 @@ def handle_data(data: dict, format="json"): else: from frappe.utils.commands import render_table - data = [["DocType", "Fields"]] + [ - [table, ", ".join(columns)] for table, columns in data.items() - ] + data = [["DocType", "Fields"]] + [[table, ", ".join(columns)] for table, columns in data.items()] render_table(data) From 9d6f82725eb9fcd4b4e51ee9c5d7230b5a9973d6 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sun, 1 Jan 2023 21:22:36 +0530 Subject: [PATCH 026/136] chore: remove tracking changes for token cache happens very frequently for email and leads to unnecessary bloat in version doctype plus token caches are only read and delete only --- frappe/integrations/doctype/token_cache/token_cache.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frappe/integrations/doctype/token_cache/token_cache.json b/frappe/integrations/doctype/token_cache/token_cache.json index c016405031..0e6601fd98 100644 --- a/frappe/integrations/doctype/token_cache/token_cache.json +++ b/frappe/integrations/doctype/token_cache/token_cache.json @@ -86,10 +86,11 @@ } ], "links": [], - "modified": "2020-11-13 13:35:53.714352", + "modified": "2023-01-01 21:01:24.405729", "modified_by": "Administrator", "module": "Integrations", "name": "Token Cache", + "naming_rule": "Expression", "owner": "Administrator", "permissions": [ { @@ -106,5 +107,5 @@ ], "sort_field": "modified", "sort_order": "DESC", - "track_changes": 1 + "states": [] } \ No newline at end of file From 52daef0dfd30b8b0a0559cefdd7acfa521f197eb Mon Sep 17 00:00:00 2001 From: phot0n Date: Sun, 1 Jan 2023 22:43:33 +0530 Subject: [PATCH 027/136] fix: get_expires_in logic for token cache --- frappe/integrations/doctype/token_cache/token_cache.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/frappe/integrations/doctype/token_cache/token_cache.py b/frappe/integrations/doctype/token_cache/token_cache.py index b01974e5ab..034b1d9107 100644 --- a/frappe/integrations/doctype/token_cache/token_cache.py +++ b/frappe/integrations/doctype/token_cache/token_cache.py @@ -52,11 +52,10 @@ class TokenCache(Document): return self def get_expires_in(self): + modified = frappe.utils.get_datetime(self.modified) + expiry_utc = modified.astimezone(pytz.utc) + timedelta(seconds=self.expires_in) now_utc = datetime.utcnow().replace(tzinfo=pytz.utc) - expiry_time = frappe.utils.get_datetime(self.modified) + timedelta(seconds=self.expires_in) - expiry_local = expiry_time.replace(tzinfo=pytz.timezone(frappe.utils.get_time_zone())) - expiry_utc = expiry_local.astimezone(pytz.utc) - return (expiry_utc - now_utc).total_seconds() + return cint((expiry_utc - now_utc).total_seconds()) def is_expired(self): return self.get_expires_in() < 0 From f50be4bbf5ae9396dabf3e5e31ed1e40e9fcf8d9 Mon Sep 17 00:00:00 2001 From: phot0n Date: Sun, 1 Jan 2023 23:00:03 +0530 Subject: [PATCH 028/136] chore: log any exception while refreshing access token in connected apps --- .../doctype/connected_app/connected_app.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/frappe/integrations/doctype/connected_app/connected_app.py b/frappe/integrations/doctype/connected_app/connected_app.py index e8193c89f2..ff2eb2dc96 100644 --- a/frappe/integrations/doctype/connected_app/connected_app.py +++ b/frappe/integrations/doctype/connected_app/connected_app.py @@ -107,11 +107,17 @@ class ConnectedApp(Document): token_cache = self.get_token_cache(user) if token_cache and token_cache.is_expired(): oauth_session = self.get_oauth2_session(user) - token = oauth_session.refresh_token( - body=f"redirect_uri={self.redirect_uri}", - token_url=self.token_uri, - refresh_token=token_cache.get_password("refresh_token"), - ) + + try: + token = oauth_session.refresh_token( + body=f"redirect_uri={self.redirect_uri}", + token_url=self.token_uri, + refresh_token=token_cache.get_password("refresh_token"), + ) + except Exception: + self.log_error("Token Refresh Error") + return None + token_cache.update_data(token) return token_cache From 324c3e24c4085d13650aa2075e2a0e02166fd433 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 2 Jan 2023 10:20:16 +0530 Subject: [PATCH 029/136] fix: replace all spaces with _ to generate fieldname --- frappe/public/js/form_builder/utils.js | 4 ++-- frappe/website/doctype/web_form/templates/web_form_row.html | 4 ++++ frappe/website/doctype/web_form/web_form.json | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 frappe/website/doctype/web_form/templates/web_form_row.html diff --git a/frappe/public/js/form_builder/utils.js b/frappe/public/js/form_builder/utils.js index d149a48c16..4c2887e4e7 100644 --- a/frappe/public/js/form_builder/utils.js +++ b/frappe/public/js/form_builder/utils.js @@ -279,7 +279,7 @@ export function scrub_field_names(fields) { if (d.fieldtype) { if (!d.fieldname) { if (d.label) { - d.fieldname = d.label.trim().toLowerCase().replace(" ", "_"); + d.fieldname = d.label.trim().toLowerCase().replaceAll(" ", "_"); if (d.fieldname.endsWith("?")) { d.fieldname = d.fieldname.slice(0, -1); } @@ -295,7 +295,7 @@ export function scrub_field_names(fields) { } } else { d.fieldname = - d.fieldtype.toLowerCase().replace(" ", "_") + + d.fieldtype.toLowerCase().replaceAll(" ", "_") + "_" + frappe.utils.get_random(4); } diff --git a/frappe/website/doctype/web_form/templates/web_form_row.html b/frappe/website/doctype/web_form/templates/web_form_row.html new file mode 100644 index 0000000000..d7014b453a --- /dev/null +++ b/frappe/website/doctype/web_form/templates/web_form_row.html @@ -0,0 +1,4 @@ + + diff --git a/frappe/website/doctype/web_form/web_form.json b/frappe/website/doctype/web_form/web_form.json index 0c2e416696..21e501481b 100644 --- a/frappe/website/doctype/web_form/web_form.json +++ b/frappe/website/doctype/web_form/web_form.json @@ -364,7 +364,7 @@ "icon": "icon-edit", "is_published_field": "published", "links": [], - "modified": "2022-12-15 17:14:44.939645", + "modified": "2023-01-02 10:19:15.680960", "modified_by": "Administrator", "module": "Website", "name": "Web Form", From f39c6e18f65eb6040f6201f531f74e250709f50d Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 2 Jan 2023 10:24:26 +0530 Subject: [PATCH 030/136] fix: do not allow to make check field mandatory --- frappe/public/js/form_builder/components/FieldProperties.vue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/public/js/form_builder/components/FieldProperties.vue b/frappe/public/js/form_builder/components/FieldProperties.vue index 23945554a7..b5597d20c5 100644 --- a/frappe/public/js/form_builder/components/FieldProperties.vue +++ b/frappe/public/js/form_builder/components/FieldProperties.vue @@ -25,6 +25,10 @@ let docfield_df = computed(() => { return false; } + if (df.fieldname === "reqd" && store.selected_field.fieldtype === "Check") { + return false; + } + if (df.fieldname === "options") { df.fieldtype = "Small Text"; df.options = ""; From 0e9d16820b6342294559fb6c9c8acffc05001109 Mon Sep 17 00:00:00 2001 From: rohitwaghchaure Date: Mon, 2 Jan 2023 11:35:21 +0530 Subject: [PATCH 031/136] fix: For Update for child table (#19436) --- frappe/model/document.py | 1 + 1 file changed, 1 insertion(+) diff --git a/frappe/model/document.py b/frappe/model/document.py index 773ee4a764..7222cf4ad6 100644 --- a/frappe/model/document.py +++ b/frappe/model/document.py @@ -178,6 +178,7 @@ class Document(BaseDocument): "*", as_dict=True, order_by="idx asc", + for_update=self.flags.for_update, ) or [] ) From 4d048cd651b2250031918e706e327124a3fa681e Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 2 Jan 2023 11:50:25 +0530 Subject: [PATCH 032/136] fix: type hint for image view closes https://github.com/frappe/frappe/issues/19426 --- frappe/core/api/file.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/core/api/file.py b/frappe/core/api/file.py index c2354516a8..e3e6a9de08 100644 --- a/frappe/core/api/file.py +++ b/frappe/core/api/file.py @@ -13,7 +13,7 @@ def unzip_file(name: str): @frappe.whitelist() -def get_attached_images(doctype: str, names: list[str]) -> frappe._dict: +def get_attached_images(doctype: str, names: list[str] | str) -> frappe._dict: """get list of image urls attached in form returns {name: ['image.jpg', 'image.png']}""" From 5ece1d7c394a80f2d63828601afe517c08cc676c Mon Sep 17 00:00:00 2001 From: Ankush Menat Date: Mon, 2 Jan 2023 12:01:32 +0530 Subject: [PATCH 033/136] fix: Allow everyone to read geo data (#19451) This is static data present in code, no need to apply permissions. closes https://github.com/frappe/frappe/issues/19394 --- frappe/geo/country_info.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/frappe/geo/country_info.py b/frappe/geo/country_info.py index 2aefa27170..3267149d4c 100644 --- a/frappe/geo/country_info.py +++ b/frappe/geo/country_info.py @@ -5,6 +5,7 @@ import json # all country info import os +from functools import lru_cache import frappe from frappe.utils.momentjs import get_all_timezones @@ -27,8 +28,13 @@ def get_all(): return all_data -@frappe.whitelist() +@frappe.whitelist(allow_guest=True) def get_country_timezone_info(): + return _get_country_timezone_info() + + +@lru_cache(maxsize=2) +def _get_country_timezone_info(): return {"country_info": get_all(), "all_timezones": get_all_timezones()} From c29f6892b3f5f4ccd7ecc48a45936986aeae9552 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 2 Jan 2023 12:03:13 +0530 Subject: [PATCH 034/136] fix: repositioned action buttons --- .../js/form_builder/form_builder.bundle.js | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/frappe/public/js/form_builder/form_builder.bundle.js b/frappe/public/js/form_builder/form_builder.bundle.js index 0b4a1a7cc2..ae19edbfc6 100644 --- a/frappe/public/js/form_builder/form_builder.bundle.js +++ b/frappe/public/js/form_builder/form_builder.bundle.js @@ -45,18 +45,26 @@ class FormBuilder { this.store.read_only = this.store.preview; this.read_only = true; }); - this.customize_form_btn = this.page.add_button(__("For Customize Form"), () => { - frappe.set_route("form-builder", this.doctype, "customize"); - }); - this.doctype_form_btn = this.page.add_button(__("For DocType Form"), () => { - frappe.set_route("form-builder", this.doctype); - }); this.reset_changes_btn = this.page.add_button(__("Reset Changes"), () => { this.store.reset_changes(); }); - this.go_to_doctype_btn = this.page.add_menu_item(__("Go to Doctype"), () => + this.go_to_doctype_list_btn = this.page.add_button( + __("Go to {0} List", [__(this.doctype)]), + () => { + window.open(`/app/${frappe.router.slug(this.doctype)}`); + } + ); + + this.customize_form_btn = this.page.add_menu_item(__("Switch to Customize Form"), () => { + frappe.set_route("form-builder", this.doctype, "customize"); + }); + this.doctype_form_btn = this.page.add_menu_item(__("Switch to DocType Form"), () => { + frappe.set_route("form-builder", this.doctype); + }); + + this.go_to_doctype_btn = this.page.add_menu_item(__("Go to DocType"), () => frappe.set_route("Form", "DocType", this.doctype) ); this.go_to_customize_form_btn = this.page.add_menu_item(__("Go to Customize Form"), () => @@ -121,9 +129,7 @@ class FormBuilder { ? __("Go to {0}", [__(this.doctype)]) : __("Go to {0} List", [__(this.doctype)]); - this.page.add_menu_item(label, () => { - window.open(`/app/${frappe.router.slug(this.doctype)}`); - }); + this.go_to_doctype_list_btn.text(label); } // toggle preview btn text From 69af3e883bc33eadf4055e100cb8e82c97f3ddd5 Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 2 Jan 2023 12:03:50 +0530 Subject: [PATCH 035/136] fix: added description for section --- frappe/public/js/form_builder/components/FormBuilder.vue | 4 ++++ frappe/public/js/form_builder/components/Section.vue | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/frappe/public/js/form_builder/components/FormBuilder.vue b/frappe/public/js/form_builder/components/FormBuilder.vue index d1bebb2210..a81632c8c1 100644 --- a/frappe/public/js/form_builder/components/FormBuilder.vue +++ b/frappe/public/js/form_builder/components/FormBuilder.vue @@ -210,6 +210,10 @@ onMounted(() => { } } + .section-description { + padding-left: 15px; + } + .section-columns { margin-top: 8px; diff --git a/frappe/public/js/form_builder/components/Section.vue b/frappe/public/js/form_builder/components/Section.vue index cbf848c10a..2cb24a8d4c 100644 --- a/frappe/public/js/form_builder/components/Section.vue +++ b/frappe/public/js/form_builder/components/Section.vue @@ -128,6 +128,7 @@ function move_sections_to_tab() { +
    {{ section.df.description }}
    Date: Mon, 2 Jan 2023 16:12:21 +0530 Subject: [PATCH 036/136] feat(minor): add use-default-authtoken to ngrok command --- frappe/commands/site.py | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 2f111200e5..fbbdde8e03 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1134,8 +1134,14 @@ def stop_recording(context): @click.option( "--bind-tls", is_flag=True, default=False, help="Returns a reference to the https tunnel." ) +@click.option( + "--use-default-authtoken", + is_flag=True, + default=False, + help="Use the auth token present in ngrok's config.", +) @pass_context -def start_ngrok(context, bind_tls): +def start_ngrok(context, bind_tls, use_default_authtoken): """Start a ngrok tunnel to your local development server.""" from pyngrok import ngrok @@ -1143,13 +1149,15 @@ def start_ngrok(context, bind_tls): frappe.init(site=site) ngrok_authtoken = frappe.conf.ngrok_authtoken - if not ngrok_authtoken: - click.echo( - f"{click.style('ngrok_authtoken', bold=True)} not found in site config. Please register for a free ngrok account at: https://dashboard.ngrok.com/signup and place the obtained authtoken in site_config.json file.\n", - err=True, - ) - sys.exit(1) - ngrok.set_auth_token(ngrok_authtoken) + if not use_default_authtoken: + if not ngrok_authtoken: + click.echo( + f"\n{click.style('ngrok_authtoken', fg='yellow')} not found in site config.\n" + "Please register for a free ngrok account at: https://dashboard.ngrok.com/signup and place the obtained authtoken in the site config.", + ) + sys.exit(1) + + ngrok.set_auth_token(ngrok_authtoken) port = frappe.conf.http_port or frappe.conf.webserver_port tunnel = ngrok.connect(addr=str(port), host_header=site, bind_tls=bind_tls) From 2937b09a220cb2f061a396083cddc8e23a0186a0 Mon Sep 17 00:00:00 2001 From: Shariq Ansari <30859809+shariquerik@users.noreply.github.com> Date: Mon, 2 Jan 2023 16:32:27 +0530 Subject: [PATCH 037/136] fix: hide tooltip content on webform --- frappe/public/scss/website/web_form.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frappe/public/scss/website/web_form.scss b/frappe/public/scss/website/web_form.scss index 8ecf753384..e1ca8b8f58 100644 --- a/frappe/public/scss/website/web_form.scss +++ b/frappe/public/scss/website/web_form.scss @@ -18,6 +18,10 @@ .page_content { max-width: 800px; margin: auto; + + .tooltip-content { + display: none; + } h1 { font-size: 2.25rem; From 81066d7562e54d71831497a57aa24a84a3d2b0c3 Mon Sep 17 00:00:00 2001 From: Sagar Vora Date: Mon, 2 Jan 2023 15:30:09 +0530 Subject: [PATCH 038/136] fix: only allow system manager to create email templates --- frappe/email/doctype/email_template/email_template.json | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frappe/email/doctype/email_template/email_template.json b/frappe/email/doctype/email_template/email_template.json index c6ec971da4..00f1428475 100644 --- a/frappe/email/doctype/email_template/email_template.json +++ b/frappe/email/doctype/email_template/email_template.json @@ -57,18 +57,16 @@ ], "icon": "fa fa-comment", "links": [], - "modified": "2022-01-04 14:12:50.321633", + "modified": "2023-01-02 03:56:48.437280", "modified_by": "Administrator", "module": "Email", "name": "Email Template", + "naming_rule": "Set by user", "owner": "Administrator", "permissions": [ { - "create": 1, "read": 1, - "role": "All", - "share": 1, - "write": 1 + "role": "All" }, { "create": 1, @@ -85,5 +83,6 @@ ], "sort_field": "modified", "sort_order": "DESC", + "states": [], "track_changes": 1 } \ No newline at end of file From eac4f2a58c1fa306d4c2c42d950ec0d1ea0e9bcc Mon Sep 17 00:00:00 2001 From: Shariq Ansari Date: Mon, 2 Jan 2023 20:30:25 +0530 Subject: [PATCH 039/136] fix: added label & description to column --- .../js/form_builder/components/Column.vue | 118 ++++++++++++------ .../form_builder/components/FormBuilder.vue | 8 ++ .../js/form_builder/components/Section.vue | 1 + frappe/public/js/frappe/form/column.js | 12 +- frappe/public/scss/desk/form.scss | 15 +++ 5 files changed, 115 insertions(+), 39 deletions(-) diff --git a/frappe/public/js/form_builder/components/Column.vue b/frappe/public/js/form_builder/components/Column.vue index 3f108c06ba..dab1ec4c5e 100644 --- a/frappe/public/js/form_builder/components/Column.vue +++ b/frappe/public/js/form_builder/components/Column.vue @@ -1,6 +1,7 @@