vassili/seitime-frappe
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Merge pull request #28271 from rmehta/fix-oauth-page

Browse source 
fix(style): fix oauth authorisation page and standardise error responses
This commit is contained in:
Rushabh Mehta 2024-11-03 10:42:04 +05:30 • committed by GitHub
commit 6da9d2a808
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 64 additions and 66 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
frappe/app.py
View file

@ -28,6 +28,7 @@ from frappe.middlewares import StaticDataMiddleware
from frappe.utils import CallbackManager, cint, get_site_name
from frappe.utils.data import escape_html
from frappe.utils.error import log_error_snapshot
from frappe.website.page_renderers.error_page import ErrorPage
from frappe.website.serve import get_response
_site = None
@ -339,7 +340,6 @@ def make_form_dict(request: Request):
def handle_exception(e):
response = None
http_status_code = getattr(e, "http_status_code", 500)
return_as_message = False
accept_header = frappe.get_request_header("Accept") or ""
respond_as_json = (
frappe.get_request_header("Accept")
@ -347,8 +347,6 @@ def handle_exception(e):
or (frappe.local.request.path.startswith("/api/") and not accept_header.startswith("text"))
)
allow_traceback = frappe.get_system_settings("allow_error_traceback") if frappe.db else False
if not frappe.session.user:
# If session creation fails then user won't be unset. This causes a lot of code that
# assumes presence of this to fail. Session creation fails => guest or expired login
@ -371,45 +369,33 @@ def handle_exception(e):
http_status_code = 508
elif http_status_code == 401:
frappe.respond_as_web_page(
_("Session Expired"),
_("Your session has expired, please login again to continue."),
response = ErrorPage(
http_status_code=http_status_code,
indicator_color="red",
)
return_as_message = True
title=_("Session Expired"),
message=_("Your session has expired, please login again to continue."),
).render()
elif http_status_code == 403:
frappe.respond_as_web_page(
_("Not Permitted"),
_("You do not have enough permissions to complete the action"),
response = ErrorPage(
http_status_code=http_status_code,
indicator_color="red",
)
return_as_message = True
title=_("Not Permitted"),
message=_("You do not have enough permissions to complete the action"),
).render()
elif http_status_code == 404:
frappe.respond_as_web_page(
_("Not Found"),
_("The resource you are looking for is not available"),
response = ErrorPage(
http_status_code=http_status_code,
indicator_color="red",
)
return_as_message = True
title=_("Not Found"),
message=_("The resource you are looking for is not available"),
).render()
elif http_status_code == 429:
response = frappe.rate_limiter.respond()
else:
traceback = "<pre>" + escape_html(frappe.get_traceback()) + "</pre>"
# disable traceback in production if flag is set
if frappe.local.flags.disable_traceback or not allow_traceback and not frappe.local.dev_server:
traceback = ""
frappe.respond_as_web_page(
"Server Error", traceback, http_status_code=http_status_code, indicator_color="red", width=640
)
return_as_message = True
response = ErrorPage(
http_status_code=http_status_code, title=_("Server Error"), message=_("Uncaught Exception")
).render()
if e.__class__ == frappe.AuthenticationError:
if hasattr(frappe.local, "login_manager"):
@ -418,9 +404,6 @@ def handle_exception(e):
if http_status_code >= 500:
log_error_snapshot(e)
if return_as_message:
response = get_response("message", http_status_code=http_status_code)
if frappe.conf.get("developer_mode") and not respond_as_json:
# don't fail silently for non-json response errors
print(frappe.get_traceback())

2
frappe/integrations/oauth2.py
View file

@ -74,7 +74,7 @@ def approve(*args, **kwargs):
@frappe.whitelist(allow_guest=True)
def authorize(**kwargs):
success_url = "/api/method/frappe.integrations.oauth2.approve?" + encode_params(sanitize_kwargs(kwargs))
failure_url = frappe.form_dict["redirect_uri"] + "?error=access_denied"
failure_url = frappe.form_dict.get("redirect_uri", "") + "?error=access_denied"
if frappe.session.user == "Guest":
# Force login, redirect to preauth again.

2
frappe/public/scss/desk/sidebar.scss
View file

@ -57,7 +57,7 @@ body {
.body-sidebar-placeholder {
display: none;
width: 53px;
width: 50px;
height: 100vh;
}
}

6
frappe/public/scss/website/portal.scss
View file

@ -56,6 +56,12 @@
font-weight: var(--weight-semibold);
font-size: var(--text-lg);
color: var(--text-color);
p {
font-weight: normal;
font-size: var(--text-base);
color: var(--text-light);
}
}
}

42
frappe/templates/includes/oauth_confirmation.html
View file

@ -1,21 +1,24 @@
{% if not error %}
<div class="panel panel-default">
<div class="panel-heading">
<div class="panel-title h3">{{ _(client_id) }}</div>
<p class="panel-subtitle">{{ _("wants to access the following details from your account") }}</p>
</div>
<div class="panel-body">
<ul>
{% for dtl in details %}
<li>{{ _(dtl.title()) }}</li>
{% endfor %}
</ul>
<div class="action-buttons d-flex">
<button id="deny" class="btn btn-sm btn-light btn-block mr-3">{{ _("Deny") }}</button>
<button id="allow" class="btn btn-sm btn-primary btn-block">{{ _("Allow") }}</button>
</div>
</div>
<div class="portal-container">
<div class="portal-section">
<p>{{ _(client_id) }} {{ _("wants to access the following details from your account") }}</p>
</div>
<div class="portal-items">
{% for dtl in details %}
<div class="portal-section">
{{ _(dtl.title()) }}
</div>
{% endfor %}
</div>
<div class="portal-section">
<div class="action-buttons" style="display: flex; flex-direction: row-reverse; width: 100%;">
<button id="allow" class="btn btn-sm btn-primary">{{ _("Allow") }}</button>
<button id="deny" class="btn btn-sm btn-light mr-3">{{ _("Deny") }}</button>
</div>
</div>
</div>
<script type="text/javascript">
frappe.ready(function() {
$('#allow').on('click', function(event) {
@ -34,11 +37,6 @@
<div class="panel-body">
<p>{{ _("An unexpected error occurred while authorizing {}.").format(client_id) }}</p>
<h4>{{ error }}</h4>
<ul class="list-inline">
<li>
<button class="btn btn-sm btn-light">{{ _("OK") }}</button>
</li>
</ul>
</div>
</div>
{% endif %}
{% endif %}

13
frappe/website/page_renderers/error_page.py
View file

@ -2,16 +2,21 @@ from frappe.website.page_renderers.template_page import TemplatePage
class ErrorPage(TemplatePage):
def __init__(self, path=None, http_status_code=None, exception=None):
def __init__(self, path=None, http_status_code=None, exception=None, title=None, message=None):
path = "error"
super().__init__(path=path, http_status_code=http_status_code)
self.exception = exception
self.http_status_code = http_status_code
self.title = title
self.message = message
def can_render(self):
return True
def init_context(self):
super().init_context()
self.context.http_status_code = getattr(self.exception, "http_status_code", None) or 500
self.context.error_title = getattr(self.exception, "title", None)
self.context.error_message = getattr(self.exception, "message", None)
self.context.http_status_code = (
self.http_status_code or getattr(self.exception, "http_status_code", None) or 500
)
self.context.title = self.title or getattr(self.exception, "title", None)
self.context.message = self.message or getattr(self.exception, "message", None)

4
frappe/www/error.html
View file

@ -33,10 +33,10 @@
<div class="error-page">
<div>
<h4>
{{ _("Error: {0}").format(http_status_code) }}
{{ title or _("Server Error") }}
</h4>
<div class="details">
<p>{{ error_message }}</p>
<p>{{ http_status_code }}: {{ message }}</p>
<button class="btn btn-xs btn-secondary text-muted small view-error" >
{{ _("Show Error") }}

8
frappe/www/error.py
View file

@ -11,9 +11,13 @@ def get_context(context):
return
allow_traceback = frappe.get_system_settings("allow_error_traceback") if frappe.db else False
if frappe.local.flags.disable_traceback and not frappe.local.dev_server:
allow_traceback = False
context.error_title = context.error_title or _("Uncaught Server Exception")
context.error_message = context.error_message or _("There was an error building this page")
if not context.title:
context.title = _("Server Error")
if not context.message:
context.message = _("There was an error building this page")
return {
"error": frappe.get_traceback().replace("<", "&lt;").replace(">", "&gt;") if allow_traceback else ""

4
frappe/www/message.html
View file

@ -13,7 +13,9 @@
</style>
<div class='error-page'>
<div>
<h4 class='page-card-head'>{{ title or _("Message") }}</h4>
{% if title %}
<h4 class='page-card-head'>{{ title }}</h4>
{% endif %}
<div class="details">
{% block message_body %}
{% if message %}