From 6084775ed0307145753f84245dfec81aeb00df68 Mon Sep 17 00:00:00 2001 From: Anand Doshi Date: Fri, 18 Sep 2015 17:59:23 +0530 Subject: [PATCH] Added CSRF token verification for desk --- frappe/__init__.py | 2 -- frappe/app.py | 6 +++++- frappe/auth.py | 18 +++++++++++----- frappe/exceptions.py | 5 ++++- frappe/public/js/frappe/request.js | 8 +++---- frappe/sessions.py | 27 +++++++++++++++++++----- frappe/templates/includes/login/login.js | 1 + frappe/templates/pages/desk.py | 6 +++++- frappe/utils/response.py | 2 +- frappe/website/js/website.js | 5 ----- socketio.js | 4 ++-- 11 files changed, 56 insertions(+), 28 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index d00a3e64e6..2ab0c18038 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -66,7 +66,6 @@ db = local("db") conf = local("conf") form = form_dict = local("form_dict") request = local("request") -request_method = local("request_method") response = local("response") session = local("session") user = local("user") @@ -109,7 +108,6 @@ def init(site, sites_path=None): local.sites_path = sites_path local.site_path = os.path.join(sites_path, site) - local.request_method = request.method if request else None local.request_ip = None local.response = _dict({"docs":[]}) local.task_id = None diff --git a/frappe/app.py b/frappe/app.py index 6d6b34b005..f1bbb28519 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -99,8 +99,12 @@ def application(request): if frappe.local.is_ajax or 'application/json' in request.headers.get('Accept', ''): response = frappe.utils.response.report_error(http_status_code) else: + traceback = "
"+frappe.get_traceback()+"
" + if frappe.local.flags.disable_traceback: + traceback = "" + frappe.respond_as_web_page("Server Error", - "
"+frappe.get_traceback()+"
", + traceback, http_status_code=http_status_code) response = frappe.website.render.render("message", http_status_code=http_status_code) diff --git a/frappe/auth.py b/frappe/auth.py index 7d450dd78b..39cf78c306 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -37,17 +37,14 @@ class HTTPRequest: # load cookies frappe.local.cookie_manager = CookieManager() - # override request method. All request to be of type POST, but if _type == "POST" then commit - if frappe.form_dict.get("_type"): - frappe.local.request_method = frappe.form_dict.get("_type") - del frappe.form_dict["_type"] - # set db self.connect() # login frappe.local.login_manager = LoginManager() + self.validate_csrf_token() + # write out latest cookies frappe.local.cookie_manager.init_cookies() @@ -58,6 +55,15 @@ class HTTPRequest: if frappe.form_dict.get('cmd')=='login': frappe.local.login_manager.run_trigger('on_session_creation') + def validate_csrf_token(self): + if frappe.local.request and frappe.local.request.method=="POST": + if not frappe.local.session.data.csrf_token: + # not via boot + return + + if frappe.local.session.data.csrf_token != frappe.get_request_header("X-Frappe-CSRF-Token"): + frappe.local.flags.disable_traceback = True + frappe.throw(_("Invalid Request"), frappe.CSRFTokenError) def set_lang(self, lang_codes): from frappe.translate import guess_language @@ -81,8 +87,10 @@ class LoginManager: if frappe.local.form_dict.get('cmd')=='login' or frappe.local.request.path=="/api/method/login": self.login() + self.resume = False else: try: + self.resume = True self.make_session(resume=True) self.set_user_info(resume=True) except AttributeError: diff --git a/frappe/exceptions.py b/frappe/exceptions.py index a5b5ac03a6..17c12d2c36 100644 --- a/frappe/exceptions.py +++ b/frappe/exceptions.py @@ -35,6 +35,9 @@ class UnsupportedMediaType(Exception): class Redirect(Exception): http_status_code = 301 +class CSRFTokenError(Exception): + http_status_code = 400 + class DuplicateEntryError(NameError):pass class DataError(ValidationError): pass class UnknownDomainError(Exception): pass @@ -53,4 +56,4 @@ class EmptyTableError(ValidationError): pass class LinkExistsError(ValidationError): pass class InvalidEmailAddressError(ValidationError): pass class TemplateNotFoundError(ValidationError): pass -class UniqueValidationError(ValidationError): pass \ No newline at end of file +class UniqueValidationError(ValidationError): pass diff --git a/frappe/public/js/frappe/request.js b/frappe/public/js/frappe/request.js index b364a5b0cc..6139e6d03a 100644 --- a/frappe/public/js/frappe/request.js +++ b/frappe/public/js/frappe/request.js @@ -62,9 +62,6 @@ frappe.call = function(opts) { frappe.request.call = function(opts) { frappe.request.prepare(opts); - // all requests will be post, set _type as POST for commit - opts.args._type = opts.type; - var statusCode = { 200: function(data, xhr) { if(typeof data === "string") data = JSON.parse(data); @@ -121,9 +118,10 @@ frappe.request.call = function(opts) { var ajax_args = { url: opts.url || frappe.request.url, data: opts.args, - type: 'POST', + type: opts.type, dataType: opts.dataType || 'json', - async: opts.async + async: opts.async, + headers: { "X-Frappe-CSRF-Token": frappe.boot.csrf_token } }; frappe.last_request = ajax_args.data; diff --git a/frappe/sessions.py b/frappe/sessions.py index 4de81b577e..46b323fc97 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -68,6 +68,7 @@ def delete_session(sid=None, user=None): frappe.cache().hdel("session", sid) frappe.cache().hdel("last_db_session_update", sid) frappe.db.sql("""delete from tabSessions where sid=%s""", sid) + frappe.db.commit() def clear_all_sessions(): """This effectively logs out all users""" @@ -127,8 +128,19 @@ def get(): bootinfo["lang"] = frappe.translate.get_user_lang() bootinfo["dev_server"] = os.environ.get('DEV_SERVER', False) bootinfo["disable_async"] = frappe.conf.disable_async + return bootinfo +def get_csrf_token(): + if not frappe.local.session.data.csrf_token: + generate_csrf_token() + + return frappe.local.session.data.csrf_token + +def generate_csrf_token(): + frappe.local.session.data.csrf_token = frappe.generate_hash() + frappe.local.session_obj.update(force=True) + class Session: def __init__(self, user, resume=False, full_name=None, user_type=None): self.sid = cstr(frappe.form_dict.get('sid') or @@ -145,6 +157,7 @@ class Session: if resume: self.resume() + else: if self.user: self.start() @@ -168,7 +181,7 @@ class Session: "full_name": self.full_name, "user_type": self.user_type, "device": self.device, - "session_country": get_geo_ip_country(frappe.local.request_ip) if frappe.local.request_ip else None + "session_country": get_geo_ip_country(frappe.local.request_ip) if frappe.local.request_ip else None, }) # insert session @@ -178,6 +191,7 @@ class Session: # update user frappe.db.sql("""UPDATE tabUser SET last_login = %s, last_ip = %s where name=%s""", (frappe.utils.now(), frappe.local.request_ip, self.data['user'])) + frappe.db.commit() def insert_session_record(self): @@ -194,10 +208,12 @@ class Session: import frappe data = self.get_session_record() + if data: # set language self.data.update({'data': data, 'user':data.user, 'sid': self.sid}) self.user = data.user + self.device = data.device else: self.start_as_guest() @@ -300,12 +316,13 @@ class Session: return updated_in_db def get_expiry_period(device="desktop"): - if device=="desktop": - key = "session_expiry" - default = "06:00:00" - else: + if device=="mobile": key = "session_expiry_mobile" default = "720:00:00" + else: + key = "session_expiry" + default = "06:00:00" + exp_sec = frappe.defaults.get_global_default(key) or default # incase seconds is missing diff --git a/frappe/templates/includes/login/login.js b/frappe/templates/includes/login/login.js index 859d910e9b..758797a7c6 100644 --- a/frappe/templates/includes/login/login.js +++ b/frappe/templates/includes/login/login.js @@ -14,6 +14,7 @@ login.bind_events = function() { args.cmd = "login"; args.usr = ($("#login_email").val() || "").trim(); args.pwd = $("#login_password").val(); + args.device = "desktop"; if(!args.usr || !args.pwd) { frappe.msgprint(__("Both login and password required")); return false; diff --git a/frappe/templates/pages/desk.py b/frappe/templates/pages/desk.py index c8d262ca75..f1ce2a8daa 100644 --- a/frappe/templates/pages/desk.py +++ b/frappe/templates/pages/desk.py @@ -19,10 +19,14 @@ def get_context(context): hooks = frappe.get_hooks() boot = frappe.sessions.get() - boot_json = frappe.as_json(boot) + + # this needs commit + boot["csrf_token"] = frappe.sessions.get_csrf_token() frappe.db.commit() + boot_json = frappe.as_json(boot) + # remove script tags from boot boot_json = re.sub("\[^<]*\", "", boot_json) diff --git a/frappe/utils/response.py b/frappe/utils/response.py index f4d1c3722b..4f017d4327 100644 --- a/frappe/utils/response.py +++ b/frappe/utils/response.py @@ -18,7 +18,7 @@ from werkzeug.wrappers import Response from werkzeug.exceptions import NotFound, Forbidden def report_error(status_code): - if status_code!=404 or frappe.conf.logging: + if (status_code!=404 or frappe.conf.logging) and not frappe.local.flags.disable_traceback: frappe.errprint(frappe.utils.get_traceback()) response = build_response("json") diff --git a/frappe/website/js/website.js b/frappe/website/js/website.js index 5adf938279..77bd6ebd08 100644 --- a/frappe/website/js/website.js +++ b/frappe/website/js/website.js @@ -66,11 +66,6 @@ $.extend(frappe, { if(!opts.args) opts.args = {}; - // get or post? - if(!opts.args._type) { - opts.args._type = opts.type || "GET"; - } - // method if(opts.method) { opts.args.cmd = opts.method; diff --git a/socketio.js b/socketio.js index 496d28b615..cf9b795e1a 100644 --- a/socketio.js +++ b/socketio.js @@ -32,7 +32,7 @@ io.on('connection', function(socket){ return; } // console.log("firing get_user_info"); - request.post(get_url(socket, '/api/method/frappe.async.get_user_info')) + request.get(get_url(socket, '/api/method/frappe.async.get_user_info')) .type('form') .send({ sid: sid @@ -63,7 +63,7 @@ io.on('connection', function(socket){ socket.on('doc_subscribe', function(doctype, docname) { // console.log('trying to subscribe', doctype, docname) - request.post(get_url(socket, '/api/method/frappe.async.can_subscribe_doc')) + request.get(get_url(socket, '/api/method/frappe.async.can_subscribe_doc')) .type('form') .send({ sid: sid,