From a121b90d7fc30d32a73a69e8d5e37c377f42fea2 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 22 Jan 2025 16:35:53 +0530 Subject: [PATCH 1/5] feat: allow created a session for a fixed duration via `bench browse` Signed-off-by: Akhil Narang --- frappe/auth.py | 5 ++++- frappe/commands/site.py | 5 ++++- frappe/sessions.py | 7 ++++++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index c77d50ab93..fde5af0031 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -219,7 +219,10 @@ class LoginManager: def make_session(self, resume=False): # start session frappe.local.session_obj = Session( - user=self.user, resume=resume, full_name=self.full_name, user_type=self.user_type + user=self.user, + resume=resume, + full_name=self.full_name, + user_type=self.user_type, ) # reset user if changed to Guest diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 5a8c53e840..6cc2440dc1 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1175,8 +1175,9 @@ def publish_realtime(context: CliCtxObj, event, message, room, user, doctype, do @click.command("browse") @click.argument("site", required=False) @click.option("--user", required=False, help="Login as user") +@click.option("--duration", required=False, help="Session duration (in hh:mm:ss format)") @pass_context -def browse(context: CliCtxObj, site, user=None): +def browse(context: CliCtxObj, site, user=None, duration=None): """Opens the site on web browser""" from frappe.auth import CookieManager, LoginManager @@ -1192,6 +1193,8 @@ def browse(context: CliCtxObj, site, user=None): frappe.init(site) frappe.connect() + frappe.flags.session_duration = duration + sid = "" if user: if not frappe.db.exists("User", user): diff --git a/frappe/sessions.py b/frappe/sessions.py index 6329b1320d..9c1d8752d5 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -246,11 +246,13 @@ class Session: self.sid = self.data.sid = sid self.data.data.user = self.user self.data.data.session_ip = frappe.local.request_ip + if frappe.flags.session_duration: + self.data.data.fixed_duration = True if self.user != "Guest": self.data.data.update( { "last_updated": frappe.utils.now(), - "session_expiry": get_expiry_period(), + "session_expiry": frappe.flags.session_duration or get_expiry_period(), "full_name": self.full_name, "user_type": self.user_type, } @@ -386,6 +388,9 @@ class Session: if frappe.session.user == "Guest": return + if self.data.data.fixed_duration: + return + now = frappe.utils.now_datetime() # update session in db From 332e22f00b3559c96d04023f5242530f31d19a2c Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 22 Jan 2025 16:39:20 +0530 Subject: [PATCH 2/5] refactor: fix typo in `impersonated` Signed-off-by: Akhil Narang --- frappe/auth.py | 2 +- frappe/sessions.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index fde5af0031..c7dc0d17a5 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -350,7 +350,7 @@ class LoginManager: current_user = frappe.session.user self.login_as(user) # Flag this session as impersonated session, so other code can log this. - frappe.local.session_obj.set_impersonsated(current_user) + frappe.local.session_obj.set_impersonated(current_user) def logout(self, arg="", user=None): if not user: diff --git a/frappe/sessions.py b/frappe/sessions.py index 9c1d8752d5..b890d2837c 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -425,7 +425,7 @@ class Session: return updated_in_db - def set_impersonsated(self, original_user): + def set_impersonated(self, original_user): self.data.data.impersonated_by = original_user # Forcefully flush session self.update(force=True) From 1dc767f671a12e643c305a2de1fc32fcc8a97198 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Wed, 22 Jan 2025 16:45:34 +0530 Subject: [PATCH 3/5] feat(browse): allow passing a user for `impersonated_by` Signed-off-by: Akhil Narang --- frappe/commands/site.py | 10 +++++++++- frappe/sessions.py | 4 ++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 6cc2440dc1..66f2d5e244 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1176,8 +1176,15 @@ def publish_realtime(context: CliCtxObj, event, message, room, user, doctype, do @click.argument("site", required=False) @click.option("--user", required=False, help="Login as user") @click.option("--duration", required=False, help="Session duration (in hh:mm:ss format)") +@click.option("--user-for-audit", required=False, help="The user to mention in audit trail") @pass_context -def browse(context: CliCtxObj, site, user=None, duration=None): +def browse( + context: CliCtxObj, + site, + user: str | None = None, + duration: str | None = None, + user_for_audit: str | None = None, +): """Opens the site on web browser""" from frappe.auth import CookieManager, LoginManager @@ -1194,6 +1201,7 @@ def browse(context: CliCtxObj, site, user=None, duration=None): frappe.connect() frappe.flags.session_duration = duration + frappe.flags.audit_user = user_for_audit sid = "" if user: diff --git a/frappe/sessions.py b/frappe/sessions.py index b890d2837c..51df41bff1 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -248,6 +248,10 @@ class Session: self.data.data.session_ip = frappe.local.request_ip if frappe.flags.session_duration: self.data.data.fixed_duration = True + + if frappe.flags.audit_user: + self.data.data.impersonated_by = frappe.flags.audit_user + if self.user != "Guest": self.data.data.update( { From 15065a93e301bfb6e370f15abdb573f47004ebc1 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Thu, 23 Jan 2025 13:03:39 +0530 Subject: [PATCH 4/5] refactor: don't use impersonate directly, use similar logic This will allow impersonating as well Signed-off-by: Akhil Narang --- frappe/auth.py | 15 ++++++---- frappe/commands/site.py | 5 +--- frappe/core/doctype/version/version.py | 3 ++ .../version_timeline_content_builder.js | 6 ++++ frappe/sessions.py | 28 +++++++++++-------- 5 files changed, 36 insertions(+), 21 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index c7dc0d17a5..2a50c75cac 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -163,12 +163,12 @@ class LoginManager: frappe.form_dict.pop("pwd", None) self.post_login() - def post_login(self): + def post_login(self, duration: str | None = None, audit_user: str | None = None): self.run_trigger("on_login") validate_ip_address(self.user) self.validate_hour() self.get_user_info() - self.make_session() + self.make_session(duration=duration, audit_user=audit_user) self.setup_boot_cache() self.set_user_info() @@ -216,13 +216,15 @@ class LoginManager: def clear_preferred_language(self): frappe.local.cookie_manager.delete_cookie("preferred_language") - def make_session(self, resume=False): + def make_session(self, resume: bool = False, duration: str | None = None, audit_user: str | None = None): # start session frappe.local.session_obj = Session( user=self.user, resume=resume, full_name=self.full_name, user_type=self.user_type, + duration=duration, + audit_user=audit_user, ) # reset user if changed to Guest @@ -342,13 +344,14 @@ class LoginManager: """login as guest""" self.login_as("Guest") - def login_as(self, user): + def login_as(self, user: str, duration: str | None = None, audit_user: str | None = None): self.user = user - self.post_login() + self.post_login(duration, audit_user) def impersonate(self, user): current_user = frappe.session.user - self.login_as(user) + session_data = frappe.local.session_obj.data.data + self.login_as(user, duration=session_data.duration, audit_user=session_data.audit_user) # Flag this session as impersonated session, so other code can log this. frappe.local.session_obj.set_impersonated(current_user) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 66f2d5e244..0c748d038a 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1200,9 +1200,6 @@ def browse( frappe.init(site) frappe.connect() - frappe.flags.session_duration = duration - frappe.flags.audit_user = user_for_audit - sid = "" if user: if not frappe.db.exists("User", user): @@ -1213,7 +1210,7 @@ def browse( frappe.utils.set_request(path="/") frappe.local.cookie_manager = CookieManager() frappe.local.login_manager = LoginManager() - frappe.local.login_manager.login_as(user) + frappe.local.login_manager.login_as(user, duration, user_for_audit) sid = f"/app?sid={frappe.session.sid}" else: click.echo("Please enable developer mode to login as a user") diff --git a/frappe/core/doctype/version/version.py b/frappe/core/doctype/version/version.py index 39eff56a75..3ad33021c7 100644 --- a/frappe/core/doctype/version/version.py +++ b/frappe/core/doctype/version/version.py @@ -37,6 +37,9 @@ class Version(Document): if impersonator := frappe.session.data.get("impersonated_by"): data["impersonated_by"] = impersonator + if audit_user := frappe.session.data.get("audit_user"): + data["audit_user"] = audit_user + def set_diff(self, old: Document, new: Document) -> bool: """Set the data property with the diff of the docs if present""" diff = get_diff(old, new) diff --git a/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js b/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js index 43d55fe5da..7e6a5526c7 100644 --- a/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js +++ b/frappe/public/js/frappe/form/footer/version_timeline_content_builder.js @@ -246,6 +246,12 @@ function get_version_timeline_content(version_doc, frm) { const impersonated_msg = __("Impersonated by {0}", [get_user_link(impersonated_by)]); out = out.map((message) => `${message} · ${impersonated_msg.bold()}`); } + + const audit_user = data.audit_user; + if (audit_user) { + const audit_msg = __("[Action taken by {0}]", [audit_user]); + out = out.map((message) => `${message} · ${audit_msg.bold()}`); + } return out; } diff --git a/frappe/sessions.py b/frappe/sessions.py index 51df41bff1..1b5d04f8b5 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -205,7 +205,15 @@ def generate_csrf_token(): class Session: __slots__ = ("_update_in_cache", "data", "full_name", "sid", "time_diff", "user", "user_type") - def __init__(self, user, resume=False, full_name=None, user_type=None): + def __init__( + self, + user: str, + resume: bool = False, + full_name: str | None = None, + user_type: str | None = None, + duration: str | None = None, + audit_user: str | None = None, + ): self.sid = cstr( frappe.form_dict.pop("sid", None) or unquote(frappe.request.cookies.get("sid", "Guest")) ) @@ -225,7 +233,7 @@ class Session: else: if self.user: self.validate_user() - self.start() + self.start(duration, audit_user) def validate_user(self): if not frappe.get_cached_value("User", self.user, "enabled"): @@ -234,7 +242,7 @@ class Session: frappe.ValidationError, ) - def start(self): + def start(self, duration: str | None = None, audit_user: str | None = None): """start a new session""" # generate sid if self.user == "Guest": @@ -246,17 +254,17 @@ class Session: self.sid = self.data.sid = sid self.data.data.user = self.user self.data.data.session_ip = frappe.local.request_ip - if frappe.flags.session_duration: + if duration: self.data.data.fixed_duration = True - if frappe.flags.audit_user: - self.data.data.impersonated_by = frappe.flags.audit_user + if audit_user: + self.data.data.audit_user = audit_user if self.user != "Guest": self.data.data.update( { "last_updated": frappe.utils.now(), - "session_expiry": frappe.flags.session_duration or get_expiry_period(), + "session_expiry": duration or get_expiry_period(), "full_name": self.full_name, "user_type": self.user_type, } @@ -392,9 +400,6 @@ class Session: if frappe.session.user == "Guest": return - if self.data.data.fixed_duration: - return - now = frappe.utils.now_datetime() # update session in db @@ -406,7 +411,8 @@ class Session: if ( force or (time_diff is None) or (time_diff > 600) or self._update_in_cache ) and not frappe.flags.read_only: - self.data.data.last_updated = now + if not self.data.data.fixed_duration: + self.data.data.last_updated = now self.data.data.lang = str(frappe.lang) Sessions = frappe.qb.DocType("Sessions") From a9c1c49fff749b57fc1cb30734220be0ada6ad2d Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Fri, 24 Jan 2025 17:57:21 +0530 Subject: [PATCH 5/5] refactor: use an alternate key for handling expiry This allows for less changes to update() + allows impersonated sessions to not end later Signed-off-by: Akhil Narang --- frappe/auth.py | 16 +++++++++------- frappe/commands/site.py | 10 +++++++--- frappe/sessions.py | 22 +++++++++++++--------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/frappe/auth.py b/frappe/auth.py index 2a50c75cac..dd20fdacb0 100644 --- a/frappe/auth.py +++ b/frappe/auth.py @@ -163,12 +163,12 @@ class LoginManager: frappe.form_dict.pop("pwd", None) self.post_login() - def post_login(self, duration: str | None = None, audit_user: str | None = None): + def post_login(self, session_end: str | None = None, audit_user: str | None = None): self.run_trigger("on_login") validate_ip_address(self.user) self.validate_hour() self.get_user_info() - self.make_session(duration=duration, audit_user=audit_user) + self.make_session(session_end=session_end, audit_user=audit_user) self.setup_boot_cache() self.set_user_info() @@ -216,14 +216,16 @@ class LoginManager: def clear_preferred_language(self): frappe.local.cookie_manager.delete_cookie("preferred_language") - def make_session(self, resume: bool = False, duration: str | None = None, audit_user: str | None = None): + def make_session( + self, resume: bool = False, session_end: str | None = None, audit_user: str | None = None + ): # start session frappe.local.session_obj = Session( user=self.user, resume=resume, full_name=self.full_name, user_type=self.user_type, - duration=duration, + session_end=session_end, audit_user=audit_user, ) @@ -344,14 +346,14 @@ class LoginManager: """login as guest""" self.login_as("Guest") - def login_as(self, user: str, duration: str | None = None, audit_user: str | None = None): + def login_as(self, user: str, session_end: str | None = None, audit_user: str | None = None): self.user = user - self.post_login(duration, audit_user) + self.post_login(session_end, audit_user) def impersonate(self, user): current_user = frappe.session.user session_data = frappe.local.session_obj.data.data - self.login_as(user, duration=session_data.duration, audit_user=session_data.audit_user) + self.login_as(user, session_end=session_data.session_end, audit_user=session_data.audit_user) # Flag this session as impersonated session, so other code can log this. frappe.local.session_obj.set_impersonated(current_user) diff --git a/frappe/commands/site.py b/frappe/commands/site.py index 0c748d038a..7a858f62ec 100644 --- a/frappe/commands/site.py +++ b/frappe/commands/site.py @@ -1175,14 +1175,18 @@ def publish_realtime(context: CliCtxObj, event, message, room, user, doctype, do @click.command("browse") @click.argument("site", required=False) @click.option("--user", required=False, help="Login as user") -@click.option("--duration", required=False, help="Session duration (in hh:mm:ss format)") +@click.option( + "--session-end", + required=False, + help="Session end (in ISO8601 format and timezone-aware - 2025-01-24T12:26:29.200853+00:00)", +) @click.option("--user-for-audit", required=False, help="The user to mention in audit trail") @pass_context def browse( context: CliCtxObj, site, user: str | None = None, - duration: str | None = None, + session_end: str | None = None, user_for_audit: str | None = None, ): """Opens the site on web browser""" @@ -1210,7 +1214,7 @@ def browse( frappe.utils.set_request(path="/") frappe.local.cookie_manager = CookieManager() frappe.local.login_manager = LoginManager() - frappe.local.login_manager.login_as(user, duration, user_for_audit) + frappe.local.login_manager.login_as(user, session_end, user_for_audit) sid = f"/app?sid={frappe.session.sid}" else: click.echo("Please enable developer mode to login as a user") diff --git a/frappe/sessions.py b/frappe/sessions.py index 1b5d04f8b5..8404405148 100644 --- a/frappe/sessions.py +++ b/frappe/sessions.py @@ -8,6 +8,7 @@ permission, homepage, default variables, system defaults etc """ import json +from datetime import datetime, timezone from urllib.parse import unquote import redis @@ -211,7 +212,7 @@ class Session: resume: bool = False, full_name: str | None = None, user_type: str | None = None, - duration: str | None = None, + session_end: str | None = None, audit_user: str | None = None, ): self.sid = cstr( @@ -233,7 +234,7 @@ class Session: else: if self.user: self.validate_user() - self.start(duration, audit_user) + self.start(session_end, audit_user) def validate_user(self): if not frappe.get_cached_value("User", self.user, "enabled"): @@ -242,7 +243,7 @@ class Session: frappe.ValidationError, ) - def start(self, duration: str | None = None, audit_user: str | None = None): + def start(self, session_end: str | None = None, audit_user: str | None = None): """start a new session""" # generate sid if self.user == "Guest": @@ -254,8 +255,9 @@ class Session: self.sid = self.data.sid = sid self.data.data.user = self.user self.data.data.session_ip = frappe.local.request_ip - if duration: - self.data.data.fixed_duration = True + + if session_end: + self.data.data.session_end = session_end if audit_user: self.data.data.audit_user = audit_user @@ -264,7 +266,7 @@ class Session: self.data.data.update( { "last_updated": frappe.utils.now(), - "session_expiry": duration or get_expiry_period(), + "session_expiry": get_expiry_period(), "full_name": self.full_name, "user_type": self.user_type, } @@ -361,7 +363,10 @@ class Session: ) expiry = get_expiry_in_seconds(session_data.get("session_expiry")) - if self.time_diff > expiry: + if self.time_diff > expiry or ( + (session_end := session_data.get("session_end")) + and datetime.now(tz=timezone.utc) > datetime.fromisoformat(session_end) + ): self._delete_session() data = None @@ -411,8 +416,7 @@ class Session: if ( force or (time_diff is None) or (time_diff > 600) or self._update_in_cache ) and not frappe.flags.read_only: - if not self.data.data.fixed_duration: - self.data.data.last_updated = now + self.data.data.last_updated = now self.data.data.lang = str(frappe.lang) Sessions = frappe.qb.DocType("Sessions")